Make kubelet restarting as a windows service #1752

zylxjtu · 2025-05-16T21:30:55Z

Change description

To support features such as windows graceful shutdown, we will need to make kubelet (itself) running as a windows service. this means we will have to remove the dependency on nssm which will hijack the call SCM made to the running executable. However, the benefit of using nssm is that it can load the envs/config changes dynamically so that without updating the service itself, through the control of config files, the service will run differently. This was used in the CAPZ set up for kublet to dynamically adopt the changes passed from kubeadm. With this PR, we still use nssm to start kubelet in the beginning to apply dynamic changes made through kubeadm. but we will stop/delete the service and re-install/restart the kubelet with the same parameters through SCM, in this way, the kubelet itself will be running as windows service. that's the reason we will need to stop and delete the original service and re-install a new one because the service will be different (except the same service name of "kubelet")

Ideally, we should totally remove the dependency of nssm, maybe this is still the future direction, but I have tried quite a few times and had difficulties to make kubelet (started from SC) fit well with the cloud-init and join the CAPZ cluster. I believe it is achievable, but it probably will make things trickier so maybe this could be a future direction that we can target with.

Related issues

Fixes #

Additional context

sriramandev · 2025-05-19T07:05:32Z

@laozc Can you please take at view of this PR?

sriramandev · 2025-05-19T07:09:02Z

Do we have a issue related to this PR? Also can you please elaborate the description. Looking at trying to understand whats the core issue thats being addressed here.

marosset · 2025-05-20T00:18:29Z

/lgtm
This is related to https://github.com/kubernetes/enhancements/tree/master/keps/sig-windows/4802-windows-node-shutdown but I don't see us discussing logging in that KEP.
@zylxjtu do you remember where we had those discussions?

jsturtevant

Why does the service need to deleted and restarted in the script?

jsturtevant · 2025-05-20T15:23:32Z

images/capi/ansible/windows/roles/kubernetes/templates/RestartKubelet.ps1

+# limitations under the License.
+
+# From https://github.com/kubernetes-sigs/sig-windows-tools/blob/master/kubeadm/scripts/PrepareNode.ps1
+# Does not support kubeadm KUBELET_KUBEADM_ARGS which is used by Cluster API to pass extra user args


This is no longer accurate?

will remove the comments which do not apply anymore

jsturtevant · 2025-05-20T15:23:56Z

images/capi/ansible/windows/roles/kubernetes/templates/RestartKubelet.ps1

