Skip to content

Conversation

voelzmo
Copy link
Member

@voelzmo voelzmo commented Feb 21, 2024

How to categorize this PR?

/area auto-scaling
/kind bug

What this PR does / why we need it:
Remove the containerName: "*" catch-all definition from the containerPolicies for the prometheus VPAs and move the configuration into the containers explicitly.

VPA doesn't actually merge these configurations: as soon as you specify a containerPolicy for a specific container name, all configuration under containerName: "*" is ignored. This can lead to unwanted cases, for example here you could assume that VPA was configured to scale only requests, but not limits for the prometheus container, but this was not the case. This configuration would only be set for all containers not explicitly mentioned in the containerPolicies section with their name.

Release note:

NONE

@gardener-prow gardener-prow bot added area/auto-scaling Auto-scaling (CA/HPA/VPA/HVPA, predominantly control plane, but also otherwise) related kind/bug Bug labels Feb 21, 2024
@gardener-prow gardener-prow bot requested review from acumino and timebertt February 21, 2024 13:15
@gardener-prow gardener-prow bot added cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Feb 21, 2024
@voelzmo
Copy link
Member Author

voelzmo commented Feb 21, 2024

/cc @rfranzke @nickytd @oliver-goetz

@voelzmo voelzmo force-pushed the fix/prometheus-vpa-configuration branch from 765afe2 to 158d201 Compare February 21, 2024 13:23
Copy link
Member

@rfranzke rfranzke left a comment

Choose a reason for hiding this comment

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

Thanks!

/lgtm
/approve

@gardener-prow gardener-prow bot added the lgtm Indicates that a PR is ready to be merged. label Feb 21, 2024
Copy link
Contributor

gardener-prow bot commented Feb 21, 2024

LGTM label has been added.

Git tree hash: edbc48ef2c888a4fa8df403062f4e3e648856726

Copy link
Contributor

gardener-prow bot commented Feb 21, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rfranzke

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

The pull request process is described here

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

@rfranzke
Copy link
Member

/cherry-pick release-v1.89

@gardener-prow gardener-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 21, 2024
@gardener-ci-robot
Copy link
Contributor

@rfranzke: once the present PR merges, I will cherry-pick it on top of release-v1.89 in a new PR and assign it to you.

In response to this:

/cherry-pick release-v1.89

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/test-infra repository.

@gardener-prow gardener-prow bot merged commit 1f02d3a into gardener:master Feb 21, 2024
@gardener-ci-robot
Copy link
Contributor

@rfranzke: new pull request created: #9199

In response to this:

/cherry-pick release-v1.89

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/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/auto-scaling Auto-scaling (CA/HPA/VPA/HVPA, predominantly control plane, but also otherwise) related cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. kind/bug Bug lgtm Indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants