From 90b3f1efcf0d4ed6dd4bc57de5e2c7d56b1d73d9 Mon Sep 17 00:00:00 2001 From: "beili.zhou" Date: Tue, 15 Aug 2017 14:39:58 -0400 Subject: [PATCH] [APPC-143] OAM restart operation with stop error Fix OAM restart operation does not report stop failure. And also remove unused LCM OperationStatus and related comments. Issue-Id: APPC-143 Change-Id: Ia2580717ef515856725e3e50c63b17404f123325 Signed-off-by: beili.zhou --- .../appc/listener/LCM/impl/ListenerImpl.java | 1 - .../appc/listener/LCM/model/InputBody.java | 16 ----- .../GenericProviderOperationRequestFormatter.java | 34 ----------- .../listener/LCM/operation/OperationStatus.java | 69 ---------------------- .../appc/oam/processor/BaseActionRunnable.java | 48 +++++++++++++-- .../appc/oam/processor/OamRestartProcessor.java | 11 ++-- .../appc/oam/processor/BaseActionRunnableTest.java | 47 ++++++++++++++- 7 files changed, 92 insertions(+), 134 deletions(-) delete mode 100644 appc-event-listener/appc-event-listener-bundle/src/main/java/org/openecomp/appc/listener/LCM/operation/OperationStatus.java diff --git a/appc-event-listener/appc-event-listener-bundle/src/main/java/org/openecomp/appc/listener/LCM/impl/ListenerImpl.java b/appc-event-listener/appc-event-listener-bundle/src/main/java/org/openecomp/appc/listener/LCM/impl/ListenerImpl.java index d0ba7fbf5..c9ac104f3 100644 --- a/appc-event-listener/appc-event-listener-bundle/src/main/java/org/openecomp/appc/listener/LCM/impl/ListenerImpl.java +++ b/appc-event-listener/appc-event-listener-bundle/src/main/java/org/openecomp/appc/listener/LCM/impl/ListenerImpl.java @@ -87,7 +87,6 @@ public class ListenerImpl extends AbstractListener { if (isValid(incoming)) { String requestIdWithSubId = getRequestIdWithSubId(incoming.getBody()); LOG.info("Acknowledging Message: " + requestIdWithSubId); -// dmaap.postStatus(incoming.toOutgoing(OperationStatus.PENDING)); } } for (DmaapIncomingMessage incoming : messages) { diff --git a/appc-event-listener/appc-event-listener-bundle/src/main/java/org/openecomp/appc/listener/LCM/model/InputBody.java b/appc-event-listener/appc-event-listener-bundle/src/main/java/org/openecomp/appc/listener/LCM/model/InputBody.java index 88e515b0d..9808daf56 100644 --- a/appc-event-listener/appc-event-listener-bundle/src/main/java/org/openecomp/appc/listener/LCM/model/InputBody.java +++ b/appc-event-listener/appc-event-listener-bundle/src/main/java/org/openecomp/appc/listener/LCM/model/InputBody.java @@ -24,14 +24,11 @@ package org.openecomp.appc.listener.LCM.model; -import org.openecomp.appc.listener.LCM.operation.OperationStatus; - import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.databind.annotation.JsonSerialize; - @JsonSerialize(include = JsonSerialize.Inclusion.NON_NULL) @JsonIgnoreProperties(ignoreUnknown = true) public class InputBody { @@ -85,19 +82,6 @@ public class InputBody { this.commonHeader = commonHeader; } -// public String toOutgoing(OperationStatus operationStatus) { -// OutputBody out = new OutputBody(this); -// out.setStatus(new ResponseStatus(operationStatus.getCode(), operationStatus.getValue())); -// return out.toResponse().toString(); -// } - -// public String toOutgoing(OperationStatus operationStatus,String islocked) { -// OutputBody out = new OutputBody(this); -// out.setStatus(new ResponseStatus(operationStatus.getCode(), operationStatus.getValue())); -// out.setLocked(islocked); -// return out.toResponse().toString(); -// } - @JsonIgnore public boolean isValid() { return getCommonHeader() != null; diff --git a/appc-event-listener/appc-event-listener-bundle/src/main/java/org/openecomp/appc/listener/LCM/operation/GenericProviderOperationRequestFormatter.java b/appc-event-listener/appc-event-listener-bundle/src/main/java/org/openecomp/appc/listener/LCM/operation/GenericProviderOperationRequestFormatter.java index e68d23bb6..dc8b5eab7 100644 --- a/appc-event-listener/appc-event-listener-bundle/src/main/java/org/openecomp/appc/listener/LCM/operation/GenericProviderOperationRequestFormatter.java +++ b/appc-event-listener/appc-event-listener-bundle/src/main/java/org/openecomp/appc/listener/LCM/operation/GenericProviderOperationRequestFormatter.java @@ -36,8 +36,6 @@ import com.att.eelf.configuration.EELFManager; import java.net.URL; - - public class GenericProviderOperationRequestFormatter implements ProviderOperationRequestFormatter { private final EELFLogger LOG = EELFManager.getInstance().getLogger(GenericProviderOperationRequestFormatter.class); @@ -52,44 +50,12 @@ public class GenericProviderOperationRequestFormatter implements ProviderOperati return url.getPath() + ":"+rpcName; } -// private String convertActionToUrl(String action) { -// String res = ""; -// switch (action) { -// case "SoftwareUpload": -// res = "software-upload"; -// break; -// case "LiveUpgrade": -// res = "live-upgrade"; -// break; -// case "HealthCheck": -// res = "health-check"; -// break; -// case "CheckLock": -// res = "check-lock"; -// break; -// default : -// res = action; -// } -// return res.toLowerCase(); -// } - @Override public String buildRequest(InputBody msg) { JSONObject jsonObject = Mapper.toJsonObject(msg); return String.format(TEMPLATE, jsonObject.toString()); } -/* @Override - public OperationStatus getOperationStatus(JSONObject responseBody) throws APPCException { - try { - JSONObject status = responseBody.getJSONObject("output").getJSONObject("status"); - return new OperationStatus(String.valueOf(status.getInt("code")), status.getString("message")); - } catch (Exception e) { - LOG.error("Unknown error processing failed response from provider. Json not in expected format", e); - throw new APPCException("APPC has an unknown RPC error"); - } - }*/ - @Override public ResponseStatus getResponseStatus(JsonNode responseBody)throws APPCException{ try { diff --git a/appc-event-listener/appc-event-listener-bundle/src/main/java/org/openecomp/appc/listener/LCM/operation/OperationStatus.java b/appc-event-listener/appc-event-listener-bundle/src/main/java/org/openecomp/appc/listener/LCM/operation/OperationStatus.java deleted file mode 100644 index e60c90916..000000000 --- a/appc-event-listener/appc-event-listener-bundle/src/main/java/org/openecomp/appc/listener/LCM/operation/OperationStatus.java +++ /dev/null @@ -1,69 +0,0 @@ -/*- - * ============LICENSE_START======================================================= - * ONAP : APPC - * ================================================================================ - * Copyright (C) 2017 AT&T Intellectual Property. All rights reserved. - * ================================================================================ - * Copyright (C) 2017 Amdocs - * ============================================================================= - * 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. - * See the License for the specific language governing permissions and - * limitations under the License. - * - * ECOMP is a trademark and service mark of AT&T Intellectual Property. - * ============LICENSE_END========================================================= - */ - -package org.openecomp.appc.listener.LCM.operation; - - -public class OperationStatus { - public final static OperationStatus PENDING = new OperationStatus("PENDING", "PENDING"); - public final static OperationStatus ACTIVE = new OperationStatus("ACTIVE", "ACTIVE"); - public final static OperationStatus SUCCESS = new OperationStatus("SUCCESS", "SUCCESS"); - public final static OperationStatus FAILURE = new OperationStatus("FAILURE", "FAILURE"); - - private String code; - private String value; - - public OperationStatus() { - } - - - public OperationStatus(String code, String value) { - this.code = code; - this.value = value; - } - - public String getCode() { - return code; - } - - public void setCode(String code) { - this.code = code; - } - - public String getValue() { - return value; - } - - public void setValue(String value) { - this.value = value; - } - - public boolean isSucceeded() { - if (code == null) { - return false; - } - int intCode = Integer.parseInt(code); - return (intCode >= 200) && (intCode < 300); // All 2xx statuses are success - } -} diff --git a/appc-oam/appc-oam-bundle/src/main/java/org/openecomp/appc/oam/processor/BaseActionRunnable.java b/appc-oam/appc-oam-bundle/src/main/java/org/openecomp/appc/oam/processor/BaseActionRunnable.java index 5df2c805e..2ffad6993 100644 --- a/appc-oam/appc-oam-bundle/src/main/java/org/openecomp/appc/oam/processor/BaseActionRunnable.java +++ b/appc-oam/appc-oam-bundle/src/main/java/org/openecomp/appc/oam/processor/BaseActionRunnable.java @@ -51,6 +51,8 @@ abstract class BaseActionRunnable extends BaseCommon implements Runnable { final String ABORT_MESSAGE_FORMAT = "Aborting %s operation."; /** Timeout message format with flexible operation name */ final String TIMEOUT_MESSAGE_FORMAT = "%s operation has reached timeout %d milliseconds."; + /** Failure message format with flexible number of bundles */ + final String BUNDLE_OPERATION_FAILED_FORMAT = "%d bundle(s) failed, see logs for details."; private boolean isWaiting = false; private AppcOamStates currentState; @@ -65,6 +67,11 @@ abstract class BaseActionRunnable extends BaseCommon implements Runnable { BaseProcessor myParent; Map> bundleNameToFuture = new HashMap<>(); + /** + * Constructor + * + * @param parent BaseProcessor who has called this constructor. + */ BaseActionRunnable(BaseProcessor parent) { super(parent.logger, parent.configurationHelper, parent.stateHelper, parent.operationHelper); @@ -76,6 +83,9 @@ abstract class BaseActionRunnable extends BaseCommon implements Runnable { setTimeoutValues(); } + /** + * Set timeout in milliseconds + */ void setTimeoutValues() { Integer timeoutSeconds = myParent.timeoutSeconds; if (timeoutSeconds == null) { @@ -120,6 +130,10 @@ abstract class BaseActionRunnable extends BaseCommon implements Runnable { } } + /** + * Keep waiting to be override by children classes for different behaviors. + * Timeout is validated here. + */ void keepWaiting() { logDebug(String.format("%s runnable waiting, current state is %s.", actionName, currentState == null ? "null" : currentState.toString())); @@ -127,6 +141,12 @@ abstract class BaseActionRunnable extends BaseCommon implements Runnable { isTimeout("keepWaiting"); } + /** + * Check if the timeout milliseconds has reached. + * + * @param parentName String of the caller, for logging purpose. + * @return true if the timeout has reached, otherwise false. + */ boolean isTimeout(String parentName) { logDebug(String.format("%s task isTimeout called from %s", actionName, parentName)); if (doTimeoutChecking @@ -141,6 +161,23 @@ abstract class BaseActionRunnable extends BaseCommon implements Runnable { return false; } + /** + * Check if all bundle operations are successful through BundleHelper. + * If there's failed bundler operation, set error status and trigger postAction with Error state. + * + * @return true if bundler operations have failure, otherwise false. + */ + boolean hasBundleOperationFailure() { + long failedTask = myParent.bundleHelper.getFailedMetrics(bundleNameToFuture); + if (failedTask == 0) { + return false; + } + + setStatus(OAMCommandStatus.UNEXPECTED_ERROR, String.format(BUNDLE_OPERATION_FAILED_FORMAT, failedTask)); + postAction(AppcOamStates.Error); + return true; + } + /** * Set class status to REJECTED with abort message. */ @@ -150,6 +187,7 @@ abstract class BaseActionRunnable extends BaseCommon implements Runnable { /** * Final handling. The thread is cancelled. + * * @param setState boolean to indicate if set OAM state or not */ void postDoAction(boolean setState) { @@ -157,7 +195,8 @@ abstract class BaseActionRunnable extends BaseCommon implements Runnable { } /** - * Handling for after doAction. does post notification, issue audit log and set OAM state based on input + * Handling for after doAction. does post notification, issue audit log and set OAM state based on input. + * * @param state of AppcOamState to be set as OAM state when it is not null. */ void postAction(AppcOamStates state) { @@ -174,6 +213,7 @@ abstract class BaseActionRunnable extends BaseCommon implements Runnable { /** * Check state + * * @return true if final state reached, otherwise return false */ boolean checkState() { @@ -186,11 +226,7 @@ abstract class BaseActionRunnable extends BaseCommon implements Runnable { return false; } - long failedTask = myParent.bundleHelper.getFailedMetrics(bundleNameToFuture); - if (failedTask != 0) { - String errorMsg = failedTask + " bundle(s) failed, see logs for details."; - setStatus(OAMCommandStatus.UNEXPECTED_ERROR, errorMsg); - postAction(AppcOamStates.Error); + if (hasBundleOperationFailure()) { return true; } diff --git a/appc-oam/appc-oam-bundle/src/main/java/org/openecomp/appc/oam/processor/OamRestartProcessor.java b/appc-oam/appc-oam-bundle/src/main/java/org/openecomp/appc/oam/processor/OamRestartProcessor.java index 529d25004..e9f0ada56 100644 --- a/appc-oam/appc-oam-bundle/src/main/java/org/openecomp/appc/oam/processor/OamRestartProcessor.java +++ b/appc-oam/appc-oam-bundle/src/main/java/org/openecomp/appc/oam/processor/OamRestartProcessor.java @@ -45,14 +45,14 @@ public class OamRestartProcessor extends BaseProcessor { *
-Stopped: check if all bundle state reached stopped *
-ToStart: call bundles start *
-Started: action is full completed - *
-Timeout: indication of timeout reached + *
-Error: indication of error, such as timeout reached, bundler operation failure and etc. */ private enum ActionPhases { ToStop, Stopped, ToStart, Started, - Timeout + Error } /** @@ -137,7 +137,7 @@ public class OamRestartProcessor extends BaseProcessor { AppcOam.RPC.start, bundleNameToFuture, myParent.asyncTaskHelper); currentPhase = ActionPhases.Started; break; - case Timeout: + case Error: // do nothing break; default: @@ -148,8 +148,9 @@ public class OamRestartProcessor extends BaseProcessor { return false; } - if (isTimeout("restart doAction")) { - currentPhase = ActionPhases.Timeout; + if (isTimeout("restart doAction") + || hasBundleOperationFailure()) { + currentPhase = ActionPhases.Error; return true; } if (isBundleOperationCompleted) { diff --git a/appc-oam/appc-oam-bundle/src/test/java/org/openecomp/appc/oam/processor/BaseActionRunnableTest.java b/appc-oam/appc-oam-bundle/src/test/java/org/openecomp/appc/oam/processor/BaseActionRunnableTest.java index 5fd51ef11..70adca0fc 100644 --- a/appc-oam/appc-oam-bundle/src/test/java/org/openecomp/appc/oam/processor/BaseActionRunnableTest.java +++ b/appc-oam/appc-oam-bundle/src/test/java/org/openecomp/appc/oam/processor/BaseActionRunnableTest.java @@ -36,6 +36,7 @@ import org.openecomp.appc.i18n.Msg; import org.openecomp.appc.oam.AppcOam; import org.openecomp.appc.oam.OAMCommandStatus; import org.openecomp.appc.oam.util.AsyncTaskHelper; +import org.openecomp.appc.oam.util.BundleHelper; import org.openecomp.appc.oam.util.ConfigurationHelper; import org.openecomp.appc.oam.util.OperationHelper; import org.openecomp.appc.oam.util.StateHelper; @@ -46,6 +47,7 @@ import java.util.Date; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyInt; +import static org.mockito.Matchers.anyMap; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; @@ -101,6 +103,7 @@ public class BaseActionRunnableTest { private OperationHelper mockOperHelper = mock(OperationHelper.class); private ConfigurationHelper mockConfigHelper = mock(ConfigurationHelper.class); private Configuration mockConfig = mock(Configuration.class); + private BundleHelper mockBundleHelper = mock(BundleHelper.class); @SuppressWarnings("ResultOfMethodCallIgnored") @Before @@ -113,8 +116,9 @@ public class BaseActionRunnableTest { testProcessor = spy( new TestProcessor(mockLogger, mockConfigHelper, mockStateHelper, null, mockOperHelper)); - testBaseAcionRunnable = spy(new TestAbc(testProcessor)); + Whitebox.setInternalState(testProcessor, "bundleHelper", mockBundleHelper); + testBaseAcionRunnable = spy(new TestAbc(testProcessor)); Whitebox.setInternalState(testBaseAcionRunnable, "commonHeader", mock(CommonHeader.class)); } @@ -163,13 +167,13 @@ public class BaseActionRunnableTest { Whitebox.setInternalState(testBaseAcionRunnable, "doActionResult", true); // with checkState return true - Mockito.doReturn(targetState).when(mockStateHelper).getBundlesState(); + Mockito.doReturn(true).when(testBaseAcionRunnable).checkState(); testBaseAcionRunnable.run(); Assert.assertFalse("isWaiting should still be false", Whitebox.getInternalState(testBaseAcionRunnable, "isWaiting")); // with checkState return false - Mockito.doReturn(AppcOamStates.Started).when(mockStateHelper).getBundlesState(); + Mockito.doReturn(false).when(testBaseAcionRunnable).checkState(); testBaseAcionRunnable.run(); Assert.assertTrue("isWaiting should still be true", Whitebox.getInternalState(testBaseAcionRunnable, "isWaiting")); @@ -191,6 +195,27 @@ public class BaseActionRunnableTest { @Test public void testCheckState() throws Exception { + // 1. with isTimeout true + Mockito.doReturn(true).when(testBaseAcionRunnable).isTimeout("checkState"); + Assert.assertTrue("Should return true", testBaseAcionRunnable.checkState()); + + // 2. with isTimeout false and + Mockito.doReturn(false).when(testBaseAcionRunnable).isTimeout("checkState"); + + // 2.1 with task not all done + Mockito.doReturn(false).when(mockBundleHelper).isAllTaskDone(any()); + Assert.assertFalse("Should return false", testBaseAcionRunnable.checkState()); + + // 2. 2 with task all done + Mockito.doReturn(true).when(mockBundleHelper).isAllTaskDone(any()); + + // 2.2.1 with has bundle failure + Mockito.doReturn(true).when(testBaseAcionRunnable).hasBundleOperationFailure(); + Assert.assertTrue("Should return true", testBaseAcionRunnable.checkState()); + + // 2.2.2 with no bundle failure + Mockito.doReturn(false).when(testBaseAcionRunnable).hasBundleOperationFailure(); + Mockito.doReturn(targetState).when(mockStateHelper).getBundlesState(); Assert.assertTrue("Should return true", testBaseAcionRunnable.checkState()); @@ -240,4 +265,20 @@ public class BaseActionRunnableTest { testBaseAcionRunnable.status.getMessage().endsWith( String.format(testBaseAcionRunnable.TIMEOUT_MESSAGE_FORMAT, testRpc.name(), timeoutMs))); } + + @SuppressWarnings("unchecked") + @Test + public void testHasBundleOperationFailure() throws Exception { + Mockito.when(mockBundleHelper.getFailedMetrics(anyMap())).thenReturn(Long.valueOf("0")); + Assert.assertFalse("should return false", testBaseAcionRunnable.hasBundleOperationFailure()); + + Mockito.when(mockStateHelper.getCurrentOamState()).thenReturn(AppcOamStates.Restarting); + long failedNumber = 1; + Mockito.doReturn(failedNumber).when(mockBundleHelper).getFailedMetrics(anyMap()); + Assert.assertTrue("should return true", testBaseAcionRunnable.hasBundleOperationFailure()); + Mockito.verify(testBaseAcionRunnable, + times(1)).setStatus(OAMCommandStatus.UNEXPECTED_ERROR, + String.format(testBaseAcionRunnable.BUNDLE_OPERATION_FAILED_FORMAT, failedNumber)); + Mockito.verify(testBaseAcionRunnable, times(1)).postAction(AppcOamStates.Error); + } } -- 2.16.6