fix: Unmark daemoned nodes after stopping them (#5005)

Signed-off-by: Simon Behar <simbeh7@gmail.com>

test/e2e/daemon_pod_test.go

@@ -0,0 +1,60 @@
+// +build functional
+
+package e2e
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/suite"
+	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	"github.com/argoproj/argo/v3/pkg/apis/workflow/v1alpha1"
+	"github.com/argoproj/argo/v3/test/e2e/fixtures"
+)
+
+type DaemonPodSuite struct {
+	fixtures.E2ESuite
+}
+
+func (s *DaemonPodSuite) TestWorkflowCompletesIfContainsDaemonPod() {
+	s.Given().
+		Workflow(`apiVersion: argoproj.io/v1alpha1
+kind: Workflow
+metadata:
+  generateName: whalesay-
+  labels:
+    argo-e2e: true
+spec:
+  entrypoint: whalesay
+  templates:
+  - name: whalesay
+    dag:
+      tasks:
+        - name: redis
+          template: redis-tmpl
+        - name: whale
+          dependencies: [redis]
+          template: whale-tmpl
+  - name: redis-tmpl
+    daemon: true
+    container:
+      image: argoproj/argosay:v2
+      args: ["sleep", "100s"]
+  - name: whale-tmpl
+    container:
+      image: argoproj/argosay:v2
+      args: ["echo", "hello world"]
+`).
+		When().
+		SubmitWorkflow().
+		WaitForWorkflow(fixtures.ToBeSucceeded, "to be succeeded").
+		Then().
+		ExpectWorkflow(func(t *testing.T, metadata *v1.ObjectMeta, status *v1alpha1.WorkflowStatus) {
+			assert.False(t, status.FinishedAt.IsZero())
+		})
+}
+
+func TestDaemonPodSuite(t *testing.T) {
+	suite.Run(t, new(DaemonPodSuite))
+}

test/e2e/fixtures/when.go

@@ -172,6 +172,7 @@ var ToBeRunning Condition = func(wf *wfv1.Workflow) bool {
 		return node.Phase == wfv1.NodeRunning
 	})
 }
+var ToBeSucceeded Condition = func(wf *wfv1.Workflow) bool { return wf.Status.Phase == wfv1.WorkflowSucceeded }
 
 // `ToBeDone` replaces `ToFinish` which also makes sure the workflow is both complete not pending archiving.
 // This additional check is not needed for most use case, however in `AfterTest` we delete the workflow and this

workflow/controller/exec_control.go

@@ -120,6 +120,10 @@ func (woc *wfOperationCtx) killDaemonedChildren(ctx context.Context, nodeID stri
 				firstErr = err
 			}
 		}
+
+		childNode.Daemoned = nil
+		woc.wf.Status.Nodes[childNode.ID] = childNode
+		woc.updated = true
 	}
 	return firstErr
 }

workflow/controller/exec_control_test.go

@@ -0,0 +1,33 @@
+package controller
+
+import (
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"k8s.io/utils/pointer"
+
+	"github.com/argoproj/argo/v3/pkg/apis/workflow/v1alpha1"
+)
+
+func TestKillDaemonChildrenUnmarkPod(t *testing.T) {
+	cancel, controller := newController()
+	defer cancel()
+
+	woc := newWorkflowOperationCtx(&v1alpha1.Workflow{
+		Status: v1alpha1.WorkflowStatus{
+			Nodes: v1alpha1.Nodes{
+				"a": v1alpha1.NodeStatus{
+					ID:         "a",
+					BoundaryID: "a",
+					Daemoned:   pointer.BoolPtr(true),
+				},
+			},
+		},
+	}, controller)
+
+	assert.NotNil(t, woc.wf.Status.Nodes["a"].Daemoned)
+	// Error will be that it cannot find the pod, but we only care about the node status for this test
+	_ = woc.killDaemonedChildren(context.Background(), "a")
+	assert.Nil(t, woc.wf.Status.Nodes["a"].Daemoned)
+}