Skip to content
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.

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

Bump CNI plugins to v1.4.0 #9249

Merged
merged 1 commit into from Feb 7, 2024
Merged

Conversation

brandond
Copy link
Contributor

@brandond brandond commented Jan 16, 2024

Proposed Changes

Bump CNI plugins to v1.4.0

Ref: containernetworking/plugins@v1.3.0...v1.4.0

Types of Changes

version bump

Verification

check versions

Testing

Linked Issues

User-Facing Change

Further Comments

@brandond brandond requested a review from a team as a code owner January 16, 2024 21:52
Copy link

codecov bot commented Jan 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (9a70021) 45.19% compared to head (974adc7) 40.51%.
Report is 4 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #9249      +/-   ##
==========================================
- Coverage   45.19%   40.51%   -4.68%     
==========================================
  Files         154      154              
  Lines       16555    16555              
==========================================
- Hits         7482     6708     -774     
- Misses       7861     8699     +838     
+ Partials     1212     1148      -64     
Flag Coverage Δ
e2etests ?
inttests 37.68% <ø> (-0.02%) ⬇️
unittests 14.53% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

dereknola
dereknola previously approved these changes Jan 16, 2024
@manuelbuil
Copy link
Contributor

I have doubts about adding macvlan to the set of K3s CNI plugins. Firstly because it does not serve a general purpose use case but a very specific one: user installs Multus with a manifest which does not provide CNI binaries and wants to use macvlan CNI binary. I think K3s design philosophy is to support general Kubernetes use cases and this is not one of them.

Secondly, this creates a precedent. What if a user now wants to use the "dhcp" ipam CNI binary for Multus? Or the "vlan" CNI binary? If we do this, I'd say we should add all of them: https://www.cni.dev/plugins/current/. However, this would also be at odds with K3s design philosophy of being minimal and only including what's really needed.

Instead, I suggest to create some integration with Multus. I think we decided that adding extra CNI options is not desirable in K3s (maybe we can revisit this decision for Multus?). But maybe we can document how to deploy the multus chart living in rke2-charts changing a few values to make it work in K3s. Note that the multus chart over there can install all the required CNI plugins (we can also skip the ones we don't want): https://github.com/rancher/rke2-charts/blob/main/charts/rke2-multus/rke2-multus/v4.0.2-build2023081107/values.yaml#L119-L126. BTW, I'm surprised to see Multus integration in the roadmap: https://github.com/k3s-io/k3s/blob/master/ROADMAP.md

@brandond
Copy link
Contributor Author

I agree the precedent is possibly dangerous, but on the other hand the difference in footprint is small; oddly enough it is exactly 32k.

-rwxr-xr-x 1 0 0 4186112 Dec 22 00:04 /bin/cni
-rwxr-xr-x 1 0 0 4218880 Jan 17 20:38 /bin/cni

We have taken on additional plugins on a case by case basis in the past, see for example the firewall and bandwidth plugins we added in #7424.

@brandond brandond changed the title Bump CNI plugins to v1.4.0 and add macvlan plugin Bump CNI plugins to v1.4.0 Jan 25, 2024
@brandond brandond requested a review from a team January 25, 2024 18:00
@brandond
Copy link
Contributor Author

Have decided not to add macvlan, so this is now just a version bump.

scripts/package-cli Outdated Show resolved Hide resolved
vitorsavian
vitorsavian previously approved these changes Jan 26, 2024
Ref: rancher/plugins@v1.3.0-k3s1...v1.4.0-k3s2

Signed-off-by: Brad Davidson <brad.davidson@rancher.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants