Retry for yang module upgrade operation

- Added retry code for yang module upgrade failed

Issue-ID: CPS-1802

Signed-off-by: sourabh_sourabh <sourabh.sourabh@est.tech>
Change-Id: Ida68d45f926ed48b92f4b5a3b82e641d778303ce
Signed-off-by: sourabh_sourabh <sourabh.sourabh@est.tech>

11 files changed, 172 insertions, 106 deletions

diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServiceImpl.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServiceImpl.java
index db7b12cbc..f0336900f 100755
--- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServiceImpl.java
+++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServiceImpl.java
@@ -60,6 +60,7 @@ import org.onap.cps.ncmp.api.impl.inventory.CompositeStateBuilder;
 import org.onap.cps.ncmp.api.impl.inventory.CompositeStateUtils;
 import org.onap.cps.ncmp.api.impl.inventory.DataStoreSyncState;
 import org.onap.cps.ncmp.api.impl.inventory.InventoryPersistence;
+import org.onap.cps.ncmp.api.impl.inventory.sync.ModuleOperationsUtils;
 import org.onap.cps.ncmp.api.impl.operations.DmiDataOperations;
 import org.onap.cps.ncmp.api.impl.operations.OperationType;
 import org.onap.cps.ncmp.api.impl.trustlevel.TrustLevel;
@@ -409,7 +410,8 @@ public class NetworkCmProxyDataServiceImpl implements NetworkCmProxyDataService
         final NcmpServiceCmHandle ncmpServiceCmHandle = new NcmpServiceCmHandle();
         ncmpServiceCmHandle.setCmHandleId(cmHandleId);
         final String moduleSetTag = dmiPluginRegistration.getUpgradedCmHandles().getModuleSetTag();
-        final String lockReasonWithModuleSetTag = MessageFormat.format("ModuleSetTag: {0}", moduleSetTag);
+        final String lockReasonWithModuleSetTag = MessageFormat.format(
+                ModuleOperationsUtils.MODULE_SET_TAG_MESSAGE_FORMAT, moduleSetTag);
         ncmpServiceCmHandle.setCompositeState(new CompositeStateBuilder().withCmHandleState(CmHandleState.READY)
                 .withLockReason(MODULE_UPGRADE, lockReasonWithModuleSetTag).build());
         return YangModelCmHandle.toYangModelCmHandle(dmiPluginRegistration.getDmiPlugin(),
diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/inventory/sync/DataSyncWatchdog.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/inventory/sync/DataSyncWatchdog.java
index 49804adc1..6f089a57f 100644
--- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/inventory/sync/DataSyncWatchdog.java
+++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/inventory/sync/DataSyncWatchdog.java
@@ -48,7 +48,7 @@ public class DataSyncWatchdog {
 
     private final CpsDataService cpsDataService;
 
-    private final SyncUtils syncUtils;
+    private final ModuleOperationsUtils moduleOperationsUtils;
 
     private final IMap<String, Boolean> dataSyncSemaphores;
 
@@ -58,13 +58,13 @@ public class DataSyncWatchdog {
      */
     @Scheduled(fixedDelayString = "${ncmp.timers.cm-handle-data-sync.sleep-time-ms:30000}")
     public void executeUnSynchronizedReadyCmHandlePoll() {
-        syncUtils.getUnsynchronizedReadyCmHandles().forEach(unSynchronizedReadyCmHandle -> {
+        moduleOperationsUtils.getUnsynchronizedReadyCmHandles().forEach(unSynchronizedReadyCmHandle -> {
             final String cmHandleId = unSynchronizedReadyCmHandle.getId();
             if (hasPushedIntoSemaphoreMap(cmHandleId)) {
                 log.debug("Executing data sync on {}", cmHandleId);
                 final CompositeState compositeState = inventoryPersistence
                         .getCmHandleState(cmHandleId);
-                final String resourceData = syncUtils.getResourceData(cmHandleId);
+                final String resourceData = moduleOperationsUtils.getResourceData(cmHandleId);
                 if (resourceData == null) {
                     log.debug("Error retrieving resource data for Cm-Handle: {}", cmHandleId);
                 } else {
diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/inventory/sync/SyncUtils.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/inventory/sync/ModuleOperationsUtils.java
index ab85c2127..25ea3bc1e 100644
--- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/inventory/sync/SyncUtils.java
+++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/inventory/sync/ModuleOperationsUtils.java
@@ -29,6 +29,7 @@ import java.time.OffsetDateTime;
 import java.time.format.DateTimeFormatter;
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
@@ -54,12 +55,18 @@ import org.springframework.stereotype.Service;
 @Slf4j
 @Service
 @RequiredArgsConstructor
-public class SyncUtils {
+public class ModuleOperationsUtils {
 
     private final CmHandleQueries cmHandleQueries;
     private final DmiDataOperations dmiDataOperations;
     private final JsonObjectMapper jsonObjectMapper;
-    private static final Pattern retryAttemptPattern = Pattern.compile("^Attempt #(\\d+) failed:");
+    private static final String RETRY_ATTEMPT_KEY = "attempt";
+    public static final String MODULE_SET_TAG_KEY = "moduleSetTag";
+    public static final String MODULE_SET_TAG_MESSAGE_FORMAT = "Upgrade to ModuleSetTag: {0}";
+    private static final String UPGRADE_FORMAT = "Upgrade to ModuleSetTag: %s";
+    private static final String UPGRADE_FAILED_FORMAT = UPGRADE_FORMAT + " Attempt #%d failed: %s";
+    private static final Pattern retryAttemptPattern = Pattern.compile("Attempt #(\\d+) failed:.+");
+    private static final Pattern moduleSetTagPattern = Pattern.compile("Upgrade to ModuleSetTag: (\\S+)");
 
     /**
      * Query data nodes for cm handles with an "ADVISED" cm handle state.
@@ -87,14 +94,11 @@ public class SyncUtils {
         for (final DataNode unsynchronizedCmHandle : unsynchronizedCmHandles) {
             final String cmHandleId = unsynchronizedCmHandle.getLeaves().get("id").toString();
             if (cmHandleQueries.cmHandleHasState(cmHandleId, CmHandleState.READY)) {
-                yangModelCmHandles.addAll(
-                        convertCmHandlesDataNodesToYangModelCmHandles(
+                yangModelCmHandles.addAll(convertCmHandlesDataNodesToYangModelCmHandles(
                                 Collections.singletonList(unsynchronizedCmHandle)));
             }
         }
-
         Collections.shuffle(yangModelCmHandles);
-
         return yangModelCmHandles;
     }
 
@@ -121,17 +125,43 @@ public class SyncUtils {
                                                    final LockReasonCategory lockReasonCategory,
                                                    final String errorMessage) {
         int attempt = 1;
-        if (compositeState.getLockReason() != null) {
-            final Matcher matcher = retryAttemptPattern.matcher(compositeState.getLockReason().getDetails());
-            if (matcher.find()) {
-                attempt = 1 + Integer.parseInt(matcher.group(1));
-            }
+        final Map<String, String> compositeStateDetails
+                = getLockedCompositeStateDetails(compositeState.getLockReason());
+        if (!compositeStateDetails.isEmpty()) {
+            attempt = 1 + Integer.parseInt(compositeStateDetails.get(RETRY_ATTEMPT_KEY));
         }
         compositeState.setLockReason(CompositeState.LockReason.builder()
-                .details(String.format("Attempt #%d failed: %s", attempt, errorMessage))
+                .details(String.format(UPGRADE_FAILED_FORMAT,
+                        compositeStateDetails.get(MODULE_SET_TAG_KEY), attempt, errorMessage))
                 .lockReasonCategory(lockReasonCategory).build());
     }
 
+    /**
+     * Extract lock reason details as key-value pair.
+     *
+     * @param compositeStateLockReason lock reason having all the details
+     * @return a map of lock reason details
+     */
+    public static Map<String, String> getLockedCompositeStateDetails(final CompositeState.LockReason
+                                                                             compositeStateLockReason) {
+        if (compositeStateLockReason != null) {
+            final Map<String, String> compositeStateDetails = new HashMap<>(2);
+            final String lockedCompositeStateReasonDetails = compositeStateLockReason.getDetails();
+            final Matcher retryAttemptMatcher = retryAttemptPattern.matcher(lockedCompositeStateReasonDetails);
+            if (retryAttemptMatcher.find()) {
+                final int attemptsRegexGroupId = 1;
+                compositeStateDetails.put(RETRY_ATTEMPT_KEY, retryAttemptMatcher.group(attemptsRegexGroupId));
+            }
+            final Matcher moduleSetTagMatcher = moduleSetTagPattern.matcher(lockedCompositeStateReasonDetails);
+            if (moduleSetTagMatcher.find()) {
+                final int moduleSetTagRegexGroupId = 1;
+                compositeStateDetails.put(MODULE_SET_TAG_KEY, moduleSetTagMatcher.group(moduleSetTagRegexGroupId));
+            }
+            return compositeStateDetails;
+        }
+        return Collections.emptyMap();
+    }
+
 
     /**
      * Check if a module sync retry is needed.
@@ -146,29 +176,12 @@ public class SyncUtils {
 
         final boolean failedDuringModuleSync = LockReasonCategory.MODULE_SYNC_FAILED
                 == lockReason.getLockReasonCategory();
-        final boolean moduleUpgrade = LockReasonCategory.MODULE_UPGRADE
+        final boolean failedDuringModuleUpgrade = LockReasonCategory.MODULE_UPGRADE_FAILED
                 == lockReason.getLockReasonCategory();
 
-        if (failedDuringModuleSync) {
-            final int timeInMinutesUntilNextAttempt;
-            final Matcher matcher = retryAttemptPattern.matcher(lockReason.getDetails());
-            if (matcher.find()) {
-                timeInMinutesUntilNextAttempt = (int) Math.pow(2, Integer.parseInt(matcher.group(1)));
-            } else {
-                timeInMinutesUntilNextAttempt = 1;
-                log.info("First Attempt: no current attempts found.");
-            }
-            final int timeSinceLastAttempt = (int) Duration.between(time, OffsetDateTime.now()).toMinutes();
-            if (timeInMinutesUntilNextAttempt >= timeSinceLastAttempt) {
-                log.info("Time until next attempt is {} minutes: ",
-                        timeInMinutesUntilNextAttempt - timeSinceLastAttempt);
-                return false;
-            }
-            log.info("Retry due now");
-            return true;
-        } else if (moduleUpgrade) {
-            log.info("Locked for module upgrade.");
-            return true;
+        if (failedDuringModuleSync || failedDuringModuleUpgrade) {
+            log.info("Locked for module {}.", failedDuringModuleSync ? "sync" : "upgrade");
+            return isRetryDue(lockReason, time);
         }
         log.info("Locked for other reason");
         return false;
@@ -191,6 +204,19 @@ public class SyncUtils {
         return null;
     }
 
+    /**
+     * Checks if cm handle state module is in upgrade or upgrade failed.
+     *
+     * @param compositeState current lock reason of  cm handle
+     * @return true or false based on lock reason category
+     */
+    public static boolean isInUpgradeOrUpgradeFailed(final CompositeState compositeState) {
+        return compositeState.getLockReason() != null
+                && (LockReasonCategory.MODULE_UPGRADE.equals(compositeState.getLockReason().getLockReasonCategory())
+                || LockReasonCategory.MODULE_UPGRADE_FAILED.equals(compositeState.getLockReason()
+                .getLockReasonCategory()));
+    }
+
     private String getFirstResource(final Object responseBody) {
         final String jsonObjectAsString = jsonObjectMapper.asJsonString(responseBody);
         final JsonNode overallJsonNode = jsonObjectMapper.convertToJsonNode(jsonObjectAsString);
@@ -199,10 +225,29 @@ public class SyncUtils {
         return jsonObjectMapper.asJsonString(Map.of(firstElement.getKey(), firstElement.getValue()));
     }
 
-    private static List<YangModelCmHandle> convertCmHandlesDataNodesToYangModelCmHandles(
+    private List<YangModelCmHandle> convertCmHandlesDataNodesToYangModelCmHandles(
             final List<DataNode> cmHandlesAsDataNodeList) {
         return cmHandlesAsDataNodeList.stream()
                 .map(cmHandle -> YangDataConverter.convertCmHandleToYangModel(cmHandle,
                         cmHandle.getLeaves().get("id").toString())).toList();
     }
+
+    private boolean isRetryDue(final CompositeState.LockReason compositeStateLockReason, final OffsetDateTime time) {
+        final int timeInMinutesUntilNextAttempt;
+        final Map<String, String> compositeStateDetails = getLockedCompositeStateDetails(compositeStateLockReason);
+        if (compositeStateDetails.isEmpty()) {
+            timeInMinutesUntilNextAttempt = 1;
+            log.info("First Attempt: no current attempts found.");
+        } else {
+            timeInMinutesUntilNextAttempt = (int) Math.pow(2, Integer.parseInt(compositeStateDetails
+                    .get(RETRY_ATTEMPT_KEY)));
+        }
+        final int timeSinceLastAttempt = (int) Duration.between(time, OffsetDateTime.now()).toMinutes();
+        if (timeInMinutesUntilNextAttempt >= timeSinceLastAttempt) {
+            log.info("Time until next attempt is {} minutes: ", timeInMinutesUntilNextAttempt - timeSinceLastAttempt);
+            return false;
+        }
+        log.info("Retry due now");
+        return true;
+    }
 }
diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/inventory/sync/ModuleSyncService.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/inventory/sync/ModuleSyncService.java
index d191a5467..841368c0d 100644
--- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/inventory/sync/ModuleSyncService.java
+++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/inventory/sync/ModuleSyncService.java
@@ -41,7 +41,6 @@ import org.onap.cps.api.CpsModuleService;
 import org.onap.cps.ncmp.api.impl.inventory.CmHandleQueries;
 import org.onap.cps.ncmp.api.impl.inventory.CmHandleState;
 import org.onap.cps.ncmp.api.impl.inventory.CompositeState;
-import org.onap.cps.ncmp.api.impl.inventory.LockReasonCategory;
 import org.onap.cps.ncmp.api.impl.operations.DmiModelOperations;
 import org.onap.cps.ncmp.api.impl.utils.YangDataConverter;
 import org.onap.cps.ncmp.api.impl.yangmodels.YangModelCmHandle;
@@ -77,10 +76,11 @@ public class ModuleSyncService {
         final String moduleSetTag;
         final String cmHandleId = yangModelCmHandle.getId();
         final CompositeState compositeState = yangModelCmHandle.getCompositeState();
-        final boolean inUpgrade = isInUpgrade(compositeState);
+        final boolean inUpgrade = ModuleOperationsUtils.isInUpgradeOrUpgradeFailed(compositeState);
 
         if (inUpgrade) {
-            moduleSetTag = extractModuleSetTag(compositeState);
+            moduleSetTag = ModuleOperationsUtils.getLockedCompositeStateDetails(compositeState.getLockReason())
+                    .get(ModuleOperationsUtils.MODULE_SET_TAG_KEY);
         } else {
             moduleSetTag = yangModelCmHandle.getModuleSetTag();
         }
@@ -174,12 +174,4 @@ public class ModuleSyncService {
         moduleSetTagCache.put(moduleSetTag, moduleReferencesFromExistingCmHandle);
     }
 
-    private static String extractModuleSetTag(final CompositeState compositeState) {
-        return compositeState.getLockReason().getDetails().split(":")[1].trim();
-    }
-
-    private static boolean isInUpgrade(final CompositeState compositeState) {
-        return compositeState.getLockReason() != null && LockReasonCategory.MODULE_UPGRADE.equals(
-                compositeState.getLockReason().getLockReasonCategory());
-    }
 }
diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/inventory/sync/ModuleSyncTasks.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/inventory/sync/ModuleSyncTasks.java
index facaf155f..896316a49 100644
--- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/inventory/sync/ModuleSyncTasks.java
+++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/inventory/sync/ModuleSyncTasks.java
@@ -44,7 +44,7 @@ import org.springframework.stereotype.Component;
 @Slf4j
 public class ModuleSyncTasks {
     private final InventoryPersistence inventoryPersistence;
-    private final SyncUtils syncUtils;
+    private final ModuleOperationsUtils moduleOperationsUtils;
     private final ModuleSyncService moduleSyncService;
     private final LcmEventsCmHandleStateHandler lcmEventsCmHandleStateHandler;
     private final IMap<String, Object> moduleSyncStartedOnCmHandles;
@@ -73,9 +73,14 @@ public class ModuleSyncTasks {
                     yangModelCmHandle.getCompositeState().setLockReason(null);
                     cmHandelStatePerCmHandle.put(yangModelCmHandle, CmHandleState.READY);
                 } catch (final Exception e) {
-                    log.warn("Processing of {} module sync failed due to reason {}.", cmHandleId, e.getMessage());
-                    syncUtils.updateLockReasonDetailsAndAttempts(compositeState, LockReasonCategory.MODULE_SYNC_FAILED,
-                            e.getMessage());
+                    log.warn("Processing of {} module failed due to reason {}.", cmHandleId, e.getMessage());
+                    if (ModuleOperationsUtils.isInUpgradeOrUpgradeFailed(compositeState)) {
+                        moduleOperationsUtils.updateLockReasonDetailsAndAttempts(compositeState,
+                                LockReasonCategory.MODULE_UPGRADE_FAILED, e.getMessage());
+                    } else {
+                        moduleOperationsUtils.updateLockReasonDetailsAndAttempts(compositeState,
+                                LockReasonCategory.MODULE_SYNC_FAILED, e.getMessage());
+                    }
                     setCmHandleStateLocked(yangModelCmHandle, compositeState.getLockReason());
                     cmHandelStatePerCmHandle.put(yangModelCmHandle, CmHandleState.LOCKED);
                 }
@@ -98,7 +103,7 @@ public class ModuleSyncTasks {
         final Map<YangModelCmHandle, CmHandleState> cmHandleStatePerCmHandle = new HashMap<>(failedCmHandles.size());
         for (final YangModelCmHandle failedCmHandle : failedCmHandles) {
             final CompositeState compositeState = failedCmHandle.getCompositeState();
-            final boolean isReadyForRetry = syncUtils.needsModuleSyncRetryOrUpgrade(compositeState);
+            final boolean isReadyForRetry = moduleOperationsUtils.needsModuleSyncRetryOrUpgrade(compositeState);
             log.info("Retry for cmHandleId : {} is {}", failedCmHandle.getId(), isReadyForRetry);
             if (isReadyForRetry) {
                 final String resetCmHandleId = failedCmHandle.getId();
diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/inventory/sync/ModuleSyncWatchdog.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/inventory/sync/ModuleSyncWatchdog.java
index bf005051f..249232d23 100644
--- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/inventory/sync/ModuleSyncWatchdog.java
+++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/inventory/sync/ModuleSyncWatchdog.java
@@ -43,7 +43,7 @@ import org.springframework.stereotype.Service;
 @Service
 public class ModuleSyncWatchdog {
 
-    private final SyncUtils syncUtils;
+    private final ModuleOperationsUtils moduleOperationsUtils;
     private final BlockingQueue<DataNode> moduleSyncWorkQueue;
     private final IMap<String, Object> moduleSyncStartedOnCmHandles;
     private final ModuleSyncTasks moduleSyncTasks;
@@ -88,7 +88,8 @@ public class ModuleSyncWatchdog {
     @Scheduled(fixedDelayString = "${ncmp.timers.locked-modules-sync.sleep-time-ms:300000}")
     public void resetPreviouslyFailedCmHandles() {
         log.info("Processing module sync retry-watchdog waking up.");
-        final List<YangModelCmHandle> failedCmHandles = syncUtils.getCmHandlesThatFailedModelSyncOrUpgrade();
+        final List<YangModelCmHandle> failedCmHandles
+                = moduleOperationsUtils.getCmHandlesThatFailedModelSyncOrUpgrade();
         log.info("Retrying {} cmHandles", failedCmHandles.size());
         moduleSyncTasks.resetFailedCmHandles(failedCmHandles);
     }
@@ -104,7 +105,7 @@ public class ModuleSyncWatchdog {
 
     private void populateWorkQueueIfNeeded() {
         if (moduleSyncWorkQueue.isEmpty()) {
-            final List<DataNode> advisedCmHandles = syncUtils.getAdvisedCmHandles();
+            final List<DataNode> advisedCmHandles = moduleOperationsUtils.getAdvisedCmHandles();
             log.info("Processing module sync fetched {} advised cm handles from DB", advisedCmHandles.size());
             for (final DataNode advisedCmHandle : advisedCmHandles) {
                 if (!moduleSyncWorkQueue.offer(advisedCmHandle)) {
diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/inventory/sync/DataSyncWatchdogSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/inventory/sync/DataSyncWatchdogSpec.groovy
index 65dfc0579..0ffb56791 100644
--- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/inventory/sync/DataSyncWatchdogSpec.groovy
+++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/inventory/sync/DataSyncWatchdogSpec.groovy
@@ -24,8 +24,6 @@ import static org.onap.cps.ncmp.api.impl.ncmppersistence.NcmpPersistence.NFP_OPE
 
 import com.hazelcast.map.IMap
 import org.onap.cps.api.CpsDataService
-import org.onap.cps.ncmp.api.impl.inventory.sync.DataSyncWatchdog
-import org.onap.cps.ncmp.api.impl.inventory.sync.SyncUtils
 import org.onap.cps.ncmp.api.impl.yangmodels.YangModelCmHandle
 import org.onap.cps.ncmp.api.impl.inventory.CmHandleState
 import org.onap.cps.ncmp.api.impl.inventory.CompositeState
@@ -39,7 +37,7 @@ class DataSyncWatchdogSpec extends Specification {
 
     def mockCpsDataService = Mock(CpsDataService)
 
-    def mockSyncUtils = Mock(SyncUtils)
+    def mockSyncUtils = Mock(ModuleOperationsUtils)
 
     def mockDataSyncSemaphores = Mock(IMap<String,Boolean>)
 
diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/inventory/sync/SyncUtilsSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/inventory/sync/ModuleOperationsUtilsSpec.groovy
index 025d9482d..099fc5ac1 100644
--- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/inventory/sync/SyncUtilsSpec.groovy
+++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/inventory/sync/ModuleOperationsUtilsSpec.groovy
@@ -22,15 +22,14 @@
 package org.onap.cps.ncmp.api.impl.inventory.sync
 
 import static org.onap.cps.ncmp.api.impl.inventory.LockReasonCategory.LOCKED_MISBEHAVING
+import static org.onap.cps.ncmp.api.impl.inventory.LockReasonCategory.MODULE_UPGRADE
 import static org.onap.cps.ncmp.api.impl.operations.DatastoreType.PASSTHROUGH_OPERATIONAL
 import static org.onap.cps.ncmp.api.impl.inventory.LockReasonCategory.MODULE_SYNC_FAILED
-import static org.onap.cps.ncmp.api.impl.inventory.LockReasonCategory.MODULE_UPGRADE
 import static org.onap.cps.ncmp.api.impl.inventory.LockReasonCategory.MODULE_UPGRADE_FAILED
 
 import ch.qos.logback.classic.Level
 import ch.qos.logback.classic.Logger
 import ch.qos.logback.core.read.ListAppender
-import org.onap.cps.ncmp.api.impl.inventory.sync.SyncUtils
 import org.slf4j.LoggerFactory
 import org.springframework.context.annotation.AnnotationConfigApplicationContext
 import com.fasterxml.jackson.databind.JsonNode
@@ -51,7 +50,7 @@ import java.time.OffsetDateTime
 import java.time.format.DateTimeFormatter
 import java.util.stream.Collectors
 
-class SyncUtilsSpec extends Specification{
+class ModuleOperationsUtilsSpec extends Specification{
 
     def mockCmHandleQueries = Mock(CmHandleQueries)
 
@@ -59,7 +58,7 @@ class SyncUtilsSpec extends Specification{
 
     def jsonObjectMapper = new JsonObjectMapper(new ObjectMapper())
 
-    def objectUnderTest = new SyncUtils(mockCmHandleQueries, mockDmiDataOperations, jsonObjectMapper)
+    def objectUnderTest = new ModuleOperationsUtils(mockCmHandleQueries, mockDmiDataOperations, jsonObjectMapper)
 
     def static neverUpdatedBefore = '1900-01-01T00:00:00.000+0100'
 
@@ -71,7 +70,7 @@ class SyncUtilsSpec extends Specification{
 
     def applicationContext = new AnnotationConfigApplicationContext()
 
-    def logger = (Logger) LoggerFactory.getLogger(SyncUtils)
+    def logger = (Logger) LoggerFactory.getLogger(ModuleOperationsUtils)
     def loggingListAppender
 
     void setup() {
@@ -83,7 +82,7 @@ class SyncUtilsSpec extends Specification{
     }
 
     void cleanup() {
-        ((Logger) LoggerFactory.getLogger(SyncUtils.class)).detachAndStopAllAppenders()
+        ((Logger) LoggerFactory.getLogger(ModuleOperationsUtils.class)).detachAndStopAllAppenders()
         applicationContext.close()
     }
 
@@ -107,7 +106,7 @@ class SyncUtilsSpec extends Specification{
             objectUnderTest.updateLockReasonDetailsAndAttempts(compositeState, MODULE_SYNC_FAILED, 'new error message')
         then: 'the composite state lock reason and details are updated'
             assert compositeState.lockReason.lockReasonCategory == MODULE_SYNC_FAILED
-            assert compositeState.lockReason.details == expectedDetails
+            assert compositeState.lockReason.details.contains(expectedDetails)
         where:
             scenario         | lockReason                                                                                   || expectedDetails
             'does not exist' | null                                                                                         || 'Attempt #1 failed: new error message'
@@ -143,28 +142,28 @@ class SyncUtilsSpec extends Specification{
             def logs = loggingListAppender.list.toString()
             assert logs.contains(logReason)
         where: 'the following parameters are used'
-            scenario                                    | lastUpdateMinutesAgo | lockDetails          | logReason                               || retryExpected
-            'never attempted before'                    | -1                   | 'fist attempt:'      | 'First Attempt:'                        || true
-            '1st attempt, last attempt > 2 minute ago'  | 3                    | 'Attempt #1 failed:' | 'Retry due now'                         || true
-            '2nd attempt, last attempt < 4 minutes ago' | 1                    | 'Attempt #2 failed:' | 'Time until next attempt is 3 minutes:' || false
-            '2nd attempt, last attempt > 4 minutes ago' | 5                    | 'Attempt #2 failed:' | 'Retry due now'                         || true
+            scenario                                    | lastUpdateMinutesAgo | lockDetails                     | logReason                               || retryExpected
+            'never attempted before'                    | -1                   | 'Fist attempt:'                 | 'First Attempt:'                        || true
+            '1st attempt, last attempt > 2 minute ago'  | 3                    | 'Attempt #1 failed: some error' | 'Retry due now'                         || true
+            '2nd attempt, last attempt < 4 minutes ago' | 1                    | 'Attempt #2 failed: some error' | 'Time until next attempt is 3 minutes:' || false
+            '2nd attempt, last attempt > 4 minutes ago' | 5                    | 'Attempt #2 failed: some error' | 'Retry due now'                         || true
     }
 
-    def 'Retry Locked Cm-Handle with other lock reasons (category) #lockReasonCategory'() {
+    def 'Retry Locked Cm-Handle with lock reasons (category) #lockReasonCategory'() {
         when: 'checking to see if cm handle is ready for retry'
-        def result = objectUnderTest.needsModuleSyncRetryOrUpgrade(new CompositeStateBuilder()
+            def result = objectUnderTest.needsModuleSyncRetryOrUpgrade(new CompositeStateBuilder()
                 .withLockReason(lockReasonCategory, 'some details')
                 .withLastUpdatedTime(nowAsString).build())
         then: 'verify retry attempts'
-        assert result == retryAttempt
+            assert !result
         and: 'logs contain related information'
-        def logs = loggingListAppender.list.toString()
-        assert logs.contains(logReason)
+            def logs = loggingListAppender.list.toString()
+            assert logs.contains(logReason)
         where: 'the following lock reasons occurred'
-        scenario             | lockReasonCategory || logReason                    | retryAttempt
-        'module upgrade'     | MODULE_UPGRADE     || 'Locked for module upgrade.' | true
-        'module sync failed' | MODULE_SYNC_FAILED || 'First Attempt:'             | false
-        'lock misbehaving'   | LOCKED_MISBEHAVING || 'Locked for other reason'    | false
+            scenario             | lockReasonCategory    || logReason
+            'module upgrade'     | MODULE_UPGRADE_FAILED || 'First Attempt:'
+            'module sync failed' | MODULE_SYNC_FAILED    || 'First Attempt:'
+            'lock misbehaving'   | LOCKED_MISBEHAVING    || 'Locked for other reason'
     }
 
     def 'Get a Cm-Handle where #scenario'() {
@@ -195,4 +194,18 @@ class SyncUtilsSpec extends Specification{
         then: 'the returned data is correct'
             result == jsonString
     }
+
+    def 'Extract module set tag and number of attempt when lock reason contains #scenario'() {
+        expect: 'lock reason details are extracted correctly'
+            def result = objectUnderTest.getLockedCompositeStateDetails(new CompositeStateBuilder().withLockReason(MODULE_UPGRADE, lockReasonDetails).build().lockReason)
+        and: 'the result contains the correct moduleSetTag'
+            assert result['moduleSetTag'] == expectedModuleSetTag
+        and: 'the result contains the correct number of attempts'
+            assert result['attempt'] == expectedNumberOfAttempts
+        where: 'the following scenarios are used'
+            scenario                                     | lockReasonDetails                                                           || expectedModuleSetTag | expectedNumberOfAttempts
+            'module set tag only'                        | 'Upgrade to ModuleSetTag: targetModuleSetTag'                               || 'targetModuleSetTag' | null
+            'number of attempts only'                    | 'Attempt #1 failed: some error'                                             || null                 | '1'
+            'number of attempts and module set tag both' | 'Upgrade to ModuleSetTag: targetModuleSetTag Attempt #1 failed: some error' || 'targetModuleSetTag' | '1'
+    }
 }
diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/inventory/sync/ModuleSyncServiceSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/inventory/sync/ModuleSyncServiceSpec.groovy
index 18b87af9a..5384f31cc 100644
--- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/inventory/sync/ModuleSyncServiceSpec.groovy
+++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/inventory/sync/ModuleSyncServiceSpec.groovy
@@ -20,7 +20,6 @@
 
 package org.onap.cps.ncmp.api.impl.inventory.sync
 
-import static org.onap.cps.ncmp.api.impl.inventory.LockReasonCategory.LOCKED_MISBEHAVING
 import static org.onap.cps.ncmp.api.impl.ncmppersistence.NcmpPersistence.NFP_OPERATIONAL_DATASTORE_DATASPACE_NAME
 import static org.onap.cps.ncmp.api.impl.inventory.LockReasonCategory.MODULE_UPGRADE
 
@@ -31,7 +30,6 @@ import org.onap.cps.api.CpsAdminService
 import org.onap.cps.api.CpsDataService
 import org.onap.cps.api.CpsModuleService
 import org.onap.cps.spi.model.DataNodeBuilder
-import org.onap.cps.ncmp.api.impl.inventory.sync.ModuleSyncService
 import org.onap.cps.ncmp.api.impl.operations.DmiModelOperations
 import org.onap.cps.ncmp.api.impl.yangmodels.YangModelCmHandle
 import org.onap.cps.ncmp.api.impl.inventory.CmHandleQueries
@@ -90,7 +88,7 @@ class ModuleSyncServiceSpec extends Specification {
     def 'Upgrade model for an existing cm handle with Module Set Tag where the modules are #scenario'() {
         given: 'a cm handle being upgraded to module set tag: tag-1'
             def ncmpServiceCmHandle = new NcmpServiceCmHandle()
-            ncmpServiceCmHandle.setCompositeState(new CompositeStateBuilder().withLockReason(MODULE_UPGRADE, 'new moduleSetTag: tag-1').build())
+            ncmpServiceCmHandle.setCompositeState(new CompositeStateBuilder().withLockReason(MODULE_UPGRADE, 'Upgrade to ModuleSetTag: tag-1').build())
             def dmiServiceName = 'some service name'
             ncmpServiceCmHandle.cmHandleId = 'upgraded-ch'
             def yangModelCmHandle = YangModelCmHandle.toYangModelCmHandle(dmiServiceName, '', '', ncmpServiceCmHandle,'tag-1')
@@ -98,7 +96,7 @@ class ModuleSyncServiceSpec extends Specification {
             def moduleReferences =  [ new ModuleReference('module1','1') ]
         and: 'cache or DMI operations returns some module references for upgraded cm handle'
             if (populateCache) {
-                mockModuleSetTagCache.put('tag-1',moduleReferences)
+                mockModuleSetTagCache.put('tag-1', moduleReferences)
             } else {
                 mockDmiModelOperations.getModuleReferences(yangModelCmHandle) >> moduleReferences
             }
@@ -127,7 +125,7 @@ class ModuleSyncServiceSpec extends Specification {
         given: 'a cm handle that is ready but locked for upgrade'
             def ncmpServiceCmHandle = new NcmpServiceCmHandle()
             ncmpServiceCmHandle.setCompositeState(new CompositeStateBuilder()
-                .withLockReason(MODULE_UPGRADE, 'new moduleSetTag: targetModuleSetTag').build())
+                .withLockReason(MODULE_UPGRADE, 'Upgrade to ModuleSetTag: targetModuleSetTag').build())
             ncmpServiceCmHandle.setCmHandleId('cmHandleId-1')
             def yangModelCmHandle = YangModelCmHandle.toYangModelCmHandle('some service name', '', '', ncmpServiceCmHandle, 'targetModuleSetTag')
             mockCmHandleQueries.cmHandleHasState('cmHandleId-1', CmHandleState.READY) >> true
@@ -170,11 +168,6 @@ class ModuleSyncServiceSpec extends Specification {
             result == unsupportedOperationException
     }
 
-    def 'Extract module set tag'() {
-        expect: 'the module set tag is extracted correctly'
-            assert 'targetModuleSetTag' == objectUnderTest.extractModuleSetTag(new CompositeStateBuilder().withLockReason(MODULE_UPGRADE, 'new moduleSetTag: targetModuleSetTag').build())
-    }
-
     def toModuleReferences(moduleReferenceAsMap) {
         def moduleReferences = [].withDefault { [:] }
         moduleReferenceAsMap.forEach(property ->
diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/inventory/sync/ModuleSyncTasksSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/inventory/sync/ModuleSyncTasksSpec.groovy
index 0d927551d..3bdac1855 100644
--- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/inventory/sync/ModuleSyncTasksSpec.groovy
+++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/inventory/sync/ModuleSyncTasksSpec.groovy
@@ -21,6 +21,10 @@
 
 package org.onap.cps.ncmp.api.impl.inventory.sync
 
+
+import static org.onap.cps.ncmp.api.impl.inventory.LockReasonCategory.MODULE_SYNC_FAILED
+import static org.onap.cps.ncmp.api.impl.inventory.LockReasonCategory.MODULE_UPGRADE_FAILED
+
 import ch.qos.logback.classic.Level
 import ch.qos.logback.classic.Logger
 import ch.qos.logback.classic.spi.ILoggingEvent
@@ -31,9 +35,6 @@ import com.hazelcast.map.IMap
 import org.junit.jupiter.api.AfterEach
 import org.junit.jupiter.api.BeforeEach
 import org.onap.cps.ncmp.api.impl.events.lcm.LcmEventsCmHandleStateHandler
-import org.onap.cps.ncmp.api.impl.inventory.sync.ModuleSyncService
-import org.onap.cps.ncmp.api.impl.inventory.sync.ModuleSyncTasks
-import org.onap.cps.ncmp.api.impl.inventory.sync.SyncUtils
 import org.onap.cps.ncmp.api.impl.yangmodels.YangModelCmHandle
 import org.onap.cps.ncmp.api.impl.inventory.CmHandleState
 import org.onap.cps.ncmp.api.impl.inventory.CompositeState
@@ -62,7 +63,7 @@ class ModuleSyncTasksSpec extends Specification {
 
     def mockInventoryPersistence = Mock(InventoryPersistence)
 
-    def mockSyncUtils = Mock(SyncUtils)
+    def mockSyncUtils = Mock(ModuleOperationsUtils)
 
     def mockModuleSyncService = Mock(ModuleSyncService)
 
@@ -79,8 +80,8 @@ class ModuleSyncTasksSpec extends Specification {
 
     def 'Module Sync ADVISED cm handles.'() {
         given: 'cm handles in an ADVISED state'
-            def cmHandle1 = advisedCmHandleAsDataNode('cm-handle-1')
-            def cmHandle2 = advisedCmHandleAsDataNode('cm-handle-2')
+            def cmHandle1 = cmHandleAsDataNodeByIdAndState('cm-handle-1', CmHandleState.ADVISED)
+            def cmHandle2 = cmHandleAsDataNodeByIdAndState('cm-handle-2', CmHandleState.ADVISED)
         and: 'the inventory persistence cm handle returns a ADVISED state for the any handle'
             mockInventoryPersistence.getCmHandleState(_) >> new CompositeState(cmHandleState: CmHandleState.ADVISED)
         when: 'module sync poll is executed'
@@ -101,7 +102,7 @@ class ModuleSyncTasksSpec extends Specification {
 
     def 'Module Sync ADVISED cm handle with failure during sync.'() {
         given: 'a cm handle in an ADVISED state'
-            def cmHandle = advisedCmHandleAsDataNode('cm-handle')
+            def cmHandle = cmHandleAsDataNodeByIdAndState('cm-handle', CmHandleState.ADVISED)
         and: 'the inventory persistence cm handle returns a ADVISED state for the cm handle'
             def cmHandleState = new CompositeState(cmHandleState: CmHandleState.ADVISED)
             1 * mockInventoryPersistence.getCmHandleState('cm-handle') >> cmHandleState
@@ -110,7 +111,7 @@ class ModuleSyncTasksSpec extends Specification {
         when: 'module sync is executed'
             objectUnderTest.performModuleSync([cmHandle], batchCount)
         then: 'update lock reason, details and attempts is invoked'
-            1 * mockSyncUtils.updateLockReasonDetailsAndAttempts(cmHandleState, LockReasonCategory.MODULE_SYNC_FAILED, 'some exception')
+            1 * mockSyncUtils.updateLockReasonDetailsAndAttempts(cmHandleState, MODULE_SYNC_FAILED, 'some exception')
         and: 'the state handler is called to update the state to LOCKED'
             1 * mockLcmEventsCmHandleStateHandler.updateCmHandleStateBatch(_) >> { args ->
                 assertBatch(args, ['cm-handle'], CmHandleState.LOCKED)
@@ -119,6 +120,25 @@ class ModuleSyncTasksSpec extends Specification {
             assert batchCount.get() == 4
     }
 
+    def 'Failed cm handle during #scenario.'() {
+        given: 'a cm handle in LOCKED state'
+            def cmHandle = cmHandleAsDataNodeByIdAndState('cm-handle', CmHandleState.LOCKED)
+        and: 'the inventory persistence cm handle returns a LOCKED state with reason for the cm handle'
+            def expectedCmHandleState = new CompositeState(cmHandleState: cmHandleState, lockReason: CompositeState
+                .LockReason.builder().lockReasonCategory(lockReasonCategory).details(lockReasonDetails).build())
+            1 * mockInventoryPersistence.getCmHandleState('cm-handle') >> expectedCmHandleState
+        and: 'module sync service attempts to sync/upgrade the cm handle and throws an exception'
+            1 * mockModuleSyncService.syncAndCreateOrUpgradeSchemaSetAndAnchor(*_) >> { throw new Exception('some exception') }
+        when: 'module sync is executed'
+            objectUnderTest.performModuleSync([cmHandle], batchCount)
+        then: 'update lock reason, details and attempts is invoked'
+            1 * mockSyncUtils.updateLockReasonDetailsAndAttempts(expectedCmHandleState, expectedLockReasonCategory, 'some exception')
+        where:
+            scenario         | cmHandleState        | lockReasonCategory    | lockReasonDetails                              || expectedLockReasonCategory
+            'module upgrade' | CmHandleState.LOCKED | MODULE_UPGRADE_FAILED | 'Upgrade to ModuleSetTag: some-module-set-tag' || MODULE_UPGRADE_FAILED
+            'module sync'    | CmHandleState.LOCKED | MODULE_SYNC_FAILED    | 'some lock details'                            || MODULE_SYNC_FAILED
+    }
+
     def 'Reset failed CM Handles #scenario.'() {
         given: 'cm handles in an locked state'
             def lockedState = new CompositeStateBuilder().withCmHandleState(CmHandleState.LOCKED)
@@ -147,7 +167,7 @@ class ModuleSyncTasksSpec extends Specification {
 
     def 'Module Sync ADVISED cm handle without entry in progress map.'() {
         given: 'cm handles in an ADVISED state'
-            def cmHandle1 = advisedCmHandleAsDataNode('cm-handle-1')
+            def cmHandle1 = cmHandleAsDataNodeByIdAndState('cm-handle-1', CmHandleState.ADVISED)
         and: 'the inventory persistence cm handle returns a ADVISED state for the any handle'
             mockInventoryPersistence.getCmHandleState(_) >> new CompositeState(cmHandleState: CmHandleState.ADVISED)
         and: 'entry in progress map for other cm handle'
@@ -183,8 +203,8 @@ class ModuleSyncTasksSpec extends Specification {
             assert loggingEvent == null
     }
 
-    def advisedCmHandleAsDataNode(cmHandleId) {
-        return new DataNode(anchorName: cmHandleId, leaves: ['id': cmHandleId, 'cm-handle-state': 'ADVISED'])
+    def cmHandleAsDataNodeByIdAndState(cmHandleId, cmHandleState) {
+        return new DataNode(anchorName: cmHandleId, leaves: ['id': cmHandleId, 'cm-handle-state': cmHandleState])
     }
 
     def assertYamgModelCmHandleArgument(args, expectedCmHandleId) {
diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/inventory/sync/ModuleSyncWatchdogSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/inventory/sync/ModuleSyncWatchdogSpec.groovy
index 7425a52ff..1752a17a1 100644
--- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/inventory/sync/ModuleSyncWatchdogSpec.groovy
+++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/inventory/sync/ModuleSyncWatchdogSpec.groovy
@@ -22,9 +22,6 @@
 package org.onap.cps.ncmp.api.impl.inventory.sync
 
 import com.hazelcast.map.IMap
-import org.onap.cps.ncmp.api.impl.inventory.sync.ModuleSyncTasks
-import org.onap.cps.ncmp.api.impl.inventory.sync.ModuleSyncWatchdog
-import org.onap.cps.ncmp.api.impl.inventory.sync.SyncUtils
 import org.onap.cps.ncmp.api.impl.yangmodels.YangModelCmHandle
 import org.onap.cps.ncmp.api.impl.inventory.sync.executor.AsyncTaskExecutor
 import java.util.concurrent.ArrayBlockingQueue
@@ -33,7 +30,7 @@ import spock.lang.Specification
 
 class ModuleSyncWatchdogSpec extends Specification {
 
-    def mockSyncUtils = Mock(SyncUtils)
+    def mockSyncUtils = Mock(ModuleOperationsUtils)
 
     def static testQueueCapacity = 50 + 2 * ModuleSyncWatchdog.MODULE_SYNC_BATCH_SIZE