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

Add note about putting a flanneld binary in /opt/bin #1563

Merged
merged 1 commit into from
May 13, 2022

Conversation

radiosilence
Copy link
Contributor

@radiosilence radiosilence commented May 5, 2022

Description

It took me ages to figure this out

@radiosilence radiosilence changed the title Add note about putting a flanneld in the right place Add note about putting a flanneld binary in /opt/bin May 6, 2022
@radiosilence
Copy link
Contributor Author

Not sure how a doc change could cause tests to fail :)

@luthermonson
Copy link
Contributor

ya it's not you, the e2e tests are broke

@luthermonson luthermonson merged commit 7a28364 into flannel-io:master May 13, 2022
@Thyroxine
Copy link

Thyroxine commented May 14, 2022

Hello!

Why do we need the manual installation of the raw flanneld binary onto the nodes?
We already have flanneld inside the Docker container (pod template).

It is very confusing addition.

We've recently deployed a number of K8s clusters using Flannel CNI plugin by kubeadm 1.23, and everything is working perfectly by just applying the manifest, as stated in the previous version of the docs.

Flannel Pods are starting automatically after node reboot, as expected.
Network is working flawlessly too without manual installation of binaries.

@discostur
Copy link

Doesn't make any sense ... why was this merged @luthermonson ?

@cprivitere
Copy link

Just gonna +1 this question, what's up with this requirement, @luthermonson ?

@primeos-work
Copy link

I'm also confused by this change. Shouldn't it get reverted?

I guess, if necessary, it could be documented somewhere else in the README for alternative setups but it really doesn't seem to belong in front of kubectl apply -f https://raw.githubusercontent.com/flannel-io/flannel/master/Documentation/kube-flannel.yml.

kube-flannel.yml declares a kube-flannel container that currently uses the docker.io/rancher/mirrored-flannelcni-flannel:v0.19.0 image which already contains /opt/bin/flanneld:

[michael@groot ~]$ /opt/bin/flanneld --version
-bash: /opt/bin/flanneld: No such file or directory
[michael@groot ~]$ podman run -it --entrypoint /bin/sh docker.io/rancher/mirrored-flannelcni-flannel:v0.19.0
/ # /opt/bin/flanneld --version
v0.19.0

So the advice of installing /opt/bin/flanneld on the K8s node must simply be wrong in this context.

@cprivitere
Copy link

Is this one of those things where a University program is submitting bad changes to see if people notice, maybe?

lkiesow added a commit to lkiesow/flannel that referenced this pull request Sep 2, 2022
As several people already pointed out on pull request flannel-io#1563, the
instructions added in there are not necessary. In fact, blindly following
these instructions actually caused some trouble for me since I ran into
issue flannel-io#1418 and started debugging that problem before realizing that I
don't actually need the binary in `/opt/bin`.

The pull request also does not provide any reasoning for why this could
be necessary. Given that, and given that many people install Flannel
without doing this, it's probably fair to remove this again to prevent
more people running into the issue I had.

This reverts commit a93a407.
lkiesow added a commit to lkiesow/flannel that referenced this pull request Sep 2, 2022
As several people already pointed out on pull request flannel-io#1563, the
instructions added in there are not necessary. In fact, blindly following
these instructions actually caused some trouble for me since I ran into
issue flannel-io#1418 and started debugging that problem before realizing that I
don't actually need the binary in `/opt/bin`.

The pull request also does not provide any reasoning for why this could
be necessary. Given that, and given that many people install Flannel
without doing this, it's probably fair to remove this again to prevent
more people running into the issue I had.

This reverts commit a93a407.
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.

6 participants