Add KEP 4802 - windows node shutdown #4813

zylxjtu · 2024-08-28T22:29:08Z

One-line PR description:
This is to implement node shutdown feature for windows node, comparable with linux part, which is describe in KEP-2000

Issue link: Windows node gracefully shutdown #4802

Other comments:
code PR: Windows node graceful shutdown kubernetes#127404

linux-foundation-easycla · 2024-08-28T22:29:13Z

The committers listed above are authorized under a signed CLA.

✅ login: zylxjtu / name: yuanliang (8d72fb3)

k8s-ci-robot · 2024-08-28T22:29:17Z

Welcome @zylxjtu!

It looks like this is your first PR to kubernetes/enhancements 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/enhancements has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2024-08-28T22:29:18Z

Hi @zylxjtu. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

kannon92 · 2024-08-30T20:46:38Z

/ok-to-test

pacoxu · 2024-09-20T10:19:37Z

/sig node

enj

PRR shadow.

keps/sig-windows/4802-windows-node-shutdown/README.md

enj · 2024-10-07T22:18:09Z

keps/sig-windows/4802-windows-node-shutdown/README.md

+
+n/a
+
+## Production Readiness Review Questionnaire


This KEP's markdown does not seem to be following the standard template. Please update it to the regular template.

updated with new template format

enj · 2024-10-07T22:22:36Z

keps/sig-windows/4802-windows-node-shutdown/README.md

+  with and without the feature, are necessary. At the very least, think about
+  conversion tests if API types are being modified.
+
+    *   n/a


I don't see how this feature could be meaningfully tested without an e2e.

we do have e2e test, please refer to the attached code PR link, will add more clarification for this

enj · 2024-10-07T22:25:25Z

keps/sig-windows/4802-windows-node-shutdown/README.md

+This feature should not impact rollouts.
+
+* **What specific metrics should inform a rollback?**
+
+N/A.


I think this section is more asking 'if there is bug in the feature' how would an admin figure out that this feature was the cause of it.

enj · 2024-10-07T22:26:05Z

keps/sig-windows/4802-windows-node-shutdown/README.md

+  - [ ] Other (treat as last resort)
+    - Details:
+
+N/A


I think you need at least logs since those can be persisted across reboots.

enj · 2024-10-07T22:27:43Z

keps/sig-windows/4802-windows-node-shutdown/README.md

+
+Check if the feature gate and kubelet config settings are enabled on a node.
+
+* **What are the SLIs (Service Level Indicators) an operator can use to determine


How can someone using this feature know that it is working for their instance? is missing from the KEP template.

enj

PRR shadow

enj · 2024-10-09T12:31:41Z

keps/sig-windows/4802-windows-node-shutdown/README.md

+
+- [X] Feature gate (also fill in values in `kep.yaml`)
+  - Feature gate name: `WindowsGracefulNodeShutdown`
+  - Components depending on the feature gate:


enj · 2024-10-09T12:33:52Z

keps/sig-windows/4802-windows-node-shutdown/README.md

+###### Were upgrade and rollback tested? Was the upgrade->downgrade->upgrade path tested?
+
+The feature is part of kubelet config so updating kubelet config should
+enable/disable the feature; upgrade/downgrade is N/A.


This is basically how all features work so upgrade and downgrade apply as normal.

enj · 2024-10-09T12:34:26Z

keps/sig-windows/4802-windows-node-shutdown/README.md

+
+n/a
+
+The failure of the roll out will behave like disbling this feature, operators can check the log to get more specific info


Which logs? What information will they contain?

it should be the default kubelet log, add example for the error information

enj · 2024-10-09T12:34:51Z

keps/sig-windows/4802-windows-node-shutdown/README.md

+
+###### How can a rollout or rollback fail? Can it impact already running workloads?
+
+This fail of rollout will behave like disabling this feature, it wil not impact running workloads during rollout/rollback.


I don't understand what this is saying.

Remove the confusion part

deads2k · 2024-10-09T20:01:16Z

PRR looks good. Thanks.

/approve

marosset · 2024-10-09T21:25:21Z

keps/prod-readiness/sig-windows/4802.yaml

+# of http://git.k8s.io/enhancements/OWNERS_ALIASES
+kep-number: 4802
+alpha:
+  approver: "@johnbelamaric"


Suggested change

approver: "@johnbelamaric"

approver: "@deads2k"

marosset · 2024-10-09T21:25:43Z

keps/sig-windows/4802-windows-node-shutdown/README.md

+- [x] (R) Design details are appropriately documented
+- [x] (R) Test plan is in place, giving consideration to SIG Architecture and SIG Testing input
+- [x] (R) Graduation criteria is in place
+- [] (R) Production readiness review completed


Suggested change

- [] (R) Production readiness review completed

- [x] (R) Production readiness review completed

marosset

/lgtm
/approve
/hold for @mrunalp / node review

Looks good for Windows!

k8s-ci-robot · 2024-10-09T21:29:13Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, marosset, zylxjtu

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:

~~keps/prod-readiness/OWNERS~~ [deads2k]
~~keps/sig-windows/OWNERS~~ [marosset]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

mrunalp · 2024-10-10T01:53:57Z

/lgtm

mrunalp · 2024-10-10T01:54:35Z

/unhold

k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. kind/kep Categorizes KEP tracking issues and PRs modifying the KEP directory sig/windows Categorizes an issue or PR as relevant to SIG Windows. labels Aug 28, 2024

