Refactor node annotations #23772
Merged
Refactor node annotations #23772
Conversation
This file contains 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
maintainer-s-little-helper
bot
added
the
dont-merge/needs-release-note-label
github-actions
bot
added
the
kind/community-contribution
marseel
added a commit
to marseel/cilium-cli
that referenced
this pull request
Feb 15, 2023
Related PR: cilium/cilium#23772 Signed-off-by: Marcel Zięba <marseel@gmail.com>
squeed
requested changes
Feb 16, 2023
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.
We should only do this if, as mentioned in the original issue, clean-cilium-state is set.
There was a problem hiding this comment.
For now, I removed this part and only pushed refactoring + new function for removing node annotations. I will work on this in separate PR.
marseel
added a commit
to marseel/cilium-cli
that referenced
this pull request
Feb 16, 2023
Related PR: cilium/cilium#23772 Signed-off-by: Marcel Zieba <marcel.zieba@isovalent.com>
marseel
added a commit
to marseel/cilium-cli
that referenced
this pull request
Feb 20, 2023
Related PR: cilium/cilium#23772 Signed-off-by: Marcel Zieba <marcel.zieba@isovalent.com>
marseel
force-pushed
the
fix_k8s_annotations
branch
from
619bc17
to
add4be2
Compare
marseel
changed the title
marseel
force-pushed
the
fix_k8s_annotations
branch
from
add4be2
to
b17058c
Compare
thorn3r
reviewed
Feb 21, 2023
pkg/k8s/annotate_test.go
Outdated
| package k8s | ||
|
|
||
| import ( | ||
| "github.com/cilium/cilium/pkg/annotation" |
There was a problem hiding this comment.
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>
marseel
force-pushed
the
fix_k8s_annotations
branch
from
b17058c
to
bb23a50
Compare
|
/test |
tklauser
pushed a commit
to cilium/cilium-cli
that referenced
this pull request
Feb 21, 2023
Related PR: cilium/cilium#23772 Signed-off-by: Marcel Zieba <marcel.zieba@isovalent.com>
squeed
approved these changes
Feb 22, 2023
squeed
added
the
release-note/misc
maintainer-s-little-helper
bot
removed
the
dont-merge/needs-release-note-label
squeed
added
dont-merge/needs-release-note-label
maintainer-s-little-helper
bot
removed
the
dont-merge/needs-release-note-label
|
Agreed; that seems to be a known flake. Marking as ready-to-merge. |
squeed
added
the
ready-to-merge
christarazi
added
the
sig/k8s
michi-covalent
pushed a commit
to michi-covalent/cilium
that referenced
this pull request
May 30, 2023
Related PR: cilium#23772 Signed-off-by: Marcel Zieba <marcel.zieba@isovalent.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Refactor node annotations and add function for removing node's annotations.
Related: #22306