fix(controller): Fix node status when daemon pod deleted but its children nodes are still running #4683
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
simster7
reviewed
Dec 9, 2020
Comment on lines
+694
to
+697
| node := d.getTaskNode(taskName) | ||
| if node != nil { | ||
| return true, true, nil | ||
| } |
There was a problem hiding this comment.
Can you explain this particular change more? Not sure if we want this
There was a problem hiding this comment.
The case for this code is "explained" in the unittest.
…d. Fixes (argoproj#4652) Signed-off-by: lons <lonsdale8734@gmail.com>
…rgoproj#4665) Signed-off-by: lons <lonsdale8734@gmail.com>
lonsdale8734
force-pushed
the
fix-daemon-pod
branch
from
fcc8fb8 to
9c0dbfd
Compare
alexec
reviewed
Dec 18, 2020
workflow/controller/operator.go
Outdated
| @@ -917,6 +917,7 @@ func (woc *wfOperationCtx) podReconciliation() error { | |||
|
|
|||
| node.Message = "pod deleted" | |||
| node.Phase = wfv1.NodeError | |||
| node.Daemoned = nil | |||
There was a problem hiding this comment.
what is this needed for please?
There was a problem hiding this comment.
When daemon pod deleted and Daemoned flag in node is true, there is a bug that the Deaemoned flag never removed and the whole workflow will never completed.
alexec
reviewed
Dec 18, 2020
| @@ -1069,6 +1070,7 @@ func (woc *wfOperationCtx) assessNodeStatus(pod *apiv1.Pod, node *wfv1.NodeStatu | |||
| if pod.DeletionTimestamp != nil { | |||
| // pod is being terminated | |||
| newPhase = wfv1.NodeError | |||
| newDaemonStatus = pointer.BoolPtr(false) | |||
lonsdale8734
changed the title
simster7
approved these changes
Jan 4, 2021
This was referenced Jan 13, 2021
Closed
simster7
pushed a commit
that referenced
this pull request
Jan 19, 2021
…dren nodes are still running (#4683) Signed-off-by: lons <lonsdale8734@gmail.com>
|
This fix is out on https://github.com/argoproj/argo/releases/tag/v2.12.5 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Checklist: