Merge pull request #45299 from yujuhong/cleanup-trivial

Automatic merge from submit-queue

More dockertools cleanup

This is part of #43234.

pkg/credentialprovider/BUILD

@@ -35,6 +35,7 @@ go_test(
     ],
     library = ":go_default_library",
     tags = ["automanaged"],
+    deps = ["//vendor/github.com/docker/engine-api/types:go_default_library"],
 )
 
 filegroup(

pkg/credentialprovider/keyring_test.go

@@ -19,7 +19,10 @@ package credentialprovider
 import (
 	"encoding/base64"
 	"fmt"
+	"reflect"
 	"testing"
+
+	dockertypes "github.com/docker/engine-api/types"
 )
 
 func TestUrlsMatch(t *testing.T) {
@@ -499,3 +502,117 @@ func TestLazyKeyring(t *testing.T) {
 		t.Errorf("Unexpected number of Provide calls: %v", provider.Count)
 	}
 }
+
+func TestDockerKeyringLookup(t *testing.T) {
+	ada := LazyAuthConfiguration{
+		AuthConfig: dockertypes.AuthConfig{
+			Username: "ada",
+			Password: "smash",
+			Email:    "ada@example.com",
+		},
+	}
+
+	grace := LazyAuthConfiguration{
+		AuthConfig: dockertypes.AuthConfig{
+			Username: "grace",
+			Password: "squash",
+			Email:    "grace@example.com",
+		},
+	}
+
+	dk := &BasicDockerKeyring{}
+	dk.Add(DockerConfig{
+		"bar.example.com/pong": DockerConfigEntry{
+			Username: grace.Username,
+			Password: grace.Password,
+			Email:    grace.Email,
+		},
+		"bar.example.com": DockerConfigEntry{
+			Username: ada.Username,
+			Password: ada.Password,
+			Email:    ada.Email,
+		},
+	})
+
+	tests := []struct {
+		image string
+		match []LazyAuthConfiguration
+		ok    bool
+	}{
+		// direct match
+		{"bar.example.com", []LazyAuthConfiguration{ada}, true},
+
+		// direct match deeper than other possible matches
+		{"bar.example.com/pong", []LazyAuthConfiguration{grace, ada}, true},
+
+		// no direct match, deeper path ignored
+		{"bar.example.com/ping", []LazyAuthConfiguration{ada}, true},
+
+		// match first part of path token
+		{"bar.example.com/pongz", []LazyAuthConfiguration{grace, ada}, true},
+
+		// match regardless of sub-path
+		{"bar.example.com/pong/pang", []LazyAuthConfiguration{grace, ada}, true},
+
+		// no host match
+		{"example.com", []LazyAuthConfiguration{}, false},
+		{"foo.example.com", []LazyAuthConfiguration{}, false},
+	}
+
+	for i, tt := range tests {
+		match, ok := dk.Lookup(tt.image)
+		if tt.ok != ok {
+			t.Errorf("case %d: expected ok=%t, got %t", i, tt.ok, ok)
+		}
+
+		if !reflect.DeepEqual(tt.match, match) {
+			t.Errorf("case %d: expected match=%#v, got %#v", i, tt.match, match)
+		}
+	}
+}
+
+// This validates that dockercfg entries with a scheme and url path are properly matched
+// by images that only match the hostname.
+// NOTE: the above covers the case of a more specific match trumping just hostname.
+func TestIssue3797(t *testing.T) {
+	rex := LazyAuthConfiguration{
+		AuthConfig: dockertypes.AuthConfig{
+			Username: "rex",
+			Password: "tiny arms",
+			Email:    "rex@example.com",
+		},
+	}
+
+	dk := &BasicDockerKeyring{}
+	dk.Add(DockerConfig{
+		"https://quay.io/v1/": DockerConfigEntry{
+			Username: rex.Username,
+			Password: rex.Password,
+			Email:    rex.Email,
+		},
+	})
+
+	tests := []struct {
+		image string
+		match []LazyAuthConfiguration
+		ok    bool
+	}{
+		// direct match
+		{"quay.io", []LazyAuthConfiguration{rex}, true},
+
+		// partial matches
+		{"quay.io/foo", []LazyAuthConfiguration{rex}, true},
+		{"quay.io/foo/bar", []LazyAuthConfiguration{rex}, true},
+	}
+
+	for i, tt := range tests {
+		match, ok := dk.Lookup(tt.image)
+		if tt.ok != ok {
+			t.Errorf("case %d: expected ok=%t, got %t", i, tt.ok, ok)
+		}
+
+		if !reflect.DeepEqual(tt.match, match) {
+			t.Errorf("case %d: expected match=%#v, got %#v", i, tt.match, match)
+		}
+	}
+}

pkg/kubelet/dockershim/BUILD

@@ -23,6 +23,7 @@ go_library(
         "docker_streaming.go",
         "exec.go",
         "helpers.go",
+        "helpers_linux.go",
         "naming.go",
         "security_context.go",
     ],

pkg/kubelet/dockershim/docker_container.go

@@ -151,7 +151,7 @@ func (ds *dockerService) CreateContainer(podSandboxID string, config *runtimeapi
 		if rOpts != nil {
 			hc.Resources = dockercontainer.Resources{
 				Memory:     rOpts.MemoryLimitInBytes,
-				MemorySwap: dockertools.DefaultMemorySwap(),
+				MemorySwap: DefaultMemorySwap(),
 				CPUShares:  rOpts.CpuShares,
 				CPUQuota:   rOpts.CpuQuota,
 				CPUPeriod:  rOpts.CpuPeriod,

pkg/kubelet/dockershim/docker_sandbox.go

@@ -578,7 +578,7 @@ func sharesHostIpc(container *dockertypes.ContainerJSON) bool {
 
 func setSandboxResources(hc *dockercontainer.HostConfig) {
 	hc.Resources = dockercontainer.Resources{
-		MemorySwap: dockertools.DefaultMemorySwap(),
+		MemorySwap: DefaultMemorySwap(),
 		CPUShares:  defaultSandboxCPUshares,
 		// Use docker's default cpu quota/period.
 	}

pkg/kubelet/dockershim/helpers_linux.go

@@ -16,7 +16,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package dockertools
+package dockershim
 
 func DefaultMemorySwap() int64 {
 	return 0

pkg/kubelet/dockershim/helpers_unsupported.go

@@ -16,7 +16,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package dockertools
+package dockershim
 
 func DefaultMemorySwap() int64 {
 	return -1

pkg/kubelet/dockershim/helpers_windows.go

@@ -16,7 +16,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package dockertools
+package dockershim
 
 func DefaultMemorySwap() int64 {
 	return 0

pkg/kubelet/dockershim/naming.go

@@ -23,7 +23,6 @@ import (
 	"strings"
 
 	runtimeapi "k8s.io/kubernetes/pkg/kubelet/api/v1alpha1/runtime"
-	"k8s.io/kubernetes/pkg/kubelet/dockertools"
 	"k8s.io/kubernetes/pkg/kubelet/leaky"
 )
 
@@ -51,9 +50,9 @@ const (
 	// Delimiter used to construct docker container names.
 	nameDelimiter = "_"
 	// DockerImageIDPrefix is the prefix of image id in container status.
-	DockerImageIDPrefix = dockertools.DockerPrefix
+	DockerImageIDPrefix = "docker://"
 	// DockerPullableImageIDPrefix is the prefix of pullable image id in container status.
-	DockerPullableImageIDPrefix = dockertools.DockerPullablePrefix
+	DockerPullableImageIDPrefix = "docker-pullable://"
 )
 
 func makeSandboxName(s *runtimeapi.PodSandboxConfig) string {

pkg/kubelet/dockertools/BUILD

@@ -12,8 +12,6 @@ go_library(
     name = "go_default_library",
     srcs = [
         "docker.go",
-        "docker_manager.go",
-        "docker_manager_linux.go",
         "fake_docker_client.go",
         "instrumented_docker.go",
         "kube_docker_client.go",
@@ -24,7 +22,6 @@ go_library(
         "//pkg/credentialprovider:go_default_library",
         "//pkg/kubelet/container:go_default_library",
         "//pkg/kubelet/images:go_default_library",
-        "//pkg/kubelet/leaky:go_default_library",
         "//pkg/kubelet/metrics:go_default_library",
         "//vendor/github.com/docker/distribution/digest:go_default_library",
         "//vendor/github.com/docker/distribution/reference:go_default_library",

pkg/kubelet/dockertools/docker.go

@@ -37,15 +37,21 @@ import (
 	"k8s.io/kubernetes/pkg/credentialprovider"
 	kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
 	"k8s.io/kubernetes/pkg/kubelet/images"
-	"k8s.io/kubernetes/pkg/kubelet/leaky"
 )
 
 const (
-	PodInfraContainerName = leaky.PodInfraContainerName
-	DockerPrefix          = "docker://"
-	DockerPullablePrefix  = "docker-pullable://"
 	LogSuffix          = "log"
 	ext4MaxFileNameLen = 255
+
+	DockerType = "docker"
+
+	// https://docs.docker.com/engine/reference/api/docker_remote_api/
+	// docker version should be at least 1.10.x
+	minimumDockerAPIVersion = "1.22"
+
+	statusRunningPrefix = "Up"
+	statusExitedPrefix  = "Exited"
+	statusCreatedPrefix = "Created"
 )
 
 // DockerInterface is an abstract interface for testability.  It abstracts the interface of docker client.

pkg/kubelet/dockertools/docker_manager.go

@@ -1,29 +0,0 @@
-/*
-Copyright 2015 The Kubernetes Authors.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-package dockertools
-
-const (
-	DockerType = "docker"
-
-	// https://docs.docker.com/engine/reference/api/docker_remote_api/
-	// docker version should be at least 1.10.x
-	minimumDockerAPIVersion = "1.22"
-
-	statusRunningPrefix = "Up"
-	statusExitedPrefix  = "Exited"
-	statusCreatedPrefix = "Created"
-)

pkg/kubelet/dockertools/docker_test.go

@@ -544,168 +544,6 @@ func TestPullWithSecrets(t *testing.T) {
 	}
 }
 
-func TestDockerKeyringLookupFails(t *testing.T) {
-	fakeKeyring := &credentialprovider.FakeKeyring{}
-	fakeClient := NewFakeDockerClient()
-	fakeClient.InjectError("pull", fmt.Errorf("test error"))
-
-	dp := dockerPuller{
-		client:  fakeClient,
-		keyring: fakeKeyring,
-	}
-
-	err := dp.Pull("host/repository/image:version", []v1.Secret{})
-	if err == nil {
-		t.Errorf("unexpected non-error")
-	}
-	msg := "image pull failed for host/repository/image:version, this may be because there are no credentials on this request.  details: (test error)"
-	if err.Error() != msg {
-		t.Errorf("expected: %s, saw: %s", msg, err.Error())
-	}
-}
-
-func TestDockerKeyringLookup(t *testing.T) {
-	ada := credentialprovider.LazyAuthConfiguration{
-		AuthConfig: dockertypes.AuthConfig{
-			Username: "ada",
-			Password: "smash",
-			Email:    "ada@example.com",
-		},
-	}
-
-	grace := credentialprovider.LazyAuthConfiguration{
-		AuthConfig: dockertypes.AuthConfig{
-			Username: "grace",
-			Password: "squash",
-			Email:    "grace@example.com",
-		},
-	}
-
-	dk := &credentialprovider.BasicDockerKeyring{}
-	dk.Add(credentialprovider.DockerConfig{
-		"bar.example.com/pong": credentialprovider.DockerConfigEntry{
-			Username: grace.Username,
-			Password: grace.Password,
-			Email:    grace.Email,
-		},
-		"bar.example.com": credentialprovider.DockerConfigEntry{
-			Username: ada.Username,
-			Password: ada.Password,
-			Email:    ada.Email,
-		},
-	})
-
-	tests := []struct {
-		image string
-		match []credentialprovider.LazyAuthConfiguration
-		ok    bool
-	}{
-		// direct match
-		{"bar.example.com", []credentialprovider.LazyAuthConfiguration{ada}, true},
-
-		// direct match deeper than other possible matches
-		{"bar.example.com/pong", []credentialprovider.LazyAuthConfiguration{grace, ada}, true},
-
-		// no direct match, deeper path ignored
-		{"bar.example.com/ping", []credentialprovider.LazyAuthConfiguration{ada}, true},
-
-		// match first part of path token
-		{"bar.example.com/pongz", []credentialprovider.LazyAuthConfiguration{grace, ada}, true},
-
-		// match regardless of sub-path
-		{"bar.example.com/pong/pang", []credentialprovider.LazyAuthConfiguration{grace, ada}, true},
-
-		// no host match
-		{"example.com", []credentialprovider.LazyAuthConfiguration{}, false},
-		{"foo.example.com", []credentialprovider.LazyAuthConfiguration{}, false},
-	}
-
-	for i, tt := range tests {
-		match, ok := dk.Lookup(tt.image)
-		if tt.ok != ok {
-			t.Errorf("case %d: expected ok=%t, got %t", i, tt.ok, ok)
-		}
-
-		if !reflect.DeepEqual(tt.match, match) {
-			t.Errorf("case %d: expected match=%#v, got %#v", i, tt.match, match)
-		}
-	}
-}
-
-// This validates that dockercfg entries with a scheme and url path are properly matched
-// by images that only match the hostname.
-// NOTE: the above covers the case of a more specific match trumping just hostname.
-func TestIssue3797(t *testing.T) {
-	rex := credentialprovider.LazyAuthConfiguration{
-		AuthConfig: dockertypes.AuthConfig{
-			Username: "rex",
-			Password: "tiny arms",
-			Email:    "rex@example.com",
-		},
-	}
-
-	dk := &credentialprovider.BasicDockerKeyring{}
-	dk.Add(credentialprovider.DockerConfig{
-		"https://quay.io/v1/": credentialprovider.DockerConfigEntry{
-			Username: rex.Username,
-			Password: rex.Password,
-			Email:    rex.Email,
-		},
-	})
-
-	tests := []struct {
-		image string
-		match []credentialprovider.LazyAuthConfiguration
-		ok    bool
-	}{
-		// direct match
-		{"quay.io", []credentialprovider.LazyAuthConfiguration{rex}, true},
-
-		// partial matches
-		{"quay.io/foo", []credentialprovider.LazyAuthConfiguration{rex}, true},
-		{"quay.io/foo/bar", []credentialprovider.LazyAuthConfiguration{rex}, true},
-	}
-
-	for i, tt := range tests {
-		match, ok := dk.Lookup(tt.image)
-		if tt.ok != ok {
-			t.Errorf("case %d: expected ok=%t, got %t", i, tt.ok, ok)
-		}
-
-		if !reflect.DeepEqual(tt.match, match) {
-			t.Errorf("case %d: expected match=%#v, got %#v", i, tt.match, match)
-		}
-	}
-}
-
-type imageTrackingDockerClient struct {
-	*FakeDockerClient
-	imageName string
-}
-
-func (f *imageTrackingDockerClient) InspectImageByID(name string) (image *dockertypes.ImageInspect, err error) {
-	image, err = f.FakeDockerClient.InspectImageByID(name)
-	f.imageName = name
-	return
-}
-
-func (f *imageTrackingDockerClient) InspectImageByRef(name string) (image *dockertypes.ImageInspect, err error) {
-	image, err = f.FakeDockerClient.InspectImageByRef(name)
-	f.imageName = name
-	return
-}
-
-func TestGetImageRef(t *testing.T) {
-	cl := &imageTrackingDockerClient{NewFakeDockerClient(), ""}
-	puller := &dockerPuller{
-		client: cl,
-	}
-	_, _ = puller.GetImageRef("abc:123")
-	if cl.imageName != "abc:123" {
-		t.Errorf("expected inspection of image abc:123, instead inspected image %v", cl.imageName)
-	}
-}
-
 const letterBytes = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ"
 
 func randStringBytes(n int) string {

test/e2e_node/image_id_test.go

@@ -19,7 +19,6 @@ package e2e_node
 import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/kubernetes/pkg/api/v1"
-	"k8s.io/kubernetes/pkg/kubelet/dockertools"
 	"k8s.io/kubernetes/test/e2e/framework"
 
 	"github.com/davecgh/go-spew/spew"
@@ -62,6 +61,6 @@ var _ = framework.KubeDescribe("ImageID", func() {
 			return
 		}
 
-		Expect(status.ContainerStatuses[0].ImageID).To(Equal(dockertools.DockerPullablePrefix + busyBoxImage))
+		Expect(status.ContainerStatuses[0].ImageID).To(ContainSubstring(busyBoxImage))
 	})
 })