Skip to content
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

[2.23] backport: disable podCIDR allocation from control-plane when using calico #10715

Merged
merged 1 commit into from
Dec 13, 2023

Conversation

VannTen
Copy link
Contributor

@VannTen VannTen commented Dec 12, 2023

What type of PR is this?
/kind bug

What this PR does / why we need it:
Backport of #10639

Special notes for your reviewer:

Should I drop the breaking change (defining calico_ipam_host_local to false by default instead of relying on undefined) ?. It's unlikely to be breaking in practice, but it's theoretically possible.

Does this PR introduce a user-facing change?:

kube-controller-manager will no longer assign pod CIDRs to cluster nodes when using calico (with its default IPAM)
calico_ipam_host_local now has a default value of `false`
action required: users using a non-true value for calico_ipam_host_local will need to change it to `true`

…rnetes-sigs#10639)

* Disable control plane allocating podCIDR for nodes when using calico

Calico does not use the .spec.podCIDR field for its IP address
management.
Furthermore, it can false positives from the kube controller manager if
kube_network_node_prefix and calico_pool_blocksize are unaligned, which
is the case with the default shipped by kubespray.

If the subnets obtained from using kube_network_node_prefix are bigger,
this would result at some point in the control plane thinking it does
not have subnets left for a new node, while calico will work without
problems.

Explicitely set a default value of false for calico_ipam_host_local to
facilitate its use in templates.

* Don't default to kube_network_node_prefix for calico_pool_blocksize

They have different semantics: kube_network_node_prefix is intended to
be the size of the subnet for all pods on a node, while there can be
more than on calico block of the specified size (they are allocated on
demand).

Besides, this commit does not actually change anything, because the
current code is buggy: we don't ever default to
kube_network_node_prefix, since the variable is defined in the role
defaults.
@k8s-ci-robot k8s-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Dec 12, 2023
@k8s-ci-robot k8s-ci-robot requested review from EppO and oomichi December 12, 2023 16:47
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Dec 12, 2023
@VannTen VannTen changed the base branch from master to release-2.23 December 12, 2023 16:48
@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Dec 12, 2023
@VannTen
Copy link
Contributor Author

VannTen commented Dec 12, 2023

/retest-required

@yankay
Copy link
Member

yankay commented Dec 13, 2023

Thanks @VannTen
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 13, 2023
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mzaian, VannTen

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 13, 2023
@k8s-ci-robot k8s-ci-robot merged commit e4be213 into kubernetes-sigs:release-2.23 Dec 13, 2023
59 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/bug Categorizes issue or PR as related to a bug. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants