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.

Sign up for GitHub

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

Jump to bottom

Set "Multi Scheduling Profiles" to "implementable" #1483

Merged
merged 1 commit into from
Jan 27, 2020
Merged

Set "Multi Scheduling Profiles" to "implementable" #1483

merged 1 commit into from
Jan 27, 2020

Conversation

alculquicondor
Copy link
Member

@alculquicondor alculquicondor commented Jan 21, 2020 •
edited
Loading

Add Test Plan and Alpha Graduation criteria.

Also:

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 21, 2020
@k8s-ci-robot k8s-ci-robot requested review from ahg-g and k82cn January 21, 2020 19:40
@alculquicondor alculquicondor force-pushed the multi-scheduling-profiles branch from 485ef34 to 1b4685f Compare January 21, 2020 19:41
@k8s-ci-robot k8s-ci-robot added kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. labels Jan 21, 2020
@alculquicondor
Copy link
Member Author

/sig testing

@k8s-ci-robot k8s-ci-robot added the sig/testing Categorizes an issue or PR as relevant to SIG Testing. label Jan 21, 2020
@alculquicondor
Copy link
Member Author

alculquicondor commented Jan 21, 2020 •
edited
Loading

/assign @liggitt

for architecture

/assign @Huang-Wei

for scheduling

@alculquicondor alculquicondor mentioned this pull request Jan 21, 2020
Closed
@alculquicondor alculquicondor force-pushed the multi-scheduling-profiles branch from 38fdc91 to a32704c Compare January 22, 2020 17:53
@alculquicondor
Copy link
Member Author

@liggitt Friendly ping

ahg-g
ahg-g reviewed Jan 24, 2020
View reviewed changes
keps/sig-scheduling/20200114-multi-scheduling-profiles.md Outdated

TODO
- [ ] New `v1alpha2` ComponentConfig API, with conversion from `v1alpha1` and defaults.
- [ ] New feature gate.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@liggitt do we really need a feature gate? it looks to me like this is mostly a refactoring, and I am not sure a feature gate helps too much in gating a refactoring.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True. Given that users have to explicitly use kubescheduler.config.k8s.io/v1alpha2 API, having a feature gate just adds a redundant step. The API is a gate on its own. Unless we have to adhere to some standard practice?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the feature gate and added a note in the following paragraph.

keps/sig-scheduling/20200114-multi-scheduling-profiles.md Outdated Show resolved Hide resolved
@alculquicondor alculquicondor force-pushed the multi-scheduling-profiles branch 4 times, most recently from 9af47a7 to 85a4c67 Compare January 25, 2020 17:26
@liggitt
Copy link
Member

liggitt commented Jan 27, 2020

proposed component config lgtm

@ahg-g
Copy link
Member

ahg-g commented Jan 27, 2020

/hold
/lgtm

@Huang-Wei please take a final look, the deadline is today I think.

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 27, 2020
@k8s-ci-robot k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jan 27, 2020
Huang-Wei
Huang-Wei reviewed Jan 27, 2020
View reviewed changes
Copy link
Member

@Huang-Wei Huang-Wei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @alculquicondor.

Please fix the typo and use the chance to squash the commits.

keps/sig-scheduling/20200114-multi-scheduling-profiles.md Outdated
@@ -158,14 +158,21 @@ type KubeSchedulerProfile struct {
}
```

Note that we remove `AlgorithmSource` from the new API. It's functionality becomes redundant to
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. Done.

@alculquicondor alculquicondor force-pushed the multi-scheduling-profiles branch from 85a4c67 to ead3e26 Compare January 27, 2020 18:51
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 27, 2020
@alculquicondor
Add Test Plan and Alpha Graduation criteria.
bf978ca 
Also specify validation of Plugins.QueueSort
and remove AlgorithmSource.

Signed-off-by: Aldo Culquicondor <[email protected]>
@alculquicondor alculquicondor force-pushed the multi-scheduling-profiles branch from ead3e26 to bf978ca Compare January 27, 2020 18:54
@Huang-Wei
Copy link
Member

/lgtm
/hold cancel

@k8s-ci-robot k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Jan 27, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ahg-g, alculquicondor, Huang-Wei

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit 351505b into kubernetes:master Jan 27, 2020
@k8s-ci-robot k8s-ci-robot added this to the v1.18 milestone Jan 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ahg-g ahg-g ahg-g left review comments

@Huang-Wei Huang-Wei Huang-Wei left review comments

@liggitt liggitt liggitt left review comments

@k82cn k82cn Awaiting requested review from k82cn

Assignees

@Huang-Wei Huang-Wei

@ahg-g ahg-g

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
v1.18
Development

Successfully merging this pull request may close these issues.
5 participants
@alculquicondor @liggitt @ahg-g @Huang-Wei @k8s-ci-robot