Retry mechanism (with back off algorithm) is removed with more frequent watchdog poll

- Increased watchdog frequency for locked cm handle.
- Removed retry backoff algorithm for locked cm handle.

Issue-ID: CPS-2395
Change-Id: I54d0ec8f9de53a7d181639c14aaaa93736f03e19
Signed-off-by: sourabh_sourabh <sourabh.sourabh@est.tech>

5 files changed, 104 insertions, 185 deletions

diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/inventory/sync/ModuleOperationsUtils.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/inventory/sync/ModuleOperationsUtils.java
index 97f1e8e70f..aae1769968 100644
--- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/inventory/sync/ModuleOperationsUtils.java
+++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/inventory/sync/ModuleOperationsUtils.java
@@ -22,9 +22,6 @@
 package org.onap.cps.ncmp.impl.inventory.sync;
 
 import com.fasterxml.jackson.databind.JsonNode;
-import java.time.Duration;
-import java.time.OffsetDateTime;
-import java.time.format.DateTimeFormatter;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -62,8 +59,7 @@ public class ModuleOperationsUtils {
     private static final String RETRY_ATTEMPT_KEY = "attempt";
     private static final String MODULE_SET_TAG_KEY = "moduleSetTag";
     public static final String MODULE_SET_TAG_MESSAGE_FORMAT = "Upgrade to ModuleSetTag: %s";
-    private static final String LOCK_REASON_DETAILS_MSG_FORMAT =
-            MODULE_SET_TAG_MESSAGE_FORMAT + " Attempt #%d failed: %s";
+    private static final String LOCK_REASON_DETAILS_MSG_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+)");
     private static final String CPS_PATH_CM_HANDLES_MODEL_SYNC_FAILED_OR_UPGRADE = """
@@ -119,23 +115,25 @@ public class ModuleOperationsUtils {
     }
 
     /**
-     * Update Composite State attempts counter and set new lock reason and details.
+     * Updates the lock reason message and attempt counter for the provided CompositeState.
+     * This method increments the attempt counter and updates the lock reason message,
+     * including the module set tag if available.
      *
-     * @param lockReasonCategory lock reason category
-     * @param errorMessage       error message
+     * @param compositeState     the composite state of the CM handle
+     * @param lockReasonCategory the lock reason category for the CM handle
+     * @param errorMessage       the error message to include in the lock reason message
      */
-    public void updateLockReasonDetailsAndAttempts(final CompositeState compositeState,
-                                                   final LockReasonCategory lockReasonCategory,
-                                                   final String errorMessage) {
-        int attempt = 1;
-        final Map<String, String> compositeStateDetails
-                = getLockedCompositeStateDetails(compositeState.getLockReason());
-        if (!compositeStateDetails.isEmpty() && compositeStateDetails.containsKey(RETRY_ATTEMPT_KEY)) {
-            attempt = 1 + Integer.parseInt(compositeStateDetails.get(RETRY_ATTEMPT_KEY));
-        }
-        final String moduleSetTag = compositeStateDetails.getOrDefault(MODULE_SET_TAG_KEY, "");
+    public void updateLockReasonWithAttempts(final CompositeState compositeState,
+                                             final LockReasonCategory lockReasonCategory,
+                                             final String errorMessage) {
+        final Map<String, String> lockedStateDetails = getLockedCompositeStateDetails(compositeState.getLockReason());
+        final int nextAttemptCount = calculateNextAttemptCount(lockedStateDetails);
+        final String moduleSetTag = lockedStateDetails.getOrDefault(MODULE_SET_TAG_KEY, "");
+
+        final String lockReasonMessage = buildLockReasonDetails(moduleSetTag, nextAttemptCount, errorMessage);
+
         compositeState.setLockReason(CompositeState.LockReason.builder()
-                .details(String.format(LOCK_REASON_DETAILS_MSG_FORMAT, moduleSetTag, attempt, errorMessage))
+                .details(lockReasonMessage)
                 .lockReasonCategory(lockReasonCategory)
                 .build());
     }
@@ -166,37 +164,6 @@ public class ModuleOperationsUtils {
         return Collections.emptyMap();
     }
 
-
-    /**
-     * Check if a module sync retry is needed.
-     *
-     * @param compositeState the composite state currently in the locked state
-     * @return if the retry mechanism should be attempted
-     */
-    public boolean needsModuleSyncRetryOrUpgrade(final CompositeState compositeState) {
-        final OffsetDateTime time = OffsetDateTime.parse(compositeState.getLastUpdateTime(),
-                DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss.SSSZ"));
-        final CompositeState.LockReason lockReason = compositeState.getLockReason();
-
-        final boolean moduleUpgrade = LockReasonCategory.MODULE_UPGRADE == lockReason.getLockReasonCategory();
-        if (moduleUpgrade) {
-            log.info("Locked for module upgrade");
-            return true;
-        }
-
-        final boolean failedDuringModuleSync = LockReasonCategory.MODULE_SYNC_FAILED
-                == lockReason.getLockReasonCategory();
-        final boolean failedDuringModuleUpgrade = LockReasonCategory.MODULE_UPGRADE_FAILED
-                == lockReason.getLockReasonCategory();
-
-        if (failedDuringModuleSync || failedDuringModuleUpgrade) {
-            log.info("Locked for module {} (last attempt failed).", failedDuringModuleSync ? "sync" : "upgrade");
-            return isRetryDue(lockReason, time);
-        }
-        log.info("Locked for other reason");
-        return false;
-    }
-
     /**
      * Get the Resource Data from Node through DMI Passthrough service.
      *
@@ -242,22 +209,18 @@ public class ModuleOperationsUtils {
         return cmHandlesAsDataNodeList.stream().map(YangDataConverter::toYangModelCmHandle).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;
+    private int calculateNextAttemptCount(final Map<String, String> compositeStateDetails) {
+        return compositeStateDetails.containsKey(RETRY_ATTEMPT_KEY)
+                ? 1 + Integer.parseInt(compositeStateDetails.get(RETRY_ATTEMPT_KEY))
+                : 1;
+    }
+
+    private String buildLockReasonDetails(final String moduleSetTag, final int attempt, final String errorMessage) {
+        if (moduleSetTag.isEmpty()) {
+            return String.format(LOCK_REASON_DETAILS_MSG_FORMAT, attempt, errorMessage);
         }
-        log.info("Retry due now");
-        return true;
+        return String.format(MODULE_SET_TAG_MESSAGE_FORMAT + " " + LOCK_REASON_DETAILS_MSG_FORMAT,
+                moduleSetTag, attempt, errorMessage);
     }
+
 }
diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/inventory/sync/ModuleSyncTasks.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/inventory/sync/ModuleSyncTasks.java
index 8e5c9f3066..c6deb79d4d 100644
--- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/inventory/sync/ModuleSyncTasks.java
+++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/inventory/sync/ModuleSyncTasks.java
@@ -79,7 +79,7 @@ public class ModuleSyncTasks {
                     log.warn("Processing of {} module failed due to reason {}.", cmHandleId, e.getMessage());
                     final LockReasonCategory lockReasonCategory = inUpgrade ? LockReasonCategory.MODULE_UPGRADE_FAILED
                             : LockReasonCategory.MODULE_SYNC_FAILED;
-                    moduleOperationsUtils.updateLockReasonDetailsAndAttempts(compositeState,
+                    moduleOperationsUtils.updateLockReasonWithAttempts(compositeState,
                             lockReasonCategory, e.getMessage());
                     setCmHandleStateLocked(yangModelCmHandle, compositeState.getLockReason());
                     cmHandelStatePerCmHandle.put(yangModelCmHandle, CmHandleState.LOCKED);
@@ -95,23 +95,23 @@ public class ModuleSyncTasks {
     }
 
     /**
-     * Reset state to "ADVISED" for any previously failed cm handles.
+     * Resets the state of failed CM handles and updates their status to ADVISED for retry.
+
+     * This method processes a collection of failed CM handles, logs their lock reason, and resets their state
+     * to ADVISED. Once reset, it updates the CM handle states in a batch to allow for re-attempt by the module-sync
+     * watchdog.
      *
-     * @param failedCmHandles previously failed (locked) cm handles
+     * @param failedCmHandles a collection of CM handles that have failed and need their state reset
      */
     public void resetFailedCmHandles(final Collection<YangModelCmHandle> failedCmHandles) {
         final Map<YangModelCmHandle, CmHandleState> cmHandleStatePerCmHandle = new HashMap<>(failedCmHandles.size());
         for (final YangModelCmHandle failedCmHandle : failedCmHandles) {
             final CompositeState compositeState = failedCmHandle.getCompositeState();
-            final boolean isReadyForRetry = moduleOperationsUtils.needsModuleSyncRetryOrUpgrade(compositeState);
-            log.info("Retry for cmHandleId : {} is {}", failedCmHandle.getId(), isReadyForRetry);
-            if (isReadyForRetry) {
-                final String resetCmHandleId = failedCmHandle.getId();
-                log.debug("Reset cm handle {} state to ADVISED to be re-attempted by module-sync watchdog",
-                        resetCmHandleId);
-                cmHandleStatePerCmHandle.put(failedCmHandle, CmHandleState.ADVISED);
-                removeResetCmHandleFromModuleSyncMap(resetCmHandleId);
-            }
+            final String resetCmHandleId = failedCmHandle.getId();
+            log.debug("Resetting CM handle {} state to ADVISED for retry by the module-sync watchdog. Lock reason: {}",
+                    failedCmHandle.getId(), compositeState.getLockReason().getLockReasonCategory().name());
+            cmHandleStatePerCmHandle.put(failedCmHandle, CmHandleState.ADVISED);
+            removeResetCmHandleFromModuleSyncMap(resetCmHandleId);
         }
         lcmEventsCmHandleStateHandler.updateCmHandleStateBatch(cmHandleStatePerCmHandle);
     }
diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/inventory/sync/ModuleSyncWatchdog.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/inventory/sync/ModuleSyncWatchdog.java
index cc724e1f07..bc7d6cdf67 100644
--- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/inventory/sync/ModuleSyncWatchdog.java
+++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/inventory/sync/ModuleSyncWatchdog.java
@@ -82,7 +82,7 @@ public class ModuleSyncWatchdog {
     /**
      * Find any failed (locked) cm handles and change state back to 'ADVISED'.
      */
-    @Scheduled(fixedDelayString = "${ncmp.timers.locked-modules-sync.sleep-time-ms:300000}")
+    @Scheduled(fixedDelayString = "${ncmp.timers.locked-modules-sync.sleep-time-ms:15000}")
     public void resetPreviouslyFailedCmHandles() {
         log.info("Processing module sync retry-watchdog waking up.");
         final Collection<YangModelCmHandle> failedCmHandles
diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/inventory/sync/ModuleOperationsUtilsSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/inventory/sync/ModuleOperationsUtilsSpec.groovy
index babe8101dd..65b7ff6d6f 100644
--- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/inventory/sync/ModuleOperationsUtilsSpec.groovy
+++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/inventory/sync/ModuleOperationsUtilsSpec.groovy
@@ -40,12 +40,8 @@ import org.springframework.context.annotation.AnnotationConfigApplicationContext
 import org.springframework.http.HttpStatus
 import org.springframework.http.ResponseEntity
 import spock.lang.Specification
-
-import java.time.OffsetDateTime
-import java.time.format.DateTimeFormatter
 import java.util.stream.Collectors
 
-import static org.onap.cps.ncmp.impl.inventory.models.LockReasonCategory.LOCKED_MISBEHAVING
 import static org.onap.cps.ncmp.impl.inventory.models.LockReasonCategory.MODULE_SYNC_FAILED
 import static org.onap.cps.ncmp.impl.inventory.models.LockReasonCategory.MODULE_UPGRADE
 import static org.onap.cps.ncmp.impl.inventory.models.LockReasonCategory.MODULE_UPGRADE_FAILED
@@ -60,17 +56,12 @@ class ModuleOperationsUtilsSpec extends Specification{
 
     def objectUnderTest = new ModuleOperationsUtils(mockCmHandleQueries, mockDmiDataOperations, jsonObjectMapper)
 
-    def static neverUpdatedBefore = '1900-01-01T00:00:00.000+0100'
-
-    def static now = OffsetDateTime.now()
-
-    def static nowAsString = DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss.SSSZ").format(now)
-
     def static dataNode = new DataNode(leaves: ['id': 'cm-handle-123'])
 
     def applicationContext = new AnnotationConfigApplicationContext()
 
     def logger = (Logger) LoggerFactory.getLogger(ModuleOperationsUtils)
+
     def loggingListAppender
 
     void setup() {
@@ -103,7 +94,7 @@ class ModuleOperationsUtilsSpec extends Specification{
         given: 'A locked state'
             def compositeState = new CompositeState(lockReason: lockReason)
         when: 'update cm handle details and attempts is called'
-            objectUnderTest.updateLockReasonDetailsAndAttempts(compositeState, MODULE_SYNC_FAILED, 'new error message')
+            objectUnderTest.updateLockReasonWithAttempts(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.contains(expectedDetails)
@@ -118,14 +109,14 @@ class ModuleOperationsUtilsSpec extends Specification{
             def compositeState = new CompositeStateBuilder().withCmHandleState(CmHandleState.LOCKED)
                 .withLockReason(MODULE_UPGRADE, "Upgrade to ModuleSetTag: " + moduleSetTag).build()
         when: 'update cm handle details'
-            objectUnderTest.updateLockReasonDetailsAndAttempts(compositeState, MODULE_UPGRADE_FAILED, 'new error message')
+            objectUnderTest.updateLockReasonWithAttempts(compositeState, MODULE_UPGRADE_FAILED, 'new error message')
         then: 'the composite state lock reason and details are updated'
             assert compositeState.lockReason.lockReasonCategory == MODULE_UPGRADE_FAILED
-            assert compositeState.lockReason.details.contains("Upgrade to ModuleSetTag: " + expectedDetails)
+            assert compositeState.lockReason.details.contains(expectedDetails)
         where:
             scenario               | moduleSetTag       || expectedDetails
-            'a module set tag'     | 'someModuleSetTag' || 'someModuleSetTag'
-            'empty module set tag' | ''                 || ''
+            'a module set tag'     | 'someModuleSetTag' || 'Upgrade to ModuleSetTag: someModuleSetTag'
+            'empty module set tag' | ''                 || 'Attempt'
     }
 
     def 'Get all locked cm-Handles where lock reasons are model sync failed or upgrade'() {
@@ -135,49 +126,9 @@ class ModuleOperationsUtilsSpec extends Specification{
         when: 'get locked Misbehaving cm handle is called'
             def result = objectUnderTest.getCmHandlesThatFailedModelSyncOrUpgrade()
         then: 'the returned cm handle collection is the correct size'
-            result.size() == 1
+            assert result.size() == 1
         and: 'the correct cm handle is returned'
-            result[0].id == 'cm-handle-123'
-    }
-
-    def 'Retry Locked Cm-Handle where the last update time is #scenario'() {
-        given: 'Last update was #lastUpdateMinutesAgo minutes ago (-1 means never)'
-            def lastUpdatedTime = DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss.SSSZ").format(now.minusMinutes(lastUpdateMinutesAgo))
-            if (lastUpdateMinutesAgo < 0 ) {
-                lastUpdatedTime = neverUpdatedBefore
-            }
-        when: 'checking to see if cm handle is ready for retry'
-         def result = objectUnderTest.needsModuleSyncRetryOrUpgrade(new CompositeStateBuilder()
-                .withLockReason(MODULE_SYNC_FAILED, lockDetails)
-                .withLastUpdatedTime(lastUpdatedTime).build())
-        then: 'retry is only attempted when expected'
-            assert result == retryExpected
-        and: 'logs contain related information'
-            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: 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 lock reasons (category) #lockReasonCategory'() {
-        when: 'checking to see if cm handle is ready for retry'
-            def result = objectUnderTest.needsModuleSyncRetryOrUpgrade(new CompositeStateBuilder()
-                .withLockReason(lockReasonCategory, 'some details')
-                .withLastUpdatedTime(nowAsString).build())
-        then: 'verify retry attempts'
-            assert !result
-        and: 'logs contain related information'
-            def logs = loggingListAppender.list.toString()
-            assert logs.contains(logReason)
-        where: 'the following lock reasons occurred'
-            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'
+            assert result[0].id == 'cm-handle-123'
     }
 
     def 'Get a Cm-Handle where #scenario'() {
@@ -197,19 +148,25 @@ class ModuleOperationsUtilsSpec extends Specification{
             'all Cm-Handle synchronized'               | []                      | false            || 0                    | [] as Set
     }
 
-    def 'Get resource data through DMI Operations #scenario'() {
-        given: 'the inventory persistence service returns a collection of data nodes'
+    def 'Retrieve resource data from DMI operations for #scenario'() {
+        given: 'a JSON string representing the resource data'
             def jsonString = '{"stores:bookstore":{"categories":[{"code":"01"}]}}'
-            JsonNode jsonNode = jsonObjectMapper.convertToJsonNode(jsonString);
-            def responseEntity = new ResponseEntity<>(jsonNode, HttpStatus.OK)
+            JsonNode jsonNode = jsonObjectMapper.convertToJsonNode(jsonString)
+        and: 'DMI operations are mocked to return a response based on the scenario'
+            def responseEntity = new ResponseEntity<>(statusCode == HttpStatus.OK ? jsonNode : null, statusCode)
             mockDmiDataOperations.getAllResourceDataFromDmi('cm-handle-123', _) >> responseEntity
         when: 'get resource data is called'
             def result = objectUnderTest.getResourceData('cm-handle-123')
-        then: 'the returned data is correct'
-            result == jsonString
+        then: 'the returned data matches the expected result'
+            assert result == expectedResult
+        where:
+            scenario                              | statusCode                       | expectedResult
+            'successful response'                 | HttpStatus.OK                    | '{"stores:bookstore":{"categories":[{"code":"01"}]}}'
+            'response with not found status'      | HttpStatus.NOT_FOUND             | null
+            'response with internal server error' | HttpStatus.INTERNAL_SERVER_ERROR | null
     }
 
-    def 'Extract module set tag and number of attempt when lock reason contains #scenario'() {
+        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'
diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/inventory/sync/ModuleSyncTasksSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/inventory/sync/ModuleSyncTasksSpec.groovy
index ee49f2f901..160744a7d7 100644
--- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/inventory/sync/ModuleSyncTasksSpec.groovy
+++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/inventory/sync/ModuleSyncTasksSpec.groovy
@@ -32,16 +32,15 @@ import org.onap.cps.ncmp.api.inventory.models.CompositeState
 import org.onap.cps.ncmp.api.inventory.models.CompositeStateBuilder
 import org.onap.cps.ncmp.impl.inventory.InventoryPersistence
 import org.onap.cps.ncmp.impl.inventory.models.CmHandleState
-import org.onap.cps.ncmp.impl.inventory.models.LockReasonCategory
 import org.onap.cps.ncmp.impl.inventory.models.YangModelCmHandle
 import org.onap.cps.ncmp.impl.inventory.sync.lcm.LcmEventsCmHandleStateHandler
 import org.onap.cps.spi.model.DataNode
 import org.slf4j.LoggerFactory
 import spock.lang.Specification
-
 import java.util.concurrent.atomic.AtomicInteger
 
 import static org.onap.cps.ncmp.impl.inventory.models.LockReasonCategory.MODULE_SYNC_FAILED
+import static org.onap.cps.ncmp.impl.inventory.models.LockReasonCategory.MODULE_UPGRADE
 import static org.onap.cps.ncmp.impl.inventory.models.LockReasonCategory.MODULE_UPGRADE_FAILED
 
 class ModuleSyncTasksSpec extends Specification {
@@ -96,70 +95,52 @@ class ModuleSyncTasksSpec extends Specification {
             assert batchCount.get() == 4
     }
 
-    def 'Module Sync ADVISED cm handle with failure during sync.'() {
-        given: 'a cm handle in an ADVISED state'
-            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
-        and: 'module sync service attempts to sync the cm handle and throws an exception'
-            1 * mockModuleSyncService.syncAndCreateSchemaSetAndAnchor(*_) >> { throw new Exception('some exception') }
+    def 'Handle CM handle failure during #scenario and log MODULE_UPGRADE lock reason'() {
+        given: 'a CM handle in LOCKED state with a specific lock reason'
+            def cmHandle = cmHandleAsDataNodeByIdAndState('cm-handle', CmHandleState.LOCKED)
+            def expectedCmHandleState = new CompositeState(cmHandleState: CmHandleState.LOCKED, 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'
+            mockModuleSyncService.syncAndCreateSchemaSetAndAnchor(_) >> { throw new Exception('some exception') }
+            mockModuleSyncService.syncAndUpgradeSchemaSet(_) >> { 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(cmHandleState, MODULE_SYNC_FAILED, 'some exception')
+        then: 'lock reason is updated with number of attempts'
+            1 * mockSyncUtils.updateLockReasonWithAttempts(expectedCmHandleState, expectedLockReasonCategory, 'some exception')
         and: 'the state handler is called to update the state to LOCKED'
             1 * mockLcmEventsCmHandleStateHandler.updateCmHandleStateBatch(_) >> { args ->
                 assertBatch(args, ['cm-handle'], CmHandleState.LOCKED)
             }
         and: 'batch count is decremented by one'
             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'
-            mockModuleSyncService.syncAndCreateSchemaSetAndAnchor(*_) >> { throw new Exception('some exception') }
-            mockModuleSyncService.syncAndUpgradeSchemaSet(*_) >> { 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
+            scenario         | lockReasonCategory    | lockReasonDetails                              || expectedLockReasonCategory
+            'module sync'    | MODULE_SYNC_FAILED    | 'some lock details'                            || MODULE_SYNC_FAILED
+            'module upgrade' | MODULE_UPGRADE_FAILED | 'Upgrade to ModuleSetTag: some-module-set-tag' || MODULE_UPGRADE_FAILED
+            'module upgrade' | MODULE_UPGRADE        | 'Upgrade in progress'                          || MODULE_UPGRADE_FAILED
     }
 
+
     def 'Reset failed CM Handles #scenario.'() {
         given: 'cm handles in an locked state'
             def lockedState = new CompositeStateBuilder().withCmHandleState(CmHandleState.LOCKED)
-                    .withLockReason(LockReasonCategory.MODULE_SYNC_FAILED, '').withLastUpdatedTimeNow().build()
+                .withLockReason(MODULE_SYNC_FAILED, '').withLastUpdatedTimeNow().build()
             def yangModelCmHandle1 = new YangModelCmHandle(id: 'cm-handle-1', compositeState: lockedState)
             def yangModelCmHandle2 = new YangModelCmHandle(id: 'cm-handle-2', compositeState: lockedState)
-            def expectedCmHandleStatePerCmHandle = [(yangModelCmHandle1): CmHandleState.ADVISED]
+            def expectedCmHandleStatePerCmHandle
+                    = [(yangModelCmHandle1): CmHandleState.ADVISED, (yangModelCmHandle2): CmHandleState.ADVISED]
         and: 'clear in progress map'
             resetModuleSyncStartedOnCmHandles(moduleSyncStartedOnCmHandles)
         and: 'add cm handle entry into progress map'
             moduleSyncStartedOnCmHandles.put('cm-handle-1', 'started')
             moduleSyncStartedOnCmHandles.put('cm-handle-2', 'started')
-        and: 'sync utils retry locked cm handle returns #isReadyForRetry'
-            mockSyncUtils.needsModuleSyncRetryOrUpgrade(lockedState) >>> isReadyForRetry
         when: 'resetting failed cm handles'
             objectUnderTest.resetFailedCmHandles([yangModelCmHandle1, yangModelCmHandle2])
         then: 'updated to state "ADVISED" from "READY" is called as often as there are cm handles ready for retry'
-            expectedNumberOfInvocationsToUpdateCmHandleState * mockLcmEventsCmHandleStateHandler.updateCmHandleStateBatch(expectedCmHandleStatePerCmHandle)
-        and: 'after reset performed size of in progress map'
-            assert moduleSyncStartedOnCmHandles.size() == inProgressMapSize
-        where:
-            scenario                        | isReadyForRetry | inProgressMapSize || expectedNumberOfInvocationsToUpdateCmHandleState
-            'retry locked cm handle'        | [true, false]   | 1                 || 1
-            'do not retry locked cm handle' | [false, false]  | 2                 || 0
+            1 * mockLcmEventsCmHandleStateHandler.updateCmHandleStateBatch(expectedCmHandleStatePerCmHandle)
+        and: 'after reset performed progress map is empty'
+            assert moduleSyncStartedOnCmHandles.size() == 0
     }
 
     def 'Module Sync ADVISED cm handle without entry in progress map.'() {
@@ -189,6 +170,24 @@ class ModuleSyncTasksSpec extends Specification {
             assert loggingEvent.formattedMessage == 'ch-1 removed from in progress map'
     }
 
+    def 'Sync and upgrade CM handle if in upgrade state for #scenario'() {
+        given: 'a CM handle in an upgrade state'
+            def cmHandle = cmHandleAsDataNodeByIdAndState('cm-handle', CmHandleState.LOCKED)
+            def compositeState = new CompositeState(lockReason: CompositeState.LockReason.builder().lockReasonCategory(lockReasonCategory).build())
+            1 * mockInventoryPersistence.getCmHandleState('cm-handle') >> compositeState
+        when: 'module sync is executed'
+            objectUnderTest.performModuleSync([cmHandle], batchCount)
+        then: 'the module sync service should attempt to sync and upgrade the CM handle'
+            1 * mockModuleSyncService.syncAndUpgradeSchemaSet(_) >> { args ->
+                assert args[0].id == 'cm-handle'
+            }
+        where: 'the following lock reasons are used'
+            scenario                | lockReasonCategory
+            'module upgrade'        | MODULE_UPGRADE
+            'module upgrade failed' | MODULE_UPGRADE_FAILED
+    }
+
+
     def 'Remove non-existing cm handle id from hazelcast map'() {
         given: 'hazelcast map does not contains cm handle id'
             def result = moduleSyncStartedOnCmHandles.get('non-existing-cm-handle')