-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
Use cluster name as default subnet tag for Lyft CNI #8571
Conversation
/assign @rifelpet |
Looks good |
Hm i think technically this would be considered a breaking change. If someone's subnets were only tagged with Thoughts? |
@rifelpet I would say Release Notes / Breaking Changes mention should be a good idea. Kops tries to make these network plugins work out of the box as much as possible, to make it easier to get started. I also don't think the |
ff68fbe
to
11d06da
Compare
docs/releases/1.18-NOTES.md
Outdated
@@ -6,6 +6,8 @@ | |||
|
|||
* Terraform users on AWS may need to rename some resources in their state file in order to prepare for Terraform 0.12 support. See Required Actions below. | |||
|
|||
* Lyft CNI plugin default subnet tags changed from from `Type: pod` to `KubernetesCluster: myclustername.mydns.io`. |
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.
Maybe we should describe the action a user would want to take if they're affected by this.
Subnets intended for pod IP use will need to be tagged with this new tag, and additional tag filters may need to be added to the cluster spec in order to achieve the desired set of subnets.
or something along those lines.
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.
I think should be ok now.
11d06da
to
87bbcd6
Compare
One step closer to e2e for lyft cni! /lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: hakman, rifelpet 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 |
🤞 |
The Lyft CNI uses tags to determine which subnets to use when allocating new network adapters.
Current default is not matching anything, so the cluster is broken on startup. Using the cluster name for to determine the cluster subnets seems a better solution.
The the
conflist.template
was misaligned so I reformatted it.