-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Remove references to ClusterSpec.API from nodeup #15615
Conversation
pkg/apis/nodeup/config.go
Outdated
@@ -64,6 +64,8 @@ type Config struct { | |||
KubeProxy *kops.KubeProxyConfig | |||
// Networking configures networking. | |||
Networking kops.NetworkingSpec | |||
// API controls how the Kubernetes API is exposed. | |||
API *kops.APISpec `json:",omitempty"` |
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.
Devil's advocate: are we just going to end up copying the Cluster into the nodeup.Config object? Why do we have two objects?
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.
We only copy fields that affect the configurations of nodes of the appropriate type. Changes to NodeupConfig result in nodes of the relevant instance group being selected for rolling update, whereas changes to the ClusterSpec do not. So since we only populate this new field for control-plane and api-server nodes, changes to the sub-fields we populate result in control-plane and api-server nodes being selected for update, but worker nodes not. Previously, no nodes would be selected for update, so control-plane and api-server nodes would continue to run with outdated configuration.
/retest |
bb28cb3
to
c96caa7
Compare
As discussed in office hours, this lgtm. /approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: justinsb The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
c96caa7
to
62f7faa
Compare
/retest |
/lgtm |
No description provided.