+for ($i = 0; $i -lt 10; $i++) {
+	$service = Get-Service -Name kubelet -ErrorAction SilentlyContinue
+	if ($null -eq $service) {
+		Write-Host "kbuelet service deleted successfully, restarting"


Suggested change

Write-Host "kbuelet service deleted successfully, restarting"

Write-Host "kubelet service deleted successfully, restarting"

jsturtevant · 2025-05-20T15:24:30Z

images/capi/ansible/windows/roles/kubernetes/templates/RestartKubelet.ps1

+}
+
+
+Write-Host "kbuelet service failed to restart."


Suggested change

Write-Host "kbuelet service failed to restart."

Write-Host "kubelet service failed to restart."

jsturtevant · 2025-05-20T15:25:10Z

images/capi/ansible/windows/roles/kubernetes/templates/RestartKubelet.ps1

+# Used by sc.exe to create the service 
+$KubeletCommandLine =  "`"" + "\`"" + "$env:SYSTEMDRIVE\k\kube-log-runner.exe" + "\`" " + "--log-file=/var/log/kubelet/kubelet.log " + "$env:SYSTEMDRIVE\k\kubelet.exe " + $KubeletArgListStr + "`""
+
+# Write-Output $kubeletCommandLine


Suggested change

# Write-Output $kubeletCommandLine

prefer to keep it here as the parameters (kubeletcommdnline) send to sc.exe tend to be tricky, and we can use this to check that kind of commandline is used to start the kubelet service, when we need to do some debugging

zylxjtu · 2025-05-20T18:01:39Z

/lgtm This is related to https://github.com/kubernetes/enhancements/tree/master/keps/sig-windows/4802-windows-node-shutdown but I don't see us discussing logging in that KEP. @zylxjtu do you remember where we had those discussions?

I remembered that we had some discussions at that time, but maybe it is landed in being removed from the KEP. The idea is to utilize the kube-log-runner (from the k8s repo) to redirect the log to stdout/stderr

zylxjtu · 2025-05-20T18:24:43Z

Why does the service need to deleted and restarted in the script?

This is the solution I have after a few try of other options. let me try to explain a bit here so that more people may have a better idea on what I'm trying to achieve here. To support features such as windows graceful shutdown, we will need to make kubelet (itself) running as a windows service. this means we will have to remove the dependency on nssm which will hijack the call SCM made to the running executable. However, the benefit of using nssm is that it can load the envs/config changes dynamically so that without updating the service itself, through the control of config files, the service will run differently. This was used in the CAPZ set up for kublet to dynamically adopt the changes passed from kubeadm. With this PR, we still use nssm to start kubelet in the beginning to apply dynamic changes made through kubeadm. but we will stop/delete the service and re-install/restart the kubelet with the same parameters through SCM, in this way, the kubelet itself will be running as windows service. that's the reason we will need to stop and delete the original service and re-install a new one because the service will be different (except the same service name of "kubelet")

Ideally, we should totally remove the dependency of nssm, maybe this is still the future direction, but I have tried quite a few times and had difficulties to make kubelet (started from SC) fit well with the cloud-init and join the CAPZ cluster. I believe it is achievable, but it probably will make things trickier so maybe this could be a future direction that we can target with.

jsturtevant · 2025-05-20T20:49:30Z

images/capi/ansible/windows/roles/kubernetes/tasks/nssm.yml

+- name: Create file to restart kubelet as a windows service
+  ansible.windows.win_template:
+    src: templates/RestartKubelet.ps1
+    dest: "{{ kubernetes_install_path }}\\RestartKubelet.ps1"
+


are you modify the default install to get sc?

image-builder/images/capi/packer/config/windows/common.json

Line 13 in 81195c7

"windows_service_manager": "nssm",

I would have thought this change here wouldn't be needed since the file is updated in the SC.yaml template which restarts and loads the file.

We should also update the document to say that Service manager now supports kubeadm.
https://image-builder.sigs.k8s.io/capi/windows/windows#service-manager

With this PR, we still use nssm, I might need to revert the change in sc.yaml file as it has caught a bit confusion. as replied in the above comments. totally independent on nssm could be a long term target but not achieved in this PR yet

Doesn't this section in restartkubelet do the same thing when it is installed as a service via Service manager? Otherwise I don't think it would work?

$FileContent = Get-Content -Path "$env:SYSTEMDRIVE/var/lib/kubelet/kubeadm-flags.env" $kubeAdmArgs = $FileContent.TrimStart('KUBELET_KUBEADM_ARGS=').Trim('"')

yes, startkubelet.ps1 and restartkubelet.ps1are essentially the same, expect some special escape required by sc.exe.
In these two ps1 files, both of them will read parameters from env file and use them to construct commandline parameters for kbuelet executable. The different is that. nssm will register the powershell startkubelet.ps1 itself as windows service, which mean during boot time, it will read the env file and reload the parameters. This unfortunately is not the case for sc.exe, which mean after we register the kubelet.exe as a windows service during image building, we do not have the env yet and will miss the correpsonding parameters for kubelet, but this defect kubelet will be registered as windos service and failed the node to join the cluster when booting with this image.

I had also tried to manipulate kublet to read from config files such as (kubelet.json etc) to make it somewhat "dynamic", but looks that not all kubelet commandline parameter will have equivalent support in the json/yaml config file.

So the idea here is: use nssm to start kubelet and join the cluster, as what we have done before. and after this, use the same parameters to restart the kubelet.exe as a windows service through sc.exe, this looks to be able to work stable

This unfortunately is not the case for sc.exe, which mean after we register the kubelet.exe as a windows service during image building, we do not have the env yet and will miss the correpsonding parameters for kubelet, but this defect kubelet will be registered as windos service and failed the node to join the cluster when booting with this image.

I believe this is why I ended up using NSSM in the end. This is definitely not ideal but I see why and how this is working now.

Yes, this can definitely be improved, although I would expect a good amount of efforts probably needed.

In this PR, basically, we just added one more script, which will be pretty safe. people can still choose to use it or stick with the old way if they want.

msft-linliu · 2025-05-22T21:00:51Z

images/capi/ansible/windows/roles/kubernetes/tasks/sc.yml

-      --enable-debugging-handlers --cgroups-per-qos=false --enforce-node-allocatable=`"`"
-      --resolv-conf=`"`" --log-dir={{ systemdrive.stdout | trim }}/var/log/kubelet
-      --logtostderr=false {{ additional_kubelet_args if additional_kubelet_args is defined }}
+- name: Create file to restart kubelet as a windows service


Why add this file in both sc.yml and nssm.yml? Only one is needed?

I will revert the change in sc.yaml as this is not the one we will use

k8s-ci-robot · 2025-05-23T17:23:10Z

New changes are detected. LGTM label has been removed.

zylxjtu · 2025-05-23T17:28:28Z

Do we have a issue related to this PR? Also can you please elaborate the description. Looking at trying to understand whats the core issue thats being addressed here.

Added more explanation/clarification

rzlink · 2025-05-23T17:56:30Z

images/capi/ansible/windows/roles/kubernetes/templates/RestartKubelet.ps1

+		return
+	}
+	else {
+		Write-Host "Waiting for service to be fully deleted... (attempt $($i + 1)/$Retries)"


The variable $Retries is undefined here.

rzlink · 2025-05-23T17:57:49Z

images/capi/ansible/windows/roles/kubernetes/templates/RestartKubelet.ps1

+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# Need to keey sync with StartKubelet.ps1


keep sync with

k8s-ci-robot · 2025-06-01T17:23:56Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign jsturtevant for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found 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

zylxjtu · 2025-06-01T17:27:21Z

images/capi/packer/config/windows/common.json

@@ -10,7 +10,7 @@
  "prepull": "true",
  "runtime": "containerd",
  "ssh_source_url": "",
-  "windows_service_manager": "nssm",
+  "windows_service_manager": "windows_service",


or maybe I should make it applied to azure only in the begining?

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label May 16, 2025

k8s-ci-robot requested review from drew-viles and jsturtevant May 16, 2025 21:31

k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label May 16, 2025

zylxjtu force-pushed the main branch 2 times, most recently from d820d52 to ec1e27d Compare May 18, 2025 23:12

Make kubelet as a windows service

d41acfa

zylxjtu force-pushed the main branch from ec1e27d to d41acfa Compare May 18, 2025 23:14

k8s-ci-robot assigned marosset May 20, 2025

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 20, 2025

jsturtevant reviewed May 20, 2025

View reviewed changes

msft-linliu reviewed May 22, 2025

View reviewed changes

Address the comments

c457af1

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 23, 2025

zylxjtu requested review from jsturtevant and msft-linliu May 23, 2025 17:23

rzlink reviewed May 23, 2025

View reviewed changes

Fix a few typos

3d1e6df

zylxjtu requested a review from rzlink May 27, 2025 18:03

zylxjtu commented Jun 1, 2025

View reviewed changes

zylxjtu force-pushed the main branch from c105ace to 4dd4493 Compare June 1, 2025 17:29

remove the depend on nssm

ebf66dd

zylxjtu force-pushed the main branch from 4dd4493 to ebf66dd Compare June 1, 2025 17:41

	Write-Host "kbuelet service deleted successfully, restarting"
	Write-Host "kubelet service deleted successfully, restarting"

	Write-Host "kbuelet service failed to restart."
	Write-Host "kubelet service failed to restart."

Make kubelet restarting as a windows service #1752

Are you sure you want to change the base?

Make kubelet restarting as a windows service #1752

Conversation

zylxjtu commented May 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Change description

Related issues

Additional context

Uh oh!

sriramandev commented May 19, 2025

Uh oh!

sriramandev commented May 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

marosset commented May 20, 2025

Uh oh!

jsturtevant left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zylxjtu commented May 20, 2025

Uh oh!

zylxjtu commented May 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zylxjtu May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented May 23, 2025

Uh oh!

zylxjtu commented May 23, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented Jun 1, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

zylxjtu commented May 16, 2025 •

edited

Loading

sriramandev commented May 19, 2025 •

edited

Loading

zylxjtu commented May 20, 2025 •

edited

Loading

zylxjtu May 23, 2025 •

edited

Loading