fix(controller): remove ArtifactGC finalizer when no artifacts. Fixes

#13499 (#13500)

Signed-off-by: joey <[email protected]>

test/e2e/artifacts_test.go

@@ -388,6 +388,14 @@ func (s *ArtifactsSuite) TestArtifactGC() {
 				artifactState{s3Location{bucketName: "my-bucket", derivedKey: &artifactDerivedKey{templateName: "artifact-written", artifactName: "present"}}, false, true},
 			},
 		},
+		// Workflow defined output artifact but execution failed, no artifacts to be gced
+		{
+			workflowFile:                 "@testdata/artifactgc/artgc-artifact-not-written-failed.yaml",
+			hasGC:                        true,
+			workflowShouldSucceed:        false,
+			expectedGCPodsOnWFCompletion: 0,
+			expectedArtifacts:            []artifactState{},
+		},
 	} {
 		// for each test make sure that:
 		// 1. the finalizer gets added

test/e2e/testdata/artifactgc/artgc-artifact-not-written-failed.yaml

@@ -0,0 +1,27 @@
+apiVersion: argoproj.io/v1alpha1
+kind: Workflow
+metadata:
+  generateName: artgc-artifact-not-written-failed-
+spec:
+  entrypoint: main
+  artifactGC:
+    strategy: OnWorkflowDeletion
+  templates:
+    - name: main
+      container:
+        image: argoproj/argosay:v2
+        command:
+          - sh
+          - -c
+        args:
+          - |
+            echo "intentionally not writing anything to disk and intentional failure"
+            exit 1
+      outputs:
+        artifacts:
+          - name: on-completion
+            path: /tmp/on-completion.txt
+            s3:
+              key: on-completion.txt
+            artifactGC:
+              strategy: OnWorkflowCompletion

workflow/controller/artifact_gc.go

@@ -508,7 +508,6 @@ func (woc *wfOperationCtx) processArtifactGCCompletion(ctx context.Context) erro
 		return fmt.Errorf("failed to get pods from informer: %w", err)
 	}
 
-	anyPodSuccess := false
 	for _, obj := range pods {
 		pod := obj.(*corev1.Pod)
 		if pod.Labels[common.LabelKeyComponent] != artifactGCComponent { // make sure it's an Artifact GC Pod
@@ -534,10 +533,8 @@ func (woc *wfOperationCtx) processArtifactGCCompletion(ctx context.Context) erro
 			if err != nil {
 				return err
 			}
+
 			woc.wf.Status.ArtifactGCStatus.SetArtifactGCPodRecouped(pod.Name, true)
-			if phase == corev1.PodSucceeded {
-				anyPodSuccess = true
-			}
 			woc.updated = true
 		}
 	}
@@ -548,7 +545,7 @@ func (woc *wfOperationCtx) processArtifactGCCompletion(ctx context.Context) erro
 		removeFinalizer = woc.wf.Status.ArtifactGCStatus.AllArtifactGCPodsRecouped()
 	} else {
 		// check if all artifacts have been deleted and if so remove Finalizer
-		removeFinalizer = anyPodSuccess && woc.allArtifactsDeleted()
+		removeFinalizer = woc.allArtifactsDeleted()
 	}
 	if removeFinalizer {
 		woc.log.Infof("no remaining artifacts to GC, removing artifact GC finalizer (forceFinalizerRemoval=%v)", forceFinalizerRemoval)