Windows node graceful shutdown #127404

zylxjtu · 2024-09-16T18:28:39Z

What type of PR is this?

/kind feature

What this PR does / why we need it:

This is to implement the windows node shutdown, which is proposed in here:
kubernetes/enhancements#4813

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Enabled graceful shutdown feature for Windows node

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

k8s-ci-robot · 2024-09-16T18:28:49Z

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-10-21T17:33:28Z

/ok-to-test

SergeyKanzhelev · 2024-11-04T21:29:15Z

pkg/windows/service/service.go

@@ -21,7 +21,9 @@ package service



I would suggest to put an OWNERS file in this directlory so SIG Windows can approve it.

If you do so, please also include SIG Node team as approvers

thanks, this is also what we discussed before, let me check with @marosset and @jsturtevant

Please make sure to add Node reviewers and approvers into that file since it will be Node-related code. I don't think we should block every change in this file by pkg/ approvers.

Added Owner file @SergeyKanzhelev

SergeyKanzhelev · 2024-11-04T23:25:25Z

pkg/windows/service/OWNERS

+
+approvers:
+  - sig-windows-api-reviewers
+  - mrunalp


use alias sig-node-approvers instead of explicit names

SergeyKanzhelev · 2024-11-04T23:25:47Z

pkg/windows/service/OWNERS

+# See the OWNERS docs at https://go.k8s.io/owners
+
+approvers:
+  - sig-windows-api-reviewers


why reviewers have approvals permission? Is it intentional?

Let's move all the edits to the OWNERS file out to a seperate PR

I didn't expect it will be such a struggle. I am OK with the separate PR

SergeyKanzhelev · 2024-11-04T23:26:31Z

pkg/windows/service/OWNERS

+  - SergeyKanzhelev
+reviewers:
+  - sig-windows-api-reviewers
+  - sig-node-reviewers


also add labels:

Suggested change

- sig-node-reviewers

- sig-node-reviewers

labels:

- sig/windows

- sig/node

thx. I suggest just do sig-node-approvers as suggested above and merge it already. We can add Windows approvers later

pls remove sig-windows-api-reviewers from approvers

zylxjtu · 2024-11-05T04:00:24Z

/retest

SergeyKanzhelev · 2024-11-05T04:23:42Z

pkg/windows/service/OWNERS

+  - sig-windows-api-reviewers
+  - mrunalp
+  - kannon92
+  - pacoxu
+  - SergeyKanzhelev


Suggested change

- sig-windows-api-reviewers

- mrunalp

- kannon92

- pacoxu

- SergeyKanzhelev

- sig-node-approvers

@marosset anybody from wig windows explicitly?

SergeyKanzhelev

/lgtm

k8s-ci-robot · 2024-11-05T17:52:31Z

LGTM label has been added.

Git tree hash: b7290c6f6d51736b31fe10bff66ab7f2b92bb5aa

SergeyKanzhelev · 2024-11-05T17:53:12Z

/assign @dims

@dims are you ok to approve the new OWNERS file?

zylxjtu · 2024-11-05T18:44:54Z

/retest

dims · 2024-11-05T18:53:41Z

/approve

k8s-ci-robot · 2024-11-05T18:53:53Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dims, marosset, SergeyKanzhelev, 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:

~~cmd/kubelet/OWNERS~~ [SergeyKanzhelev,dims]
~~pkg/OWNERS~~ [dims]
~~test/e2e/windows/OWNERS~~ [dims,marosset]
~~test/featuregates_linter/test_data/OWNERS~~ [SergeyKanzhelev,dims,marosset]

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

zylxjtu · 2024-11-05T19:25:16Z

/retest-required

zylxjtu · 2024-11-05T20:02:50Z

/retest

SergeyKanzhelev · 2024-11-05T21:12:17Z

/retest

k8s-ci-robot requested review from mrunalp and saschagrunert September 16, 2024 18:29

zylxjtu mentioned this pull request Sep 16, 2024

Add KEP 4802 - windows node shutdown kubernetes/enhancements#4813

Merged

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 27, 2024

