-
Notifications
You must be signed in to change notification settings - Fork 38.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
Skip node cidr mask size check for cloud allocation #68952
Conversation
/assign @bowei |
/priority important-soon |
Tested. We can now create ip alias clusters with pod cidr range smaller than /24. |
/kind bug |
@@ -110,7 +110,7 @@ func NewNodeIpamController( | |||
glog.Fatal("Controller: Must specify --cluster-cidr if --allocate-node-cidrs is set") | |||
} | |||
mask := clusterCIDR.Mask | |||
if maskSize, _ := mask.Size(); maskSize > nodeCIDRMaskSize { | |||
if maskSize, _ := mask.Size(); allocatorType != ipam.CloudAllocatorType && maskSize > nodeCIDRMaskSize { |
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's clearer reading the code to do this:
if allocatorType != ipam.CloudAllocatorType {
// Note: CloudAllocatorType does not rely on clusterCIDR or nodeCIDRMaskSize for allocation.
if maskSize, _ := mask.Size(); maskSize > nodeCIDRMaskSize {
...
}
}
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.
Updated. Thanks
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 don't see your updates? Can you put the comment at least.
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.
My bad. Please take another look. Thx.
2545a70
to
3c1763d
Compare
/ok-to-test |
3c1763d
to
67c1071
Compare
67c1071
to
4408970
Compare
4408970
to
eb449ec
Compare
eb449ec
to
2b4f91e
Compare
/retest |
Unittest added for node_ipan_controller.go.
2b4f91e
to
e3121c1
Compare
/ok-to-test |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bowei, grayluck 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 |
Thanks for review, Bowei! |
/test pull-kubernetes-kubemark-e2e-gce-big |
…52-upstream-release-1.11 Automated cherry pick of #68952: Skip node cidr mask size check for cloud allocation.
…52-upstream-release-1.12 Automated cherry pick of #68952: Skip node cidr mask size check for cloud allocation.
…52-upstream-release-1.10 Automated cherry pick of #68952: Skip node cidr mask size check for cloud allocation.
What this PR does / why we need it:
For IPAM: Do not check if maskSize per node (flag --node-cidr-mask-size) is smaller or equals to cluster cidr (--cluster-cidr), if the secondary ranges are allocated by the cloud.
When allocatorType == CloudAllocatorType, nodeCIDRMaskSize is actually not used in creating CIDR range allocator. See:
kubernetes/pkg/controller/nodeipam/ipam/cidr_allocator.go
Lines 96 to 111 in 95ab206
This check is impeding nodepools from having different pod cidr sizes.
Special notes for your reviewer:
Release note: