Skip to content

Conversation

AritraDey-Dev
Copy link
Member

@AritraDey-Dev AritraDey-Dev commented Jun 6, 2025

Deletes Cilium-related annotations from K8s nodes as part of the uninstall process.

Fixes #22306

Please ensure your pull request adheres to the following guidelines:

  • For first time contributors, read Submitting a pull request
  • All code is covered by unit and/or runtime tests where feasible.
  • All commits contain a well written commit description including a title,
    description and a Fixes: #XXX line if the commit addresses a particular
    GitHub issue.
  • If your commit description contains a Fixes: <commit-id> tag, then
    please add the commit author[s] as reviewer[s] to this issue.
  • All commits are signed off. See the section Developer’s Certificate of Origin
  • Provide a title or release-note blurb suitable for the release notes.
  • Are you a user of Cilium? Please add yourself to the Users doc
  • Thanks for contributing!

Fixes: #22306

Cilium uninstall now removes annotations from Kubernetes nodes when clean-cilium-state: true

@AritraDey-Dev AritraDey-Dev requested review from a team as code owners June 6, 2025 20:48
@AritraDey-Dev AritraDey-Dev requested review from asauber and Artyop June 6, 2025 20:48
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jun 6, 2025
@github-actions github-actions bot added cilium-cli This PR contains changes related with cilium-cli cilium-cli-exclusive This PR only impacts cilium-cli binary kind/community-contribution This was a contribution made by a community member. labels Jun 6, 2025
@asauber
Copy link
Member

asauber commented Jun 9, 2025

Hi @AritraDey-Dev, thanks for the contribution. Would you mind squashing these two commits into one?

@asauber asauber added the release-note/minor This PR changes functionality that users may find relevant to operating Cilium. label Jun 9, 2025
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jun 9, 2025
@AritraDey-Dev AritraDey-Dev force-pushed the cleanup-cilium-node-annotations branch from 9711ae3 to c9991ff Compare June 9, 2025 10:10
@AritraDey-Dev
Copy link
Member Author

Would you mind squashing these two commits into one?

done.

@AritraDey-Dev AritraDey-Dev force-pushed the cleanup-cilium-node-annotations branch from c9991ff to 7f87c0c Compare June 9, 2025 10:26
@aanm aanm enabled auto-merge June 16, 2025 09:51
@aanm
Copy link
Member

aanm commented Jun 16, 2025

/test

@joestringer
Copy link
Member

The Cilium E2E Upgrade test is consistently failing with the same symptoms, so it looks like this PR may be causing that behavior. Furthermore the last two failures for the Multi Pool IPAM tests also had the same symptoms. I think that for this PR to move forward, these failures will need to be triaged to ensure that the PR doesn't break the tests.

auto-merge was automatically disabled June 28, 2025 16:11

Head branch was pushed to by a user without write access

@AritraDey-Dev
Copy link
Member Author

Hi, could someone from the team please run a /test?I'm happy to refactor if there are still failures.

@Artyop
Copy link
Contributor

Artyop commented Jun 30, 2025

/test

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jul 1, 2025
@tklauser
Copy link
Member

tklauser commented Jul 3, 2025

@AritraDey-Dev could you please drop the merge commit? Then we should be good to merge this PR.

@tklauser tklauser added the dont-merge/needs-rebase This PR needs to be rebased because it has merge conflicts. label Jul 3, 2025
@AritraDey-Dev AritraDey-Dev force-pushed the cleanup-cilium-node-annotations branch 3 times, most recently from 1ad47e5 to 273bf33 Compare July 3, 2025 09:09
@AritraDey-Dev
Copy link
Member Author

@AritraDey-Dev could you please drop the merge commit? Then we should be good to merge this PR.

rebased. PTAL.

@tklauser tklauser removed the dont-merge/needs-rebase This PR needs to be rebased because it has merge conflicts. label Jul 3, 2025
@tklauser
Copy link
Member

tklauser commented Jul 3, 2025

/test

@tklauser
Copy link
Member

tklauser commented Jul 3, 2025

@AritraDey-Dev could you please drop the merge commit? Then we should be good to merge this PR.

rebased. PTAL.

Thanks, LGTM.

@tklauser tklauser enabled auto-merge July 3, 2025 09:18
auto-merge was automatically disabled July 3, 2025 10:06

Head branch was pushed to by a user without write access

@AritraDey-Dev AritraDey-Dev force-pushed the cleanup-cilium-node-annotations branch from 273bf33 to 85dd3aa Compare July 3, 2025 10:06
Deletes Cilium-related annotations from K8s nodes.

Fixes cilium#22306

Signed-off-by: Aritra Dey <adey01027@gmail.com>
@AritraDey-Dev AritraDey-Dev force-pushed the cleanup-cilium-node-annotations branch from 85dd3aa to 07ae783 Compare July 3, 2025 10:17
@AritraDey-Dev
Copy link
Member Author

There was a spacing issue for which tests were failing i have fixed it.

@tklauser
Copy link
Member

tklauser commented Jul 3, 2025

/test

@tklauser tklauser enabled auto-merge July 3, 2025 11:15
@AritraDey-Dev
Copy link
Member Author

These tests passed last time. I believe they are flaky. Could you please retrigger this workflow?

@joestringer
Copy link
Member

Please investigate failures before asking to re-run the tests. The failure looks like it is consistent (text copy below). At a glance it doesn't seem related to the PR, but that doesn't mean that it will just go away if you keep re-running it:

    cilium    cilium-gp7k9    container cilium-agent is in CrashLoopBackOff, pulling previous Pod logs for further investigation:
2025-07-03T11:24:36.024058392Z time=2025-07-03T11:24:36.023804029Z level=debug msg="Skipped reading configuration file" error="Config File \"cilium\" Not Found in \"[/root]\""
2025-07-03T11:24:36.024530774Z time=2025-07-03T11:24:36.024349869Z level=info source=/go/src/github.com/cilium/cilium/pkg/option/config.go:3301 msg="Memory available for map entries (0.250% of 12550979584B): 31377448B"
2025-07-03T11:24:36.024541825Z time=2025-07-03T11:24:36.024402097Z level=debug source=/go/src/github.com/cilium/cilium/pkg/option/config.go:3308 msg="Total memory for default map entdries: 149422080"
2025-07-03T11:24:36.024545651Z time=2025-07-03T11:24:36.024412907Z level=info source=/go/src/github.com/cilium/cilium/pkg/option/config.go:3346 msg="option bpf-ct-global-tcp-max set by dynamic sizing to 131072"
2025-07-03T11:24:36.024548807Z time=2025-07-03T11:24:36.024418738Z level=info source=/go/src/github.com/cilium/cilium/pkg/option/config.go:3354 msg="option bpf-ct-global-any-max set by dynamic sizing to 65536"
<...>
2025-07-03T11:24:36.587151443Z time=2025-07-03T11:24:36.586284574Z level=error source=/go/src/github.com/cilium/cilium/vendor/github.com/cilium/hive/cell/lifecycle.go:129 msg="Start hook failed" function="cmd.newDaemonPromise.func1 (cmd/daemon_main.go:1430) (agent.controlplane.daemon)" error="daemon creation failed: WireGuard requires an IPv4 underlay"
2025-07-03T11:24:36.587162183Z time=2025-07-03T11:24:36.586305263Z level=error source=/go/src/github.com/cilium/cilium/vendor/github.com/cilium/hive/hive.go:363 msg="Failed to start hive" error="daemon creation failed: WireGuard requires an IPv4 underlay" duration=234.287186ms
<...>

@joestringer
Copy link
Member

