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: add legacy IPAM metrics back to IPAMv2 #2970

Merged
merged 1 commit into from
Sep 4, 2024
Merged

Conversation

rbtr
Copy link
Contributor

@rbtr rbtr commented Aug 28, 2024

Reason for Change:
IPAM metrics that were set by the old IPAM PoolMonitor were not set in the v2 monitor. This breaks the AMA workbooks for subnet usage monitoring on AKS 1.30+ where CNS 1.6 with v2 IPAM is deployed.

The metrics were missing because the v2 PoolMonitor does not use most of that source data anymore - it depends only on Pod count and Batch size, not the full IPAM utilization buckets and previous NNC state that the v1 implementation needed to scale up/down.

This fix creates a "metrics observer" which functionally performs what the IPAM v1 monitor used to do WRT the metrics in a portable closed scope. It is injected into the v2 monitor so that the v2 monitor does not need to be polluted with the data pulls that the v1 monitor needed and can still trigger the metrics updates.

TODO: As the v1 monitor is phased-out (hopefully in the 1.7 release of CNS), these metrics should be removed to more appropriate locations, such as moving the Allocated/Available/Assigned/etc IP state metrics in to the CNS core datastore IPConfig paths where those IP states are mutated.

Issue Fixed:

Requirements:

Notes:

@rbtr rbtr added cns Related to CNS. swift Related to SWIFT networking. fix Fixes something. release/latest Change affects latest release train labels Aug 28, 2024
@rbtr rbtr self-assigned this Aug 28, 2024
@rbtr rbtr requested a review from a team as a code owner August 28, 2024 22:50
@rbtr rbtr requested review from thatmattlong and nddq August 28, 2024 22:50
@rbtr rbtr force-pushed the fix/ipam-metrics branch from 7053b04 to 25a85a8 Compare August 28, 2024 22:52
@rbtr rbtr requested a review from a team as a code owner August 28, 2024 22:52
@rbtr
Copy link
Contributor Author

rbtr commented Aug 29, 2024

/azp run Azure Container Networking PR

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@rbtr rbtr enabled auto-merge August 29, 2024 15:14
@rbtr rbtr added this pull request to the merge queue Sep 3, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 3, 2024
@nddq nddq added this pull request to the merge queue Sep 3, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 3, 2024
@nddq nddq added this pull request to the merge queue Sep 3, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 4, 2024
@nddq nddq added this pull request to the merge queue Sep 4, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 4, 2024
Signed-off-by: Evan Baker <rbtr@users.noreply.github.com>
@rbtr rbtr enabled auto-merge September 4, 2024 15:45
@rbtr
Copy link
Contributor Author

rbtr commented Sep 4, 2024

/azp run Azure Container Networking PR

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@rbtr rbtr added this pull request to the merge queue Sep 4, 2024
Merged via the queue into master with commit ff46b57 Sep 4, 2024
14 checks passed
@rbtr rbtr deleted the fix/ipam-metrics branch September 4, 2024 18:23
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. release/latest Change affects latest release train swift Related to SWIFT networking.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants