-
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
Extend the defer phase state checks in the unit test #1397
Conversation
Since the defer phase is executed a Go 'defer' statement of a separate go routine, the test needs to account for racey scenarios where the phase state is still empty. Signed-off-by: Ivan Sim <ivan.sim@kasten.io>
@pavannd1 @viveksinghggits I don't quite understand why the controller log shows that the same blueprint is added multiple times. All of them say [36m[2022-04-21T20:11:25.785Z][0m {"File":"pkg/controller/controller.go","Function":"github.com/kanisterio/kanister/pkg/controller.(*Controller).onAddBlueprint","Line":215,"hostname":"a48311680e48","level":"info","msg":"Added blueprint test-blueprint-defer-d5hlk","time":"2022-04-21T19:54:46.738448315Z"}
[36m[2022-04-21T20:11:25.786Z][0m {"File":"pkg/controller/controller.go","Function":"github.com/kanisterio/kanister/pkg/controller.(*Controller).onAddBlueprint","Line":215,"hostname":"a48311680e48","level":"info","msg":"Added blueprint test-blueprint-defer-d5hlk","time":"2022-04-21T19:54:53.740559764Z"}
[36m[2022-04-21T20:11:25.786Z][0m {"File":"pkg/controller/controller.go","Function":"github.com/kanisterio/kanister/pkg/controller.(*Controller).onAddBlueprint","Line":215,"hostname":"a48311680e48","level":"info","msg":"Added blueprint test-blueprint-defer-d5hlk","time":"2022-04-21T19:54:54.644069197Z"}
[36m[2022-04-21T20:11:25.786Z][0m {"File":"pkg/controller/controller.go","Function":"github.com/kanisterio/kanister/pkg/controller.(*Controller).onAddBlueprint","Line":215,"hostname":"a48311680e48","level":"info","msg":"Added blueprint test-blueprint-defer-d5hlk","time":"2022-04-21T19:54:56.373873872Z"}
[36m[2022-04-21T20:11:25.786Z][0m {"File":"pkg/controller/controller.go","Function":"github.com/kanisterio/kanister/pkg/controller.(*Controller).onAddBlueprint","Line":215,"hostname":"a48311680e48","level":"info","msg":"Added blueprint test-blueprint-defer-d5hlk","time":"2022-04-21T19:54:59.714220153Z"}
[36m[2022-04-21T20:11:25.788Z][0m {"File":"pkg/controller/controller.go","Function":"github.com/kanisterio/kanister/pkg/controller.(*Controller).onAddBlueprint","Line":215,"hostname":"a48311680e48","level":"info","msg":"Added blueprint test-blueprint-defer-d5hlk","time":"2022-04-21T19:56:00.648875372Z"}
[36m[2022-04-21T20:11:25.789Z][0m {"File":"pkg/controller/controller.go","Function":"github.com/kanisterio/kanister/pkg/controller.(*Controller).onAddBlueprint","Line":215,"hostname":"a48311680e48","level":"info","msg":"Added blueprint test-blueprint-defer-d5hlk","time":"2022-04-21T19:56:03.49233199Z"}
[36m[2022-04-21T20:11:25.789Z][0m {"File":"pkg/controller/controller.go","Function":"github.com/kanisterio/kanister/pkg/controller.(*Controller).onAddBlueprint","Line":215,"hostname":"a48311680e48","level":"info","msg":"Added blueprint test-blueprint-defer-d5hlk","time":"2022-04-21T19:56:06.980262243Z"}
[36m[2022-04-21T20:11:25.789Z][0m {"File":"pkg/controller/controller.go","Function":"github.com/kanisterio/kanister/pkg/controller.(*Controller).onAddBlueprint","Line":215,"hostname":"a48311680e48","level":"info","msg":"Added blueprint test-blueprint-defer-d5hlk","time":"2022-04-21T19:56:10.743418453Z"}
[36m[2022-04-21T20:11:25.789Z][0m {"File":"pkg/controller/controller.go","Function":"github.com/kanisterio/kanister/pkg/controller.(*Controller).onAddBlueprint","Line":215,"hostname":"a48311680e48","level":"info","msg":"Added blueprint test-blueprint-defer-d5hlk","time":"2022-04-21T19:56:14.08697098Z"} |
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
I think the problem is, in the handler of blueprint we are just logging that blueprint was added but not marking the object from queue to be done. That is the reason, we are able to see stale objects as well in the queue
I will raise a PR to fix this. |
It looks like we are not even maintaining the |
Merging - got a 👍 from @pavannd1. |
Since the defer phase is executed a Go 'defer' statement of a separate go routine, the test needs to account for racey scenarios where the phase state is still empty. Signed-off-by: Ivan Sim <ivan.sim@kasten.io> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Change Overview
This PR attempts to fix the flakiness around the defer phase unit tests by extending the defer phase checks in the unit test to account for states with empty string value.
Following this log from one of the failed CI runs, notice that the defer phase of the
test-actionset-sg4qq
actionset is set to completed after the test has failed:Pull request type
Please check the type of change your PR introduces:
Test Plan