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

upgrade cni plugin version to use new feature #1323

Closed
wants to merge 1 commit into from

Conversation

a180285
Copy link

@a180285 a180285 commented Aug 28, 2024

upgrade
github.com/containernetworking/plugins v1.1.0 to v1.5.1

to use new features. Like below:
containernetworking/plugins@821982d

This new parameter allows users to remove the default vlan

Fixes: containernetworking/plugins#667

@a180285 a180285 changed the title upgrade cni plugin version upgrade cni plugin version to use new feature Aug 28, 2024
@a180285
Copy link
Author

a180285 commented Aug 29, 2024

Hi @dougbtv @s1061123. Can you give a review on this.

@dougbtv
Copy link
Member

dougbtv commented Aug 29, 2024

I think it's probably OK to get updates to the CNI requirements, but...

...question -- does this really address the issue? like, if those changes are in the resulting binaries, does that address your issue if it's just the libraries updated in Multus?

@s1061123
Copy link
Member

s1061123 commented Aug 29, 2024

Based on your comment, you want to have fix for bridge CNI pluign. But multus repository does not have any bridge CNI code, hence your issue does not fixed even if this PR is merged.

This new parameter allows users to remove the default vlan

This parameter is added in bridge CNI plugin, not multus CNI plugin. If you want to introduce it, you just update bridge CNI, as get the latest plugin from https://github.com/containernetworking/plugins/releases and install.

@a180285
Copy link
Author

a180285 commented Aug 29, 2024

Thanks @dougbtv @s1061123
Yes, I see this issue on bridge plugin. I need that fix.

Can you help show me how to update the plugin instead of update mulus?

Because I think multus depends on plugin repo, so the plugin code is built in multus, Is that wrong?

@dougbtv
Copy link
Member

dougbtv commented Aug 29, 2024

Yeah, those binaries aren't automatically included with Multus. Multus just uses some specific functionality from the plugins repo (we think for netns utilities). You would build the binaries from the containernetworking/plugins repo, and then install them onto the hosts 👍

@dougbtv
Copy link
Member

dougbtv commented Aug 29, 2024

I think this cnilib bump might be bigger than just a bump and might require some functional changes in the tests, so, let's close this out for now. Thanks for the contribution and asking the question

@dougbtv dougbtv closed this Aug 29, 2024
@a180285
Copy link
Author

a180285 commented Aug 29, 2024

Here is my config. Does that mean I should update kube-ovn or update plugin on my host?

---
apiVersion: k8s.cni.cncf.io/v1
kind: NetworkAttachmentDefinition
metadata:
  name: vlan4009
  namespace: default
spec:
  config: '{
     "cniVersion": "0.3.1",
     "name": "vlan4009",
     "type": "bridge",
     "bridge": "br0",
     "vlan": 4009,
     "ipam": {
        "type": "kube-ovn",
        "server_socket": "/run/openvswitch/kube-ovn-daemon.sock",
        "provider": "vlan4009.default"
      }
     }'

@a180285
Copy link
Author

a180285 commented Aug 29, 2024

@dougbtv Thanks for you info. If I need update cni-plugin on my host. Can you kindly provide more info, on how to upgrade cni to my host? Thanks very much.

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.

bridge: remove default pvid for veth interface
3 participants