Skip to content

ip-masq-agent: refactor into a Hive Cell #40347

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 11, 2025
Merged

ip-masq-agent: refactor into a Hive Cell #40347

merged 1 commit into from
Jul 11, 2025

Conversation

antonipp
Copy link
Contributor

@antonipp antonipp commented Jul 3, 2025

Description

This PR wraps the existing ip-masq-agent code into a Hive Cell. This goes in-line with the current efforts to modularize the codebase and is a pre-requisite for #40141.

The ip-masq-agent and its Maps used to be created inside the Legacy Daemon Cell. Both the Agent and its Maps are now modularized and are created before the Daemon Cell starts.

Testing

On top of unit tests, I also tried the PR in a real cluster. Unfortunately I don't have an easy way to setup a 1.18 environment so I backported my change to 1.17.5. I then verified that:

  • When --enable-ip-masq-agent=false:
    • The Agents start properly, there are no errors
  • When --enable-ip-masq-agent=true:
    • The Agents start properly, there are no errors. There is the "Starting ip-masq-agent" log from module=agent.controlplane.ip-masq-agent
    • cilium status shows correctly: 
      # cilium status
[...]
Masquerading:            BPF (ip-masq-agent)   [ens5, ens6]   10.128.0.0/16 [IPv4: Enabled, IPv6: Disabled]
    • The --ip-masq-agent-config-path option is taken into account and the BPF Map is populated with the expected values from the config path: 
       # cilium bpf ipmasq list
 IP PREFIX/ADDRESS
 169.254.0.0/16
 172.16.0.0/12
 192.168.0.0/16
 10.0.0.0/8
 100.64.0.0/10

ip-masq-agent: refactor into a Hive Cell

@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 Jul 3, 2025
@antonipp
Copy link
Contributor Author

antonipp commented Jul 3, 2025

/test

@antonipp
Copy link
Contributor Author

antonipp commented Jul 3, 2025 •
edited
Loading

/test

Looks like I'm not allowed to run tests on draft PRs? 😞

@HadrienPatte
Copy link
Member

/test

@antonipp antonipp marked this pull request as ready for review July 3, 2025 14:40
@antonipp antonipp requested review from a team as code owners July 3, 2025 14:40
@antonipp antonipp mentioned this pull request Jul 3, 2025
Merged
YutaroHayakawa
YutaroHayakawa requested changes Jul 3, 2025
View reviewed changes
Copy link
Member

@YutaroHayakawa YutaroHayakawa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice cleanup! Some feedbacks.

dylandreimerink
dylandreimerink approved these changes Jul 4, 2025
View reviewed changes
Copy link
Member

@dylandreimerink dylandreimerink left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@YutaroHayakawa already commented on the things I also noticed. Will approve assuming those comments are addressed before merge.

@mhofstetter mhofstetter added release-note/misc This PR makes changes that have no direct user impact. area/agent Cilium agent related. area/modularization Relates to code modularization and maintenance. labels Jul 4, 2025
@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 Jul 4, 2025
@antonipp
Copy link
Contributor Author

antonipp commented Jul 4, 2025

/test

1 similar comment
@aanm
Copy link
Member

aanm commented Jul 4, 2025

/test

@aanm
Copy link
Member

aanm commented Jul 4, 2025

/ci-runtime

@nbusseneau
Copy link
Member

/test

@antonipp
Copy link
Contributor Author

antonipp commented Jul 4, 2025

/test

YutaroHayakawa
YutaroHayakawa reviewed Jul 7, 2025
View reviewed changes
@antonipp
ip-masq-agent: refactor into a Hive Cell
17e7a4c 
Signed-off-by: Anton Ippolitov <anton.ippolitov@datadoghq.com>
@antonipp
Copy link
Contributor Author

antonipp commented Jul 8, 2025

/test

YutaroHayakawa
YutaroHayakawa approved these changes Jul 8, 2025
View reviewed changes
Copy link
Member

@YutaroHayakawa YutaroHayakawa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now, looks good to me. Thanks!

@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 Jul 9, 2025
@tklauser tklauser added this pull request to the merge queue Jul 11, 2025
Merged via the queue into cilium:main with commit 758f548 Jul 11, 2025
70 checks passed
@antonipp antonipp mentioned this pull request Aug 1, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dylandreimerink dylandreimerink dylandreimerink approved these changes

@asauber asauber asauber approved these changes

@YutaroHayakawa YutaroHayakawa YutaroHayakawa approved these changes

@thorn3r thorn3r thorn3r approved these changes

Assignees
No one assigned
Labels
area/agent Cilium agent related. area/modularization Relates to code modularization and maintenance. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@antonipp @HadrienPatte @aanm @nbusseneau @dylandreimerink @asauber @YutaroHayakawa @thorn3r @tklauser @mhofstetter