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 ml-bindport / METALLB_ML_BIND_PORT config #582

Merged
merged 1 commit into from Apr 14, 2020
Merged

Conversation

champtar
Copy link
Contributor

@champtar champtar commented Apr 14, 2020

Docker EE already uses port 7946 (likely memberlist)
Fixes #581

Copy link
Member

@rata rata left a comment

@champtar thank for the PR! Left some simple comments too

speaker/main.go Show resolved Hide resolved
speaker/main.go Show resolved Hide resolved
speaker/main.go Outdated Show resolved Hide resolved
@champtar
Copy link
Contributor Author

champtar commented Apr 14, 2020

@rata done

rata
rata approved these changes Apr 14, 2020
Copy link
Member

@rata rata left a comment

LGTM, but haven't tested it at all.

Have you tested it manually? :)

@champtar
Copy link
Contributor Author

champtar commented Apr 14, 2020

nop, not tested yet

@rata
Copy link
Member

rata commented Apr 14, 2020

@champtar please ping me when it's ready to merge (if you don't have permissions) :)

@champtar
Copy link
Contributor Author

champtar commented Apr 14, 2020

Just tested, it works !
docker.io/champtar/metallbspeaker:c3dfa2aa8113ee868edbad527e7f4fbab2d347ca
Do you want me to add the port configuration commented in the manifest ?

@rata
Copy link
Member

rata commented Apr 14, 2020

@champtar cool! Yes, adding it to the manifest commented out (maybe a comment explaining and saying it is not usually needed or something?) sounds great! :)

Docker EE already uses port 7946 (likely memberlist)
Fixes metallb#581

Signed-off-by: Etienne Champetier <echampetier@anevia.com>
@rata rata merged commit 5e654b0 into metallb:main Apr 14, 2020
@champtar champtar deleted the bindport branch Oct 2, 2020
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.

Installation conflict with Docker EE
2 participants