-
Notifications
You must be signed in to change notification settings - Fork 0
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
setting statuses, and triggering reconciliation #17
Conversation
kristofgyuracz
commented
Jan 29, 2024
- scratch implementation
- api changes
- set tenant-> collector ref and notify controller on change
- triggering for relevant changes, disowning, and checking ownership before adopting tenants and subscriptions
Signed-off-by: Peter Wilcsinszky <peter.wilcsinszky@axoflow.com>
Signed-off-by: Kristof Gyuracz <kristof.gyuracz@gmail.com>
Signed-off-by: Kristof Gyuracz <kristof.gyuracz@gmail.com>
…fore adopting tenants and subscriptions Signed-off-by: Kristof Gyuracz <kristof.gyuracz@gmail.com>
Signed-off-by: Kristóf Gyurácz <4431202+kristofgyuracz@users.noreply.github.com>
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.
Looks good to me overall, however we need to make sure we handle errors consistently.
Generally there is nothing wrong with returning an error and waiting for the next reconcile to fix it, but in this case we build a system where a misconfigured tenant/subscription should have the least possible effect on the other actors.
In this case if one subscription cannot properly be updated, then we get stuck and no configuration changes can be made anymore on the collector in general.
Update: we just want to handle subscription update errors in this special way to make sure a user updating their subscription won't be able to intentionally or accidentally block the system
42f5993
to
fccde59
Compare
Signed-off-by: Kristof Gyuracz <kristof.gyuracz@gmail.com>
fccde59
to
79aefac
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.
Overall LGTM, two minor notes only.
Signed-off-by: Kristof Gyuracz <kristof.gyuracz@gmail.com>
331bdbd
to
122806f
Compare
Signed-off-by: Kristof Gyuracz <kristof.gyuracz@gmail.com>
Signed-off-by: Peter Wilcsinszky <peter.wilcsinszky@axoflow.com>
Signed-off-by: Kristof Gyuracz <kristof.gyuracz@gmail.com>
d4f96da
to
3036752
Compare
Signed-off-by: Kristof Gyuracz <kristof.gyuracz@gmail.com>
3036752
to
8e67060
Compare
Signed-off-by: Kristof Gyuracz <kristof.gyuracz@gmail.com>
Signed-off-by: Peter Wilcsinszky <peter.wilcsinszky@axoflow.com>
Signed-off-by: Peter Wilcsinszky <peter.wilcsinszky@axoflow.com>
Signed-off-by: Peter Wilcsinszky <peter.wilcsinszky@axoflow.com>
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.
looks great, a few nits I've added: put arm64 image build back and equality checks before collector/tenant updates
This reverts commit 71834f8. Signed-off-by: Kristof Gyuracz <kristof.gyuracz@gmail.com>
b0fcce1
to
4bd14ce
Compare