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

KEP-2268: move non-graceful node shutdown to GA #4029

Merged
merged 4 commits into from
Jun 13, 2023
Merged

KEP-2268: move non-graceful node shutdown to GA #4029

merged 4 commits into from
Jun 13, 2023
+74 −24

Conversation

xing-yang
Copy link
Contributor

  • One-line PR description:
    This PR moves non-graceful node shutdown feature to GA.
  • 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 labels May 24, 2023
@k8s-ci-robot k8s-ci-robot requested review from msau42 and saad-ali May 24, 2023 01:05
@k8s-ci-robot k8s-ci-robot added sig/storage Categorizes an issue or PR as relevant to SIG Storage. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels May 24, 2023
@xing-yang xing-yang mentioned this pull request May 24, 2023
Closed
@carlory
Copy link
Member

carlory commented May 24, 2023 •
edited
Loading

It is duplicated with #4028, can you help us to review it?

@YuikoTakada YuikoTakada mentioned this pull request May 24, 2023
Closed
@YuikoTakada
Copy link
Contributor

Thank you for your PR.

@carlory carlory mentioned this pull request May 24, 2023
Merged
@YuikoTakada
Copy link
Contributor

@carlory

It is duplicated with #4028, can you help us to review it?

I've closed #4028 . so please review just this PR. Thanks.

@xing-yang xing-yang force-pushed the non-graceful-node-shutdown-ga branch from 6367497 to 4092609 Compare May 27, 2023 14:09
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels May 27, 2023
@YuikoTakada
Copy link
Contributor

https://github.com/kubernetes/enhancements/pull/4029/files#diff-0225593bb1191b37cc24ad60c172668c3df10b62f2fd748ceb1bbe85ddf078ceR199

More rigorous forms of testing—e.g., downgrade tests and scalability tests.

I don't want to discuss about detailed implementation, but do you have any idea about downgrade/scalability tests? or do you know about existing these test cases?

@xing-yang
Copy link
Contributor Author

I don't want to discuss about detailed implementation, but do you have any idea about downgrade/scalability tests? or do you know about existing these test cases?

Downgrade tests will be done manually. Scalability tests will be added.

@YuikoTakada YuikoTakada mentioned this pull request May 30, 2023
Merged
@YuikoTakada
Copy link
Contributor

Scalability tests will be added.

You've asked someone already, right?

@xing-yang
Copy link
Contributor Author

Scalability tests will be added.

You've asked someone already, right?

@sonasingh46 is working on the scalability tests.

@xing-yang
Copy link
Contributor Author

/assign @msau42

@xing-yang xing-yang force-pushed the non-graceful-node-shutdown-ga branch from 4092609 to 0c01b56 Compare June 4, 2023 14:41
@xing-yang
Copy link
Contributor Author

Review comments are addressed. Thanks.

@xing-yang xing-yang force-pushed the non-graceful-node-shutdown-ga branch from 0c01b56 to 7547265 Compare June 5, 2023 13:13
@xing-yang xing-yang force-pushed the non-graceful-node-shutdown-ga branch from 7547265 to 56298f4 Compare June 6, 2023 12:38
@gnufied
Copy link
Member

gnufied commented Jun 6, 2023

Overall looks good to me. I think we can figure out details about metric in https://github.com/kubernetes/kubernetes/pull/118480/files PR as well.

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 6, 2023
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 9, 2023
@xing-yang
Copy link
Contributor Author

All comments are addressed. Thanks.

@carlory
Copy link
Member

carlory commented Jun 9, 2023

LGTM

@xing-yang xing-yang force-pushed the non-graceful-node-shutdown-ga branch from 016758b to 032cb02 Compare June 9, 2023 13:33
@jpbetz
Copy link
Contributor

jpbetz commented Jun 9, 2023

/assign @deads2k
For PRR approval. As PRR shadow, this LGTM.

@deads2k
Copy link
Contributor

deads2k commented Jun 13, 2023

prr
/approve

thanks.

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, xing-yang

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 added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 13, 2023
@gnufied
Copy link
Member

gnufied commented Jun 13, 2023

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 13, 2023
@k8s-ci-robot k8s-ci-robot merged commit 0d29502 into kubernetes:master Jun 13, 2023
@k8s-ci-robot k8s-ci-robot added this to the v1.28 milestone Jun 13, 2023
@jpbetz jpbetz mentioned this pull request Dec 1, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gnufied gnufied gnufied left review comments

@jpbetz jpbetz jpbetz left review comments

@YuikoTakada YuikoTakada YuikoTakada left review comments

@msau42 msau42 msau42 left review comments

@carlory carlory carlory left review comments

@saad-ali saad-ali Awaiting requested review from saad-ali

Assignees

@gnufied gnufied

@deads2k deads2k

@msau42 msau42

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/storage Categorizes an issue or PR as relevant to SIG Storage. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
v1.28
Development

Successfully merging this pull request may close these issues.
8 participants
@xing-yang @carlory @YuikoTakada @gnufied @jpbetz @deads2k @k8s-ci-robot @msau42