For context, it looks like WireGuard + IPv6 underlay was just recently added into the tree (#40051), and this PR is based on an older version of the main branch from before the feature was added. Still, Cilium runs the latest tests against your PR, so this means that the code does not support the test case but the test case will run from the latest branch. The test case then fails.

Overall this means that the failure should be unrelated to this PR. In general, it can help to rebase against the tip of main whenever you update the PR to mitigate this case. In this specific case, given that this is an isolated failure and the PR looks otherwise fine (and unlikely to cause other regressions), I will bypass the merge criteria to merge the PR.

Thanks for your contribution @AritraDey-Dev .

@joestringer joestringer disabled auto-merge July 3, 2025 18:57
@joestringer joestringer merged commit e92934e into cilium:main Jul 3, 2025
66 of 68 checks passed
@joestringer
Copy link
Member

joestringer commented Jul 3, 2025

I updated the release note to remove unclear references like the word this and to communicate the functionality of this PR more clearly to users in the release notes.

@AritraDey-Dev AritraDey-Dev deleted the cleanup-cilium-node-annotations branch July 3, 2025 19:03
zocimek added a commit to zocimek/home-ops that referenced this pull request Aug 25, 2025
This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
|
[aqua:cilium/cilium-cli](https://redirect.github.com/cilium/cilium-cli)
| patch | `0.18.5` -> `0.18.6` |

---

### Release Notes

<details>
<summary>cilium/cilium-cli (aqua:cilium/cilium-cli)</summary>

###
[`v0.18.6`](https://redirect.github.com/cilium/cilium-cli/releases/tag/v0.18.6)

[Compare
Source](https://redirect.github.com/cilium/cilium-cli/compare/v0.18.5...v0.18.6)

#### What's Changed

**Minor Changes:**

- Cilium uninstall now removes annotations from Kubernetes nodes when
clean-cilium-state: true
([cilium/cilium#39931](https://redirect.github.com/cilium/cilium/issues/39931),
[@&#8203;AritraDey-Dev](https://redirect.github.com/AritraDey-Dev))
- Deprecate `v2alpha1` version of `CiliumLoadBalancerIPPool` CRD in
favor of the `v2` version
([cilium/cilium#39134](https://redirect.github.com/cilium/cilium/issues/39134),
[@&#8203;pippolo84](https://redirect.github.com/pippolo84))

**Bugfixes:**

- Fix bug where we would display the Max Seq. Number for IPsec on
32bits.
([cilium/cilium#40622](https://redirect.github.com/cilium/cilium/issues/40622),
[@&#8203;pchaigno](https://redirect.github.com/pchaigno))

**CI Changes:**

- Add l7 proxy check for `to-fqdns` connectivity test
([cilium/cilium#40549](https://redirect.github.com/cilium/cilium/issues/40549),
[@&#8203;vipul-21](https://redirect.github.com/vipul-21))
- cli: switch coredns image to registry.k8s.io, and fix renovate
([cilium/cilium#40706](https://redirect.github.com/cilium/cilium/issues/40706),
[@&#8203;giorio94](https://redirect.github.com/giorio94))
- connectivity: Allow customization of tcpdump kill timeout
([cilium/cilium#40774](https://redirect.github.com/cilium/cilium/issues/40774),
[@&#8203;gentoo-root](https://redirect.github.com/gentoo-root))
- connectivity: rework sniffer to execute tcpdump in background
([cilium/cilium#40487](https://redirect.github.com/cilium/cilium/issues/40487),
[@&#8203;smagnani96](https://redirect.github.com/smagnani96))

**Misc Changes:**

- chore(deps): update docker.io/library/golang:1.24.4 docker digest to
[`20a022e`](https://redirect.github.com/cilium/cilium-cli/commit/20a022e)
(main)
([cilium/cilium#40379](https://redirect.github.com/cilium/cilium/issues/40379),
[@&#8203;cilium-renovate](https://redirect.github.com/cilium-renovate)\[bot])
- chore(deps): update docker.io/library/golang:1.24.5 docker digest to
[`ef5b4be`](https://redirect.github.com/cilium/cilium-cli/commit/ef5b4be)
(main)
([cilium/cilium#40738](https://redirect.github.com/cilium/cilium/issues/40738),
[@&#8203;cilium-renovate](https://redirect.github.com/cilium-renovate)\[bot])
- chore(deps): update go to v1.24.5 (main)
([cilium/cilium#40496](https://redirect.github.com/cilium/cilium/issues/40496),
[@&#8203;cilium-renovate](https://redirect.github.com/cilium-renovate)\[bot])
- cilium-cli: Print partial output upon `bgp peers` errors
([cilium/cilium#40278](https://redirect.github.com/cilium/cilium/issues/40278),
[@&#8203;rastislavs](https://redirect.github.com/rastislavs))
- cilium-cli: Update default network-perf image
([cilium/cilium#40376](https://redirect.github.com/cilium/cilium/issues/40376),
[@&#8203;HadrienPatte](https://redirect.github.com/HadrienPatte))
- cilium-cli: Use slim k8s packages for connectivity tests
([cilium/cilium#40708](https://redirect.github.com/cilium/cilium/issues/40708),
[@&#8203;HadrienPatte](https://redirect.github.com/HadrienPatte))
- Fix misc typos
([cilium/cilium#40769](https://redirect.github.com/cilium/cilium/issues/40769),
[@&#8203;HadrienPatte](https://redirect.github.com/HadrienPatte))
- go.mod, vendor: pull in charts for Cilium 1.18.0 and Tetragon 1.5.0
([cilium/cilium#40823](https://redirect.github.com/cilium/cilium/issues/40823),
[@&#8203;tklauser](https://redirect.github.com/tklauser))
- Miscellaneous improvements to option.NewNamedMapOptions
([cilium/cilium#40529](https://redirect.github.com/cilium/cilium/issues/40529),
[@&#8203;giorio94](https://redirect.github.com/giorio94))
- The unableTranslateCIDRgroups variable is removed as it is not used
since the v1.17 release
([cilium/cilium#40267](https://redirect.github.com/cilium/cilium/issues/40267),
[@&#8203;Surya-7890](https://redirect.github.com/Surya-7890))
- vendor: Update github.com/google/go-github to v73
([cilium/cilium#40326](https://redirect.github.com/cilium/cilium/issues/40326),
[@&#8203;HadrienPatte](https://redirect.github.com/HadrienPatte))
- Update stable release to v0.18.5 by
[@&#8203;tklauser](https://redirect.github.com/tklauser) in
[https://github.com/cilium/cilium-cli/pull/3060](https://redirect.github.com/cilium/cilium-cli/pull/3060)
- chore(deps): update docker.io/library/golang:1.24.4 docker digest to
[`20a022e`](https://redirect.github.com/cilium/cilium-cli/commit/20a022e)
by [@&#8203;renovate](https://redirect.github.com/renovate)\[bot]
in[https://github.com/cilium/cilium-cli/pull/3061](https://redirect.github.com/cilium/cilium-cli/pull/3061)1
- Update RELEASE.md by
[@&#8203;michi-covalent](https://redirect.github.com/michi-covalent) in
[https://github.com/cilium/cilium-cli/pull/3062](https://redirect.github.com/cilium/cilium-cli/pull/3062)
- chore(deps): update golang docker tag to v1.24.5 by
[@&#8203;renovate](https://redirect.github.com/renovate)\[bot]
in[https://github.com/cilium/cilium-cli/pull/3063](https://redirect.github.com/cilium/cilium-cli/pull/3063)3
- chore(deps): update go to v1.24.5 (patch) by
[@&#8203;renovate](https://redirect.github.com/renovate)\[bot]
in[https://github.com/cilium/cilium-cli/pull/3065](https://redirect.github.com/cilium/cilium-cli/pull/3065)5
- chore(deps): update golangci/golangci-lint docker tag to v2.2.2 by
[@&#8203;renovate](https://redirect.github.com/renovate)\[bot]
in[https://github.com/cilium/cilium-cli/pull/3066](https://redirect.github.com/cilium/cilium-cli/pull/3066)6
- chore(deps): update dependency cilium/cilium to v1.17.6 by
[@&#8203;renovate](https://redirect.github.com/renovate)\[bot]
in[https://github.com/cilium/cilium-cli/pull/3068](https://redirect.github.com/cilium/cilium-cli/pull/3068)8
- chore(deps): update golang:1.24.5-alpine3.21 docker digest to
[`3ebc008`](https://redirect.github.com/cilium/cilium-cli/commit/3ebc008)
by [@&#8203;renovate](https://redirect.github.com/renovate)\[bot]
in[https://github.com/cilium/cilium-cli/pull/3067](https://redirect.github.com/cilium/cilium-cli/pull/3067)7
- chore(deps): update golang:1.24.5-alpine3.21 docker digest to
[`72ff633`](https://redirect.github.com/cilium/cilium-cli/commit/72ff633)
by [@&#8203;renovate](https://redirect.github.com/renovate)\[bot]
in[https://github.com/cilium/cilium-cli/pull/3069](https://redirect.github.com/cilium/cilium-cli/pull/3069)9
- chore(deps): update golang:1.24.5-alpine3.21 docker digest to
[`6edc205`](https://redirect.github.com/cilium/cilium-cli/commit/6edc205)
by [@&#8203;renovate](https://redirect.github.com/renovate)\[bot]
in[https://github.com/cilium/cilium-cli/pull/3070](https://redirect.github.com/cilium/cilium-cli/pull/3070)0
- chore(deps): update golangci/golangci-lint docker tag to v2.3.0 -
autoclosed by
[@&#8203;renovate](https://redirect.github.com/renovate)\[bot]
in[https://github.com/cilium/cilium-cli/pull/3071](https://redirect.github.com/cilium/cilium-cli/pull/3071)1
- chore(deps): update dependency cilium/cilium to v1.18.0 by
[@&#8203;renovate](https://redirect.github.com/renovate)\[bot]
in[https://github.com/cilium/cilium-cli/pull/3073](https://redirect.github.com/cilium/cilium-cli/pull/3073)3
- chore(deps): update docker.io/library/golang:1.24.5 docker digest to
[`ef5b4be`](https://redirect.github.com/cilium/cilium-cli/commit/ef5b4be)
by [@&#8203;renovate](https://redirect.github.com/renovate)\[bot]
in[https://github.com/cilium/cilium-cli/pull/3072](https://redirect.github.com/cilium/cilium-cli/pull/3072)2
- Prepare for v0.18.6 release by
[@&#8203;tklauser](https://redirect.github.com/tklauser) in
[https://github.com/cilium/cilium-cli/pull/3074](https://redirect.github.com/cilium/cilium-cli/pull/3074)

**Full Changelog**:
cilium/cilium-cli@v0.18.5...v0.18.6

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR was generated by [Mend Renovate](https://mend.io/renovate/).
View the [repository job
log](https://developer.mend.io/github/zocimek/home-ops).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0MS40My41IiwidXBkYXRlZEluVmVyIjoiNDEuNDYuMyIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsidHlwZS9wYXRjaCJdfQ==-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Łukasz Pospiech <zocimek@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cilium-cli This PR contains changes related with cilium-cli cilium-cli-exclusive This PR only impacts cilium-cli binary kind/community-contribution This was a contribution made by a community member. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/minor This PR changes functionality that users may find relevant to operating Cilium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clean up K8s Node annotations created by Cilium when running with clean-cilium-state: true
6 participants