-
Notifications
You must be signed in to change notification settings - Fork 13
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
Add CAPI endpoints for Certificates Refresh #699
Conversation
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.
Thanks a lot @mateoflorido! Left some minor comments.
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.
Great work overall. +1 on Hues comments and please add a short comment on why we wait before restarting. This is IMHO not obvious because k8sd
is not restarted as part of RestartControlPlaneServices
@HomayoonAlimohammadi @bschimke95 I have created a new PR for including this endpoints. I will cut a release and update the endpoints once this is merged: canonical/k8s-snap-api#10 |
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, one question on the endpoints. These are per-node endpoints that are called right? For any of the control plane(k8sd cluster) we should use capi-auth-token
instead of the node token.
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, just a final comment. Feel free to ignore if you think this is not a problem
Refactor Certificates Refresh endpoints to flush the response early and restart the services asynchronously
Overview
Add new CAPI endpoints to manage certificates refreshes
Rationale
In this pull request, we have added a new set of handlers that allow access from CAPI to the refresh certificates endpoints using the node token as the authentication mechanism. Additionally, we have modified the handler to return the certificate refresh response earlier, deferring the restart until after the response has been flushed. Also, added a couple of drive-by fixes related to the RPC endpoints for Certificates Refreshes.