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

Should it be copy flannel binary to host? #8333

Closed
zhengtianbao opened this issue Dec 24, 2021 · 3 comments · Fixed by #8714
Closed

Should it be copy flannel binary to host? #8333

zhengtianbao opened this issue Dec 24, 2021 · 3 comments · Fixed by #8714
Labels
kind/feature Categorizes issue or PR as related to a new feature.

Comments

@zhengtianbao
Copy link
Member

Why is this needed:

I notice the host-cni-bin volume not used in roles/network_plugin/flannel/templates/cni-flannel.yml.j2

- name: host-cni-bin
hostPath:
path: /opt/cni/bin

PR #5598 #5937 mentioned the reason is flannel will copy binaries to /opt/cni/bin from install-cni container.

https://github.com/coreos/flannel-cni/blob/5fc4e35d410513501919a3dd7dc8063bec1d2857/install-cni.sh#L5-L8

and now the flannel project only copy flannel binary:

https://github.com/flannel-io/flannel/blob/a55d8aab7e092249e14dc6fb7a24414a8c5d8e4b/Documentation/kube-flannel.yml#L167-L178

since flannel-io/flannel@1a62caf#diff-9ef98a851566be25d8f74efb0f2a76b10a319a8ff60d0047e9c9171d3d561216 has been merged.

What would you like to be added:

here is two options:

  1. remove the host-cni-bin unused volume.
  2. use rancher/mirrored-flannelcni-flannel-cni-plugin:v1.2 to install flannel binary to host like flannel-io/flannel did.

I personally prefer option 2 that will be consistent with upstream. I'd like to make a PR if you think it's okay 😺

@zhengtianbao zhengtianbao added the kind/feature Categorizes issue or PR as related to a new feature. label Dec 24, 2021
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Mar 24, 2022
@oomichi
Copy link
Contributor

oomichi commented Apr 13, 2022

here is two options:

1. remove the `host-cni-bin` unused volume.

2. use rancher/mirrored-flannelcni-flannel-cni-plugin:v1.2 to install flannel binary to host like flannel-io/flannel did.

I personally prefer option 2 that will be consistent with upstream. I'd like to make a PR if you think it's okay 😺

Sorry for my late response.
The option 2 seems good for me.

@floryut Could you give us your idea?

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 13, 2022
@floryut
Copy link
Member

floryut commented Apr 14, 2022

I'm also more in favor of option 2, from my pov it's always better to fit to upstream as much as possible 🙇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants