-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
Update multus version & crio conf #6444
Conversation
Package |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/cc @oomichi
@@ -1,7 +1,7 @@ | |||
--- | |||
multus_conf_file: "auto" | |||
multus_cni_conf_dir_host: "/etc/cni/net.d" | |||
multus_cni_bin_dir_host: "{{ '/usr/libexec/cni' if container_manager == 'crio' else '/opt/cni/bin' }}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was basing my changes off of this: https://github.com/intel/multus-cni/blob/c85b79f5ff5bcacaa45e2135d29e9afb6b84ed9b/images/multus-daemonset-crio.yml#L166-L198 but agree it doesn't look necessary at this time
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, didn't think that was a recent change, but not sure it is ndeeded but you tell me (well as you approved the changes I guess you're fine with it)
/lgtm |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: floryut, Miouge1 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
* 'master' of https://github.com/kubernetes-sigs/kubespray: Documentation for Ingress (kubernetes-sigs#6378) Fix ansible-lint E301 for commands fetching data (kubernetes-sigs#6465) Fix shellcheck url (kubernetes-sigs#6462) Fix ansible-lint E305 (kubernetes-sigs#6459) Fix ansible-lint E404 (kubernetes-sigs#6417) Update README.md and openstack.md (kubernetes-sigs#6455) Add noqa and disable .ansible-lint global exclusions (kubernetes-sigs#6410) Move healthz check to secure ports (kubernetes-sigs#6446) Update multus version & crio conf (kubernetes-sigs#6444) Fix remove etcd broken with etcdctl_api 3 (kubernetes-sigs#6448) update cinder csi manifests (kubernetes-sigs#6434) Update docker package to 19.03.12 (kubernetes-sigs#6439) * add proxy_env definition to remove_node.yml resolving kubernetes-sigs#6430 (kubernetes-sigs#6431)
What type of PR is this?
/kind feature
What this PR does / why we need it:
Update multus cni to latest version
Which issue(s) this PR fixes:
None
Special notes for your reviewer:
I've update cni path for crio as I think it's outdated ? every references fond for crio now default to
/opt/cni/bin
Does this PR introduce a user-facing change?: