Skip to content

iptables: Don't fail on missing ip6tables when InstallIptRules=false#43940

Merged
joestringer merged 1 commit intocilium:mainfrom
javiercardona-work:pr1
Feb 3, 2026
Merged

iptables: Don't fail on missing ip6tables when InstallIptRules=false#43940
joestringer merged 1 commit intocilium:mainfrom
javiercardona-work:pr1

Conversation

@javiercardona-work
Copy link
Copy Markdown

InstallIptRules sets whether Cilium should install any iptables in general. When disabled, Cilium should not require ip6tables kernel modules to be present. The current code fails startup if IPv6 is enabled but ip6tables is unavailable, even when iptables rules are not being installed.

This fix allows Cilium to start in environments where ip6tables is not available but iptables rule installation is disabled via configuration.

Please ensure your pull request adheres to the following guidelines:

  • For first time contributors, read Submitting a pull request
  • All code is covered by unit and/or runtime tests where feasible.
    -SKIPPED: Testing this would require setting up proper mocks for logger and both iptables interfaces. Many similar conditions in this codebase are not unit-tested directly but are covered by integration tests.
  • All commits contain a well written commit description including a title,
    description and a Fixes: #XXX line if the commit addresses a particular
    GitHub issue.
  • If your commit description contains a Fixes: <commit-id> tag, then
    please add the commit author[s] as reviewer[s] to this issue.
  • All commits are signed off. See the section Developer’s Certificate of Origin
  • Provide a title or release-note blurb suitable for the release notes.
  • Are you a user of Cilium? Please add yourself to the Users doc
  • Thanks for contributing!

Fixes: #issue-number

iptables:  Allow Cilium to start in environments where `ip6tables` is not available but iptables rule installation is disabled via configuration.

@javiercardona-work javiercardona-work requested a review from a team as a code owner January 22, 2026 21:00
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jan 22, 2026
@github-actions github-actions bot added the kind/community-contribution This was a contribution made by a community member. label Jan 22, 2026
@maintainer-s-little-helper

This comment was marked as outdated.

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Jan 22, 2026
@joestringer
Copy link
Copy Markdown
Member

Thanks, fix makes logical sense but just for awareness there are a few specific cases where Cilium currently requires ip(6)tables, tracked by #12879.

You will need to rebase against main and force-push (no merge commits please). For more info see the development guide (but note it's OK if you can't change labels, that's normal until you're a member of the org).

@joestringer joestringer added the area/misc Impacts miscellaneous areas of the code not otherwise owned by another area. label Jan 22, 2026
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Jan 23, 2026
@joamaki joamaki added the release-note/minor This PR changes functionality that users may find relevant to operating Cilium. label Jan 26, 2026
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jan 26, 2026
@joamaki
Copy link
Copy Markdown
Contributor

joamaki commented Jan 26, 2026

/test

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jan 26, 2026
@maintainer-s-little-helper

This comment was marked as outdated.

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Jan 27, 2026
@joestringer
Copy link
Copy Markdown
Member

@javiercardona-work would you mind dropping the merge commit, rebase against main and force-push to update? Then we can run the tests & mark to merge.

@joestringer joestringer removed the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jan 27, 2026
InstallIptRules sets whether Cilium should install any iptables in
general.  When disabled, Cilium should not require ip6tables kernel
modules to be present. The current code fails startup if IPv6 is enabled
but ip6tables is unavailable, even when iptables rules are not being
installed.

This fix allows Cilium to start in environments where ip6tables is not
available but iptables rule installation is disabled via configuration.

Signed-off-by: Javier Cardona <jcardona@meta.com>
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Jan 30, 2026
@joestringer
Copy link
Copy Markdown
Member

/test

@joestringer joestringer enabled auto-merge January 30, 2026 18:59
@javiercardona-work
Copy link
Copy Markdown
Author

@joestringer Can someone help me understand the failing test? I do not think it is related to this change.

@joestringer
Copy link
Copy Markdown
Member

Sure. I'll demonstrate the steps below that you can follow along for future reference. Ginkgo test results present in their own slightly different way from the other failures, but the general GitHub UI navigation stuff should be a common pattern for understanding failures. This should be pretty similar to the CI Triage guide for developers in the docs.

Triage

• Failure in Spec Teardown (AfterEach) [234.830 seconds]
K8sDatapathLRPTests
/home/runner/work/cilium/cilium/test/ginkgo-ext/scopes.go:461
  Checks local redirect policy
  /home/runner/work/cilium/cilium/test/ginkgo-ext/scopes.go:461
    LRP connectivity [AfterEach]
    /home/runner/work/cilium/cilium/test/ginkgo-ext/scopes.go:515

    Found 1 io.cilium/app=operator logs matching list of errors that must be investigated:
    2026-01-30T19:05:15.489552351Z time=2026-01-30T19:05:15.48931064Z level=error source=/go/src/github.com/cilium/cilium/pkg/logging/klog_bridge.go:175 msg="\"Error initially creating lease lock\" err=\"leases.coordination.k8s.io \\\"cilium-operator-resource-lock\\\" already exists\" lock=\"kube-system/cilium-operator-resource-lock\"" subsys=klog

    /home/runner/work/cilium/cilium/test/ginkgo-ext/scopes.go:413

Check for existing failures

Is this codepath exercised?

  • Back to the main failure link, scroll to the bottom to find artifacts
  • cilium-sysdumps-bugtool-1.34-f18-datapath-lrp has a full dump of state at the time of the failure
  • For Ginkgo failures the failure itself inside the zip in nested directories for the specific test has a cilium-sysdump.zip, which has a more comprehensive set of info
  • cilium-configmap-*.yaml file has enable-ipv6: true but no install-iptables-rules.
  • git grep for InstallIptRules, condition checked in the PR, shows that the field is enabled by default
  • Conclusion: For the failing test, the same code was exercised both before and after the change; the change should effectively be a noop for this scenario. (Anecdotally we do not test the --install-iptables-rules=false so this is expected)

@joestringer
Copy link
Copy Markdown
Member

We can follow up on the failure in #44082, it doesn't need to block this PR. I've retriggered the failing test.

@joestringer joestringer added this pull request to the merge queue Feb 3, 2026
@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Feb 3, 2026
Merged via the queue into cilium:main with commit 16e9095 Feb 3, 2026
76 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/misc Impacts miscellaneous areas of the code not otherwise owned by another area. kind/community-contribution This was a contribution made by a community member. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/minor This PR changes functionality that users may find relevant to operating Cilium.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants