-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Refactor node annotations #23772
Refactor node annotations #23772
Conversation
Related PR: cilium/cilium#23772 Signed-off-by: Marcel Zięba <marseel@gmail.com>
daemon/cmd/daemon.go
Outdated
@@ -1192,7 +1192,15 @@ func newDaemon(ctx context.Context, cleaner *daemonCleanup, | |||
node.GetInternalIPv4Router(), node.GetIPv6Router()) | |||
if err != nil { | |||
log.WithError(err).Warning("Cannot annotate k8s node with CIDR range") | |||
} else { | |||
cleaner.cleanupFuncs.Add(func() { |
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.
We should only do this if, as mentioned in the original issue, clean-cilium-state is set.
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.
For now, I removed this part and only pushed refactoring + new function for removing node annotations. I will work on this in separate PR.
Related PR: cilium/cilium#23772 Signed-off-by: Marcel Zieba <marcel.zieba@isovalent.com>
Related PR: cilium/cilium#23772 Signed-off-by: Marcel Zieba <marcel.zieba@isovalent.com>
619bc17
to
add4be2
Compare
add4be2
to
b17058c
Compare
pkg/k8s/annotate_test.go
Outdated
package k8s | ||
|
||
import ( | ||
"github.com/cilium/cilium/pkg/annotation" |
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.
got a linting error here. i think this should fix it:
goimports -w -local github.com/cilium/cilium/pkg/annotation pkg/k8s/annotate_test.go
…tions. Related: cilium#22306 Signed-off-by: Marcel Zieba <marcel.zieba@isovalent.com>
b17058c
to
bb23a50
Compare
/test |
Related PR: cilium/cilium#23772 Signed-off-by: Marcel Zieba <marcel.zieba@isovalent.com>
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.
LGTM!
CI hit a flake tracked here: #23845
Agreed; that seems to be a known flake. Marking as ready-to-merge. |
Related PR: cilium#23772 Signed-off-by: Marcel Zieba <marcel.zieba@isovalent.com>
Refactor node annotations and add function for removing node's annotations.
Related: #22306