Add a new manager for all-hosts ipset even when IPIP is not enabled#10841
Merged
mazdakn merged 5 commits intoprojectcalico:masterfrom Aug 20, 2025
Merged
Add a new manager for all-hosts ipset even when IPIP is not enabled#10841mazdakn merged 5 commits intoprojectcalico:masterfrom
mazdakn merged 5 commits intoprojectcalico:masterfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a standalone manager for the all-hosts ipset that operates independently of IPIP functionality. The change extracts ipset management from the IPIP manager and creates a dedicated hostsIPSetManager to handle all-hosts ipset updates even when IPIP is disabled.
- Extracts all-hosts ipset management from IPIP manager into a dedicated manager
- Creates
hostsIPSetManagerthat operates for both IPv4 and IPv6 without IPIP dependency - Updates tests to reflect the architectural change
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| felix/dataplane/linux/ipset_hosts_mgr.go | New dedicated manager for all-hosts ipset functionality |
| felix/dataplane/linux/ipset_hosts_mgr_test.go | Comprehensive test coverage for the new hosts ipset manager |
| felix/dataplane/linux/ipip_mgr.go | Removes all-hosts ipset management functionality from IPIP manager |
| felix/dataplane/linux/ipip_mgr_test.go | Removes ipset-related tests that moved to the new manager |
| felix/dataplane/linux/int_dataplane.go | Registers the new hosts ipset manager for both IPv4 and IPv6 |
| felix/fv/nat_outgoing_test.go | Updates test configuration to disable IPIP and configure NAT exclusions |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
caseydavenport
approved these changes
Aug 19, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Previously, we only used
all-hosts-netipset for ipip encapsulation. However, we need it now for more features listed below:IPPoolsAndHostIPsfornatOutgoingExclusionsoption,all-hosts-netipset needs to be present. Since ATM, this ipset is only managed by IPIP manager, using the mentioned value in any routing setup except IPIP leads to Felix not being able to program rules, and eventually panicing. The change made in the NAT outgoing FV test simply triggers the panic.IPPoolsAndHostIPswith IPv6 pools, would panic Felix even in IPIP, since IPIP encap does not support IPv6.This PR adds a new manager to manage
all-hosts-netipset by separating the functionality from IPIP manager. The new manager will always be running since the ipset is needed by IPIP encapsulation, NAT outgoing, and QoS Policy.Related issues/PRs
Todos
Release Note
Reminder for the reviewer
Make sure that this PR has the correct labels and milestone set.
Every PR needs one
docs-*label.docs-pr-required: This change requires a change to the documentation that has not been completed yet.docs-completed: This change has all necessary documentation completed.docs-not-required: This change has no user-facing impact and requires no docs.Every PR needs one
release-note-*label.release-note-required: This PR has user-facing changes. Most PRs should have this label.release-note-not-required: This PR has no user-facing changes.Other optional labels:
cherry-pick-candidate: This PR should be cherry-picked to an earlier release. For bug fixes only.needs-operator-pr: This PR is related to install and requires a corresponding change to the operator.