Skip to content

Conversation

joestringer
Copy link
Member

@joestringer joestringer commented May 13, 2025

I noticed in local testing that when you deploy Cilium with a custom CNI
configuration, Cilium may report a different CNI configuration file
inside the Cilium container compared with the configuration on the
underlying host despite mapping the host filesystem into the Cilium
container. This appears to be due to the use of overlayfs and because
the 'install' target (invoked during container build) installs this
config file into the filesystem of the container itself. This ultimately
obscures the true config when inspecting it from inside the Cilium
container. Remove the extra unneeded config file to reduce confusion.

@joestringer joestringer added the release-note/misc This PR makes changes that have no direct user impact. label May 13, 2025
@joestringer
Copy link
Member Author

/test

@joestringer joestringer marked this pull request as ready for review May 23, 2025 22:51
@joestringer joestringer requested a review from a team as a code owner May 23, 2025 22:51
@joestringer joestringer requested a review from squeed May 23, 2025 22:51
Copy link
Contributor

@squeed squeed left a comment

Choose a reason for hiding this comment

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

Seem reasonable.

We don't use this CNI config file anymore; should we consider removing it entirely?

@joestringer
Copy link
Member Author

Huh, yeah I guess with the daemon/cmd/cni package it's programmatic by default these days.

@joestringer joestringer marked this pull request as draft May 27, 2025 19:29
@joestringer joestringer added the dont-merge/wait-until-release Freeze window for current release is blocking non-bugfix PRs label Jun 20, 2025
@joestringer joestringer removed the dont-merge/wait-until-release Freeze window for current release is blocking non-bugfix PRs label Jul 1, 2025
@joestringer joestringer added the dont-merge/needs-rebase This PR needs to be rebased because it has merge conflicts. label Jul 25, 2025
@joestringer joestringer force-pushed the pr/joe/remove-cni-conf branch from 0dd0f8f to 422482b Compare August 1, 2025 14:03
@joestringer joestringer removed the dont-merge/needs-rebase This PR needs to be rebased because it has merge conflicts. label Aug 1, 2025
@joestringer
Copy link
Member Author

/test

I noticed in local testing that when you deploy Cilium with a custom CNI
configuration, Cilium may report a different CNI configuration file
inside the Cilium container compared with the configuration on the
underlying host despite mapping the host filesystem into the Cilium
container. This appears to be due to the use of overlayfs and because
the 'install' target (invoked during container build) installs this
config file into the filesystem of the container itself. This ultimately
obscures the true config when inspecting it from inside the Cilium
container. Remove the extra unneeded config file to reduce confusion.

Signed-off-by: Joe Stringer <joe@cilium.io>
@joestringer joestringer force-pushed the pr/joe/remove-cni-conf branch from 422482b to 9404674 Compare August 1, 2025 14:27
@joestringer
Copy link
Member Author

/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 Aug 1, 2025
@joestringer joestringer marked this pull request as ready for review August 5, 2025 00:27
@joestringer joestringer added this pull request to the merge queue Aug 5, 2025
Merged via the queue into main with commit 118497f Aug 5, 2025
302 of 303 checks passed
@joestringer joestringer deleted the pr/joe/remove-cni-conf branch August 5, 2025 00:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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
Development

Successfully merging this pull request may close these issues.

2 participants