Skip to content

Commit

Permalink
fix(controller): fix tasket warning in Non-HTTP Template scanerio (ar…
Browse files Browse the repository at this point in the history
…goproj#6518)

Signed-off-by: Saravanan Balasubramanian <[email protected]>
  • Loading branch information
sarabala1979 committed Aug 10, 2021
1 parent 863d226 commit 0670f65
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 4 deletions.
9 changes: 9 additions & 0 deletions pkg/apis/workflow/v1alpha1/workflow_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
}
Expand Down
21 changes: 21 additions & 0 deletions pkg/apis/workflow/v1alpha1/workflow_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
11 changes: 8 additions & 3 deletions workflow/controller/taskset.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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
}
Expand Down
25 changes: 24 additions & 1 deletion workflow/controller/taskset_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{})
Expand All @@ -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)
})
}

0 comments on commit 0670f65

Please sign in to comment.