k8s-ci-robot requested review from aravindhp and jsturtevant August 28, 2024 22:29

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Aug 28, 2024

k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Aug 28, 2024

zylxjtu force-pushed the master branch 2 times, most recently from f6b866b to a1fb977 Compare August 29, 2024 18:19

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Aug 29, 2024

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Aug 30, 2024

zylxjtu force-pushed the master branch from be51ebb to 8077c95 Compare September 12, 2024 18:24

k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. and removed cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Sep 12, 2024

zylxjtu force-pushed the master branch 2 times, most recently from 4eecabf to 160c48c Compare September 16, 2024 18:22

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Sep 16, 2024

zylxjtu mentioned this pull request Sep 16, 2024

Windows node graceful shutdown kubernetes/kubernetes#127404

Merged

zylxjtu force-pushed the master branch from 160c48c to e63705f Compare September 17, 2024 17:35

k8s-ci-robot added the sig/node Categorizes an issue or PR as relevant to SIG Node. label Sep 20, 2024

zylxjtu force-pushed the master branch 2 times, most recently from 764193b to 34d4400 Compare September 25, 2024 19:30

zylxjtu force-pushed the master branch from 83469c1 to 2829ace Compare October 4, 2024 18:40

zylxjtu requested a review from mrunalp October 4, 2024 18:41

enj requested changes Oct 7, 2024

View reviewed changes

zylxjtu force-pushed the master branch 2 times, most recently from f2ffa55 to f47a806 Compare October 8, 2024 18:12

zylxjtu requested a review from enj October 8, 2024 18:16

enj requested changes Oct 9, 2024

View reviewed changes

zylxjtu force-pushed the master branch from f47a806 to ddc6802 Compare October 9, 2024 16:30

zylxjtu requested a review from enj October 9, 2024 16:30

marosset reviewed Oct 9, 2024

View reviewed changes

marosset approved these changes Oct 9, 2024

View reviewed changes

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 9, 2024

k8s-ci-robot assigned marosset Oct 9, 2024

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 9, 2024

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 9, 2024

Add windows graceful node shutdown

8d72fb3

zylxjtu force-pushed the master branch from ddc6802 to 8d72fb3 Compare October 9, 2024 21:43

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 9, 2024

zylxjtu requested a review from marosset October 9, 2024 22:08

k8s-ci-robot assigned mrunalp Oct 10, 2024

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 10, 2024

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 10, 2024

k8s-ci-robot merged commit 5cb1f63 into kubernetes:master Oct 10, 2024
2 of 4 checks passed

k8s-ci-robot added this to the v1.32 milestone Oct 10, 2024

pacoxu mentioned this pull request Oct 10, 2024

Windows node gracefully shutdown #4802

Open

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add KEP 4802 - windows node shutdown #4813

Add KEP 4802 - windows node shutdown #4813

zylxjtu commented Aug 28, 2024 •

edited

Loading

linux-foundation-easycla bot commented Aug 28, 2024 •

edited

Loading

k8s-ci-robot commented Aug 28, 2024

k8s-ci-robot commented Aug 28, 2024

kannon92 commented Aug 30, 2024

pacoxu commented Sep 20, 2024

enj left a comment

enj Oct 7, 2024

zylxjtu Oct 8, 2024

enj Oct 7, 2024

zylxjtu Oct 8, 2024

enj Oct 7, 2024

zylxjtu Oct 8, 2024

enj Oct 7, 2024

zylxjtu Oct 8, 2024

enj Oct 7, 2024

zylxjtu Oct 8, 2024

enj left a comment

enj Oct 9, 2024

zylxjtu Oct 9, 2024

enj Oct 9, 2024

zylxjtu Oct 9, 2024

enj Oct 9, 2024

zylxjtu Oct 9, 2024

enj Oct 9, 2024

zylxjtu Oct 9, 2024

deads2k commented Oct 9, 2024

marosset Oct 9, 2024

marosset Oct 9, 2024

marosset left a comment

k8s-ci-robot commented Oct 9, 2024

mrunalp commented Oct 10, 2024

mrunalp commented Oct 10, 2024


		Check if the feature gate and kubelet config settings are enabled on a node.

		* **What are the SLIs (Service Level Indicators) an operator can use to determine


		n/a

		The failure of the roll out will behave like disbling this feature, operators can check the log to get more specific info


		###### How can a rollout or rollback fail? Can it impact already running workloads?

		This fail of rollout will behave like disabling this feature, it wil not impact running workloads during rollout/rollback.

	- [] (R) Production readiness review completed
	- [x] (R) Production readiness review completed

Add KEP 4802 - windows node shutdown #4813

Add KEP 4802 - windows node shutdown #4813

Conversation

zylxjtu commented Aug 28, 2024 • edited Loading

linux-foundation-easycla bot commented Aug 28, 2024 • edited Loading

k8s-ci-robot commented Aug 28, 2024

k8s-ci-robot commented Aug 28, 2024

kannon92 commented Aug 30, 2024

pacoxu commented Sep 20, 2024

enj left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

enj left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deads2k commented Oct 9, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marosset left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Oct 9, 2024

mrunalp commented Oct 10, 2024

mrunalp commented Oct 10, 2024

zylxjtu commented Aug 28, 2024 •

edited

Loading

linux-foundation-easycla bot commented Aug 28, 2024 •

edited

Loading