zylxjtu force-pushed the master branch from 9d17699 to f94d79c Compare October 17, 2024 22:00

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 17, 2024

zylxjtu force-pushed the master branch from f94d79c to a47798e Compare October 18, 2024 23:44

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Oct 18, 2024

This was referenced Oct 21, 2024

Add windows node graceful shutdown doc (draft) kubernetes/website#48474

Draft

Windows node gracefully shutdown kubernetes/enhancements#4802

Open

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 Oct 21, 2024

zylxjtu force-pushed the master branch from a47798e to 51b4e19 Compare October 21, 2024 22:35

SergeyKanzhelev reviewed Nov 4, 2024

View reviewed changes

zylxjtu force-pushed the master branch from 563a79c to ee28a16 Compare November 4, 2024 23:19

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

k8s-ci-robot requested a review from SergeyKanzhelev November 4, 2024 23:20

SergeyKanzhelev reviewed Nov 4, 2024

View reviewed changes

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 5, 2024

zylxjtu force-pushed the master branch from ee28a16 to 423bd16 Compare November 5, 2024 03:57

SergeyKanzhelev reviewed Nov 5, 2024

View reviewed changes

zylxjtu force-pushed the master branch from 423bd16 to 886003c Compare November 5, 2024 17:24

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 5, 2024

zylxjtu requested a review from SergeyKanzhelev November 5, 2024 17:27

Windows node graceful shutdown

459952a

zylxjtu force-pushed the master branch from 886003c to 459952a Compare November 5, 2024 17:46

SergeyKanzhelev approved these changes Nov 5, 2024

View reviewed changes

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

k8s-ci-robot assigned dims Nov 5, 2024

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

k8s-ci-robot merged commit be7d100 into kubernetes:master Nov 5, 2024
17 checks passed

pacoxu mentioned this pull request Nov 6, 2024

[Failing Test] gci-gce-alpha-enabled-default #128600

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Windows node graceful shutdown #127404

Windows node graceful shutdown #127404

zylxjtu commented Sep 16, 2024 •

edited

Loading

k8s-ci-robot commented Sep 16, 2024

kannon92 commented Oct 21, 2024

SergeyKanzhelev Nov 4, 2024

zylxjtu Nov 4, 2024

SergeyKanzhelev Nov 4, 2024

zylxjtu Nov 4, 2024

SergeyKanzhelev Nov 4, 2024

SergeyKanzhelev Nov 4, 2024

marosset Nov 5, 2024

SergeyKanzhelev Nov 5, 2024

SergeyKanzhelev Nov 4, 2024

zylxjtu Nov 5, 2024

SergeyKanzhelev Nov 5, 2024

zylxjtu Nov 5, 2024

SergeyKanzhelev Nov 5, 2024

zylxjtu Nov 5, 2024

zylxjtu commented Nov 5, 2024

SergeyKanzhelev Nov 5, 2024

SergeyKanzhelev Nov 5, 2024

SergeyKanzhelev left a comment

k8s-ci-robot commented Nov 5, 2024

SergeyKanzhelev commented Nov 5, 2024

zylxjtu commented Nov 5, 2024

dims commented Nov 5, 2024

k8s-ci-robot commented Nov 5, 2024

zylxjtu commented Nov 5, 2024

zylxjtu commented Nov 5, 2024

SergeyKanzhelev commented Nov 5, 2024

Windows node graceful shutdown #127404

Windows node graceful shutdown #127404

Conversation

zylxjtu commented Sep 16, 2024 • edited Loading

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

k8s-ci-robot commented Sep 16, 2024

kannon92 commented Oct 21, 2024

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zylxjtu commented Nov 5, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SergeyKanzhelev left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Nov 5, 2024

SergeyKanzhelev commented Nov 5, 2024

zylxjtu commented Nov 5, 2024

dims commented Nov 5, 2024

k8s-ci-robot commented Nov 5, 2024

zylxjtu commented Nov 5, 2024

zylxjtu commented Nov 5, 2024

SergeyKanzhelev commented Nov 5, 2024

zylxjtu commented Sep 16, 2024 •

edited

Loading