-
Notifications
You must be signed in to change notification settings - Fork 152
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.
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
Update Controller To Track Action Progress #1487
Conversation
Signed-off-by: Ivan Sim <ivan.sim@kasten.io>
Thanks for submitting this pull request 🎉. The team will review it soon and get back to you. If you haven't already, please take a moment to review our project contributing guideline and Code of Conduct document. |
Signed-off-by: Ivan Sim <ivan.sim@kasten.io>
6b8f53b
to
942ee9f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1st review with minor nits. Thorough review in progress 🦅 👀
@PrasadG193 @viveksinghggits Please review this 👍🏼 |
pkg/progress/action.go
Outdated
client versioned.Interface, | ||
actionSetName string, | ||
namespace string) error { | ||
var err error |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good 👁️. Yes, it can be removed.
Should we add progress in the additional printer columns so that someone can just figure out the progress by looking at the output of |
Signed-off-by: Ivan Sim <ivan.sim@kasten.io>
Signed-off-by: Ivan Sim <ivan.sim@kasten.io>
Signed-off-by: Ivan Sim <ivan.sim@kasten.io>
@viveksinghggits that's a good idea. Now we can $ watch k -n kanister get actionset
NAME PROGRESS LAST TRANSITION TIME STATE
multi-actions-zmfj4 100.00 2022-07-20T23:41:11Z complete
multi-actions-fl5w9 87.50 2022-07-21T02:33:12Z running |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🚀
Change Overview
This PR updates the controller to track the progress of an actionset, by assessing the phase states. The progress tracker runs in its own goroutine with a 2-second ticker. The progress computation assigns weight to a phase, per the underlying Kanister Function the supports the phase. If a Kanister Function is marked as a long-running one, its phase will be assigned the
heavy
weight. Otherwise, the phase has thenormal
weight.The actionset progress always starts at 10%, to distinguish started/running phases from idle ones, and ends at 100%.
The computation is also done on a best-effort basis, with no elaborated retries on update errors, to ensure the progress goroutine doesn't contend for resources nor contribute to performance latency. In other words, it's possible that the progress goes from 10% to 100% with no immediate updates.
Design doc : https://github.com/kanisterio/kanister/blob/master/design/progress-tracking.md
Out-of-scope: Progress computation of each phase based on log outputs are slated for future PRs.
Release note: The
actionSet
CRD must be upgraded manually, as Helm doesn't upgrade CRD.Pull request type
Please check the type of change your PR introduces:
Issues
Test Plan
Deploy this multi-actions blueprint, where each phase has different completion time:
Create this multi-actions
ActionSet
to invoke actions in the blueprint:The result should look like this:
progrress: 37.50 last transition time: 2022-06-16T16:18:01Z progress: 37.50 last transition time: 2022-06-16T16:18:03Z progress: 37.50 last transition time: 2022-06-16T16:18:05Z progress: 37.50 last transition time: 2022-06-16T16:18:07Z progress: 37.50 last transition time: 2022-06-16T16:18:07Z progress: 37.50 last transition time: 2022-06-16T16:18:07Z progress: 62.50 last transition time: 2022-06-16T16:18:09Z percent: 62.50 # ....