Skip to content

Commit

Permalink
fix: Do not create pods under shutdown strategy (#5055)
Browse files Browse the repository at this point in the history
Signed-off-by: Simon Behar <simbeh7@gmail.com>
  • Loading branch information
simster7 authored Feb 12, 2021
1 parent 75d09b0 commit 68979f6
Show file tree
Hide file tree
Showing 5 changed files with 94 additions and 7 deletions.
24 changes: 24 additions & 0 deletions test/e2e/functional/stop-terminate-2.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
generateName: stop-terminate-
labels:
argo-e2e: true
spec:
entrypoint: main
templates:
- name: main
steps:
- - name: A
template: sleep
- - name: B
template: pass

- name: sleep
container:
image: argoproj/argosay:v1
args: [ sleep, "999"]

- name: pass
container:
image: argoproj/argosay:v1
6 changes: 3 additions & 3 deletions test/e2e/functional/stop-terminate.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ spec:

- name: echo
container:
image: argoproj/argosay:v2
args: [ sleep, "999" ]
image: argoproj/argosay:v1
args: [ sleep, "999"]

- name: exit
container:
image: argoproj/argosay:v2
image: argoproj/argosay:v1
32 changes: 28 additions & 4 deletions test/e2e/signals_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func (s *SignalsSuite) TestStopBehavior() {
assert.NoError(t, err)
assert.Regexp(t, "workflow stop-terminate-.* stopped", output)
}).
WaitForWorkflow().
WaitForWorkflow(1 * time.Minute).
Then().
ExpectWorkflow(func(t *testing.T, m *metav1.ObjectMeta, status *wfv1.WorkflowStatus) {
assert.Equal(t, wfv1.WorkflowFailed, status.Phase)
Expand Down Expand Up @@ -63,7 +63,7 @@ func (s *SignalsSuite) TestTerminateBehavior() {
assert.NoError(t, err)
assert.Regexp(t, "workflow stop-terminate-.* terminated", output)
}).
WaitForWorkflow().
WaitForWorkflow(1 * time.Minute).
Then().
ExpectWorkflow(func(t *testing.T, m *metav1.ObjectMeta, status *wfv1.WorkflowStatus) {
assert.Equal(t, wfv1.WorkflowFailed, status.Phase)
Expand All @@ -78,6 +78,30 @@ func (s *SignalsSuite) TestTerminateBehavior() {
})
}

// Tests that new pods are never created once a stop shutdown strategy has been added
func (s *SignalsSuite) TestDoNotCreatePodsUnderStopBehavior() {
s.Given().
Workflow("@functional/stop-terminate-2.yaml").
When().
SubmitWorkflow().
WaitForWorkflow(fixtures.ToStart, "to start").
RunCli([]string{"stop", "@latest"}, func(t *testing.T, output string, err error) {
assert.NoError(t, err)
assert.Regexp(t, "workflow stop-terminate-.* stopped", output)
}).
WaitForWorkflow(1 * time.Minute).
Then().
ExpectWorkflow(func(t *testing.T, m *metav1.ObjectMeta, status *wfv1.WorkflowStatus) {
assert.Equal(t, wfv1.WorkflowFailed, status.Phase)
nodeStatus := status.Nodes.FindByDisplayName("A")
if assert.NotNil(t, nodeStatus) {
assert.Equal(t, wfv1.NodeFailed, nodeStatus.Phase)
}
nodeStatus = status.Nodes.FindByDisplayName("B")
assert.Nil(t, nodeStatus)
})
}

func (s *SignalsSuite) TestPropagateMaxDuration() {
s.T().Skip("too hard to get working")
s.Given().
Expand Down Expand Up @@ -106,7 +130,7 @@ spec:
`).
When().
SubmitWorkflow().
WaitForWorkflow(45 * time.Second).
WaitForWorkflow(1 * time.Minute).
Then().
ExpectWorkflow(func(t *testing.T, _ *metav1.ObjectMeta, status *wfv1.WorkflowStatus) {
assert.Equal(t, wfv1.WorkflowFailed, status.Phase)
Expand All @@ -123,7 +147,7 @@ func (s *SignalsSuite) TestSidecars() {
Workflow("@testdata/sidecar-workflow.yaml").
When().
SubmitWorkflow().
WaitForWorkflow().
WaitForWorkflow(1 * time.Minute).
Then().
ExpectWorkflow(func(t *testing.T, _ *metav1.ObjectMeta, status *wfv1.WorkflowStatus) {
assert.Equal(t, wfv1.WorkflowSucceeded, status.Phase)
Expand Down
33 changes: 33 additions & 0 deletions workflow/controller/operator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5770,6 +5770,7 @@ func TestParamAggregation(t *testing.T) {
}
}
}

func TestRetryOnDiffHost(t *testing.T) {
cancel, controller := newController()
defer cancel()
Expand Down Expand Up @@ -5836,3 +5837,35 @@ func TestRetryOnDiffHost(t *testing.T) {
}
assert.Equal(t, sourceNodeSelectorRequirement, targetNodeSelectorRequirement)
}

var noPodsWhenShutdown = `
apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
name: hello-world
spec:
entrypoint: whalesay
shutdown: "Stop"
templates:
- name: whalesay
container:
image: docker/whalesay:latest
command: [cowsay]
args: ["hello world"]
`

func TestNoPodsWhenShutdown(t *testing.T) {
wf := unmarshalWF(noPodsWhenShutdown)
cancel, controller := newController(wf)
defer cancel()

ctx := context.Background()
woc := newWorkflowOperationCtx(wf, controller)
woc.operate(ctx)

node := woc.wf.Status.Nodes.FindByDisplayName("hello-world")
if assert.NotNil(t, node) {
assert.Equal(t, wfv1.NodeSkipped, node.Phase)
assert.Contains(t, node.Message, "workflow shutdown with strategy: Stop")
}
}
6 changes: 6 additions & 0 deletions workflow/controller/workflowpod.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,12 @@ func (woc *wfOperationCtx) createWorkflowPod(ctx context.Context, nodeName strin
}
}

if !woc.execWf.Spec.Shutdown.ShouldExecute(opts.onExitPod) {
// Do not create pods if we are shutting down
woc.markNodePhase(nodeName, wfv1.NodeSkipped, fmt.Sprintf("workflow shutdown with strategy: %s", woc.execWf.Spec.Shutdown))
return nil, nil
}

tmpl = tmpl.DeepCopy()
wfSpec := woc.execWf.Spec.DeepCopy()

Expand Down

0 comments on commit 68979f6

Please sign in to comment.