-
Notifications
You must be signed in to change notification settings - Fork 7
feat: use registryMirror addon as Containerd mirror #1117
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
Conversation
a16bb7f
to
3f763d1
Compare
683a3e3
to
6e6c70b
Compare
042b7da
to
ce91230
Compare
dd251db
to
816ffaa
Compare
6a7dde3
to
71bc9ca
Compare
This PR/issue depends on:
|
71bc9ca
to
d1405b5
Compare
d1405b5
to
cd30704
Compare
cd30704
to
e8b1a8d
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.
LGTM
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.
one q. if user removes registry config from cluster spec after deployment, will it generate new containerd patch without bootstrap registry config and apply it to live cluster?
Thats correct, let me create a ticket, maybe we should have a validation webhook here to prevent this. Not sure though because there are lots of other input we expect clients to understand what changing it means and nothing prevents it there. |
c301a1d
to
6cd49d1
Compare
6cd49d1
to
0eb39bf
Compare
Makes sense. Either we block users to not remove registry config from cluster spec or we should ensure that removal will cleanup everything and it becomes connected site cluster. It would be good to have integration/e2e test for this behavior. |
What problem does this PR solve?:
Depends on #1116
Automatically sets the registryMirror as a Containerd mirror. We're not updating the Cluster object because this should not be a user controller configuration and the IP used as the mirror is determined based on the addon handler and the Service CIDRs.
Tested in a Docker cluster:
Which issue(s) this PR fixes:
Fixes #
How Has This Been Tested?:
Special notes for your reviewer: