Fix nodeAssigned event on k8s >= 1.20 #812
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Lars Ekman <lars.g.ekman@est.tech>
These tests no longer work. They rely on the now archived virtuakube, which doesn't build against kube 1.19, which ends up blocking fixing metallb for kube 1.19. See metallb#723 for discussion of where it gets in the way. At this point we should just drop this directory and re-introduce e2etest code once someone is able to work on it. We can always pull this back out of git history if it helps. The commit contents are the result of roughly: - git rm -r e2etest - go mod tidy - git add go.mod go.sum Closes metallb#192 Closes metallb#629 Closes metallb#639 Amended-By: Rodrigo Campos <rodrigo@kinvolk.io>. Kept e2e/manifests folder, it is used in inv dev-env scripts.
We just remove the virtuakube dependency from go.mod and run a go mod tidy.
Contributor
Author
rata
added a commit
to rata/metallb
that referenced
this pull request
Mar 4, 2021
It happened in the past that client-go was out of date and that broke MetalLB on new Kubernetes releases, like seen in this PR: metallb#812 When we created the 0.9 release branch no one really checked, we could have updated it by then and never have this issue in the first place. So, I propose to add this to the release-process.
rata
added a commit
to rata/metallb
that referenced
this pull request
Mar 4, 2021
It happened in the past that client-go was out of date and that broke MetalLB on new Kubernetes releases, like seen in this PR: metallb#812 When we created the 0.9 release branch no one really checked, we could have updated it by then and never have this issue in the first place. So, I propose to add this to the release-process.
Contributor
Author
|
@johananl also, please thanks all the people involved in this PR (they are not in the commits, but I mentioned them on the PR description) in the release notes! :) |
|
@rata thanks for the fix! :) |
r4j4h
added a commit
to r4j4h/minikube
that referenced
this pull request
May 14, 2021
This change isn't strictly necessary but the newer versions of metallb contain some really nice quality of life improvements, and better support newer (installed by default) versions of kubernetes better. A sample of some improvements this brings to metallb add-on installed by minikube: - [Layer2 doesn't update when when ip changes](metallb/metallb#520) - this hit me, and might be hitting others - [Allow spaces in configs](metallb/metallb#500) - quality of life - [selflink is deprecated](metallb/metallb#812) - Kubernetes deprecation (I believe seeing this is in the logs is what originally caused me to look into upgrading it)
This was referenced May 14, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As reported in #794, Kubernetes 1.20 deprecated the selfLink field. For MetalLB that caused the nodeAssigned event to fail to be sent, as it was constructed using an older client-go version that still used that field and the API server rejected the message.
The IPs are advertised and all seems to work fine in my testing, just that event is lost and you can't find it when running
kubectl describe service <my-service>to easily know which node is announcing one IP. This is particularly useful for Layer2. Also, please note that the work-around mentioned by @brianmay in the original report (#794) probably works fine, but this is something we want to fix ASAP anyways (and the workaround won't work on k8s 1.21).This PR is just a mix of these PRs: #802 #723 #724.
PR #723 updated client-go but failed the tests. PR #802 is the same, but fixing the test. To do that it forked virtuakube (a dependency not really used anymore) so we can update client-go without issues and tests work fine.
However, we don't really need to fork virtuakube and update to it client-go 1.20, we can just ditch it. That is what PR #724 was doing, but it removed some files used in
inv dev-env.So, this PR just takes the commits from there and amends them to: remove e2e directory (except the manifest used for
inv dev-env) and update client-go but dropping the virtuakube dependency and replaces hacks it added in go.mod.As PR #724 is already approved and the client-go updates are easy, I'm going to just merge this after checking it indeed passes the tests.
Thanks to @russellb @brianmay @uablrek @champtar @invidian and lot of others that were involved in the issues and PRs!
This closes:
Closes: #794
Closes: #722
Closes #192
Closes #629
Closes #639
Closes: #723
Closes: #724
Closes: #802