-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
implement autoupdate_agent_rollout reconciler #48241
implement autoupdate_agent_rollout reconciler #48241
Conversation
|
||
func (h *createUpdateHandler[T]) handle(_ context.Context, object T) (T, error) { | ||
if len(h.expect) == 0 { | ||
require.Fail(h.t, "not expecting more calls") |
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.
Is this guaranteed to be executed in the goroutine running the test? The require
helpers will fail the test and then goexit, which won't actually stop the test if it's a goroutine calling the helpers.
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.
Currently yes, and I don't plan to write multi-goroutine tests if possible. Single-routine tests are easier to await and debug. I'm losing a lot of time because of the async reconciliation in the operator tests and prefer to do everything synchronously + use require.
21b0779
to
b4e12e5
Compare
31e966f
to
54f3875
Compare
97a479d
to
3bcb41e
Compare
@hugoShaka See the table below for backport results.
|
This PR does the first implementation of the autoupdate agent rollout reconciler.
The code is not used yet, once this PR is merged I will add a controller that will run every minute in the auth server and calls the reconciler.
Sorry for the large PR (but it's 75% tests I swear 😅 ), I thought it made no sense to send
tryReconcile()
without also sendingReconcile()