-
Notifications
You must be signed in to change notification settings - Fork 40.1k
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
Modified the Filter interface to pass in nodeinfo instead of node name. #82842
Conversation
/kind bug |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ahg-g 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 |
5f0ee5c
to
5abc379
Compare
5abc379
to
57b2268
Compare
This is necessary to support preemption, which relies on passing modified nodeinfo objects to the filters to simulate evicting lower-priority pods.
57b2268
to
89f936f
Compare
LGTM overall :) hm... do we keep backward comparability for framework interface when it's GA? |
This is a little different than API objects and json, k8s doesn't offer its code as a library, and so backward compatibility here is not something that k8s have strong guarantees for unless we place it in an external repo. In any case, I think we should be careful with changing the API when its "GA". |
/assign @alculquicondor |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
Maybe we should discuss this in community with our suggestion: IMO, the original idea of scheduler framework is to support customized scheduler for difference scenarios, as we can not put everything in default scheduler which will make complex; if "k8s doesn't offer its code as a library", will that make developer confuse about scheduler framework? |
For sure, I think once we have few predicates/priorities migrated, we can establish strong guarantees around the framework API, and perhaps suggest that our versioning/deprecation policy will be similar to object APIs.
I am strongly supportive of that. However, in my opinion, we can't practically offer any guarantees on the framework interface until some of our core predicates/priorities are migrated since we are still learning what it is missing.
That is k8s policy, we have two options to offer strong guarantees around the API:
but again, we can only do that after we migrate few predicates/priorities and have a first cut that is practical to use. What do you think? |
btw, the graduation criteria for the framework is aligned with the second option. We purposely didn't graduate the framework to Beta yet, we wanted to wait until few default predicates/priorities are implemented first to have more confidence in the API. |
Totally agree with you to migrate some predicates/priorities firstly; I think that's ok to get a guidance of API before scheduling framework GA :) |
What type of PR is this?
/kind feature
What this PR does / why we need it:
This is necessary to support preemption, which relies on passing modified nodeinfo objects to the filters to simulate evicting lower-priority pods.
Does this PR introduce a user-facing change?: