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

fix: set global arp_ignore param for kindnet #3881

Merged
merged 1 commit into from
Mar 12, 2025

Conversation

shaneutt
Copy link
Member

@shaneutt shaneutt commented Mar 1, 2025

Resolves #3880

If we do decide to go this way, in my testing setting 1 appears to work properly, and since this is the first time we've been opinionated about it it seems like an opportunity to set it as restrictively as we can tolerate. However we went with a setting of 0 because that has been the historically anticipated setting.

@k8s-ci-robot k8s-ci-robot added the area/provider/docker Issues or PRs related to docker label Mar 1, 2025
@k8s-ci-robot k8s-ci-robot requested review from aojea and stmcginnis March 1, 2025 19:30
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. area/provider/nerdctl Issues or PRs related to nerdctl area/provider/podman Issues or PRs related to podman size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Mar 1, 2025
@shaneutt shaneutt marked this pull request as draft March 1, 2025 19:56
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 1, 2025
@shaneutt
Copy link
Member Author

shaneutt commented Mar 1, 2025

/test all

@shaneutt shaneutt requested a review from aojea March 10, 2025 13:08
@aojea
Copy link
Contributor

aojea commented Mar 10, 2025

just set it in https://github.com/kubernetes-sigs/kind/tree/main/images/base/files/etc/sysctl.d

@shaneutt shaneutt force-pushed the shaneutt/fix-arp-ignore branch from d5bd098 to 0671ecd Compare March 11, 2025 14:12
@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Mar 11, 2025
@shaneutt shaneutt marked this pull request as ready for review March 11, 2025 14:13
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 11, 2025
Some Linux distributions set this to 2 or higher, which will break
kindnet (specifically the ptp plugin). We now set this in our base
image in an opinionated way so that the host system no longer
decides the value as it might inadvertantly break kind networking.

Signed-off-by: Shane Utt <[email protected]>
@shaneutt shaneutt force-pushed the shaneutt/fix-arp-ignore branch from 0671ecd to 5e66ec0 Compare March 11, 2025 15:16
@shaneutt shaneutt requested a review from aojea March 11, 2025 15:19
@BenTheElder
Copy link
Member

Thanks!

FYI: this will be exercised in CI on the next PR after the image is built (postsubmit), similar to agnhost changes in kubernetes/kubernetes.

I don't expect to need to revert, but just calling out that while CI ran, really all of the e2e jobs didn't test this yet, we'll get that in the next step.

/lgtm
/approve
/hold
@aojea any remaining concerns?

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 11, 2025
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 11, 2025
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: BenTheElder, shaneutt

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 Mar 11, 2025
@aojea
Copy link
Contributor

aojea commented Mar 12, 2025

/hold cancel

Thanks @shaneutt

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 12, 2025
@k8s-ci-robot k8s-ci-robot merged commit b98dc21 into kubernetes-sigs:main Mar 12, 2025
21 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. area/provider/docker Issues or PRs related to docker area/provider/nerdctl Issues or PRs related to nerdctl area/provider/podman Issues or PRs related to podman cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. 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.

Container networking failures on hosts with net.ipv4.conf.all.arp_ignore=2
4 participants