fix: addressed comments

Signed-off-by: isubasinghe <[email protected]>
argoproj · Sep 4, 2024 · 95e439c · 95e439c
1 parent 9f9ab7b
commit 95e439c
Show file tree

Hide file tree

Showing 3 changed files with 11 additions and 9 deletions.
diff --git a/pkg/apis/workflow/v1alpha1/workflow_types.go b/pkg/apis/workflow/v1alpha1/workflow_types.go
@@ -3870,7 +3870,7 @@ func (ms *MutexStatus) LockWaiting(holderKey, lockKey string, currentHolders []s
 	if i < 0 {
 		ms.Waiting = append(ms.Waiting, MutexHolding{Mutex: lockKey, Holder: currentHolders[0]})
 		return true
-	} else if !(mutexWaiting.Holder == currentHolders[0]) {
+	} else if mutexWaiting.Holder != currentHolders[0] {
 		mutexWaiting.Holder = currentHolders[0]
 		ms.Waiting[i] = mutexWaiting
 		return true

diff --git a/workflow/controller/operator_concurrency_test.go b/workflow/controller/operator_concurrency_test.go
@@ -1050,6 +1050,11 @@ func TestSynchronizationForPendingShuttingdownWfs(t *testing.T) {
 }
 
 func TestWorkflowMemoizationWithMutex(t *testing.T) {
+	// This is needed because this test explicitly checks the behaviour
+	// of Holding
+	oldVersion := os.Getenv("HOLDER_KEY_VERSION")
+	os.Setenv("HOLDER_KEY_VERSION", "v1")
+	defer os.Setenv("HOLDER_KEY_VERSION", oldVersion)
 	wf := wfv1.MustUnmarshalWorkflow(`apiVersion: argoproj.io/v1alpha1
 kind: Workflow
 metadata:
@@ -1096,8 +1101,6 @@ spec:
 	defer cancel()
 
 	ctx := context.Background()
-
-	os.Setenv("HOLDER_KEY_VERSION", "v1")
 	woc := newWorkflowOperationCtx(wf, controller)
 	woc.operate(ctx)
 

diff --git a/workflow/sync/sync_manager.go b/workflow/sync/sync_manager.go
@@ -272,18 +272,17 @@ func (cm *Manager) ReleaseAll(wf *wfv1.Workflow) bool {
 				continue
 			}
 
-			logKey := ""
+			key := ""
 			if v1alpha1.CheckHolderKeyVersion(holding.Holder) == v1alpha1.HoldingNameV1 {
 				resourceKey := getResourceKey(wf.Namespace, wf.Name, holding.Holder)
-				logKey = resourceKey
-				syncLockHolder.release(resourceKey)
+				key = resourceKey
 			} else {
-				logKey = holding.Holder
-				syncLockHolder.release(holding.Holder)
+				key = holding.Holder
 			}
 
+			syncLockHolder.release(key)
 			wf.Status.Synchronization.Mutex.LockReleased(holding.Holder, holding.Mutex)
-			log.Infof("%s released a lock from %s", logKey, holding.Mutex)
+			log.Infof("%s released a lock from %s", key, holding.Mutex)
 		}
 
 		// Remove the pending Workflow level mutex keys