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

Pod Mutable Scheduling Directives #3840

Merged
merged 1 commit into from
Feb 7, 2023
Merged

Pod Mutable Scheduling Directives #3840

merged 1 commit into from
Feb 7, 2023

Conversation

ahg-g
Copy link
Member

@ahg-g ahg-g commented Feb 3, 2023

  • One-line PR description: pod mutable scheduling directives
  • Other comments:

@k8s-ci-robot k8s-ci-robot added 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 sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. labels Feb 3, 2023
@k8s-ci-robot k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Feb 3, 2023
@ahg-g
Copy link
Member Author

ahg-g commented Feb 3, 2023

/assign @Huang-Wei @liggitt @wojtek-t

@ahg-g ahg-g mentioned this pull request Feb 3, 2023
Closed
4 tasks
@ahg-g
Copy link
Member Author

ahg-g commented Feb 3, 2023

/cc @mwielgus

@k8s-ci-robot k8s-ci-robot requested a review from mwielgus February 3, 2023 23:02
@ahg-g ahg-g force-pushed the ahg-mutable-pods branch 6 times, most recently from b14885f to 3e476d0 Compare February 3, 2023 23:46
Huang-Wei
Huang-Wei reviewed Feb 5, 2023
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 @ahg-g !

keps/sig-scheduling/3838-pod-mutable-scheduling-directives/README.md Outdated Show resolved Hide resolved
keps/sig-scheduling/3838-pod-mutable-scheduling-directives/README.md Outdated Show resolved Hide resolved
keps/sig-scheduling/3838-pod-mutable-scheduling-directives/README.md Outdated Show resolved Hide resolved
keps/sig-scheduling/3838-pod-mutable-scheduling-directives/README.md Outdated Show resolved Hide resolved
keps/sig-scheduling/3838-pod-mutable-scheduling-directives/README.md Outdated Show resolved Hide resolved
keps/sig-scheduling/3838-pod-mutable-scheduling-directives/README.md Outdated Show resolved Hide resolved
keps/sig-scheduling/3838-pod-mutable-scheduling-directives/README.md Outdated Show resolved Hide resolved
keps/sig-scheduling/3838-pod-mutable-scheduling-directives/README.md Outdated Show resolved Hide resolved
keps/sig-scheduling/3838-pod-mutable-scheduling-directives/README.md Outdated Show resolved Hide resolved
keps/sig-scheduling/3838-pod-mutable-scheduling-directives/kep.yaml Outdated Show resolved Hide resolved
@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Feb 5, 2023
ahg-g
ahg-g commented Feb 5, 2023
View reviewed changes
Copy link
Member Author

@ahg-g ahg-g left a comment

Choose a reason for hiding this comment

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

Thanks Wei for the detailed and prompt review!

keps/sig-scheduling/3838-pod-mutable-scheduling-directives/README.md Outdated Show resolved Hide resolved
keps/sig-scheduling/3838-pod-mutable-scheduling-directives/README.md Outdated Show resolved Hide resolved
keps/sig-scheduling/3838-pod-mutable-scheduling-directives/README.md Outdated
## Proposal

The proposal is to relax update validation of scheduling directives, specifically node affinity and selector, of Pods that
are blocked on a scheduling readiness gate (i.e., not yet considered for scheduling by kube-scheduler), specifically. Note
Copy link
Member Author

Choose a reason for hiding this comment

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

updated the paragraph, but kept the original one sentence, I think it is clearer.

keps/sig-scheduling/3838-pod-mutable-scheduling-directives/README.md Outdated Show resolved Hide resolved
keps/sig-scheduling/3838-pod-mutable-scheduling-directives/README.md
to match the template, they react only to change to the pod template in the app's
spec. Moreover, the side affects of this proposal is no different from a mutating
webhook that injects affinity onto the pod.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is a risk associated with the node affinity/selector feature, not updating it. A pod can be created with affinity that causes those issues.

keps/sig-scheduling/3838-pod-mutable-scheduling-directives/kep.yaml Outdated Show resolved Hide resolved
keps/sig-scheduling/3838-pod-mutable-scheduling-directives/README.md Outdated Show resolved Hide resolved
@k8s-ci-robot k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Feb 5, 2023
@ahg-g ahg-g force-pushed the ahg-mutable-pods branch 2 times, most recently from a8da4e0 to 70b29fb Compare February 5, 2023 19:17
@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Feb 5, 2023
wojtek-t
wojtek-t reviewed Feb 6, 2023
View reviewed changes
Copy link
Member

@wojtek-t wojtek-t left a comment

Choose a reason for hiding this comment

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

Few minor comments from me - other than that LGTM.

