Restart task status.Succeeded fixes #440
Conversation
|
One note, in our original discussion @burmanm suggested that we need to ensure that the revision has updated before setting I think this is actually already done here so I haven't done anything further with that. I also wonder if there is a chance that We could feasibly use a map of sts->UpdatedReplicas to keep better track. I might experiment with implementing that and see if I can make it work. |
Miles-Garnsey
changed the title
controllers/control/jobs.go
Outdated
|
|
||
| restartedPods[st.Name] = int(status.UpdatedReplicas) | ||
| totalRestarted := 0 | ||
| for _, v := range restartedPods { |
There was a problem hiding this comment.
This code is never called with finished StSes as they jump out of the loop before this is reached.
There was a problem hiding this comment.
I know, but we want unfinished ones to be tracked too, don't we?
There was a problem hiding this comment.
We do, but now we won't track the finished ones at all. We should update the TaskConfig.Completed inside that part of the loop also. Or use a pointer and update that one in TaskConfig..
There was a problem hiding this comment.
Can you check my latest commit and confirm that's what you're after?
|
.. just add missing changelog ;) |
* Restart task completion fixes. * Add test for success tracking. * Juggle types in test so that they match. * More precise logic for tracking number of pods restarted for all STSs. * Bring back simple int for restarted pods calculation. * Ensure taskConfig.Completed is updated when the restart is completed. * Changelog.
What this PR does:
Ensure that the number of pods successfully restarted is tracked via the
UpdatedReplicasstatus field.Which issue(s) this PR fixes:
Fixes #431
Checklist