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

fix: make ipamv2 metrics resilient to missing custom resource definitions #3029

Merged
merged 1 commit into from
Oct 1, 2024

Conversation

rbtr
Copy link
Contributor

@rbtr rbtr commented Sep 24, 2024

Reason for Change:

The metrics observer added in #2970 fails fast if a source of metrics is unavailable. This prevents legacy metrics from being collected at all in scenarios like (as occurred) the ClusterSubnetState custom resource definition is not installed in the cluster.

In this change, the observer is made resilient to missing data sources or errors in the data collection, and will make a best-effort attempt to record the metrics even in case of partial data availability.

Issue Fixed:

Requirements:

Notes:

@rbtr rbtr requested a review from a team as a code owner September 24, 2024 22:52
@rbtr rbtr requested a review from thatmattlong September 24, 2024 22:52
@rbtr rbtr force-pushed the fix/ipamv2-metrics branch from b34b67d to ae2bfc9 Compare September 24, 2024 22:56
@rbtr rbtr requested review from camrynl and ZetaoZhuang September 24, 2024 22:56
@rbtr rbtr self-assigned this Sep 24, 2024
@rbtr rbtr added cns Related to CNS. fix Fixes something. labels Sep 24, 2024
@rbtr rbtr enabled auto-merge September 24, 2024 22:57
@rbtr rbtr force-pushed the fix/ipamv2-metrics branch from ae2bfc9 to d842827 Compare September 25, 2024 16:16
@rbtr rbtr requested a review from a team as a code owner September 25, 2024 16:16
…ions

Signed-off-by: Evan Baker <rbtr@users.noreply.github.com>
@rbtr rbtr force-pushed the fix/ipamv2-metrics branch from d842827 to fb81db6 Compare September 25, 2024 17:54
@rbtr
Copy link
Contributor Author

rbtr commented Sep 25, 2024

/azp run Azure Container Networking PR

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@rbtr rbtr requested a review from nddq September 30, 2024 17:03
@nddq
Copy link
Contributor

nddq commented Oct 1, 2024

lgtm, although this behavior (potentially missing data sources/ errors that affects the metrics) is unique to ipam v2 right?

@rbtr
Copy link
Contributor Author

rbtr commented Oct 1, 2024

yes, due to how ipamv2 pulls the CRs directly (since it doesn't already have them, like ipamv1 did)

@rbtr rbtr added this pull request to the merge queue Oct 1, 2024
Merged via the queue into master with commit 1edb63f Oct 1, 2024
14 checks passed
@rbtr rbtr deleted the fix/ipamv2-metrics branch October 1, 2024 23:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cns Related to CNS. fix Fixes something.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants