From 0670f652cd7ca5500aa77c682bb8b380bb4c79d3 Mon Sep 17 00:00:00 2001
From: Saravanan Balasubramanian
 <33908564+sarabala1979@users.noreply.github.com>
Date: Tue, 10 Aug 2021 14:05:30 -0700
Subject: [PATCH] fix(controller): fix tasket warning in Non-HTTP Template
 scanerio (#6518)

Signed-off-by: Saravanan Balasubramanian <sarabala1979@gmail.com>
---
 pkg/apis/workflow/v1alpha1/workflow_types.go  |  9 +++++++
 .../workflow/v1alpha1/workflow_types_test.go  | 21 ++++++++++++++++
 workflow/controller/taskset.go                | 11 +++++---
 workflow/controller/taskset_test.go           | 25 ++++++++++++++++++-
 4 files changed, 62 insertions(+), 4 deletions(-)

diff --git a/pkg/apis/workflow/v1alpha1/workflow_types.go b/pkg/apis/workflow/v1alpha1/workflow_types.go
index 81aeecbdd182..ba01dfde0b89 100644
--- a/pkg/apis/workflow/v1alpha1/workflow_types.go
+++ b/pkg/apis/workflow/v1alpha1/workflow_types.go
@@ -1351,6 +1351,15 @@ func (n Nodes) Find(f func(NodeStatus) bool) *NodeStatus {
 	return nil
 }
 
+func (n Nodes) HasHTTPNodes() bool {
+	for _, i := range n {
+		if i.Type == NodeTypeHTTP {
+			return true
+		}
+	}
+	return false
+}
+
 func NodeWithDisplayName(name string) func(n NodeStatus) bool {
 	return func(n NodeStatus) bool { return n.DisplayName == name }
 }
diff --git a/pkg/apis/workflow/v1alpha1/workflow_types_test.go b/pkg/apis/workflow/v1alpha1/workflow_types_test.go
index 2d715acace51..d2a2df207b2e 100644
--- a/pkg/apis/workflow/v1alpha1/workflow_types_test.go
+++ b/pkg/apis/workflow/v1alpha1/workflow_types_test.go
@@ -955,3 +955,24 @@ func TestTemplateGetType(t *testing.T) {
 	tmpl := Template{HTTP: &HTTP{}}
 	assert.Equal(t, TemplateTypeHTTP, tmpl.GetType())
 }
+
+func TestHasHTTPNodes(t *testing.T) {
+	nodes := Nodes{
+		"test": {
+			Type: NodeTypeHTTP,
+		},
+		"test1": {
+			Type: NodeTypeContainer,
+		},
+	}
+	assert.True(t, nodes.HasHTTPNodes())
+	nodes = Nodes{
+		"test": {
+			Type: NodeTypeSteps,
+		},
+		"test1": {
+			Type: NodeTypeContainer,
+		},
+	}
+	assert.False(t, nodes.HasHTTPNodes())
+}
diff --git a/workflow/controller/taskset.go b/workflow/controller/taskset.go
index f31c112356a3..0def937824bd 100644
--- a/workflow/controller/taskset.go
+++ b/workflow/controller/taskset.go
@@ -49,11 +49,16 @@ func (woc *wfOperationCtx) getDeleteTaskAndNodePatch() map[string]interface{} {
 }
 
 func (woc *wfOperationCtx) removeCompletedTaskSetStatus(ctx context.Context) error {
-
+	if !woc.wf.Status.Nodes.HasHTTPNodes() {
+		return nil
+	}
 	return woc.patchTaskSet(ctx, woc.getDeleteTaskAndNodePatch(), types.MergePatchType)
 }
 
 func (woc *wfOperationCtx) completeTaskSet(ctx context.Context) error {
+	if !woc.wf.Status.Nodes.HasHTTPNodes() {
+		return nil
+	}
 	patch := woc.getDeleteTaskAndNodePatch()
 	patch["metadata"] = metav1.ObjectMeta{
 		Labels: map[string]string{
@@ -93,10 +98,10 @@ func (woc *wfOperationCtx) taskSetReconciliation(ctx context.Context) error {
 			woc.updated = true
 		}
 	}
-	return woc.CreateTaskSet(ctx)
+	return woc.createTaskSet(ctx)
 }
 
-func (woc *wfOperationCtx) CreateTaskSet(ctx context.Context) error {
+func (woc *wfOperationCtx) createTaskSet(ctx context.Context) error {
 	if len(woc.taskSet) == 0 {
 		return nil
 	}
diff --git a/workflow/controller/taskset_test.go b/workflow/controller/taskset_test.go
index 330cdbce0c35..4105492adfcd 100644
--- a/workflow/controller/taskset_test.go
+++ b/workflow/controller/taskset_test.go
@@ -288,7 +288,7 @@ status:
       phase: Failed
 
     `, &ts)
-	t.Run("", func(t *testing.T) {
+	t.Run("RemoveCompletedTaskSetStatus", func(t *testing.T) {
 		cancel, controller := newController(wf, ts)
 		defer cancel()
 		_, err := controller.wfclientset.ArgoprojV1alpha1().WorkflowTaskSets("default").Create(ctx, &ts, v1.CreateOptions{})
@@ -311,3 +311,26 @@ status:
 
 	})
 }
+
+func TestNonHTTPTemplateScenario(t *testing.T) {
+	cancel, controller := newController()
+	defer cancel()
+	wf := wfv1.MustUnmarshalWorkflow(helloWorldWf)
+	woc := newWorkflowOperationCtx(wf, controller)
+	ctx := context.Background()
+	t.Run("taskSetReconciliation", func(t *testing.T) {
+		woc.operate(ctx)
+		err := woc.taskSetReconciliation(ctx)
+		assert.NoError(t, err)
+	})
+	t.Run("completeTaskSet", func(t *testing.T) {
+		woc.operate(ctx)
+		err := woc.completeTaskSet(ctx)
+		assert.NoError(t, err)
+	})
+	t.Run("removeCompletedTaskSetStatus", func(t *testing.T) {
+		woc.operate(ctx)
+		err := woc.removeCompletedTaskSetStatus(ctx)
+		assert.NoError(t, err)
+	})
+}