test: tolerate cloud-provider-uninitialized taint on Windows validation pods

[Tim Wright (timmy-wright)]

Summary

Windows E2E validation pods (podWindows, used for the servercore/nanoserver checks) had no tolerations. If the target node reported Ready=True before the Azure cloud-controller-manager removed its node.cloudprovider.kubernetes.io/uninitialized:NoSchedule taint, the validation pod stayed Pending until the 360s pod-wait timed out, failing the test with a misleading FailedScheduling error rather than a real product/VHD issue.

Investigated from build 176544987 (windows-2022-containerd-gen2): node winw06a000000 reported ready at 887.774s still carrying the uninitialized taint; the validation pod then failed to schedule and the test timed out at 1250.370s.

Change

Add a toleration for node.cloudprovider.kubernetes.io/uninitialized:NoSchedule to podWindows() so the validation pod schedules onto the target node as soon as it's Ready, without waiting on CCM taint removal.

Testing

• go build ./... and go vet ./... pass in e2e/.
• gofmt -l clean.

[github-actions]

Windows Unit Test Results

  3 files   12 suites   59s ⏱️
389 tests 389 ✅ 0 💤 0 ❌
392 runs  392 ✅ 0 💤 0 ❌

Results for commit ed3d843.

♻️ This comment has been updated with latest results.

[Copilot]

Pull request overview

This PR updates the Windows E2E helper pod spec so Windows validation pods can schedule even when the target node is still tainted with node.cloudprovider.kubernetes.io/uninitialized:NoSchedule, avoiding false-negative timeouts during Windows image validation.

Changes:

• Add an explicit pod toleration for the node.cloudprovider.kubernetes.io/uninitialized:NoSchedule taint in podWindows().
• Document why the toleration is needed (node may report Ready before CCM removes the taint).

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

e2e/kube.go:786

• PR description says to add a toleration to podWindows(), but podWindows() has been removed and replaced with a DaemonSet helper (debugDaemonsetWindows). If the intent is still just to tolerate the cloud-provider-uninitialized taint for the Windows validation pod, consider restoring a Pod-based helper and adding the toleration there (or update the PR description to match the DaemonSet approach and add corresponding DaemonSet validation/cleanup).

