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

[kube-prometheus-stack] Do not set Default Resource Requests and Limits for Prometheus Config Reloader #3812

Closed
mmianl opened this issue Sep 19, 2023 · 4 comments · Fixed by #3813
Labels
enhancement New feature or request

Comments

@mmianl
Copy link
Contributor

mmianl commented Sep 19, 2023

Is your feature request related to a problem ?

At the moment, it's not possible to unset resource requests and/or limits because the default values file specifies these values:

prometheusConfigReloader:
  resources:
      requests:
        cpu: 200m
        memory: 50Mi
      limits:
        cpu: 200m
        memory: 50Mi

The other components in the chart do not specify any default resources which makes sense because it leaves the user with the option to not set cpu and memory requests and limits which is preferred by some users (especially regarding cpu limits).

Describe the solution you'd like.

Remove the default Prometheus Config Reloader resources.

Describe alternatives you've considered.

NONE

Additional context.

No response

@mmianl mmianl added the enhancement New feature or request label Sep 19, 2023
mmianl added a commit to mmianl/helm-charts that referenced this issue Sep 19, 2023
mmianl added a commit to mmianl/helm-charts that referenced this issue Sep 19, 2023
…mits from prometheus config reloader (prometheus-community#3812)

Signed-off-by: Markus Müllner <[email protected]>
@GMartinez-Sisti
Copy link
Member

@mmianl you can override by setting it to null:

prometheusConfigReloader:
  resources: null

mmianl added a commit to mmianl/helm-charts that referenced this issue Sep 19, 2023
mmianl added a commit to mmianl/helm-charts that referenced this issue Sep 20, 2023
@mmianl
Copy link
Contributor Author

mmianl commented Sep 20, 2023

@mmianl you can override by setting it to null:

prometheusConfigReloader:
  resources: null

@GMartinez-Sisti I just tried that and it causes a nil pointer error. Also, I think that approach wouldn't allow to specify only cpu requests, but no cpu limit.

@GMartinez-Sisti
Copy link
Member

@mmianl you can override by setting it to null:

prometheusConfigReloader:
  resources: null

@GMartinez-Sisti I just tried that and it causes a nil pointer error. Also, I think that approach wouldn't allow to specify only cpu requests, but no cpu limit.

You're right. I forgot this is different and used in a different way. Not the usual resources map for a container.

@mmianl
Copy link
Contributor Author

mmianl commented Sep 20, 2023

@mmianl you can override by setting it to null:

prometheusConfigReloader:
  resources: null

@GMartinez-Sisti I just tried that and it causes a nil pointer error. Also, I think that approach wouldn't allow to specify only cpu requests, but no cpu limit.

You're right. I forgot this is different and used in a different way. Not the usual resources map for a container.

@GMartinez-Sisti I came up with this (#3813) solution, what do you think of it?

mmianl added a commit to mmianl/helm-charts that referenced this issue Sep 21, 2023
mmianl added a commit to mmianl/helm-charts that referenced this issue Sep 21, 2023
GMartinez-Sisti pushed a commit that referenced this issue Sep 21, 2023
#3813)

* [kube-prometheus-stack] remove default cpu and memory requests and limits from prometheus config reloader (#3812)

Signed-off-by: Markus Müllner <[email protected]>

* [kube-prometheus-stack] update prometheus operator deployment template (#3812)

Signed-off-by: Markus Müllner <[email protected]>

* [kube-prometheus-stack] fix prometheus operator deployment template by wrapping nullable values (#3812)

Signed-off-by: Markus Müllner <[email protected]>

* [kube-prometheus-stack] use default value instead of if in prometheus operator deployment template (#3812)

Signed-off-by: Markus Müllner <[email protected]>

---------

Signed-off-by: Markus Müllner <[email protected]>
Matiasmct pushed a commit to giffgaff/prometheus-charts-backup that referenced this issue Mar 20, 2024
prometheus-community#3813)

* [kube-prometheus-stack] remove default cpu and memory requests and limits from prometheus config reloader (prometheus-community#3812)

Signed-off-by: Markus Müllner <[email protected]>

* [kube-prometheus-stack] update prometheus operator deployment template (prometheus-community#3812)

Signed-off-by: Markus Müllner <[email protected]>

* [kube-prometheus-stack] fix prometheus operator deployment template by wrapping nullable values (prometheus-community#3812)

Signed-off-by: Markus Müllner <[email protected]>

* [kube-prometheus-stack] use default value instead of if in prometheus operator deployment template (prometheus-community#3812)

Signed-off-by: Markus Müllner <[email protected]>

---------

Signed-off-by: Markus Müllner <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants