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

Add affinity to controller.admissionWebhooks.patch webhook #11750

Open
soundarya2606 opened this issue Aug 8, 2024 · 5 comments
Open

Add affinity to controller.admissionWebhooks.patch webhook #11750

soundarya2606 opened this issue Aug 8, 2024 · 5 comments
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. needs-priority needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.

Comments

@soundarya2606
Copy link

soundarya2606 commented Aug 8, 2024

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

@soundarya2606 soundarya2606 added the kind/feature Categorizes issue or PR as related to a new feature. label Aug 8, 2024
@k8s-ci-robot
Copy link
Contributor

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. 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 needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority labels Aug 8, 2024
@longwuyuan
Copy link
Contributor

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
Copy link
Author

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
Copy link
Contributor

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.

Copy link

github-actions bot commented Sep 9, 2024

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.

@github-actions github-actions bot added the lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. label Sep 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. needs-priority needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one.
Projects
Development

Successfully merging a pull request may close this issue.

3 participants