Merge pull request #40946 from yujuhong/docker_sep

pkg/kubelet/dockershim/BUILD

@@ -43,14 +43,15 @@ go_library(
         "//pkg/kubelet/qos:go_default_library",
         "//pkg/kubelet/server/streaming:go_default_library",
         "//pkg/kubelet/types:go_default_library",
+        "//pkg/kubelet/util/cache:go_default_library",
         "//pkg/kubelet/util/ioutils:go_default_library",
         "//pkg/util/hash:go_default_library",
         "//pkg/util/term:go_default_library",
+        "//vendor:github.com/blang/semver",
         "//vendor:github.com/docker/engine-api/types",
         "//vendor:github.com/docker/engine-api/types/container",
         "//vendor:github.com/docker/engine-api/types/filters",
         "//vendor:github.com/docker/engine-api/types/strslice",
-        "//vendor:github.com/docker/engine-api/types/versions",
         "//vendor:github.com/docker/go-connections/nat",
         "//vendor:github.com/golang/glog",
         "//vendor:k8s.io/apimachinery/pkg/util/errors",
@@ -87,7 +88,9 @@ go_test(
         "//pkg/kubelet/network:go_default_library",
         "//pkg/kubelet/network/mock_network:go_default_library",
         "//pkg/kubelet/types:go_default_library",
+        "//pkg/kubelet/util/cache:go_default_library",
         "//pkg/security/apparmor:go_default_library",
+        "//vendor:github.com/blang/semver",
         "//vendor:github.com/docker/engine-api/types",
         "//vendor:github.com/docker/engine-api/types/container",
         "//vendor:github.com/golang/mock/gomock",

pkg/kubelet/dockershim/docker_container.go

@@ -107,6 +107,12 @@ func (ds *dockerService) CreateContainer(podSandboxID string, config *runtimeapi
 	// Write the sandbox ID in the labels.
 	labels[sandboxIDLabelKey] = podSandboxID
 
+	apiVersion, err := ds.getDockerAPIVersion()
+	if err != nil {
+		return "", fmt.Errorf("unable to get the docker API version: %v", err)
+	}
+	securityOptSep := getSecurityOptSeparator(apiVersion)
+
 	image := ""
 	if iSpec := config.GetImage(); iSpec != nil {
 		image = iSpec.Image
@@ -152,7 +158,7 @@ func (ds *dockerService) CreateContainer(podSandboxID string, config *runtimeapi
 		// Note: ShmSize is handled in kube_docker_client.go
 
 		// Apply security context.
-		applyContainerSecurityContext(lc, podSandboxID, createConfig.Config, hc)
+		applyContainerSecurityContext(lc, podSandboxID, createConfig.Config, hc, securityOptSep)
 	}
 
 	// Apply cgroupsParent derived from the sandbox config.
@@ -177,7 +183,7 @@ func (ds *dockerService) CreateContainer(podSandboxID string, config *runtimeapi
 	hc.Resources.Devices = devices
 
 	// Apply appArmor and seccomp options.
-	securityOpts, err := getContainerSecurityOpts(config.Metadata.Name, sandboxConfig, ds.seccompProfileRoot)
+	securityOpts, err := getContainerSecurityOpts(config.Metadata.Name, sandboxConfig, ds.seccompProfileRoot, securityOptSep)
 	if err != nil {
 		return "", fmt.Errorf("failed to generate container security options for container %q: %v", config.Metadata.Name, err)
 	}

pkg/kubelet/dockershim/docker_sandbox.go

@@ -378,7 +378,7 @@ func (ds *dockerService) ListPodSandbox(filter *runtimeapi.PodSandboxFilter) ([]
 }
 
 // applySandboxLinuxOptions applies LinuxPodSandboxConfig to dockercontainer.HostConfig and dockercontainer.ContainerCreateConfig.
-func (ds *dockerService) applySandboxLinuxOptions(hc *dockercontainer.HostConfig, lc *runtimeapi.LinuxPodSandboxConfig, createConfig *dockertypes.ContainerCreateConfig, image string) error {
+func (ds *dockerService) applySandboxLinuxOptions(hc *dockercontainer.HostConfig, lc *runtimeapi.LinuxPodSandboxConfig, createConfig *dockertypes.ContainerCreateConfig, image string, separator rune) error {
 	// Apply Cgroup options.
 	cgroupParent, err := ds.GenerateExpectedCgroupParent(lc.CgroupParent)
 	if err != nil {
@@ -386,7 +386,7 @@ func (ds *dockerService) applySandboxLinuxOptions(hc *dockercontainer.HostConfig
 	}
 	hc.CgroupParent = cgroupParent
 	// Apply security context.
-	applySandboxSecurityContext(lc, createConfig.Config, hc, ds.networkPlugin)
+	applySandboxSecurityContext(lc, createConfig.Config, hc, ds.networkPlugin, separator)
 
 	return nil
 }
@@ -401,6 +401,12 @@ func (ds *dockerService) makeSandboxDockerConfig(c *runtimeapi.PodSandboxConfig,
 	// TODO(random-liu): Deprecate this label once container metrics is directly got from CRI.
 	labels[types.KubernetesContainerNameLabel] = sandboxContainerName
 
+	apiVersion, err := ds.getDockerAPIVersion()
+	if err != nil {
+		return nil, fmt.Errorf("unable to get the docker API version: %v", err)
+	}
+	securityOptSep := getSecurityOptSeparator(apiVersion)
+
 	hc := &dockercontainer.HostConfig{}
 	createConfig := &dockertypes.ContainerCreateConfig{
 		Name: makeSandboxName(c),
@@ -422,7 +428,7 @@ func (ds *dockerService) makeSandboxDockerConfig(c *runtimeapi.PodSandboxConfig,
 
 	// Apply linux-specific options.
 	if lc := c.GetLinux(); lc != nil {
-		if err := ds.applySandboxLinuxOptions(hc, lc, createConfig, image); err != nil {
+		if err := ds.applySandboxLinuxOptions(hc, lc, createConfig, image, securityOptSep); err != nil {
 			return nil, err
 		}
 	}
@@ -443,7 +449,7 @@ func (ds *dockerService) makeSandboxDockerConfig(c *runtimeapi.PodSandboxConfig,
 	setSandboxResources(hc)
 
 	// Set security options.
-	securityOpts, err := getSandboxSecurityOpts(c, ds.seccompProfileRoot)
+	securityOpts, err := getSandboxSecurityOpts(c, ds.seccompProfileRoot, securityOptSep)
 	if err != nil {
 		return nil, fmt.Errorf("failed to generate sandbox security options for sandbox %q: %v", c.Metadata.Name, err)
 	}

pkg/kubelet/dockershim/docker_service.go

@@ -19,7 +19,10 @@ package dockershim
 import (
 	"fmt"
 	"net/http"
+	"time"
 
+	"github.com/blang/semver"
+	dockertypes "github.com/docker/engine-api/types"
 	"github.com/golang/glog"
 
 	"k8s.io/kubernetes/pkg/apis/componentconfig"
@@ -33,6 +36,7 @@ import (
 	"k8s.io/kubernetes/pkg/kubelet/network/cni"
 	"k8s.io/kubernetes/pkg/kubelet/network/kubenet"
 	"k8s.io/kubernetes/pkg/kubelet/server/streaming"
+	"k8s.io/kubernetes/pkg/kubelet/util/cache"
 )
 
 const (
@@ -61,6 +65,9 @@ const (
 	containerLogPathLabelKey    = "io.kubernetes.container.logpath"
 	sandboxIDLabelKey           = "io.kubernetes.sandbox.id"
 
+	// The expiration time of version cache.
+	versionCacheTTL = 60 * time.Second
+
 	// TODO: https://github.com/kubernetes/kubernetes/pull/31169 provides experimental
 	// defaulting of host user namespace that may be enabled when the docker daemon
 	// is using remapped UIDs.
@@ -153,6 +160,12 @@ func NewDockerService(client dockertools.DockerInterface, seccompProfileRoot str
 		glog.Infof("Setting cgroupDriver to %s", cgroupDriver)
 	}
 	ds.cgroupDriver = cgroupDriver
+	ds.versionCache = cache.NewObjectCache(
+		func() (interface{}, error) {
+			return ds.getDockerVersion()
+		},
+		versionCacheTTL,
+	)
 	return ds, nil
 }
 
@@ -180,27 +193,39 @@ type dockerService struct {
 	checkpointHandler CheckpointHandler
 	// legacyCleanup indicates whether legacy cleanup has finished or not.
 	legacyCleanup legacyCleanupFlag
+	// caches the version of the runtime.
+	// To be compatible with multiple docker versions, we need to perform
+	// version checking for some operations. Use this cache to avoid querying
+	// the docker daemon every time we need to do such checks.
+	versionCache *cache.ObjectCache
 }
 
 // Version returns the runtime name, runtime version and runtime API version
 func (ds *dockerService) Version(_ string) (*runtimeapi.VersionResponse, error) {
-	v, err := ds.client.Version()
+	v, err := ds.getDockerVersion()
 	if err != nil {
-		return nil, fmt.Errorf("docker: failed to get docker version: %v", err)
+		return nil, err
 	}
-	runtimeAPIVersion := kubeAPIVersion
-	name := dockerRuntimeName
-	// Docker API version (e.g., 1.23) is not semver compatible. Add a ".0"
-	// suffix to remedy this.
-	apiVersion := fmt.Sprintf("%s.0", v.APIVersion)
 	return &runtimeapi.VersionResponse{
-		Version:           runtimeAPIVersion,
-		RuntimeName:       name,
+		Version:           kubeAPIVersion,
+		RuntimeName:       dockerRuntimeName,
 		RuntimeVersion:    v.Version,
-		RuntimeApiVersion: apiVersion,
+		RuntimeApiVersion: v.APIVersion,
 	}, nil
 }
 
+// dockerVersion gets the version information from docker.
+func (ds *dockerService) getDockerVersion() (*dockertypes.Version, error) {
+	v, err := ds.client.Version()
+	if err != nil {
+		return nil, fmt.Errorf("failed to get docker version: %v", err)
+	}
+	// Docker API version (e.g., 1.23) is not semver compatible. Add a ".0"
+	// suffix to remedy this.
+	v.APIVersion = fmt.Sprintf("%s.0", v.APIVersion)
+	return v, nil
+}
+
 // UpdateRuntimeConfig updates the runtime config. Currently only handles podCIDR updates.
 func (ds *dockerService) UpdateRuntimeConfig(runtimeConfig *runtimeapi.RuntimeConfig) (err error) {
 	if runtimeConfig == nil {
@@ -298,3 +323,31 @@ func (ds *dockerService) GenerateExpectedCgroupParent(cgroupParent string) (stri
 	glog.V(3).Infof("Setting cgroup parent to: %q", cgroupParent)
 	return cgroupParent, nil
 }
+
+// getDockerAPIVersion gets the semver-compatible docker api version.
+func (ds *dockerService) getDockerAPIVersion() (*semver.Version, error) {
+	var dv *dockertypes.Version
+	var err error
+	if ds.versionCache != nil {
+		dv, err = ds.getDockerVersionFromCache()
+	} else {
+		dv, err = ds.getDockerVersion()
+	}
+
+	apiVersion, err := semver.Parse(dv.APIVersion)
+	if err != nil {
+		return nil, err
+	}
+	return &apiVersion, nil
+}
+
+func (ds *dockerService) getDockerVersionFromCache() (*dockertypes.Version, error) {
+	// We only store on key in the cache.
+	const dummyKey = "version"
+	value, err := ds.versionCache.Get(dummyKey)
+	dv := value.(*dockertypes.Version)
+	if err != nil {
+		return nil, err
+	}
+	return dv, nil
+}

pkg/kubelet/dockershim/docker_service_test.go

@@ -21,8 +21,11 @@ import (
 	"testing"
 	"time"
 
+	"github.com/blang/semver"
+	dockertypes "github.com/docker/engine-api/types"
 	"github.com/golang/mock/gomock"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"k8s.io/client-go/util/clock"
 	runtimeapi "k8s.io/kubernetes/pkg/kubelet/api/v1alpha1/runtime"
@@ -30,6 +33,7 @@ import (
 	"k8s.io/kubernetes/pkg/kubelet/dockertools"
 	"k8s.io/kubernetes/pkg/kubelet/network"
 	"k8s.io/kubernetes/pkg/kubelet/network/mock_network"
+	"k8s.io/kubernetes/pkg/kubelet/util/cache"
 )
 
 // newTestNetworkPlugin returns a mock plugin that implements network.NetworkPlugin
@@ -40,11 +44,22 @@ func newTestNetworkPlugin(t *testing.T) *mock_network.MockNetworkPlugin {
 
 func newTestDockerService() (*dockerService, *dockertools.FakeDockerClient, *clock.FakeClock) {
 	fakeClock := clock.NewFakeClock(time.Time{})
-	c := dockertools.NewFakeDockerClient().WithClock(fakeClock)
+	c := dockertools.NewFakeDockerClient().WithClock(fakeClock).WithVersion("1.11.2", "1.23")
 	return &dockerService{client: c, os: &containertest.FakeOS{}, networkPlugin: &network.NoopNetworkPlugin{},
 		legacyCleanup: legacyCleanupFlag{done: 1}, checkpointHandler: NewTestPersistentCheckpointHandler()}, c, fakeClock
 }
 
+func newTestDockerServiceWithVersionCache() (*dockerService, *dockertools.FakeDockerClient, *clock.FakeClock) {
+	ds, c, fakeClock := newTestDockerService()
+	ds.versionCache = cache.NewObjectCache(
+		func() (interface{}, error) {
+			return ds.getDockerVersion()
+		},
+		time.Hour*10,
+	)
+	return ds, c, fakeClock
+}
+
 // TestStatus tests the runtime status logic.
 func TestStatus(t *testing.T) {
 	ds, fDocker, _ := newTestDockerService()
@@ -90,3 +105,26 @@ func TestStatus(t *testing.T) {
 		runtimeapi.NetworkReady: false,
 	}, status)
 }
+
+func TestVersion(t *testing.T) {
+	ds, _, _ := newTestDockerService()
+
+	expectedVersion := &dockertypes.Version{Version: "1.11.2", APIVersion: "1.23.0"}
+	v, err := ds.getDockerVersion()
+	require.NoError(t, err)
+	assert.Equal(t, expectedVersion, v)
+
+	expectedAPIVersion := &semver.Version{Major: 1, Minor: 23, Patch: 0}
+	apiVersion, err := ds.getDockerAPIVersion()
+	require.NoError(t, err)
+	assert.Equal(t, expectedAPIVersion, apiVersion)
+}
+
+func TestAPIVersionWithCache(t *testing.T) {
+	ds, _, _ := newTestDockerServiceWithVersionCache()
+
+	expected := &semver.Version{Major: 1, Minor: 23, Patch: 0}
+	version, err := ds.getDockerAPIVersion()
+	require.NoError(t, err)
+	assert.Equal(t, expected, version)
+}

pkg/kubelet/dockershim/helpers.go

@@ -22,9 +22,9 @@ import (
 	"strconv"
 	"strings"
 
+	"github.com/blang/semver"
 	dockertypes "github.com/docker/engine-api/types"
 	dockerfilters "github.com/docker/engine-api/types/filters"
-	dockerapiversion "github.com/docker/engine-api/types/versions"
 	dockernat "github.com/docker/go-connections/nat"
 	"github.com/golang/glog"
 
@@ -40,25 +40,11 @@ const (
 
 var (
 	conflictRE = regexp.MustCompile(`Conflict. (?:.)+ is already in use by container ([0-9a-z]+)`)
-)
-
-// apiVersion implements kubecontainer.Version interface by implementing
-// Compare() and String(). It uses the compare function of engine-api to
-// compare docker apiversions.
-type apiVersion string
-
-func (v apiVersion) String() string {
-	return string(v)
-}
 
-func (v apiVersion) Compare(other string) (int, error) {
-	if dockerapiversion.LessThan(string(v), other) {
-		return -1, nil
-	} else if dockerapiversion.GreaterThan(string(v), other) {
-		return 1, nil
-	}
-	return 0, nil
-}
+	// Docker changes the security option separator from ':' to '=' in the 1.23
+	// API version.
+	optsSeparatorChangeVersion = semver.MustParse(dockertools.SecurityOptSeparatorChangeVersion)
+)
 
 // generateEnvList converts KeyValue list to a list of strings, in the form of
 // '<key>=<value>', which can be understood by docker.
@@ -198,7 +184,7 @@ func makePortsAndBindings(pm []*runtimeapi.PortMapping) (map[dockernat.Port]stru
 // getContainerSecurityOpt gets container security options from container and sandbox config, currently from sandbox
 // annotations.
 // It is an experimental feature and may be promoted to official runtime api in the future.
-func getContainerSecurityOpts(containerName string, sandboxConfig *runtimeapi.PodSandboxConfig, seccompProfileRoot string) ([]string, error) {
+func getContainerSecurityOpts(containerName string, sandboxConfig *runtimeapi.PodSandboxConfig, seccompProfileRoot string, separator rune) ([]string, error) {
 	appArmorOpts, err := dockertools.GetAppArmorOpts(sandboxConfig.GetAnnotations(), containerName)
 	if err != nil {
 		return nil, err
@@ -208,17 +194,13 @@ func getContainerSecurityOpts(containerName string, sandboxConfig *runtimeapi.Po
 		return nil, err
 	}
 	securityOpts := append(appArmorOpts, seccompOpts...)
-	var opts []string
-	for _, securityOpt := range securityOpts {
-		k, v := securityOpt.GetKV()
-		opts = append(opts, fmt.Sprintf("%s=%s", k, v))
-	}
-	return opts, nil
+	fmtOpts := dockertools.FmtDockerOpts(securityOpts, separator)
+	return fmtOpts, nil
 }
 
-func getSandboxSecurityOpts(sandboxConfig *runtimeapi.PodSandboxConfig, seccompProfileRoot string) ([]string, error) {
+func getSandboxSecurityOpts(sandboxConfig *runtimeapi.PodSandboxConfig, seccompProfileRoot string, separator rune) ([]string, error) {
 	// sandboxContainerName doesn't exist in the pod, so pod security options will be returned by default.
-	return getContainerSecurityOpts(sandboxContainerName, sandboxConfig, seccompProfileRoot)
+	return getContainerSecurityOpts(sandboxContainerName, sandboxConfig, seccompProfileRoot, separator)
 }
 
 func getNetworkNamespace(c *dockertypes.ContainerJSON) string {
@@ -323,3 +305,18 @@ func recoverFromCreationConflictIfNeeded(client dockertools.DockerInterface, cre
 	glog.V(2).Infof("Create the container with randomized name %s", createConfig.Name)
 	return client.CreateContainer(createConfig)
 }
+
+// getSecurityOptSeparator returns the security option separator based on the
+// docker API version.
+// TODO: Remove this function along with the relevant code when we no longer
+// need to support docker 1.10.
+func getSecurityOptSeparator(v *semver.Version) rune {
+	switch v.Compare(optsSeparatorChangeVersion) {
+	case -1:
+		// Current version is less than the API change version; use the old
+		// separator.
+		return dockertools.SecurityOptSeparatorOld
+	default:
+		return dockertools.SecurityOptSeparatorNew
+	}
+}

pkg/kubelet/dockershim/helpers_test.go

@@ -19,6 +19,7 @@ package dockershim
 import (
 	"testing"
 
+	"github.com/blang/semver"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
@@ -95,7 +96,7 @@ func TestGetContainerSecurityOpts(t *testing.T) {
 	}}
 
 	for i, test := range tests {
-		opts, err := getContainerSecurityOpts(containerName, test.config, "test/seccomp/profile/root")
+		opts, err := getContainerSecurityOpts(containerName, test.config, "test/seccomp/profile/root", '=')
 		assert.NoError(t, err, "TestCase[%d]: %s", i, test.msg)
 		assert.Len(t, opts, len(test.expectedOpts), "TestCase[%d]: %s", i, test.msg)
 		for _, opt := range test.expectedOpts {
@@ -140,7 +141,7 @@ func TestGetSandboxSecurityOpts(t *testing.T) {
 	}}
 
 	for i, test := range tests {
-		opts, err := getSandboxSecurityOpts(test.config, "test/seccomp/profile/root")
+		opts, err := getSandboxSecurityOpts(test.config, "test/seccomp/profile/root", '=')
 		assert.NoError(t, err, "TestCase[%d]: %s", i, test.msg)
 		assert.Len(t, opts, len(test.expectedOpts), "TestCase[%d]: %s", i, test.msg)
 		for _, opt := range test.expectedOpts {
@@ -236,3 +237,27 @@ func TestParsingCreationConflictError(t *testing.T) {
 	require.Len(t, matches, 2)
 	require.Equal(t, matches[1], "24666ab8c814d16f986449e504ea0159468ddf8da01897144a770f66dce0e14e")
 }
+
+func TestGetSecurityOptSeparator(t *testing.T) {
+	for c, test := range map[string]struct {
+		desc     string
+		version  *semver.Version
+		expected rune
+	}{
+		"older docker version": {
+			version:  &semver.Version{Major: 1, Minor: 22, Patch: 0},
+			expected: ':',
+		},
+		"changed docker version": {
+			version:  &semver.Version{Major: 1, Minor: 23, Patch: 0},
+			expected: '=',
+		},
+		"newer docker version": {
+			version:  &semver.Version{Major: 1, Minor: 24, Patch: 0},
+			expected: '=',
+		},
+	} {
+		actual := getSecurityOptSeparator(test.version)
+		assert.Equal(t, test.expected, actual, c)
+	}
+}

pkg/kubelet/dockershim/security_context.go

@@ -24,13 +24,12 @@ import (
 
 	"k8s.io/kubernetes/pkg/api/v1"
 	runtimeapi "k8s.io/kubernetes/pkg/kubelet/api/v1alpha1/runtime"
-	"k8s.io/kubernetes/pkg/kubelet/dockertools"
 	"k8s.io/kubernetes/pkg/kubelet/dockertools/securitycontext"
 	"k8s.io/kubernetes/pkg/kubelet/network"
 )
 
 // applySandboxSecurityContext updates docker sandbox options according to security context.
-func applySandboxSecurityContext(lc *runtimeapi.LinuxPodSandboxConfig, config *dockercontainer.Config, hc *dockercontainer.HostConfig, networkPlugin network.NetworkPlugin) {
+func applySandboxSecurityContext(lc *runtimeapi.LinuxPodSandboxConfig, config *dockercontainer.Config, hc *dockercontainer.HostConfig, networkPlugin network.NetworkPlugin, separator rune) {
 	if lc == nil {
 		return
 	}
@@ -47,19 +46,19 @@ func applySandboxSecurityContext(lc *runtimeapi.LinuxPodSandboxConfig, config *d
 	}
 
 	modifyContainerConfig(sc, config)
-	modifyHostConfig(sc, hc)
+	modifyHostConfig(sc, hc, separator)
 	modifySandboxNamespaceOptions(sc.GetNamespaceOptions(), hc, networkPlugin)
 
 }
 
 // applyContainerSecurityContext updates docker container options according to security context.
-func applyContainerSecurityContext(lc *runtimeapi.LinuxContainerConfig, sandboxID string, config *dockercontainer.Config, hc *dockercontainer.HostConfig) {
+func applyContainerSecurityContext(lc *runtimeapi.LinuxContainerConfig, sandboxID string, config *dockercontainer.Config, hc *dockercontainer.HostConfig, separator rune) {
 	if lc == nil {
 		return
 	}
 
 	modifyContainerConfig(lc.SecurityContext, config)
-	modifyHostConfig(lc.SecurityContext, hc)
+	modifyHostConfig(lc.SecurityContext, hc, separator)
 	modifyContainerNamespaceOptions(lc.SecurityContext.GetNamespaceOptions(), sandboxID, hc)
 	return
 }
@@ -78,7 +77,7 @@ func modifyContainerConfig(sc *runtimeapi.LinuxContainerSecurityContext, config
 }
 
 // modifyHostConfig applies security context config to dockercontainer.HostConfig.
-func modifyHostConfig(sc *runtimeapi.LinuxContainerSecurityContext, hostConfig *dockercontainer.HostConfig) {
+func modifyHostConfig(sc *runtimeapi.LinuxContainerSecurityContext, hostConfig *dockercontainer.HostConfig, separator rune) {
 	if sc == nil {
 		return
 	}
@@ -104,7 +103,7 @@ func modifyHostConfig(sc *runtimeapi.LinuxContainerSecurityContext, hostConfig *
 				Type:  sc.SelinuxOptions.Type,
 				Level: sc.SelinuxOptions.Level,
 			},
-			dockertools.SecurityOptSeparatorNew,
+			separator,
 		)
 	}
 }

pkg/kubelet/dockershim/security_context_test.go

@@ -127,7 +127,7 @@ func TestModifyHostConfig(t *testing.T) {
 
 	for _, tc := range cases {
 		dockerCfg := &dockercontainer.HostConfig{}
-		modifyHostConfig(tc.sc, dockerCfg)
+		modifyHostConfig(tc.sc, dockerCfg, '=')
 		assert.Equal(t, tc.expected, dockerCfg, "[Test case %q]", tc.name)
 	}
 }
@@ -157,7 +157,7 @@ func TestModifyHostConfigWithGroups(t *testing.T) {
 
 	for _, tc := range testCases {
 		dockerCfg := &dockercontainer.HostConfig{}
-		modifyHostConfig(tc.securityContext, dockerCfg)
+		modifyHostConfig(tc.securityContext, dockerCfg, '=')
 		assert.Equal(t, tc.expected, dockerCfg, "[Test case %q]", tc.name)
 	}
 }
@@ -218,7 +218,7 @@ func TestModifyHostConfigAndNamespaceOptionsForContainer(t *testing.T) {
 
 	for _, tc := range cases {
 		dockerCfg := &dockercontainer.HostConfig{}
-		modifyHostConfig(tc.sc, dockerCfg)
+		modifyHostConfig(tc.sc, dockerCfg, '=')
 		modifyContainerNamespaceOptions(tc.sc.GetNamespaceOptions(), sandboxID, dockerCfg)
 		assert.Equal(t, tc.expected, dockerCfg, "[Test case %q]", tc.name)
 	}

pkg/kubelet/dockertools/docker_manager.go

@@ -110,7 +110,7 @@ const (
 	versionCacheTTL = 60 * time.Second
 
 	// Docker changed the API for specifying options in v1.11
-	SecurityOptSeparatorChangeVersion = "1.23" // Corresponds to docker 1.11.x
+	SecurityOptSeparatorChangeVersion = "1.23.0" // Corresponds to docker 1.11.x
 	SecurityOptSeparatorOld           = ':'
 	SecurityOptSeparatorNew           = '='
 )

pkg/kubelet/dockertools/fake_docker_client.go

@@ -569,7 +569,8 @@ func (f *FakeDockerClient) PullImage(image string, auth dockertypes.AuthConfig,
 func (f *FakeDockerClient) Version() (*dockertypes.Version, error) {
 	f.Lock()
 	defer f.Unlock()
-	return &f.VersionInfo, f.popError("version")
+	v := f.VersionInfo
+	return &v, f.popError("version")
 }
 
 func (f *FakeDockerClient) Info() (*dockertypes.Info, error) {