-
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
Create etcd-manager config for each instance group #14080
Conversation
/cc @justinsb @olemarkus @rifelpet |
/retest |
Looks good. You are only using IG-specific tags for hetzner now, right? nothing changes for AWS? |
Yes, only Hetzner and flag is still up to debate. |
9c39b19
to
2b0342a
Compare
2b0342a
to
b9394fe
Compare
/hold cancel |
return err | ||
} | ||
for _, member := range etcdCluster.Members { | ||
instanceGroupName := fi.StringValue(member.InstanceGroup) |
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.
It might be that we should use the member name here. On the one hand, it's the primary key and therefore maybe "more correct", and maybe avoids the problem of then wanting to lock instance groups to members. On the other hand, it means users must be more aware of etcd member names, when they are probably more familiar with instance group names.
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.
Edit: but now I see that it the mapping in nodeup below becomes much more straightforward if we stick to instance groups...
Thanks @hakman this lgtm, and will help us be more precise with rolling updates etc. I now also understand better why you want to try to keep instance groups to a specific member - that makes a lot more sense now that I see it! /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 |
[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 |
kOps should give a way for
etcd-manager
to identify a specific volume for a specific control-plane node. This means having access to the instance group, as defined in the cluster config:kops/pkg/apis/kops/cluster.go
Lines 664 to 665 in 70c8528
This can be done either directly in
etcd-manager
, by having some hardcoded tags, or by passing a specific config for each instance group, which includes the instance group tag. My preference is for the latter.