Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: Flakey test about lifecycle hooks #11534

Merged
merged 12 commits into from
Aug 7, 2023
Merged

fix: Flakey test about lifecycle hooks #11534

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
46c1a21
fix: Flaky test about lifecycle hooks
toyamagu-2021 Aug 6, 2023
3e99181
chore: empty commit
toyamagu-2021 Aug 6, 2023
2924e2e
chore: empty commit
toyamagu-2021 Aug 6, 2023
c2ba618
chore: empty commit
toyamagu-2021 Aug 6, 2023
bf47e8f
chore: empty commit
toyamagu-2021 Aug 6, 2023
f07a3cb
chore: empty commit
toyamagu-2021 Aug 6, 2023
bb18394
chore: empty commit
toyamagu-2021 Aug 6, 2023
34a1466
chore: empty commit
toyamagu-2021 Aug 6, 2023
c10ad0c
chore: empty commit
toyamagu-2021 Aug 6, 2023
14f069a
chore: commit empty
toyamagu-2021 Aug 6, 2023
1ec76f7
fix: wait after create cm
toyamagu-2021 Aug 7, 2023
020f6ca
fix: increase sleep time
toyamagu-2021 Aug 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 27 additions & 30 deletions test/e2e/hooks_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -344,36 +344,35 @@ spec:
hooks:
running:
expression: workflow.status == "Running"
template: sleep
template: argosay-sleep-2seconds
# This hook never triggered by following test.
# To guarantee workflow does not wait forever for untriggered hooks.
failed:
expression: workflow.status == "Failed"
template: sleep
template: argosay-sleep-2seconds
templates:
- name: main
steps:
- - name: step1
template: exit0
template: argosay

- name: exit0
- name: argosay
container:
image: alpine:latest
image: argoproj/argosay:v2
command: ["/bin/sh", "-c"]
args: ["exit 0"]
- name: sleep
args: ["/bin/sleep 1; /argosay"]
- name: argosay-sleep-2seconds
container:
image: alpine:latest
image: argoproj/argosay:v2
Copy link
Member

@agilgur5 agilgur5 Aug 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what was the purpose of changing some of these from Alpine to argosay? didn't see a mention of that in the PR description

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!
Not essential, just the same as the others.
https://github.com/argoproj/argo-workflows/blob/20d0923611f1df6b7147c3547aeeff6b6bfecf18/test/e2e/hooks_test.go#L89C1-L99

Copy link
Member

@agilgur5 agilgur5 Aug 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah consistency with this rest of this file, gotcha.
I searched the rest of the codebase and found that alpine:latest is used with some frequency.
So didn't think that this one particularly stood out, but in the context of the file, it appears to have been inconsistent. Thanks for the clarification!

command: ["/bin/sh", "-c"]
args: ["/bin/sleep 2; exit 0"]
args: ["/bin/sleep 2; /argosay"]
`).When().
SubmitWorkflow().
WaitForWorkflow(fixtures.ToBeSucceeded).
Then().
ExpectWorkflow(func(t *testing.T, metadata *v1.ObjectMeta, status *v1alpha1.WorkflowStatus) {
assert.Equal(t, status.Phase, v1alpha1.WorkflowSucceeded)
// TODO: This is sometimes "1/1" which might be a bug we need to investigate later.
//assert.Equal(t, status.Progress, v1alpha1.Progress("2/2"))
assert.Equal(t, status.Progress, v1alpha1.Progress("2/2"))
assert.Equal(t, 1, int(status.Progress.N()/status.Progress.M()))
}).
ExpectWorkflowNode(func(status v1alpha1.NodeStatus) bool {
Expand All @@ -396,34 +395,32 @@ spec:
- name: main
steps:
- - name: job
template: exit0
template: argosay
hooks:
running:
expression: steps['job'].status == "Running"
template: hook
template: argosay-sleep-2seconds
failed:
expression: steps['job'].status == "Failed"
template: hook
- name: hook
script:
image: alpine:latest
command: [/bin/sh]
source: |
sleep 2
- name: exit0
script:
image: alpine:latest
command: [/bin/sh]
source: |
exit 0
template: argosay-sleep-2seconds

- name: argosay
container:
image: argoproj/argosay:v2
command: ["/bin/sh", "-c"]
args: ["/bin/sleep 1; /argosay"]
- name: argosay-sleep-2seconds
container:
image: argoproj/argosay:v2
command: ["/bin/sh", "-c"]
args: ["/bin/sleep 2; /argosay"]
`).When().
SubmitWorkflow().
WaitForWorkflow(fixtures.ToBeSucceeded).
Then().
ExpectWorkflow(func(t *testing.T, metadata *v1.ObjectMeta, status *v1alpha1.WorkflowStatus) {
assert.Equal(t, status.Phase, v1alpha1.WorkflowSucceeded)
// TODO: This is sometimes "1/1" which might be a bug we need to investigate later.
//assert.Equal(t, status.Progress, v1alpha1.Progress("2/2"))
assert.Equal(t, status.Progress, v1alpha1.Progress("2/2"))
}).
ExpectWorkflowNode(func(status v1alpha1.NodeStatus) bool {
return strings.Contains(status.Name, "job.hooks.running")
Expand Down Expand Up @@ -462,13 +459,13 @@ spec:
image: alpine:latest
command: [/bin/sh]
source: |
sleep 3
sleep 4
- name: exit0
script:
image: alpine:latest
command: [/bin/sh]
source: |
sleep 1
sleep 2
exit 0
`).When().
SubmitWorkflow().
Expand Down
4 changes: 4 additions & 0 deletions test/e2e/workflow_configmap_substitution_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package e2e

import (
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/suite"
Expand Down Expand Up @@ -52,6 +53,7 @@ spec:
"cmref-parameters",
map[string]string{"msg": "hello world"},
map[string]string{"workflows.argoproj.io/configmap-type": "Parameter"}).
Wait(1 * time.Second).
SubmitWorkflow().
WaitForWorkflow(fixtures.ToBeSucceeded).
DeleteConfigMap("cmref-parameters").
Expand Down Expand Up @@ -95,6 +97,7 @@ spec:
"cmref-parameters",
map[string]string{"msg": "hello world"},
map[string]string{"workflows.argoproj.io/configmap-type": "Parameter"}).
Wait(1 * time.Second).
SubmitWorkflow().
WaitForWorkflow(fixtures.ToBeSucceeded).
DeleteConfigMap("cmref-parameters").
Expand Down Expand Up @@ -173,6 +176,7 @@ spec:
"cmref-parameters",
map[string]string{"msg": "hello world"},
map[string]string{"workflows.argoproj.io/configmap-type": "Parameter"}).
Wait(1 * time.Second).
SubmitWorkflow().
WaitForWorkflow(fixtures.ToBeSucceeded).
DeleteConfigMap("cmref-parameters").
Expand Down
4 changes: 4 additions & 0 deletions test/e2e/workflow_inputs_orverridable_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package e2e

import (
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/suite"
Expand Down Expand Up @@ -53,6 +54,7 @@ spec:
"cmref-parameters",
map[string]string{"cmref-key": "input-value"},
map[string]string{"workflows.argoproj.io/configmap-type": "Parameter"}).
Wait(1 * time.Second).
SubmitWorkflow().
WaitForWorkflow(fixtures.ToBeSucceeded).
DeleteConfigMap("cmref-parameters").
Expand Down Expand Up @@ -105,6 +107,7 @@ spec:
"new-cmref-parameters",
map[string]string{"cmref-key": "arg-value"},
map[string]string{"workflows.argoproj.io/configmap-type": "Parameter"}).
Wait(1 * time.Second).
SubmitWorkflow().
WaitForWorkflow(fixtures.ToBeSucceeded).
DeleteConfigMap("cmref-parameters").
Expand Down Expand Up @@ -188,6 +191,7 @@ spec:
"cmref-parameters",
map[string]string{"cmref-key": "arg-value"},
map[string]string{"workflows.argoproj.io/configmap-type": "Parameter"}).
Wait(1 * time.Second).
SubmitWorkflow().
WaitForWorkflow(fixtures.ToBeSucceeded).
DeleteConfigMap("cmref-parameters").
Expand Down