search

kubernetes / ingress-nginx

Ingress-NGINX Controller for Kubernetes
https://kubernetes.github.io/ingress-nginx/
Apache License 2.0
17.26k stars 8.21k forks source link

Add affinity to controller.admissionWebhooks.patch webhook #11750

Open soundarya2606 opened 1 month ago

soundarya2606 commented 1 month ago

Ability to add affinity to controller.admissionWebhooks.patch. Right now, it only allows to use nodeSelector and tolerations through the chart. https://github.com/kubernetes/ingress-nginx/blob/main/charts/ingress-nginx/templates/admission-webhooks/job-patch/job-patchWebhook.yaml https://github.com/kubernetes/ingress-nginx/blob/main/charts/ingress-nginx/values.yaml#L807-L844

k8s-ci-robot commented 1 month ago

This issue is currently awaiting triage.

If Ingress contributors determines 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.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
longwuyuan commented 1 month ago

why has absense of affinity not impacted other users so far ? what is your use-case that upstream project needs to change the chart and add affinity for ?

soundarya2606 commented 1 month ago

why has absense of affinity not impacted other users so far ? what is your use-case that upstream project needs to change the chart and add affinity for ?

We have an use case to schedule pods on our custom nodepool and having affinity will help us to define more constraints on which nodepool to use. The helm chart allows to add affinity for ingress-nginx controller pods and it would be nice to have the same for the controller.admissionWebhooks.patch as well.

longwuyuan commented 1 month ago

Thanks. I guess I should have phrased differently.

Its a patch job and deduced implication is a short lifetime and nowhere consequential in practical terms to resource consumption or sustained networking etc, (factors which one can assume to be considered for affinity of a pod to a node or nodepool).

Further, you would be the only user expecting this and not enough details are available as to how it will impact other users. It seems harmless as just a optional key but its not worth changing the project template for a job that finishes in seconds or less.

So in this context I was curious to know what justifies a change in the project as compared to you maintaining a fork etc etc. The reason for exploring this direction is that the project is trying to simplify and avoid less-used features.

I also see you closed your PR so my opinion is not to change the project code. This as it seems like a mildly experimental tweak for a job that runs for less than a few seconds instead of a problem-solving change that helps a large number of users experiencing some scheduling problem. Unless I am missing something.

github-actions[bot] commented 6 days ago

This is stale, but we won't close it automatically, just bare in mind the maintainers may be busy with other tasks and will reach your issue ASAP. If you have any question or request to prioritize this, please reach #ingress-nginx-dev on Kubernetes Slack.