From 5baa1ed97c1d2b98952a025c3bc76f60587e9670 Mon Sep 17 00:00:00 2001 From: Jozsef Csongvai Date: Mon, 13 Jun 2022 08:53:19 -0400 Subject: Enable long-running processes in ControllerExecutionBB Instead of blocking a thread while waiting for controller response, ControllerExecutionBB is now using camunda receive task to support long running processes without increasing the camunda job timeout. A new property was added to configure the gRPC client's keep alive ping mechanism, which will identify connection issues and prevent the process getting stuck when the controller crashes. Issue-ID: SO-3953 Signed-off-by: Jozsef Csongvai Change-Id: Iaf6438dba76e715dba846bf45ef47b6a91239c4a --- .../client/cds/AbstractCDSProcessingBBUtils.java | 122 +++++++++++++++++++-- .../org/onap/so/client/cds/PayloadConstants.java | 4 + .../client/restproperties/CDSPropertiesImpl.java | 12 ++ .../BuildingBlock/ControllerExecutionBB.bpmn | 84 +++++++++----- .../impl/buildingblock/ControllerExecutionBB.java | 19 ++++ .../buildingblock/ControllerExecutionBBTest.java | 32 ++++++ .../cds/PnfConfigCdsControllerDETest.java | 2 +- .../onap/so/client/cds/CDSProcessingClient.java | 2 + .../java/org/onap/so/client/cds/CDSProperties.java | 17 ++- .../onap/so/client/cds/TestCDSPropertiesImpl.java | 5 + 10 files changed, 260 insertions(+), 39 deletions(-) diff --git a/bpmn/MSOCommonBPMN/src/main/java/org/onap/so/client/cds/AbstractCDSProcessingBBUtils.java b/bpmn/MSOCommonBPMN/src/main/java/org/onap/so/client/cds/AbstractCDSProcessingBBUtils.java index e5d8a921a5..3ed1011ee9 100644 --- a/bpmn/MSOCommonBPMN/src/main/java/org/onap/so/client/cds/AbstractCDSProcessingBBUtils.java +++ b/bpmn/MSOCommonBPMN/src/main/java/org/onap/so/client/cds/AbstractCDSProcessingBBUtils.java @@ -22,17 +22,24 @@ package org.onap.so.client.cds; +import static org.onap.so.client.cds.PayloadConstants.CONTROLLER_ERROR_MESSAGE; import com.google.protobuf.InvalidProtocolBufferException; import com.google.protobuf.Struct; import com.google.protobuf.Struct.Builder; import com.google.protobuf.util.JsonFormat; import io.grpc.Status; +import java.util.UUID; +import org.apache.commons.lang3.StringUtils; +import org.camunda.bpm.engine.MismatchingMessageCorrelationException; +import org.camunda.bpm.engine.ProcessEngine; import org.camunda.bpm.engine.delegate.DelegateExecution; +import org.camunda.bpm.engine.runtime.MessageCorrelationBuilder; import org.onap.ccsdk.cds.controllerblueprints.common.api.ActionIdentifiers; import org.onap.ccsdk.cds.controllerblueprints.common.api.CommonHeader; import org.onap.ccsdk.cds.controllerblueprints.common.api.EventType; import org.onap.ccsdk.cds.controllerblueprints.processing.api.ExecutionServiceInput; import org.onap.ccsdk.cds.controllerblueprints.processing.api.ExecutionServiceOutput; +import org.onap.logging.filter.base.ONAPComponents; import org.onap.so.bpmn.common.BuildingBlockExecution; import org.onap.so.client.PreconditionFailedException; import org.onap.so.client.RestPropertiesLoader; @@ -62,10 +69,17 @@ public class AbstractCDSProcessingBBUtils { private static final String EXEC_INPUT = "executionServiceInput"; private static final String EXECUTION_OBJECT = "executionObject"; private static final String EXCEPTION = "Exception"; + private static final String CDS_REQUEST_ID = "CDS_REQUEST_ID"; + private static final String CONTROLLER_MESSAGE = "ControllerMessage"; + + private static final String REQ_ID = "requestId"; @Autowired protected ExceptionBuilder exceptionUtil; + @Autowired + private ProcessEngine processEngine; + /** * Extracting data from execution object and building the ExecutionServiceInput Object * @@ -132,23 +146,28 @@ public class AbstractCDSProcessingBBUtils { } /** - * get the executionServiceInput object from execution and send a request to CDS Client and wait for TIMEOUT period + * get the executionServiceInput object from execution and send a request to CDS Client * * @param execution BuildingBlockExecution object */ public void sendRequestToCDSClientBB(BuildingBlockExecution execution) { - logger.trace("Start AbstractCDSProcessingBBUtils.sendRequestToCDSClient for BuildingBlockExecution object."); try { ExecutionServiceInput executionServiceInput = execution.getVariable(EXEC_INPUT); - CDSResponse cdsResponse = getCdsResponse(executionServiceInput); - execution.setVariable(CDS_STATUS, cdsResponse.status); - if (cdsResponse.payload != null) { - String payload = JsonFormat.printer().print(cdsResponse.payload); - execution.setVariable(RESPONSE_PAYLOAD, payload); + String messageCorrelationId = executionServiceInput.getCommonHeader().getSubRequestId(); + if (StringUtils.isBlank(messageCorrelationId)) { + throw new IllegalArgumentException("subRequestId can not be blank"); } - + execution.setVariable(CDS_REQUEST_ID, messageCorrelationId); + + MessageCorrelationBuilder messageCorrelationBuilder = + processEngine.getRuntimeService().createMessageCorrelation(CONTROLLER_MESSAGE) + .processInstanceVariableEquals(CDS_REQUEST_ID, messageCorrelationId); + MessageSendingHandler handler = new MessageSendingHandler(messageCorrelationBuilder); + CDSProcessingClient client = new CDSProcessingClient(handler); + handler.setClient(client); + client.sendRequest(executionServiceInput); } catch (Exception ex) { exceptionUtil.buildAndThrowWorkflowException(execution, 7000, ex); } @@ -249,6 +268,93 @@ public class AbstractCDSProcessingBBUtils { } } + private class MessageSendingHandler implements CDSProcessingListener { + + private MessageCorrelationBuilder messageCorrelationBuilder; + private AutoCloseable client; + private Logger logger = LoggerFactory.getLogger(MessageSendingHandler.class); + + MessageSendingHandler(MessageCorrelationBuilder messageCorrelationBuilder) { + this.messageCorrelationBuilder = messageCorrelationBuilder; + } + + public void setClient(AutoCloseable client) { + this.client = client; + } + + @Override + public void onMessage(ExecutionServiceOutput message) { + logger.info("Received payload from CDS: {}", message); + EventType eventType = message.getStatus().getEventType(); + + if (eventType == EventType.EVENT_COMPONENT_PROCESSING) { + return; + } + + String status = eventType == EventType.EVENT_COMPONENT_EXECUTED ? SUCCESS : FAILED; + messageCorrelationBuilder.setVariable(CDS_STATUS, status); + messageCorrelationBuilder.setVariable(CONTROLLER_ERROR_MESSAGE, message.getStatus().getErrorMessage()); + + if (message.hasPayload()) { + try { + String payload = JsonFormat.printer().print(message.getPayload()); + messageCorrelationBuilder.setVariable(RESPONSE_PAYLOAD, payload); + } catch (InvalidProtocolBufferException e) { + logger.error("Failed parsing cds response", e); + } + } + correlate(); + } + + @Override + public void onError(Throwable t) { + logger.error("Failed sending CDS request", t); + messageCorrelationBuilder.setVariable(CONTROLLER_ERROR_MESSAGE, t.getMessage()); + messageCorrelationBuilder.setVariable(CDS_STATUS, FAILED); + correlate(); + } + + /** + * When a CDS call returns before the bpmn process is in a waiting state, message correlation will fail. This + * retry logic will allow camunda some time to finish transitioning the process. + */ + private void correlate() { + try { + int remainingTries = 10; + while (!tryCorrelateMessage() && remainingTries > 0) { + logger.warn("Message correlation failed. Retries remaining: {}", remainingTries); + remainingTries--; + Thread.sleep(1000L); + } + } catch (InterruptedException e) { + logger.error("Thread interrupted during message correlation", e); + Thread.currentThread().interrupt(); + } finally { + closeClient(); + } + } + + private boolean tryCorrelateMessage() { + try { + messageCorrelationBuilder.correlate(); + logger.info("Message correlation successful"); + return true; + } catch (MismatchingMessageCorrelationException e) { + return false; + } + } + + private void closeClient() { + if (client == null) + throw new IllegalStateException("Client was not set and could not be closed"); + try { + client.close(); + } catch (Exception e) { + logger.error("Failed closing cds client", e); + } + } + } + private class CDSResponse { String status; diff --git a/bpmn/MSOCommonBPMN/src/main/java/org/onap/so/client/cds/PayloadConstants.java b/bpmn/MSOCommonBPMN/src/main/java/org/onap/so/client/cds/PayloadConstants.java index 2812de799d..019e336325 100644 --- a/bpmn/MSOCommonBPMN/src/main/java/org/onap/so/client/cds/PayloadConstants.java +++ b/bpmn/MSOCommonBPMN/src/main/java/org/onap/so/client/cds/PayloadConstants.java @@ -52,4 +52,8 @@ public final class PayloadConstants { public final static String PNF_UUID = "pnfUuid"; public final static String SERVICE_INSTANCE_ID = "serviceInstanceId"; public final static String MODEL_UUID = "modelUuid"; + + public final static String TIMEOUT_CONTROLLER_MESSAGE = "timeoutControllerMessage"; + public final static String CONTROLLER_ERROR_MESSAGE = "controllerErrorMessage"; + public final static String CONTROLLER_MSG_TIMEOUT_REACHED = "controllerMessageTimeoutReached"; } diff --git a/bpmn/MSOCommonBPMN/src/main/java/org/onap/so/client/restproperties/CDSPropertiesImpl.java b/bpmn/MSOCommonBPMN/src/main/java/org/onap/so/client/restproperties/CDSPropertiesImpl.java index 2efd74ddc5..ec0eb37b95 100644 --- a/bpmn/MSOCommonBPMN/src/main/java/org/onap/so/client/restproperties/CDSPropertiesImpl.java +++ b/bpmn/MSOCommonBPMN/src/main/java/org/onap/so/client/restproperties/CDSPropertiesImpl.java @@ -22,6 +22,7 @@ package org.onap.so.client.restproperties; import java.net.URL; import java.util.Objects; +import org.apache.commons.lang3.StringUtils; import org.onap.so.bpmn.core.UrnPropertiesReader; import org.onap.so.client.cds.CDSProperties; @@ -31,6 +32,8 @@ public class CDSPropertiesImpl implements CDSProperties { private static final String PORT = "cds.port"; private static final String AUTH = "cds.auth"; private static final String TIMEOUT = "cds.timeout"; + private static final String KEEP_ALIVE_PING_MINUTES = "keep-alive-ping-minutes"; + private static final long GRPC_SERVER_DEFAULT_MIN_ALLOWED_PING_INTERVAL = 5; public CDSPropertiesImpl() { // Needed for service loader @@ -90,4 +93,13 @@ public class CDSPropertiesImpl implements CDSProperties { public boolean getUseBasicAuth() { return true; } + + @Override + public long getKeepAlivePingMinutes() { + String value = UrnPropertiesReader.getVariable(KEEP_ALIVE_PING_MINUTES); + if (StringUtils.isBlank(value)) { + return GRPC_SERVER_DEFAULT_MIN_ALLOWED_PING_INTERVAL + 1L; + } + return Long.parseLong(Objects.requireNonNull(value)); + } } diff --git a/bpmn/so-bpmn-building-blocks/src/main/resources/subprocess/BuildingBlock/ControllerExecutionBB.bpmn b/bpmn/so-bpmn-building-blocks/src/main/resources/subprocess/BuildingBlock/ControllerExecutionBB.bpmn index 065d7e0c4b..e04d281c8b 100644 --- a/bpmn/so-bpmn-building-blocks/src/main/resources/subprocess/BuildingBlock/ControllerExecutionBB.bpmn +++ b/bpmn/so-bpmn-building-blocks/src/main/resources/subprocess/BuildingBlock/ControllerExecutionBB.bpmn @@ -1,5 +1,5 @@ - + SequenceFlow_0gmfit3 @@ -14,24 +14,19 @@ SequenceFlow_07tqu82 SequenceFlow_1mkhog2 - + SequenceFlow_0vzx2yr SequenceFlow_05qembo - - SequenceFlow_05qembo + + SequenceFlow_01kp408 SequenceFlow_07tqu82 - SequenceFlow_15gxql1 + SequenceFlow_1szkurj #{execution.getVariable("ControllerStatus").equals("Success")} - - SequenceFlow_15gxql1 - - - SequenceFlow_0gmfit3 SequenceFlow_1lspfyy @@ -79,8 +74,29 @@ #{execution.getVariable("ControllerStatus").equals("Success")} + + SequenceFlow_05qembo + SequenceFlow_01kp408 + + + + + + SequenceFlow_13ddk47 + + #{execution.getVariable("timeoutControllerMessage")} + + + + SequenceFlow_1szkurj + SequenceFlow_13ddk47 + + + + + @@ -93,9 +109,9 @@ - + - + @@ -134,13 +150,6 @@ - - - - - - - @@ -149,8 +158,8 @@ - - + + @@ -186,7 +195,7 @@ - + @@ -194,12 +203,6 @@ - - - - - - @@ -224,6 +227,31 @@ + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/bpmn/so-bpmn-tasks/src/main/java/org/onap/so/bpmn/infrastructure/decisionpoint/impl/buildingblock/ControllerExecutionBB.java b/bpmn/so-bpmn-tasks/src/main/java/org/onap/so/bpmn/infrastructure/decisionpoint/impl/buildingblock/ControllerExecutionBB.java index 92be824691..c5536106fe 100644 --- a/bpmn/so-bpmn-tasks/src/main/java/org/onap/so/bpmn/infrastructure/decisionpoint/impl/buildingblock/ControllerExecutionBB.java +++ b/bpmn/so-bpmn-tasks/src/main/java/org/onap/so/bpmn/infrastructure/decisionpoint/impl/buildingblock/ControllerExecutionBB.java @@ -26,11 +26,13 @@ import org.onap.so.bpmn.common.BuildingBlockExecution; import org.onap.so.bpmn.infrastructure.decisionpoint.api.ControllerContext; import org.onap.so.bpmn.infrastructure.decisionpoint.api.ControllerRunnable; import org.onap.so.bpmn.infrastructure.decisionpoint.impl.AbstractControllerExecution; +import org.onap.so.client.cds.PayloadConstants; import org.onap.so.db.catalog.beans.ControllerSelectionReference; import org.onap.so.db.catalog.beans.PnfResourceCustomization; import org.onap.so.db.catalog.beans.VnfResourceCustomization; import org.onap.so.db.catalog.client.CatalogDbClient; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Value; import org.springframework.stereotype.Component; /** @@ -52,8 +54,11 @@ public class ControllerExecutionBB extends AbstractControllerExecution controllerContext = buildControllerContext(execution); controllerExecute(controllerContext); } @@ -151,4 +156,18 @@ public class ControllerExecutionBB extends AbstractControllerExecution errMsgCaptor = ArgumentCaptor.forClass(String.class); + verify(exceptionBuilder, times(1)).buildAndThrowWorkflowException(any(BuildingBlockExecution.class), anyInt(), + errMsgCaptor.capture(), any()); + + assertTrue(errMsgCaptor.getValue().contains("timeout")); + } } diff --git a/bpmn/so-bpmn-tasks/src/test/java/org/onap/so/bpmn/infrastructure/decisionpoint/impl/camunda/controller/cds/PnfConfigCdsControllerDETest.java b/bpmn/so-bpmn-tasks/src/test/java/org/onap/so/bpmn/infrastructure/decisionpoint/impl/camunda/controller/cds/PnfConfigCdsControllerDETest.java index d8f607f6d9..3c3dc839c8 100644 --- a/bpmn/so-bpmn-tasks/src/test/java/org/onap/so/bpmn/infrastructure/decisionpoint/impl/camunda/controller/cds/PnfConfigCdsControllerDETest.java +++ b/bpmn/so-bpmn-tasks/src/test/java/org/onap/so/bpmn/infrastructure/decisionpoint/impl/camunda/controller/cds/PnfConfigCdsControllerDETest.java @@ -50,7 +50,7 @@ public class PnfConfigCdsControllerDETest { @MockBean private ControllerPreparable preparable; - @Mock + @MockBean private AbstractCDSProcessingBBUtils abstractCDSProcessingBBUtils; @Test diff --git a/common/src/main/java/org/onap/so/client/cds/CDSProcessingClient.java b/common/src/main/java/org/onap/so/client/cds/CDSProcessingClient.java index e40b936daa..6e27b85863 100644 --- a/common/src/main/java/org/onap/so/client/cds/CDSProcessingClient.java +++ b/common/src/main/java/org/onap/so/client/cds/CDSProcessingClient.java @@ -28,6 +28,7 @@ import java.security.KeyStore; import java.security.KeyStoreException; import java.security.NoSuchAlgorithmException; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; import javax.net.ssl.SSLException; import javax.net.ssl.TrustManagerFactory; import org.onap.ccsdk.cds.controllerblueprints.processing.api.ExecutionServiceInput; @@ -107,6 +108,7 @@ public class CDSProcessingClient implements AutoCloseable { log.info("Configure Basic authentication"); builder.intercept(new BasicAuthClientInterceptor(props)).usePlaintext(); } + builder.keepAliveTime(props.getKeepAlivePingMinutes(), TimeUnit.MINUTES); this.channel = builder.build(); this.handler = new CDSProcessingHandler(listener); log.info("CDSProcessingClient started"); diff --git a/common/src/main/java/org/onap/so/client/cds/CDSProperties.java b/common/src/main/java/org/onap/so/client/cds/CDSProperties.java index db566fa3de..f47a70976b 100644 --- a/common/src/main/java/org/onap/so/client/cds/CDSProperties.java +++ b/common/src/main/java/org/onap/so/client/cds/CDSProperties.java @@ -7,9 +7,9 @@ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. @@ -20,6 +20,7 @@ package org.onap.so.client.cds; +import java.util.concurrent.TimeUnit; import org.onap.so.client.RestProperties; public interface CDSProperties extends RestProperties { @@ -35,4 +36,16 @@ public interface CDSProperties extends RestProperties { boolean getUseSSL(); boolean getUseBasicAuth(); + + /** + * Gets grpc keep alive ping interval, which is useful for detecting connection issues when the server dies + * abruptly. If the value is set lower than what is allowed by the server (default 5 min), the connection will be + * closed after a few pings. + * + * If no value is set this method will default to 6 min (server default minimum + 1) + * + * @see io.grpc.netty.NettyChannelBuilder#keepAliveTime(long, TimeUnit) + * @return + */ + long getKeepAlivePingMinutes(); } diff --git a/common/src/test/java/org/onap/so/client/cds/TestCDSPropertiesImpl.java b/common/src/test/java/org/onap/so/client/cds/TestCDSPropertiesImpl.java index 41238e539e..76dc6ad4ea 100644 --- a/common/src/test/java/org/onap/so/client/cds/TestCDSPropertiesImpl.java +++ b/common/src/test/java/org/onap/so/client/cds/TestCDSPropertiesImpl.java @@ -82,4 +82,9 @@ public class TestCDSPropertiesImpl implements CDSProperties { public boolean getUseBasicAuth() { return true; } + + @Override + public long getKeepAlivePingMinutes() { + return 6L; + } } -- cgit 1.2.3-korg