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

ipam: when a CiliumNode is removed, delete node label from metrics. #27713

Conversation

tommyp1ckles
Copy link
Contributor

@tommyp1ckles tommyp1ckles commented Aug 25, 2023

Affects:

  • operator_ipam_available_ips
  • operator_ipam_used_ips
  • operator_ipam_needed_ips

Which have the label "target_name", previously when a Node was deleted the metric continued to be emitted by the Prometheus exporter, leading to confusing sum() values across a cluster.

@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 Aug 25, 2023
@tommyp1ckles tommyp1ckles added release-note/bug This PR fixes an issue in a previous release of Cilium. area/metrics Impacts statistics / metrics gathering, eg via Prometheus. labels Aug 25, 2023
@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 Aug 25, 2023
@tommyp1ckles tommyp1ckles marked this pull request as ready for review August 25, 2023 20:17
@tommyp1ckles tommyp1ckles requested a review from a team as a code owner August 25, 2023 20:17
@christarazi christarazi added sig/ipam IP address management, including cloud IPAM area/ipam Impacts IP address management functionality. needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch labels Aug 25, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from main in 1.14.2 Aug 25, 2023
Copy link
Member

@christarazi christarazi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL about DeleteLabelValues. I thought it wasn't possible. Thanks for the fix.

A few CI failures around build. Also, it would be good to put a Fixes tag for the commit that this PR is fixing so we know where to backport this to. I'm pretty sure it's only for 1.14 so I've added that already.

@tommyp1ckles tommyp1ckles force-pushed the pr/tp/fix-operator-ipam-target-node-metrics-on-delete branch 2 times, most recently from e195ff4 to 4f2c3dd Compare August 25, 2023 21:02
@tommyp1ckles
Copy link
Contributor Author

/test

@tommyp1ckles tommyp1ckles force-pushed the pr/tp/fix-operator-ipam-target-node-metrics-on-delete branch 2 times, most recently from 0088d79 to 1aeacb9 Compare August 25, 2023 21:05
@tommyp1ckles
Copy link
Contributor Author

/test

Affects:

* operator_ipam_available_ips
* operator_ipam_used_ips
* operator_ipam_needed_ips

Which have the label "target_name", previously when a Node was deleted
the metric continued to be emitted by the Prometheus exporter, leading
to confusing sum() values across a cluster.

Fixes changes in cilium#24776

Signed-off-by: Tom Hadlaw <tom.hadlaw@isovalent.com>
@tommyp1ckles tommyp1ckles force-pushed the pr/tp/fix-operator-ipam-target-node-metrics-on-delete branch from 1aeacb9 to c536bc1 Compare August 25, 2023 21:28
@tommyp1ckles
Copy link
Contributor Author

/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 Aug 29, 2023
@aditighag aditighag merged commit 5b7b3bb into cilium:main Aug 29, 2023
60 checks passed
@jibi jibi mentioned this pull request Sep 4, 2023
16 tasks
@jibi jibi added backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. and removed needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch labels Sep 4, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from main to Backport pending to v1.14 in 1.14.2 Sep 4, 2023
@jibi jibi added backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. and removed backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. labels Sep 7, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.14 to Backport done to v1.14 in 1.14.2 Sep 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ipam Impacts IP address management functionality. area/metrics Impacts statistics / metrics gathering, eg via Prometheus. backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/bug This PR fixes an issue in a previous release of Cilium. sig/ipam IP address management, including cloud IPAM
Projects
No open projects
1.14.2
Backport done to v1.14
Development

Successfully merging this pull request may close these issues.

None yet

4 participants