Skip to content

Commit

Permalink
fix: fix e2e and unit tests
Browse files Browse the repository at this point in the history 
Signed-off-by: toyamagu-2021 <toyamagu2021@gmail.com>
  • Loading branch information
@toyamagu-2021
toyamagu-2021 committed Oct 9, 2023
1 parent 8402a1a commit 847a341
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 16 deletions.
19 changes: 15 additions & 4 deletions test/e2e/hooks_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -611,28 +611,39 @@ spec:
assert.Equal(t, status.Progress, v1alpha1.Progress("2/4"))
}).
ExpectWorkflowNode(func(status v1alpha1.NodeStatus) bool {
return strings.Contains(status.Name, "test-workflow-level-hooks-with-retry.hooks.running")
return status.Name == "test-workflow-level-hooks-with-retry.hooks.running"
}, func(t *testing.T, status *v1alpha1.NodeStatus, pod *apiv1.Pod) {
assert.Equal(t, v1alpha1.NodeSucceeded, status.Phase)
assert.Equal(t, true, status.NodeFlag.Hooked)
assert.Equal(t, false, status.NodeFlag.Retried)
}).
ExpectWorkflowNode(func(status v1alpha1.NodeStatus) bool {
return strings.Contains(status.Name, "test-workflow-level-hooks-with-retry.hooks.failed")
return status.Name == "test-workflow-level-hooks-with-retry.hooks.failed"
}, func(t *testing.T, status *v1alpha1.NodeStatus, pod *apiv1.Pod) {
assert.Equal(t, v1alpha1.NodeSucceeded, status.Phase)
assert.Equal(t, true, status.NodeFlag.Hooked)
assert.Equal(t, false, status.NodeFlag.Retried)
}).
ExpectWorkflowNode(func(status v1alpha1.NodeStatus) bool {
return strings.Contains(status.Name, "test-workflow-level-hooks-with-retry(0)")
return status.Name == "test-workflow-level-hooks-with-retry"
}, func(t *testing.T, status *v1alpha1.NodeStatus, pod *apiv1.Pod) {
assert.Equal(t, v1alpha1.NodeFailed, status.Phase)
assert.Equal(t, v1alpha1.NodeTypeRetry, status.Type)
assert.Nil(t, status.NodeFlag)
}).
ExpectWorkflowNode(func(status v1alpha1.NodeStatus) bool {
return status.Name == "test-workflow-level-hooks-with-retry(0)"
}, func(t *testing.T, status *v1alpha1.NodeStatus, pod *apiv1.Pod) {
assert.Equal(t, v1alpha1.NodeFailed, status.Phase)
assert.Equal(t, false, status.NodeFlag.Hooked)
assert.Equal(t, true, status.NodeFlag.Retried)
}).
ExpectWorkflowNode(func(status v1alpha1.NodeStatus) bool {
return strings.Contains(status.Name, "test-workflow-level-hooks-with-retry(1)")
return status.Name == "test-workflow-level-hooks-with-retry(1)"
}, func(t *testing.T, status *v1alpha1.NodeStatus, pod *apiv1.Pod) {
assert.Equal(t, v1alpha1.NodeFailed, status.Phase)
assert.Equal(t, false, status.NodeFlag.Hooked)
assert.Equal(t, true, status.NodeFlag.Retried)
})
}

