fix: do not delete pvc when max parallelism has been reached. Fixes #…

…11119 (#11138) Signed-off-by: Abraham Bah <[email protected]>
argoproj · Jun 25, 2023 · aa2b66a · aa2b66a
1 parent f180335
commit aa2b66a
Show file tree

Hide file tree

Showing 3 changed files with 102 additions and 8 deletions.
diff --git a/test/e2e/testdata/loops-steps-limited-parallelism-pvc.yaml b/test/e2e/testdata/loops-steps-limited-parallelism-pvc.yaml
@@ -0,0 +1,78 @@
+apiVersion: argoproj.io/v1alpha1
+kind: Workflow
+metadata:
+  name: loop-fail-3
+spec:
+  entrypoint: constrained-loops-with-volumes
+  parallelism: 1
+  volumeClaimTemplates:
+  - metadata:
+      name: one
+    spec:
+      accessModes: [ "ReadWriteOnce" ]
+      resources:
+        requests:
+          storage: 1Gi
+  - metadata:
+      name: two
+    spec:
+      accessModes: [ "ReadWriteOnce" ]
+      resources:
+        requests:
+          storage: 1Gi
+
+
+  templates:
+  - name: constrained-loops-with-volumes
+    steps:
+    - - name: write-and-read-msg-from-pvc
+        template: write-and-read-msg-from-pvc
+        arguments:
+          parameters:
+          - name: pvc
+            value: '{{item}}'
+        withItems: 
+          - one
+          - two
+  - name: write-and-read-msg-from-pvc
+    inputs:
+      parameters:
+      - name: pvc
+    steps:
+    - - name: write-msg
+        template: write-msg
+        arguments:
+          parameters:
+          - name: pvc
+            value: '{{inputs.parameters.pvc}}'        
+    - - name: read-msg
+        template: read-msg
+        arguments:
+          parameters:
+          - name: pvc
+            value: '{{inputs.parameters.pvc}}'
+  - name: write-msg
+    inputs:
+      parameters:
+      - name: pvc
+    script:
+      image: busybox:stable
+      command: ["sh"]
+      source: | 
+        echo "Hello! i'm writing to pvc; '{{inputs.parameters.pvc}}'" >> /pvc/msg
+      volumeMounts:
+      - name: '{{inputs.parameters.pvc}}'
+        mountPath: /pvc
+  - name: read-msg
+    inputs:
+      parameters:
+      - name: pvc
+    script:
+      image: busybox:stable
+      command: ["sh"]
+      source: | 
+        echo "Found the following message;"
+        cat /pvc/msg
+      volumeMounts:
+      - name: '{{inputs.parameters.pvc}}'
+        mountPath: /pvc
diff --git a/test/e2e/workflow_template_test.go b/test/e2e/workflow_template_test.go
@@ -90,6 +90,20 @@ func (s *WorkflowTemplateSuite) TestSubmitWorkflowTemplateResourceUnquotedExpres
 		})
 }
 
+func (s *WorkflowTemplateSuite) TestSubmitWorkflowTemplateWithParallelStepsRequiringPVC() {
+	s.Given().
+		WorkflowTemplate("@testdata/loops-steps-limited-parallelism-pvc.yaml").
+		When().
+		CreateWorkflowTemplates().
+		SubmitWorkflowsFromWorkflowTemplates().
+		WaitForWorkflow().
+		Then().
+		ExpectWorkflow(func(t *testing.T, metadata *v1.ObjectMeta, status *v1alpha1.WorkflowStatus) {
+			assert.Equal(t, status.Phase, v1alpha1.WorkflowSucceeded)
+		}).
+		ExpectPVCDeleted()
+}
+
 func (s *WorkflowTemplateSuite) TestWorkflowTemplateInvalidOnExit() {
 	s.Given().
 		WorkflowTemplate("@testdata/workflow-template-invalid-onexit.yaml").

diff --git a/workflow/controller/operator.go b/workflow/controller/operator.go
@@ -358,12 +358,13 @@ func (woc *wfOperationCtx) operate(ctx context.Context) {
 		default:
 			if !errorsutil.IsTransientErr(err) && !woc.wf.Status.Phase.Completed() && os.Getenv("BUBBLE_ENTRY_TEMPLATE_ERR") != "false" {
 				woc.markWorkflowError(ctx, x)
+
+				// Garbage collect PVCs if Entrypoint template execution returns error
+				if err := woc.deletePVCs(ctx); err != nil {
+					woc.log.WithError(err).Warn("failed to delete PVCs")
+				}
 			}
 		}
-		// Garbage collect PVCs if Entrypoint template execution returns error
-		if err := woc.deletePVCs(ctx); err != nil {
-			woc.log.WithError(err).Warn("failed to delete PVCs")
-		}
 		return
 	}
 
@@ -433,12 +434,13 @@ func (woc *wfOperationCtx) operate(ctx context.Context) {
 			default:
 				if !errorsutil.IsTransientErr(err) && !woc.wf.Status.Phase.Completed() && os.Getenv("BUBBLE_ENTRY_TEMPLATE_ERR") != "false" {
 					woc.markWorkflowError(ctx, x)
+
+					// Garbage collect PVCs if Onexit template execution returns error
+					if err := woc.deletePVCs(ctx); err != nil {
+						woc.log.WithError(err).Warn("failed to delete PVCs")
+					}
 				}
 			}
-			// Garbage collect PVCs if Onexit template execution returns error
-			if err := woc.deletePVCs(ctx); err != nil {
-				woc.log.WithError(err).Warn("failed to delete PVCs")
-			}
 			return
 		}