func debugDaemonsetWindows(s *Scenario, podName string, imageName string) *appsv1.DaemonSet {
	deploymentName := fmt.Sprintf("%s-test-%s-pod", s.Runtime.VM.KubeName, podName)
	return &appsv1.DaemonSet{
		TypeMeta: metav1.TypeMeta{
			Kind:       "DaemonSet",
			APIVersion: "apps/v1",
		},

e2e/kube.go:809

• Typo in comment: "nanoserve" should be "nanoserver".

							// this should exist on both servercore and nanoserve

[r2k1]

I traced the recurring failure to the Windows pause image.

Build 176544987 shows the full chain in Test_Windows2022CachingRegression:

• The node reported Ready=True with node.cloudprovider.kubernetes.io/uninitialized:NoSchedule.
• The scheduler assigned cloud-node-manager-windows at 23:25:40Z.
• At 23:25:46Z, kubelet reported FailedCreatePodSandBox for mcr.microsoft.com/oss/v2/kubernetes/pause:3.10.2-windows-ltsc2022-amd64: not found.
• CSE package v0.0.52 creates that tag by adding -windows-<version>-amd64 to the supplied pause image.
• MCR contains the legacy Windows tag for 3.10.1, but it does not contain the generated 3.10.2 tag.
• Commit 45f5643549 in fix: update pause to 3.10.2 #9128 changed the Windows pause image from 3.10.1 to 3.10.2. The E2E history changes from green to red across that commit.

cloud-node-manager removes the taint after node initialization. Containerd could not create its pod sandbox, so the manager never ran and the taint remained. Scheduler retries repeat the same image-pull error.

This test uses v0.0.52 on purpose. It covers Windows VHDs built before local caching. Moving the test to the current CSE removes that compatibility coverage.

Please replace the toleration with a compatibility fix:

1. Publish the missing 3.10.2-windows-<version>-amd64 aliases for supported Windows versions.
2. Until those aliases exist, keep the Windows pause image at 3.10.1 for legacy CSE packages.

WaitUntilNodeReady must also require removal of the cloud-provider taint. On timeout, the test can print the final node state and cloud-node-manager events.

The validation-pod toleration weakens the readiness check without repairing node initialization.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

e2e/kube.go:474

• This adds a toleration for the cloud-provider-uninitialized taint inside getPodTolerations(), and that helper is now used by podHTTPServerLinux() and daemonsetDebug() too. This broadens behavior beyond the PR description/title (Windows validation pods) and may mask issues by allowing those other workloads to schedule before CCM taint removal. Consider splitting helpers (e.g., base test-taint tolerations vs. Windows validation tolerations that additionally include the uninitialized taint) and use the uninitialized toleration only where needed.

		{
			Key:      "node.cloudprovider.kubernetes.io/uninitialized",
			Operator: corev1.TolerationOpExists,
			Effect:   corev1.TaintEffectNoSchedule,
		},

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Suppressed comments (4)

e2e/kube.go:777

• debugPodWindows creates a Pod but sets TypeMeta to Kind: DaemonSet / APIVersion: apps/v1. This can lead to API server decoding/validation issues for the Pod create request (and is misleading even if ignored).

		TypeMeta: metav1.TypeMeta{
			Kind:       "DaemonSet",
			APIVersion: "apps/v1",
		},

e2e/kube.go:758

• The comment above podHTTPServerLinux still describes tolerating only the test taints, but the pod now uses getPodTolerations() which also includes the cloud-provider-uninitialized toleration. This is now inaccurate.

			// Set Tolerations to tolerate the node with test taints "testkey1=value1:NoSchedule,testkey2=value2:NoSchedule".
			// This is to ensure that the pod can be scheduled on the node with the taints.
			// It won't affect other pods running on the same node.

e2e/kube.go:475

• PR description says the change is limited to Windows validation pods, but getPodTolerations() is now used by Linux validation pods and daemonsetDebug() as well, meaning the new node.cloudprovider.kubernetes.io/uninitialized toleration applies more broadly. Please confirm this is intended and either update the PR description or scope the toleration to the Windows validation pods only.

		{
			Key:      "node.cloudprovider.kubernetes.io/uninitialized",
			Operator: corev1.TolerationOpExists,
			Effect:   corev1.TaintEffectNoSchedule,
		},
	}

e2e/kube.go:456

• The comment on getPodTolerations says it only tolerates the test taints, but the function now also tolerates node.cloudprovider.kubernetes.io/uninitialized. Please update the comment so it matches behavior.

This issue also appears in the following locations of the same file:

• line 470
• line 756

	// Set Tolerations to tolerate the node with test taints "testkey1=value1:NoSchedule,testkey2=value2:NoSchedule".
	// This is to ensure that the pod can be scheduled on the node with the taints.
	// It won't affect other pods running on the same node.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (3)

e2e/validation.go:29

• The retry sleep/backoff block has a few issues: the comment misspells "exponential" and doesn't match the actual sequence (currently 1s, 2s, 4s…), the log prints a time.Duration with %d while claiming "seconds", and it sleeps even after the final attempt (adding unnecessary delay before failing). Consider computing an explicit seconds integer, only sleeping when another retry remains, and updating the comment accordingly.

		// crude expenential backoff: 2s, 4s, 8s, ...
		retryBackoff := time.Duration(1 << uint(i))
		if err != nil {
			s.T.Logf("sleeping %d seconds before retrying pod %q", retryBackoff, pod.Name)
			time.Sleep(retryBackoff * time.Second)

e2e/kube.go:475

• PR description/title indicate adding the cloud-provider-uninitialized toleration specifically for the Windows validation pod, but it’s currently included in getPodTolerations(), which is also used by podHTTPServerLinux() and daemonsetDebug(). That broadens the behavioral change to Linux validation and debug workloads too. If the broader scope is intended, update the PR description and the getPodTolerations() comment to reflect it; if it’s not intended, consider keeping test taint tolerations shared and adding the uninitialized toleration only in the Windows pod builder.

func getPodTolerations() []corev1.Toleration {
	// Set Tolerations to tolerate the node with test taints "testkey1=value1:NoSchedule,testkey2=value2:NoSchedule".
	// This is to ensure that the pod can be scheduled on the node with the taints.
	// It won't affect other pods running on the same node.
	return []corev1.Toleration{
		{
			Key:      "testkey1",
			Operator: corev1.TolerationOpEqual,
			Value:    "value1",
			Effect:   corev1.TaintEffectNoSchedule,
		},
		{
			Key:      "testkey2",
			Operator: corev1.TolerationOpEqual,
			Value:    "value2",
			Effect:   corev1.TaintEffectNoSchedule,
		},
		{
			Key:      "node.cloudprovider.kubernetes.io/uninitialized",
			Operator: corev1.TolerationOpExists,
			Effect:   corev1.TaintEffectNoSchedule,
		},
	}

e2e/kube.go:773

• debugPodWindows() is used as the primary Windows validation pod builder (ValidateNodeCanRunAPod), so the "debug" prefix is misleading. Consider renaming back to podWindows()/validationPodWindows() (and similarly rename the local "deploymentName" variable, since this is a Pod name) to make intent clearer for future maintainers.

func debugPodWindows(s *Scenario, podName string, imageName string) *corev1.Pod {
	deploymentName := fmt.Sprintf("%s-test-%s-pod", s.Runtime.VM.KubeName, podName)
	return &corev1.Pod{

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (5)

e2e/kube.go:760

• This comment says the pod only tolerates the synthetic test taints, but the pod now uses getPodTolerations() which also includes the node.cloudprovider.kubernetes.io/uninitialized:NoSchedule toleration. Please update the comment so it reflects the actual scheduling behavior.

			// Set Tolerations to tolerate the node with test taints "testkey1=value1:NoSchedule,testkey2=value2:NoSchedule".
			// This is to ensure that the pod can be scheduled on the node with the taints.
			// It won't affect other pods running on the same node.
			Tolerations:  getPodTolerations(),
			NodeSelector: getNodeSelectorForScenario(s),

e2e/kube.go:456

• The comment above getPodTolerations no longer matches what the function returns: it now also tolerates node.cloudprovider.kubernetes.io/uninitialized:NoSchedule, but the comment only mentions the synthetic test taints. Please update the comment to include the uninitialized taint so readers don’t miss the scheduling behavior change.

This issue also appears on line 756 of the same file.

	// Set Tolerations to tolerate the node with test taints "testkey1=value1:NoSchedule,testkey2=value2:NoSchedule".
	// This is to ensure that the pod can be scheduled on the node with the taints.
	// It won't affect other pods running on the same node.

e2e/validation.go:29

• The retry loop is off-by-one and can log impossible attempt counts (e.g., 4/3) because i starts at 1, the loop condition is i <= maxRetries, and the log uses i+1. This also changes semantics from “max attempts” to “max retries”, which can significantly extend E2E runtime since each attempt can block up to 6 minutes in WaitUntilPodRunning. Consider rewriting this as a clear attempt-based loop (maxAttempts) and using context-aware backoff sleep; also fix the "expenential" typo while touching this block.

	for i <= maxRetries && err != nil {
		// crude expenential backoff: 2s, 4s, 8s, ...
		retryBackoff := time.Duration(1 << uint(i))
		s.T.Logf("sleeping %d seconds before retrying pod %q", retryBackoff, pod.Name)
		time.Sleep(retryBackoff * time.Second)

e2e/kube.go:789

• Typo in comment: "nanoserve" should be "nanoserver".

					ImagePullPolicy: "IfNotPresent",
					// this should exist on both servercore and nanoserve
					Command: []string{"cmd", "/c", "ping", "-t", "localhost"},

e2e/kube.go:772

• deploymentName is used as a Pod name here, not a Deployment name. Renaming it to something like podFullName avoids confusion when debugging Kubernetes objects.

	deploymentName := fmt.Sprintf("%s-test-%s-pod", s.Runtime.VM.KubeName, podName)