Expand Down
18 changes: 17 additions & 1 deletion test/e2e/retry_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@ import (

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/suite"
apiv1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"github.com/argoproj/argo-workflows/v3/pkg/apis/workflow/v1alpha1"
wfv1 "github.com/argoproj/argo-workflows/v3/pkg/apis/workflow/v1alpha1"
"github.com/argoproj/argo-workflows/v3/test/e2e/fixtures"
)
Expand All @@ -23,7 +25,7 @@ func (s *RetryTestSuite) TestRetryLimit() {
s.Given().
Workflow(`
metadata:
generateName: test-retry-limit-
name: test-retry-limit
spec:
entrypoint: main
templates:
Expand All @@ -46,6 +48,20 @@ spec:
ExpectWorkflow(func(t *testing.T, _ *metav1.ObjectMeta, status *wfv1.WorkflowStatus) {
assert.Equal(t, wfv1.WorkflowPhase("Failed"), status.Phase)
assert.Equal(t, "No more retries left", status.Message)
assert.Equal(t, v1alpha1.Progress("0/1"), status.Progress)
}).
ExpectWorkflowNode(func(status v1alpha1.NodeStatus) bool {
return status.Name == "test-retry-limit"
}, func(t *testing.T, status *v1alpha1.NodeStatus, pod *apiv1.Pod) {
assert.Equal(t, v1alpha1.NodeFailed, status.Phase)
assert.Equal(t, v1alpha1.NodeTypeRetry, status.Type)
assert.Nil(t, status.NodeFlag)
}).
ExpectWorkflowNode(func(status v1alpha1.NodeStatus) bool {
return status.Name == "test-retry-limit(0)"
}, func(t *testing.T, status *v1alpha1.NodeStatus, pod *apiv1.Pod) {
assert.Equal(t, v1alpha1.NodeFailed, status.Phase)
assert.Equal(t, true, status.NodeFlag.Retried)
})
}

Expand Down
2 changes: 1 addition & 1 deletion workflow/controller/operator.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2044,7 +2044,7 @@ func (woc *wfOperationCtx) executeTemplate(ctx context.Context, nodeName string,
if processedTmpl.Synchronization != nil {
woc.controller.syncManager.Release(woc.wf, node.ID, processedTmpl.Synchronization)
}
lastChildNode := getChildNodeIndex(retryParentNode, woc.wf.Status.Nodes, -1)
_, lastChildNode := getChildNodeIdsAndLastRetriedNode(retryParentNode, woc.wf.Status.Nodes)
if lastChildNode != nil {
retryParentNode.Outputs = lastChildNode.Outputs.DeepCopy()
woc.wf.Status.Nodes.Set(node.ID, *retryParentNode)
Expand Down
36 changes: 26 additions & 10 deletions workflow/controller/operator_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -624,7 +624,7 @@ func TestProcessNodeRetriesOnErrors(t *testing.T) {
// Add child nodes.
for i := 0; i < 2; i++ {
childNode := fmt.Sprintf("%s(%d)", nodeName, i)
woc.initializeNode(childNode, wfv1.NodeTypePod, "", &wfv1.WorkflowStep{}, "", wfv1.NodeRunning, &wfv1.NodeFlag{})
woc.initializeNode(childNode, wfv1.NodeTypePod, "", &wfv1.WorkflowStep{}, "", wfv1.NodeRunning, &wfv1.NodeFlag{Retried: true})
woc.addChildNode(nodeName, childNode)
}

Expand Down Expand Up @@ -657,7 +657,7 @@ func TestProcessNodeRetriesOnErrors(t *testing.T) {

// Add a third node that has errored.
childNode := fmt.Sprintf("%s(%d)", nodeName, 3)
woc.initializeNode(childNode, wfv1.NodeTypePod, "", &wfv1.WorkflowStep{}, "", wfv1.NodeError, &wfv1.NodeFlag{})
woc.initializeNode(childNode, wfv1.NodeTypePod, "", &wfv1.WorkflowStep{}, "", wfv1.NodeError, &wfv1.NodeFlag{Retried: true})
woc.addChildNode(nodeName, childNode)
n, err = woc.wf.GetNodeByName(nodeName)
assert.NoError(t, err)
Expand Down Expand Up @@ -696,7 +696,7 @@ func TestProcessNodeRetriesOnTransientErrors(t *testing.T) {
// Add child nodes.
for i := 0; i < 2; i++ {
childNode := fmt.Sprintf("%s(%d)", nodeName, i)
woc.initializeNode(childNode, wfv1.NodeTypePod, "", &wfv1.WorkflowStep{}, "", wfv1.NodeRunning, &wfv1.NodeFlag{})
woc.initializeNode(childNode, wfv1.NodeTypePod, "", &wfv1.WorkflowStep{}, "", wfv1.NodeRunning, &wfv1.NodeFlag{Retried: true})
woc.addChildNode(nodeName, childNode)
}

Expand Down Expand Up @@ -734,7 +734,7 @@ func TestProcessNodeRetriesOnTransientErrors(t *testing.T) {

// Add a third node that has errored.
childNode := fmt.Sprintf("%s(%d)", nodeName, 3)
woc.initializeNode(childNode, wfv1.NodeTypePod, "", &wfv1.WorkflowStep{}, "", wfv1.NodeError, &wfv1.NodeFlag{})
woc.initializeNode(childNode, wfv1.NodeTypePod, "", &wfv1.WorkflowStep{}, "", wfv1.NodeError, &wfv1.NodeFlag{Retried: true})
woc.addChildNode(nodeName, childNode)
n, err = woc.wf.GetNodeByName(nodeName)
assert.NoError(t, err)
Expand Down Expand Up @@ -775,7 +775,7 @@ func TestProcessNodeRetriesWithBackoff(t *testing.T) {
lastChild := getChildNodeIndex(node, woc.wf.Status.Nodes, -1)
assert.Nil(t, lastChild)

woc.initializeNode(nodeName+"(0)", wfv1.NodeTypePod, "", &wfv1.WorkflowStep{}, "", wfv1.NodeRunning, &wfv1.NodeFlag{})
woc.initializeNode(nodeName+"(0)", wfv1.NodeTypePod, "", &wfv1.WorkflowStep{}, "", wfv1.NodeRunning, &wfv1.NodeFlag{Retried: true})
woc.addChildNode(nodeName, nodeName+"(0)")

n, err := woc.wf.GetNodeByName(nodeName)
Expand Down Expand Up @@ -830,7 +830,7 @@ func TestProcessNodeRetriesWithExponentialBackoff(t *testing.T) {
lastChild := getChildNodeIndex(node, woc.wf.Status.Nodes, -1)
require.Nil(lastChild)

woc.initializeNode(nodeName+"(0)", wfv1.NodeTypePod, "", &wfv1.WorkflowStep{}, "", wfv1.NodeFailed, &wfv1.NodeFlag{})
woc.initializeNode(nodeName+"(0)", wfv1.NodeTypePod, "", &wfv1.WorkflowStep{}, "", wfv1.NodeFailed, &wfv1.NodeFlag{Retried: true})
woc.addChildNode(nodeName, nodeName+"(0)")

n, err := woc.wf.GetNodeByName(nodeName)
Expand All @@ -847,7 +847,7 @@ func TestProcessNodeRetriesWithExponentialBackoff(t *testing.T) {
require.LessOrEqual(backoff, 300)
require.Less(295, backoff)

woc.initializeNode(nodeName+"(1)", wfv1.NodeTypePod, "", &wfv1.WorkflowStep{}, "", wfv1.NodeError, &wfv1.NodeFlag{})
woc.initializeNode(nodeName+"(1)", wfv1.NodeTypePod, "", &wfv1.WorkflowStep{}, "", wfv1.NodeError, &wfv1.NodeFlag{Retried: true})
woc.addChildNode(nodeName, nodeName+"(1)")
n, err = woc.wf.GetNodeByName(nodeName)
assert.NoError(t, err)
Expand Down Expand Up @@ -908,7 +908,7 @@ func TestProcessNodesNoRetryWithError(t *testing.T) {
// Add the parent node for retries.
nodeName := "test-node"
nodeID := woc.wf.NodeID(nodeName)
node := woc.initializeNode(nodeName, wfv1.NodeTypeRetry, "", &wfv1.WorkflowStep{}, "", wfv1.NodeRunning, &wfv1.NodeFlag{})
node := woc.initializeNode(nodeName, wfv1.NodeTypeRetry, "", &wfv1.WorkflowStep{}, "", wfv1.NodeRunning, &wfv1.NodeFlag{Retried: true})
retries := wfv1.RetryStrategy{}
retries.Limit = intstrutil.ParsePtr("2")
retries.RetryPolicy = wfv1.RetryPolicyOnFailure
Expand All @@ -923,7 +923,7 @@ func TestProcessNodesNoRetryWithError(t *testing.T) {
// Add child nodes.
for i := 0; i < 2; i++ {
childNode := fmt.Sprintf("%s(%d)", nodeName, i)
woc.initializeNode(childNode, wfv1.NodeTypePod, "", &wfv1.WorkflowStep{}, "", wfv1.NodeRunning, &wfv1.NodeFlag{})
woc.initializeNode(childNode, wfv1.NodeTypePod, "", &wfv1.WorkflowStep{}, "", wfv1.NodeRunning, &wfv1.NodeFlag{Retried: true})
woc.addChildNode(nodeName, childNode)
}

Expand Down Expand Up @@ -1040,6 +1040,8 @@ status:
templateName: retry-backoff
templateScope: local/retry-backoff-s69z6
type: Pod
nodeFlag:
retried: true
retry-backoff-s69z6-1807967148:
displayName: retry-backoff-s69z6(0)
finishedAt: "2020-05-05T15:18:43Z"
Expand Down Expand Up @@ -1071,6 +1073,8 @@ status:
templateName: retry-backoff
templateScope: local/retry-backoff-s69z6
type: Pod
nodeFlag:
retried: true
phase: Running
resourcesDuration:
cpu: 5
Expand Down Expand Up @@ -4545,6 +4549,8 @@ status:
templateName: echo
templateScope: local/echo-wngc4
type: Pod
nodeFlag:
retried: true
phase: Running
startedAt: "2020-05-07T17:40:57Z"
`
Expand Down Expand Up @@ -4638,6 +4644,8 @@ status:
templateName: echo
templateScope: local/echo-r6v49
type: Pod
nodeFlag:
retried: true
phase: Running
resourcesDuration:
cpu: 1
Expand Down Expand Up @@ -5489,7 +5497,7 @@ func TestPropagateMaxDurationProcess(t *testing.T) {
woc.wf.Status.Nodes[woc.wf.NodeID(nodeName)] = *node

childNode := fmt.Sprintf("%s(%d)", nodeName, 0)
woc.initializeNode(childNode, wfv1.NodeTypePod, "", &wfv1.WorkflowStep{}, "", wfv1.NodeFailed, &wfv1.NodeFlag{})
woc.initializeNode(childNode, wfv1.NodeTypePod, "", &wfv1.WorkflowStep{}, "", wfv1.NodeFailed, &wfv1.NodeFlag{Retried: true})
woc.addChildNode(nodeName, childNode)

var opts executeTemplateOpts
Expand Down Expand Up @@ -7969,6 +7977,8 @@ status:
templateName: retry-script
templateScope: local/retry-script-9z9pv
type: Pod
nodeFlag:
retried: true
retry-script-9z9pv-2346402485:
boundaryID: retry-script-9z9pv
children:
Expand Down Expand Up @@ -8004,6 +8014,8 @@ status:
templateName: retry-script
templateScope: local/retry-script-9z9pv
type: Pod
nodeFlag:
retried: true
retry-script-9z9pv-3940097040:
boundaryID: retry-script-9z9pv
children:
Expand Down Expand Up @@ -8157,6 +8169,8 @@ status:
templateName: retry-script
templateScope: local/retry-script-9z9pv
type: Pod
nodeFlag:
retried: true
retry-script-9z9pv-2346402485:
boundaryID: retry-script-9z9pv
children:
Expand Down Expand Up @@ -8192,6 +8206,8 @@ status:
templateName: retry-script
templateScope: local/retry-script-9z9pv
type: Pod
nodeFlag:
retried: true
retry-script-9z9pv-3940097040:
boundaryID: retry-script-9z9pv
children:
Expand Down

0 comments on commit 847a341

Please sign in to comment.