Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove no longer needed modifyContainerPIDNamespaceOverrides #86783

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 0 additions & 1 deletion pkg/kubelet/dockershim/helpers_linux.go
Expand Up @@ -120,7 +120,6 @@ func (ds *dockerService) updateCreateConfig(
if err := applyContainerSecurityContext(lc, podSandboxID, createConfig.Config, createConfig.HostConfig, securityOptSep); err != nil {
return fmt.Errorf("failed to apply container security context for container %q: %v", config.Metadata.Name, err)
}
modifyContainerPIDNamespaceOverrides(apiVersion, createConfig.HostConfig, podSandboxID)
}

// Apply cgroupsParent derived from the sandbox config.
Expand Down
13 changes: 0 additions & 13 deletions pkg/kubelet/dockershim/security_context.go
Expand Up @@ -19,9 +19,7 @@ package dockershim
import (
"fmt"
"strconv"
"strings"

"github.com/blang/semver"
dockercontainer "github.com/docker/docker/api/types/container"

runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1alpha2"
Expand Down Expand Up @@ -204,14 +202,3 @@ func modifyHostOptionsForContainer(nsOpts *runtimeapi.NamespaceOption, podSandbo
hc.UTSMode = namespaceModeHost
}
}

// modifyPIDNamespaceOverrides implements a temporary override for the default PID namespace sharing for Docker:
// 1. Docker engine prior to API Version 1.24 doesn't support attaching to another container's
// PID namespace, and it didn't stabilize until 1.26. This check can be removed when Kubernetes'
// minimum Docker version is at least 1.13.1 (API version 1.26).
// TODO(verb): remove entirely once these two conditions are satisfied
func modifyContainerPIDNamespaceOverrides(version *semver.Version, hc *dockercontainer.HostConfig, podSandboxID string) {
if version.LT(semver.Version{Major: 1, Minor: 26}) && strings.HasPrefix(string(hc.PidMode), "container:") {
hc.PidMode = ""
}
}
51 changes: 0 additions & 51 deletions pkg/kubelet/dockershim/security_context_test.go
Expand Up @@ -21,7 +21,6 @@ import (
"strconv"
"testing"

"github.com/blang/semver"
dockercontainer "github.com/docker/docker/api/types/container"
"github.com/stretchr/testify/assert"

Expand Down Expand Up @@ -404,56 +403,6 @@ func TestModifyContainerNamespaceOptions(t *testing.T) {
}
}

func TestModifyContainerNamespacePIDOverride(t *testing.T) {
cases := []struct {
name string
version *semver.Version
input, expected dockercontainer.PidMode
}{
{
name: "mode:CONTAINER docker:NEW",
version: &semver.Version{Major: 1, Minor: 26},
input: "",
expected: "",
},
{
name: "mode:CONTAINER docker:OLD",
version: &semver.Version{Major: 1, Minor: 25},
input: "",
expected: "",
},
{
name: "mode:HOST docker:NEW",
version: &semver.Version{Major: 1, Minor: 26},
input: "host",
expected: "host",
},
{
name: "mode:HOST docker:OLD",
version: &semver.Version{Major: 1, Minor: 25},
input: "host",
expected: "host",
},
{
name: "mode:POD docker:NEW",
version: &semver.Version{Major: 1, Minor: 26},
input: "container:sandbox",
expected: "container:sandbox",
},
{
name: "mode:POD docker:OLD",
version: &semver.Version{Major: 1, Minor: 25},
input: "container:sandbox",
expected: "",
},
}
for _, tc := range cases {
dockerCfg := &dockercontainer.HostConfig{PidMode: tc.input}
modifyContainerPIDNamespaceOverrides(tc.version, dockerCfg, "sandbox")
assert.Equal(t, tc.expected, dockerCfg.PidMode, "[Test case %q]", tc.name)
}
}

func fullValidSecurityContext() *runtimeapi.LinuxContainerSecurityContext {
return &runtimeapi.LinuxContainerSecurityContext{
Privileged: true,
Expand Down