Skip to content

Prefer highest value on duplicate container metrics#1779

Closed
dippynark wants to merge 1 commit into
kubernetes-sigs:masterfrom
dippynark:improve-duplicate-container-handling
Closed

Prefer highest value on duplicate container metrics#1779
dippynark wants to merge 1 commit into
kubernetes-sigs:masterfrom
dippynark:improve-duplicate-container-handling

Conversation

@dippynark

@dippynark dippynark commented Mar 19, 2026

Copy link
Copy Markdown

What this PR does / why we need it:

If kubelet fails to cleanup cgroups from a previous instance of a container (e.g. due to some zombie process), cAdvisor will continue to expose its metrics (see kubernetes/kubernetes#134518 (comment) for more details).

The following shows an example of a kubelet v1.30.10 returning multiple metrics for the same container (same namespace/name but different UID):

$ kubectl get --raw /api/v1/nodes/[NODE_NAME]/proxy/metrics/resource | grep prometheus-prometheus-k8s-0
...
container_cpu_usage_seconds_total{container="prometheus",namespace="monitoring-system",pod="prometheus-prometheus-k8s-0"} 994631.562157 1773934038841
container_cpu_usage_seconds_total{container="prometheus",namespace="monitoring-system",pod="prometheus-prometheus-k8s-0"} 295207.606798 1773934038962
container_memory_working_set_bytes{container="prometheus",namespace="monitoring-system",pod="prometheus-prometheus-k8s-0"} 5.690306969e+10 1173934038841
container_memory_working_set_bytes{container="prometheus",namespace="monitoring-system",pod="prometheus-prometheus-k8s-0"} 3.3157259264e+10 1773934038962
container_start_time_seconds{container="prometheus",namespace="monitoring-system",pod="prometheus-prometheus-k8s-0"} 1.7691949565992033e+09

Currently Metrics Server will silently override these duplicates giving unpredictable results. This PR improves consistency by always picking the highest value when there are duplicates. In particular, if we are using these metrics to scale memory vertically with VPA, we prefer to overprovision than underpovision.

Note that doing this with CPU does not necessarily help since it is calculated as a rate, however we do this anyway just to make the response consistent (and once container_cpu_usage_seconds_total for a new instance catches up with the old one CPU utilisation for the latest container instance will be calculated properly).

This PR coupled with #1778 makes Metrics Server more resilient to the root cause which should be fixed by this PR.

@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 19, 2026
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Mar 19, 2026
@k8s-ci-robot

Copy link
Copy Markdown
Contributor

This issue is currently awaiting triage.

If metrics-server contributors determine this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Mar 19, 2026
@github-project-automation github-project-automation Bot moved this to Needs Triage in SIG Instrumentation Mar 19, 2026
@k8s-ci-robot

Copy link
Copy Markdown
Contributor

Hi @dippynark. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Tip

We noticed you've done this a few times! Consider joining the org to skip this step and gain /lgtm and other bot rights. We recommend asking approvers on your previous PRs to sponsor you.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Mar 19, 2026
@dippynark dippynark force-pushed the improve-duplicate-container-handling branch from 7b5896b to bd13c20 Compare March 19, 2026 14:03
@dippynark dippynark closed this Mar 20, 2026
@github-project-automation github-project-automation Bot moved this from Needs Triage to Closed in SIG Instrumentation Mar 20, 2026
@dippynark dippynark reopened this Mar 22, 2026
@github-project-automation github-project-automation Bot moved this from Closed to In Progress in SIG Instrumentation Mar 22, 2026
@dippynark dippynark changed the title WIP: Prefer most recent timestamp on duplicate container metrics Prefer highest value on duplicate container metrics Mar 22, 2026
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 22, 2026
@dippynark dippynark changed the title Prefer highest value on duplicate container metrics WIP: Prefer highest value on duplicate container metrics Mar 22, 2026
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 22, 2026
@dippynark dippynark changed the title WIP: Prefer highest value on duplicate container metrics Prefer highest value on duplicate container metrics Mar 22, 2026
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 22, 2026
@dippynark

Copy link
Copy Markdown
Author

/assign @serathius

@dippynark dippynark force-pushed the improve-duplicate-container-handling branch from 7326bba to eae4273 Compare March 23, 2026 17:50
@k8s-ci-robot

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dippynark
Once this PR has been reviewed and has the lgtm label, please ask for approval from serathius. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@dippynark

Copy link
Copy Markdown
Author

This is caused by a kubelet bug fixed in the following PR: kubernetes/kubernetes#137942

@dippynark dippynark closed this Apr 1, 2026
@github-project-automation github-project-automation Bot moved this from In Progress to Closed in SIG Instrumentation Apr 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants