Merge pull request #110140 from marosset/hpc-sandbox-config-fixes

Fixing issue in generatePodSandboxWindowsConfig for hostProcess containers

pkg/kubelet/container/helpers_test.go

@@ -699,3 +699,216 @@ func TestShouldRecordEvent(t *testing.T) {
 	_, actual = innerEventRecorder.shouldRecordEvent(nilObj)
 	assert.Equal(t, false, actual, "should not panic if the typed nil was used, see https://github.com/kubernetes/kubernetes/issues/95552")
 }
+
+func TestHasWindowsHostProcessContainer(t *testing.T) {
+	trueVar := true
+	falseVar := false
+	const containerName = "container"
+
+	testCases := []struct {
+		name           string
+		podSpec        *v1.PodSpec
+		expectedResult bool
+	}{
+		{
+			name: "hostprocess not set anywhere",
+			podSpec: &v1.PodSpec{
+				Containers: []v1.Container{{
+					Name: containerName,
+				}},
+			},
+			expectedResult: false,
+		},
+		{
+			name: "pod with hostprocess=false",
+			podSpec: &v1.PodSpec{
+				HostNetwork: true,
+				SecurityContext: &v1.PodSecurityContext{
+					WindowsOptions: &v1.WindowsSecurityContextOptions{
+						HostProcess: &falseVar,
+					},
+				},
+				Containers: []v1.Container{{
+					Name: containerName,
+				}},
+			},
+			expectedResult: false,
+		},
+		{
+			name: "pod with hostprocess=true",
+			podSpec: &v1.PodSpec{
+				HostNetwork: true,
+				SecurityContext: &v1.PodSecurityContext{
+					WindowsOptions: &v1.WindowsSecurityContextOptions{
+						HostProcess: &trueVar,
+					},
+				},
+				Containers: []v1.Container{{
+					Name: containerName,
+				}},
+			},
+			expectedResult: true,
+		},
+		{
+			name: "container with hostprocess=false",
+			podSpec: &v1.PodSpec{
+				HostNetwork: true,
+				Containers: []v1.Container{{
+					Name: containerName,
+					SecurityContext: &v1.SecurityContext{
+						WindowsOptions: &v1.WindowsSecurityContextOptions{
+							HostProcess: &falseVar,
+						},
+					},
+				}},
+			},
+			expectedResult: false,
+		},
+		{
+			name: "container with hostprocess=true",
+			podSpec: &v1.PodSpec{
+				HostNetwork: true,
+				Containers: []v1.Container{{
+					Name: containerName,
+					SecurityContext: &v1.SecurityContext{
+						WindowsOptions: &v1.WindowsSecurityContextOptions{
+							HostProcess: &trueVar,
+						},
+					},
+				}},
+			},
+			expectedResult: true,
+		},
+		{
+			name: "pod with hostprocess=false, container with hostprocess=true",
+			podSpec: &v1.PodSpec{
+				HostNetwork: true,
+				SecurityContext: &v1.PodSecurityContext{
+					WindowsOptions: &v1.WindowsSecurityContextOptions{
+						HostProcess: &falseVar,
+					},
+				},
+				Containers: []v1.Container{{
+					Name: containerName,
+					SecurityContext: &v1.SecurityContext{
+						WindowsOptions: &v1.WindowsSecurityContextOptions{
+							HostProcess: &trueVar,
+						},
+					},
+				}},
+			},
+			expectedResult: true,
+		},
+		{
+			name: "pod with hostprocess=true, container with hostprocess=flase",
+			podSpec: &v1.PodSpec{
+				HostNetwork: true,
+				SecurityContext: &v1.PodSecurityContext{
+					WindowsOptions: &v1.WindowsSecurityContextOptions{
+						HostProcess: &trueVar,
+					},
+				},
+				Containers: []v1.Container{{
+					Name: containerName,
+					SecurityContext: &v1.SecurityContext{
+						WindowsOptions: &v1.WindowsSecurityContextOptions{
+							HostProcess: &falseVar,
+						},
+					},
+				}},
+			},
+			expectedResult: false,
+		},
+		{
+			name: "containers with hostproces=mixed",
+			podSpec: &v1.PodSpec{
+				Containers: []v1.Container{
+					{
+						Name: containerName,
+						SecurityContext: &v1.SecurityContext{
+							WindowsOptions: &v1.WindowsSecurityContextOptions{
+								HostProcess: &falseVar,
+							},
+						},
+					},
+					{
+						Name: containerName,
+						SecurityContext: &v1.SecurityContext{
+							WindowsOptions: &v1.WindowsSecurityContextOptions{
+								HostProcess: &trueVar,
+							},
+						},
+					},
+				},
+			},
+			expectedResult: true,
+		},
+		{
+			name: "pod with hostProcess=false, containers with hostproces=mixed",
+			podSpec: &v1.PodSpec{
+				SecurityContext: &v1.PodSecurityContext{
+					WindowsOptions: &v1.WindowsSecurityContextOptions{
+						HostProcess: &falseVar,
+					},
+				},
+				Containers: []v1.Container{
+					{
+						Name: containerName,
+						SecurityContext: &v1.SecurityContext{
+							WindowsOptions: &v1.WindowsSecurityContextOptions{
+								HostProcess: &falseVar,
+							},
+						},
+					},
+					{
+						Name: containerName,
+						SecurityContext: &v1.SecurityContext{
+							WindowsOptions: &v1.WindowsSecurityContextOptions{
+								HostProcess: &trueVar,
+							},
+						},
+					},
+				},
+			},
+			expectedResult: true,
+		},
+		{
+			name: "pod with hostProcess=true, containers with hostproces=mixed",
+			podSpec: &v1.PodSpec{
+				SecurityContext: &v1.PodSecurityContext{
+					WindowsOptions: &v1.WindowsSecurityContextOptions{
+						HostProcess: &trueVar,
+					},
+				},
+				Containers: []v1.Container{
+					{
+						Name: containerName,
+						SecurityContext: &v1.SecurityContext{
+							WindowsOptions: &v1.WindowsSecurityContextOptions{
+								HostProcess: &falseVar,
+							},
+						},
+					},
+					{
+						Name: containerName,
+						SecurityContext: &v1.SecurityContext{
+							WindowsOptions: &v1.WindowsSecurityContextOptions{
+								HostProcess: &trueVar,
+							},
+						},
+					},
+				},
+			},
+			expectedResult: true,
+		},
+	}
+
+	for _, testCase := range testCases {
+		t.Run(testCase.name, func(t *testing.T) {
+			pod := &v1.Pod{}
+			pod.Spec = *testCase.podSpec
+			result := HasWindowsHostProcessContainer(pod)
+			assert.Equal(t, result, testCase.expectedResult)
+		})
+	}
+}

pkg/kubelet/kuberuntime/kuberuntime_sandbox.go

@@ -228,6 +228,29 @@ func (m *kubeGenericRuntimeManager) generatePodSandboxWindowsConfig(pod *v1.Pod)
 		SecurityContext: &runtimeapi.WindowsSandboxSecurityContext{},
 	}
 
+	// If all of the containers in a pod are HostProcess containers, set the pod's HostProcess field
+	// explicitly because the container runtime requires this information at sandbox creation time.
+	if kubecontainer.HasWindowsHostProcessContainer(pod) {
+		// Pods containing HostProcess containers should fail to schedule if feature is not
+		// enabled instead of trying to schedule containers as regular containers as stated in
+		// PRR review.
+		if !utilfeature.DefaultFeatureGate.Enabled(features.WindowsHostProcessContainers) {
+			return nil, fmt.Errorf("pod contains HostProcess containers but feature 'WindowsHostProcessContainers' is not enabled")
+		}
+
+		// At present Windows all containers in a Windows pod must be HostProcess containers
+		// and HostNetwork is required to be set.
+		if !kubecontainer.AllContainersAreWindowsHostProcess(pod) {
+			return nil, fmt.Errorf("pod must not contain both HostProcess and non-HostProcess containers")
+		}
+
+		if !kubecontainer.IsHostNetworkPod(pod) {
+			return nil, fmt.Errorf("hostNetwork is required if Pod contains HostProcess containers")
+		}
+
+		wc.SecurityContext.HostProcess = true
+	}
+
 	sc := pod.Spec.SecurityContext
 	if sc == nil || sc.WindowsOptions == nil {
 		return wc, nil
@@ -243,26 +266,10 @@ func (m *kubeGenericRuntimeManager) generatePodSandboxWindowsConfig(pod *v1.Pod)
 	}
 
 	if kubecontainer.HasWindowsHostProcessContainer(pod) {
-		// Pods containing HostProcess containers should fail to schedule if feature is not
-		// enabled instead of trying to schedule containers as regular containers as stated in
-		// PRR review.
-		if !utilfeature.DefaultFeatureGate.Enabled(features.WindowsHostProcessContainers) {
-			return nil, fmt.Errorf("pod contains HostProcess containers but feature 'WindowsHostProcessContainers' is not enabled")
-		}
 
 		if wo.HostProcess != nil && !*wo.HostProcess {
 			return nil, fmt.Errorf("pod must not contain any HostProcess containers if Pod's WindowsOptions.HostProcess is set to false")
 		}
-		// At present Windows all containers in a Windows pod must be HostProcess containers
-		// and HostNetwork is required to be set.
-		if !kubecontainer.AllContainersAreWindowsHostProcess(pod) {
-			return nil, fmt.Errorf("pod must not contain both HostProcess and non-HostProcess containers")
-		}
-		if !kubecontainer.IsHostNetworkPod(pod) {
-			return nil, fmt.Errorf("hostNetwork is required if Pod contains HostProcess containers")
-		}
-
-		wc.SecurityContext.HostProcess = true
 	}
 
 	return wc, nil

pkg/kubelet/kuberuntime/kuberuntime_sandbox_test.go

@@ -345,6 +345,27 @@ func TestGeneratePodSandboxWindowsConfig(t *testing.T) {
 			expectedWindowsConfig: nil,
 			expectedError:         fmt.Errorf("pod must not contain any HostProcess containers if Pod's WindowsOptions.HostProcess is set to false"),
 		},
+		{
+			name:                      "Pod's security context doesn't specify HostProcess containers but Container's security context does",
+			hostProcessFeatureEnabled: true,
+			podSpec: &v1.PodSpec{
+				HostNetwork: true,
+				Containers: []v1.Container{{
+					Name: containerName,
+					SecurityContext: &v1.SecurityContext{
+						WindowsOptions: &v1.WindowsSecurityContextOptions{
+							HostProcess: &trueVar,
+						},
+					},
+				}},
+			},
+			expectedWindowsConfig: &runtimeapi.WindowsPodSandboxConfig{
+				SecurityContext: &runtimeapi.WindowsSandboxSecurityContext{
+					HostProcess: true,
+				},
+			},
+			expectedError: nil,
+		},
 	}
 
 	for _, testCase := range testCases {