Skip to content

Commit

Permalink
Merge pull request kubernetes#110140 from marosset/hpc-sandbox-config…
Browse files Browse the repository at this point in the history 
…-fixes

Fixing issue in generatePodSandboxWindowsConfig for hostProcess containers
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot committed Jun 28, 2022
2 parents 7c8721a + 0c60888 commit 10bea49
Show file tree
Hide file tree
Showing 3 changed files with 257 additions and 16 deletions.
213 changes: 213 additions & 0 deletions pkg/kubelet/container/helpers_test.go
View file
Expand Up @@ -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)
})
}
}
39 changes: 23 additions & 16 deletions pkg/kubelet/kuberuntime/kuberuntime_sandbox.go
View file
Expand Up @@ -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
Expand All @@ -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
Expand Down
21 changes: 21 additions & 0 deletions pkg/kubelet/kuberuntime/kuberuntime_sandbox_test.go
View file
Expand Up @@ -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 {
Expand Down

0 comments on commit 10bea49

Please sign in to comment.