stakater / Reloader

A Kubernetes controller to watch changes in ConfigMap and Secrets and do rolling upgrades on Pods with their associated Deployment, StatefulSet, DaemonSet and DeploymentConfig – [✩Star] if you're using it!
https://docs.stakater.com/reloader/
Apache License 2.0
7.47k stars 498 forks source link

[ENHANCE] #639 , Add resources to the Reloader deployment. #692

Closed kunj-bosamia closed 3 months ago

kunj-bosamia commented 3 months ago

Memory -> request & limits Cpu -> requests & limits are added to the reloader deployment.

github-actions[bot] commented 3 months ago

@bnallapeta Image is available for testing. docker pull ghcr.io/stakater/docs/reloader:v1.0.110

kunj-bosamia commented 3 months ago

Related issue -> https://github.com/stakater/Reloader/issues/639

MuneebAijaz commented 3 months ago

hi @kunj-bosamia thanks for the PR, i'd prefer if the resources are added to plain manifests only and to not change the chart values. There's a helm template command in workflows, you can add this change in that command instead. https://github.com/stakater/Reloader/blob/master/.github/workflows/push.yaml#L212

github-actions[bot] commented 3 months ago

@kunj-bosamia Images are available for testing. docker pull ghcr.io/stakater/reloader:SNAPSHOT-PR-692-85dfee8d\ndocker pull ghcr.io/stakater/reloader:SNAPSHOT-PR-692-UBI-85dfee8d

bnallapeta commented 3 months ago

@kunj-bosamia While it's a good practice to set resource requests and limits, I believe it's better to provide documentation on how to set these values instead of enforcing them. The default values may not be suitable for every user's environment. For example, clusters with 1000 deployments might find the set limits inadequate, and vice versa. Providing clear guidance allows users to configure their resources based on their specific needs and cluster capacity, balancing best practices with flexibility.

Additionally, our current documentation includes parameters that users can pass during the Helm install command here. They can also set it in values.yaml, where recommended values are commented out.

What do you think?

kunj-bosamia commented 3 months ago

Yeah I understand your point of view , the people who are doing helm install to install the reloader app in their k8s environment will have a control to edit the resources.

I think most of the users directly do a

kubectl apply -f https://raw.githubusercontent.com/stakater/Reloader/master/deployments/kubernetes/reloader.yaml

this is what I did in my k8s env , I know it's bad practice but we can get away with it.

My point being at least this relaoser.yaml file should have resources defined in it. I will make changes in my PR to include resources in just reloader.yaml and not the helm chart.

github-actions[bot] commented 3 months ago

@kunj-bosamia Images are available for testing. docker pull ghcr.io/stakater/reloader:SNAPSHOT-PR-692-f167a9ba\ndocker pull ghcr.io/stakater/reloader:SNAPSHOT-PR-692-UBI-f167a9ba

bnallapeta commented 3 months ago

@kunj-bosamia Sure. It makes sense to set moderate requests and limits as a safety net.

Also, could you please add this in docs under this section? It'll help users know they can tweak it as needed.

kunj-bosamia commented 3 months ago

@kunj-bosamia Sure. It makes sense to set moderate requests and limits as a safety net.

Also, could you please add this in docs under this section? It'll help users know they can tweak it as needed.

Added :+1:

github-actions[bot] commented 3 months ago

@kunj-bosamia Images are available for testing. docker pull ghcr.io/stakater/reloader:SNAPSHOT-PR-692-d1abc2ee\ndocker pull ghcr.io/stakater/reloader:SNAPSHOT-PR-692-UBI-d1abc2ee

github-actions[bot] commented 3 months ago

@kunj-bosamia Images are available for testing. docker pull ghcr.io/stakater/reloader:SNAPSHOT-PR-692-0db4652d\ndocker pull ghcr.io/stakater/reloader:SNAPSHOT-PR-692-UBI-0db4652d

github-actions[bot] commented 3 months ago

@kunj-bosamia Images are available for testing. docker pull ghcr.io/stakater/reloader:SNAPSHOT-PR-692-953957f1\ndocker pull ghcr.io/stakater/reloader:SNAPSHOT-PR-692-UBI-953957f1