CNI: improve plugin chaining #24956
Merged
CNI: improve plugin chaining #24956
Conversation
This file contains 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
maintainer-s-little-helper
bot
added
the
dont-merge/needs-release-note-label
|
fyi @sofat1989, do you want to see if this covers your use case? |
squeed
added
the
release-note/minor
maintainer-s-little-helper
bot
removed
the
dont-merge/needs-release-note-label
squeed
added
area/cni
maintainer-s-little-helper
bot
removed
dont-merge/needs-release-note-label
squeed
force-pushed
the
cni-better-chaining
branch
from
70f051a
to
59276bf
Compare
|
as far as I can tell, the test failure isn't caused by this PR: the pod's CNI plugin execution returned without any issues, meaning that nothing this PR touched failed. Unfortunately there's not enough information to debug; #24982 will improve that situation. |
asauber
approved these changes
Apr 20, 2023
asauber
reviewed
Apr 20, 2023
squeed
force-pushed
the
cni-better-chaining
branch
from
59276bf
to
edd77e9
Compare
zacharysarah
approved these changes
Apr 21, 2023
squeed
force-pushed
the
cni-better-chaining
branch
from
edd77e9
to
dad6331
Compare
qmonnet
approved these changes
Apr 21, 2023
|
/test Job 'Cilium-PR-K8s-1.27-kernel-net-next' failed: Click to show.Test NameFailure OutputJenkins URL: https://jenkins.cilium.io/job/Cilium-PR-K8s-1.27-kernel-net-next/108/ If it is a flake and a GitHub issue doesn't already exist to track it, comment |
youngnick
approved these changes
May 1, 2023
Just some capitalized errors that were annoying the linter. Signed-off-by: Casey Callendrello <cdc@isovalent.com>
We determined exactly how to chain by reading the CNI network name. This is fragile, and means that end-users can't easily configure chaining for their own networks. This keeps the existing logic but adds an explicit chaining-mode field in the CNI configuration. Additionally: remove the "portmap" plugin, since all it did was signify that the network "portmap" was the same as "cilium". There are cleaner ways to handle that. Signed-off-by: Casey Callendrello <cdc@isovalent.com>
Signed-off-by: Casey Callendrello <cdc@isovalent.com>
People would like to automatically inject Cilium in to CNI configuration files. Add an option to the daemon that watches for networks of a given name and injects Cilium into it. We already supported this in a limited sense with aws-cni, but let's generalize it to support arbitrary CNI networks. Signed-off-by: Casey Callendrello <cdc@isovalent.com>
Add the option to set a CNI chaining target explicitly. Also, clean up some of the defaulting around cni chaining mode. Signed-off-by: Casey Callendrello <cdc@isovalent.com>
There was a logic error when handling an unset chaining mode. Signed-off-by: Casey Callendrello <cdc@isovalent.com>
squeed
force-pushed
the
cni-better-chaining
branch
from
dad6331
to
0f75ecd
Compare
|
/test |
maintainer-s-little-helper
bot
added
the
ready-to-merge
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.
The Cilium agent already supports a mode where it waits for a certain CNI network to exist, then injects itself as a chained plugin, and writes a new file. However, this was only only supported for AWS. Instead, let's make this functionality generic.
This PR makes these changes:
The end result is a cleaner setup for end-users who would like to chain Cilium.
Fixes: #22286
Inspired-by: #22358