keps/sig-scheduling/3838-pod-mutable-scheduling-directives/README.md Outdated Show resolved Hide resolved
keps/sig-scheduling/3838-pod-mutable-scheduling-directives/README.md Outdated Show resolved Hide resolved
keps/sig-scheduling/3838-pod-mutable-scheduling-directives/README.md Outdated Show resolved Hide resolved
keps/sig-scheduling/3838-pod-mutable-scheduling-directives/README.md Outdated Show resolved Hide resolved
keps/sig-scheduling/3838-pod-mutable-scheduling-directives/README.md Show resolved Hide resolved
keps/sig-scheduling/3838-pod-mutable-scheduling-directives/README.md Show resolved Hide resolved
ahg-g
ahg-g commented Feb 6, 2023
View reviewed changes
Copy link
Member Author

@ahg-g ahg-g left a comment

Choose a reason for hiding this comment

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

Thanks @wojtek-t and @liggitt

keps/sig-scheduling/3838-pod-mutable-scheduling-directives/README.md Outdated Show resolved Hide resolved
keps/sig-scheduling/3838-pod-mutable-scheduling-directives/README.md Outdated Show resolved Hide resolved
keps/sig-scheduling/3838-pod-mutable-scheduling-directives/README.md Outdated Show resolved Hide resolved
keps/sig-scheduling/3838-pod-mutable-scheduling-directives/README.md Outdated Show resolved Hide resolved
keps/sig-scheduling/3838-pod-mutable-scheduling-directives/README.md Outdated Show resolved Hide resolved
keps/sig-scheduling/3838-pod-mutable-scheduling-directives/README.md Outdated Show resolved Hide resolved
keps/sig-scheduling/3838-pod-mutable-scheduling-directives/README.md Outdated Show resolved Hide resolved
keps/sig-scheduling/3838-pod-mutable-scheduling-directives/README.md Show resolved Hide resolved
keps/sig-scheduling/3838-pod-mutable-scheduling-directives/README.md Outdated Show resolved Hide resolved
@liggitt
Copy link
Member

liggitt commented Feb 6, 2023

details about the allowed mutations lgtm

@ahg-g ahg-g mentioned this pull request Feb 6, 2023
Closed
alculquicondor
alculquicondor reviewed Feb 6, 2023
View reviewed changes
Copy link
Member

@alculquicondor alculquicondor left a comment

Choose a reason for hiding this comment

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

Just some clarifications

keps/sig-scheduling/3838-pod-mutable-scheduling-directives/README.md Outdated Show resolved Hide resolved
keps/sig-scheduling/3838-pod-mutable-scheduling-directives/README.md Show resolved Hide resolved
keps/sig-scheduling/3838-pod-mutable-scheduling-directives/README.md Show resolved Hide resolved
keps/sig-scheduling/3838-pod-mutable-scheduling-directives/README.md Outdated Show resolved Hide resolved
@ahg-g
Copy link
Member Author

ahg-g commented Feb 7, 2023

@Huang-Wei i think all comments are addressed

@Huang-Wei Huang-Wei mentioned this pull request Feb 7, 2023
Merged
wojtek-t
wojtek-t reviewed Feb 7, 2023
View reviewed changes
Copy link
Member

@wojtek-t wojtek-t left a comment

Choose a reason for hiding this comment

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

Just one nit, but it's good to go from PRR perspective.

/approve PRR

keps/sig-scheduling/3838-pod-mutable-scheduling-directives/README.md Outdated Show resolved Hide resolved
@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 7, 2023
@ahg-g
Copy link
Member Author

ahg-g commented Feb 7, 2023

Squashed.

alculquicondor
alculquicondor reviewed Feb 7, 2023
View reviewed changes
Copy link
Member

@alculquicondor alculquicondor left a comment

Choose a reason for hiding this comment

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

LGTM

keps/sig-scheduling/3838-pod-mutable-scheduling-directives/README.md Outdated Show resolved Hide resolved
keps/sig-scheduling/3838-pod-mutable-scheduling-directives/README.md Show resolved Hide resolved
Huang-Wei
Huang-Wei approved these changes Feb 7, 2023
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.

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 7, 2023
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

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

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 4fa0282 into kubernetes:master Feb 7, 2023
@k8s-ci-robot k8s-ci-robot added this to the v1.27 milestone Feb 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wojtek-t wojtek-t wojtek-t left review comments

@alculquicondor alculquicondor alculquicondor left review comments

@liggitt liggitt liggitt left review comments

@Huang-Wei Huang-Wei Huang-Wei approved these changes

@mwielgus mwielgus Awaiting requested review from mwielgus

Assignees

@liggitt liggitt

@Huang-Wei Huang-Wei

@wojtek-t wojtek-t

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. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
v1.27
Development

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