Migrate unit tests for image pulling credentials and error handling

Also remove the dockertools package completely.

pkg/kubelet/api/testing/fake_image_service.go

@@ -18,6 +18,9 @@ package testing
 
 import (
 	"sync"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
 
 	runtimeapi "k8s.io/kubernetes/pkg/kubelet/api/v1alpha1/runtime"
 	"k8s.io/kubernetes/pkg/kubelet/util/sliceutils"
@@ -29,6 +32,8 @@ type FakeImageService struct {
 	FakeImageSize uint64
 	Called        []string
 	Images        map[string]*runtimeapi.Image
+
+	pulledImages []*pulledImage
 }
 
 func (r *FakeImageService) SetFakeImages(images []string) {
@@ -97,6 +102,7 @@ func (r *FakeImageService) PullImage(image *runtimeapi.ImageSpec, auth *runtimea
 
 	r.Called = append(r.Called, "PullImage")
 
+	r.pulledImages = append(r.pulledImages, &pulledImage{imageSpec: image, authConfig: auth})
 	// ImageID should be randomized for real container runtime, but here just use
 	// image's name for easily making fake images.
 	imageID := image.Image
@@ -128,3 +134,15 @@ func (r *FakeImageService) ImageFsInfo() (*runtimeapi.FsInfo, error) {
 
 	return nil, nil
 }
+
+func (r *FakeImageService) AssertImagePulledWithAuth(t *testing.T, image *runtimeapi.ImageSpec, auth *runtimeapi.AuthConfig, failMsg string) {
+	r.Lock()
+	defer r.Unlock()
+	expected := &pulledImage{imageSpec: image, authConfig: auth}
+	assert.Contains(t, r.pulledImages, expected, failMsg)
+}
+
+type pulledImage struct {
+	imageSpec  *runtimeapi.ImageSpec
+	authConfig *runtimeapi.AuthConfig
+}

pkg/kubelet/dockershim/docker_image.go

@@ -18,10 +18,12 @@ package dockershim
 
 import (
 	"fmt"
+	"net/http"
 
+	"github.com/docker/docker/pkg/jsonmessage"
 	dockertypes "github.com/docker/engine-api/types"
-	runtimeapi "k8s.io/kubernetes/pkg/kubelet/api/v1alpha1/runtime"
 
+	runtimeapi "k8s.io/kubernetes/pkg/kubelet/api/v1alpha1/runtime"
 	"k8s.io/kubernetes/pkg/kubelet/dockershim/libdocker"
 )
 
@@ -80,7 +82,7 @@ func (ds *dockerService) PullImage(image *runtimeapi.ImageSpec, auth *runtimeapi
 		dockertypes.ImagePullOptions{},
 	)
 	if err != nil {
-		return "", err
+		return "", filterHTTPError(err, image.Image)
 	}
 
 	return getImageRef(ds.client, image.Image)
@@ -127,3 +129,18 @@ func getImageRef(client libdocker.Interface, image string) (string, error) {
 func (ds *dockerService) ImageFsInfo() (*runtimeapi.FsInfo, error) {
 	return nil, fmt.Errorf("not implemented")
 }
+
+func filterHTTPError(err error, image string) error {
+	// docker/docker/pull/11314 prints detailed error info for docker pull.
+	// When it hits 502, it returns a verbose html output including an inline svg,
+	// which makes the output of kubectl get pods much harder to parse.
+	// Here converts such verbose output to a concise one.
+	jerr, ok := err.(*jsonmessage.JSONError)
+	if ok && (jerr.Code == http.StatusBadGateway ||
+		jerr.Code == http.StatusServiceUnavailable ||
+		jerr.Code == http.StatusGatewayTimeout) {
+		return fmt.Errorf("RegistryUnavailable: %v", err)
+	}
+	return err
+
+}

pkg/kubelet/dockershim/docker_image_test.go

@@ -17,12 +17,14 @@ limitations under the License.
 package dockershim
 
 import (
+	"fmt"
 	"testing"
 
+	"github.com/docker/docker/pkg/jsonmessage"
 	dockertypes "github.com/docker/engine-api/types"
+	"github.com/stretchr/testify/assert"
 
 	runtimeapi "k8s.io/kubernetes/pkg/kubelet/api/v1alpha1/runtime"
-
 	"k8s.io/kubernetes/pkg/kubelet/dockershim/libdocker"
 )
 
@@ -44,3 +46,29 @@ func TestRemoveImageWithMultipleTags(t *testing.T) {
 		libdocker.NewCalledDetail("remove_image", []interface{}{"foo", dockertypes.ImageRemoveOptions{PruneChildren: true}}),
 		libdocker.NewCalledDetail("remove_image", []interface{}{"bar", dockertypes.ImageRemoveOptions{PruneChildren: true}}))
 }
+
+func TestPullWithJSONError(t *testing.T) {
+	ds, fakeDocker, _ := newTestDockerService()
+	tests := map[string]struct {
+		image         *runtimeapi.ImageSpec
+		err           error
+		expectedError string
+	}{
+		"Json error": {
+			&runtimeapi.ImageSpec{Image: "ubuntu"},
+			&jsonmessage.JSONError{Code: 50, Message: "Json error"},
+			"Json error",
+		},
+		"Bad gateway": {
+			&runtimeapi.ImageSpec{Image: "ubuntu"},
+			&jsonmessage.JSONError{Code: 502, Message: "<!doctype html>\n<html class=\"no-js\" lang=\"\">\n    <head>\n  </head>\n    <body>\n   <h1>Oops, there was an error!</h1>\n        <p>We have been contacted of this error, feel free to check out <a href=\"http://status.docker.com/\">status.docker.com</a>\n           to see if there is a bigger issue.</p>\n\n    </body>\n</html>"},
+			"RegistryUnavailable",
+		},
+	}
+	for key, test := range tests {
+		fakeDocker.InjectError("pull", test.err)
+		_, err := ds.PullImage(test.image, &runtimeapi.AuthConfig{})
+		assert.Error(t, err, fmt.Sprintf("TestCase [%s]", key))
+		assert.Contains(t, err.Error(), test.expectedError)
+	}
+}

pkg/kubelet/dockertools/BUILD

@@ -1,54 +0,0 @@
-package(default_visibility = ["//visibility:public"])
-
-licenses(["notice"])
-
-load(
-    "@io_bazel_rules_go//go:def.bzl",
-    "go_library",
-    "go_test",
-)
-
-go_library(
-    name = "go_default_library",
-    srcs = ["docker.go"],
-    tags = ["automanaged"],
-    deps = [
-        "//pkg/api/v1:go_default_library",
-        "//pkg/credentialprovider:go_default_library",
-        "//pkg/kubelet/dockershim/libdocker:go_default_library",
-        "//pkg/kubelet/images:go_default_library",
-        "//vendor/github.com/docker/docker/pkg/jsonmessage:go_default_library",
-        "//vendor/github.com/docker/engine-api/types:go_default_library",
-        "//vendor/github.com/golang/glog:go_default_library",
-        "//vendor/k8s.io/apimachinery/pkg/util/errors:go_default_library",
-    ],
-)
-
-go_test(
-    name = "go_default_test",
-    srcs = ["docker_test.go"],
-    library = ":go_default_library",
-    tags = [
-        "automanaged",
-    ],
-    deps = [
-        "//pkg/api/v1:go_default_library",
-        "//pkg/credentialprovider:go_default_library",
-        "//pkg/kubelet/dockershim/libdocker:go_default_library",
-        "//pkg/kubelet/images:go_default_library",
-        "//vendor/github.com/docker/docker/pkg/jsonmessage:go_default_library",
-    ],
-)
-
-filegroup(
-    name = "package-srcs",
-    srcs = glob(["**"]),
-    tags = ["automanaged"],
-    visibility = ["//visibility:private"],
-)
-
-filegroup(
-    name = "all-srcs",
-    srcs = [":package-srcs"],
-    tags = ["automanaged"],
-)

pkg/kubelet/dockertools/OWNERS

@@ -1,3 +0,0 @@
-approvers:
-- Random-Liu
-- yujuhong

pkg/kubelet/dockertools/docker.go

@@ -1,141 +0,0 @@
-/*
-Copyright 2014 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
-
-import (
-	"fmt"
-	"net/http"
-	"strings"
-
-	"github.com/docker/docker/pkg/jsonmessage"
-	dockertypes "github.com/docker/engine-api/types"
-	"github.com/golang/glog"
-	utilerrors "k8s.io/apimachinery/pkg/util/errors"
-	"k8s.io/kubernetes/pkg/api/v1"
-	"k8s.io/kubernetes/pkg/credentialprovider"
-	"k8s.io/kubernetes/pkg/kubelet/dockershim/libdocker"
-	"k8s.io/kubernetes/pkg/kubelet/images"
-)
-
-// DockerPuller is an abstract interface for testability.  It abstracts image pull operations.
-// DockerPuller is *not* in use anywhere in the codebase.
-// TODO: Examine whether we can migrate the unit tests and remove the code.
-type DockerPuller interface {
-	Pull(image string, secrets []v1.Secret) error
-	GetImageRef(image string) (string, error)
-}
-
-// dockerPuller is the default implementation of DockerPuller.
-type dockerPuller struct {
-	client  libdocker.Interface
-	keyring credentialprovider.DockerKeyring
-}
-
-// newDockerPuller creates a new instance of the default implementation of DockerPuller.
-func newDockerPuller(client libdocker.Interface) DockerPuller {
-	return &dockerPuller{
-		client:  client,
-		keyring: credentialprovider.NewDockerKeyring(),
-	}
-}
-
-func filterHTTPError(err error, image string) error {
-	// docker/docker/pull/11314 prints detailed error info for docker pull.
-	// When it hits 502, it returns a verbose html output including an inline svg,
-	// which makes the output of kubectl get pods much harder to parse.
-	// Here converts such verbose output to a concise one.
-	jerr, ok := err.(*jsonmessage.JSONError)
-	if ok && (jerr.Code == http.StatusBadGateway ||
-		jerr.Code == http.StatusServiceUnavailable ||
-		jerr.Code == http.StatusGatewayTimeout) {
-		glog.V(2).Infof("Pulling image %q failed: %v", image, err)
-		return images.RegistryUnavailable
-	} else {
-		return err
-	}
-}
-
-func (p dockerPuller) Pull(image string, secrets []v1.Secret) error {
-	keyring, err := credentialprovider.MakeDockerKeyring(secrets, p.keyring)
-	if err != nil {
-		return err
-	}
-
-	// The only used image pull option RegistryAuth will be set in kube_docker_client
-	opts := dockertypes.ImagePullOptions{}
-
-	creds, haveCredentials := keyring.Lookup(image)
-	if !haveCredentials {
-		glog.V(1).Infof("Pulling image %s without credentials", image)
-
-		err := p.client.PullImage(image, dockertypes.AuthConfig{}, opts)
-		if err == nil {
-			// Sometimes PullImage failed with no error returned.
-			imageRef, ierr := p.GetImageRef(image)
-			if ierr != nil {
-				glog.Warningf("Failed to inspect image %s: %v", image, ierr)
-			}
-			if imageRef == "" {
-				return fmt.Errorf("image pull failed for unknown error")
-			}
-			return nil
-		}
-
-		// Image spec: [<registry>/]<repository>/<image>[:<version] so we count '/'
-		explicitRegistry := (strings.Count(image, "/") == 2)
-		// Hack, look for a private registry, and decorate the error with the lack of
-		// credentials.  This is heuristic, and really probably could be done better
-		// by talking to the registry API directly from the kubelet here.
-		if explicitRegistry {
-			return fmt.Errorf("image pull failed for %s, this may be because there are no credentials on this request.  details: (%v)", image, err)
-		}
-
-		return filterHTTPError(err, image)
-	}
-
-	var pullErrs []error
-	for _, currentCreds := range creds {
-		err = p.client.PullImage(image, credentialprovider.LazyProvide(currentCreds), opts)
-		// If there was no error, return success
-		if err == nil {
-			return nil
-		}
-
-		pullErrs = append(pullErrs, filterHTTPError(err, image))
-	}
-
-	return utilerrors.NewAggregate(pullErrs)
-}
-
-func (p dockerPuller) GetImageRef(image string) (string, error) {
-	resp, err := p.client.InspectImageByRef(image)
-	if err == nil {
-		if resp == nil {
-			return "", nil
-		}
-
-		imageRef := resp.ID
-		if len(resp.RepoDigests) > 0 {
-			imageRef = resp.RepoDigests[0]
-		}
-		return imageRef, nil
-	}
-	if libdocker.IsImageNotFoundError(err) {
-		return "", nil
-	}
-	return "", err
-}

pkg/kubelet/dockertools/docker_test.go

@@ -1,179 +0,0 @@
-/*
-Copyright 2014 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
-
-import (
-	"encoding/json"
-	"strings"
-	"testing"
-
-	"github.com/docker/docker/pkg/jsonmessage"
-	"k8s.io/kubernetes/pkg/api/v1"
-	"k8s.io/kubernetes/pkg/credentialprovider"
-	"k8s.io/kubernetes/pkg/kubelet/dockershim/libdocker"
-	"k8s.io/kubernetes/pkg/kubelet/images"
-)
-
-// TODO: Examine the tests and see if they can be migrated to kuberuntime.
-func TestPullWithNoSecrets(t *testing.T) {
-	tests := []struct {
-		imageName     string
-		expectedImage string
-	}{
-		{"ubuntu", "ubuntu using {}"},
-		{"ubuntu:2342", "ubuntu:2342 using {}"},
-		{"ubuntu:latest", "ubuntu:latest using {}"},
-		{"foo/bar:445566", "foo/bar:445566 using {}"},
-		{"registry.example.com:5000/foobar", "registry.example.com:5000/foobar using {}"},
-		{"registry.example.com:5000/foobar:5342", "registry.example.com:5000/foobar:5342 using {}"},
-		{"registry.example.com:5000/foobar:latest", "registry.example.com:5000/foobar:latest using {}"},
-	}
-	for _, test := range tests {
-		fakeKeyring := &credentialprovider.FakeKeyring{}
-		fakeClient := libdocker.NewFakeDockerClient()
-
-		dp := dockerPuller{
-			client:  fakeClient,
-			keyring: fakeKeyring,
-		}
-
-		err := dp.Pull(test.imageName, []v1.Secret{})
-		if err != nil {
-			t.Errorf("unexpected non-nil err: %s", err)
-			continue
-		}
-
-		if err := fakeClient.AssertImagesPulled([]string{test.imageName}); err != nil {
-			t.Errorf("images pulled do not match the expected: %v", err)
-		}
-	}
-}
-
-func TestPullWithJSONError(t *testing.T) {
-	tests := map[string]struct {
-		imageName     string
-		err           error
-		expectedError string
-	}{
-		"Json error": {
-			"ubuntu",
-			&jsonmessage.JSONError{Code: 50, Message: "Json error"},
-			"Json error",
-		},
-		"Bad gateway": {
-			"ubuntu",
-			&jsonmessage.JSONError{Code: 502, Message: "<!doctype html>\n<html class=\"no-js\" lang=\"\">\n    <head>\n  </head>\n    <body>\n   <h1>Oops, there was an error!</h1>\n        <p>We have been contacted of this error, feel free to check out <a href=\"http://status.docker.com/\">status.docker.com</a>\n           to see if there is a bigger issue.</p>\n\n    </body>\n</html>"},
-			images.RegistryUnavailable.Error(),
-		},
-	}
-	for i, test := range tests {
-		fakeKeyring := &credentialprovider.FakeKeyring{}
-		fakeClient := libdocker.NewFakeDockerClient()
-		fakeClient.InjectError("pull", test.err)
-
-		puller := &dockerPuller{
-			client:  fakeClient,
-			keyring: fakeKeyring,
-		}
-		err := puller.Pull(test.imageName, []v1.Secret{})
-		if err == nil || !strings.Contains(err.Error(), test.expectedError) {
-			t.Errorf("%s: expect error %s, got : %s", i, test.expectedError, err)
-			continue
-		}
-	}
-}
-
-func TestPullWithSecrets(t *testing.T) {
-	// auth value is equivalent to: "username":"passed-user","password":"passed-password"
-	dockerCfg := map[string]map[string]string{"index.docker.io/v1/": {"email": "passed-email", "auth": "cGFzc2VkLXVzZXI6cGFzc2VkLXBhc3N3b3Jk"}}
-	dockercfgContent, err := json.Marshal(dockerCfg)
-	if err != nil {
-		t.Errorf("unexpected error: %v", err)
-	}
-
-	dockerConfigJson := map[string]map[string]map[string]string{"auths": dockerCfg}
-	dockerConfigJsonContent, err := json.Marshal(dockerConfigJson)
-	if err != nil {
-		t.Errorf("unexpected error: %v", err)
-	}
-
-	tests := map[string]struct {
-		imageName           string
-		passedSecrets       []v1.Secret
-		builtInDockerConfig credentialprovider.DockerConfig
-		expectedPulls       []string
-	}{
-		"no matching secrets": {
-			"ubuntu",
-			[]v1.Secret{},
-			credentialprovider.DockerConfig(map[string]credentialprovider.DockerConfigEntry{}),
-			[]string{"ubuntu using {}"},
-		},
-		"default keyring secrets": {
-			"ubuntu",
-			[]v1.Secret{},
-			credentialprovider.DockerConfig(map[string]credentialprovider.DockerConfigEntry{
-				"index.docker.io/v1/": {Username: "built-in", Password: "password", Email: "email", Provider: nil},
-			}),
-			[]string{`ubuntu using {"username":"built-in","password":"password","email":"email"}`},
-		},
-		"default keyring secrets unused": {
-			"ubuntu",
-			[]v1.Secret{},
-			credentialprovider.DockerConfig(map[string]credentialprovider.DockerConfigEntry{
-				"extraneous": {Username: "built-in", Password: "password", Email: "email", Provider: nil},
-			}),
-			[]string{`ubuntu using {}`},
-		},
-		"builtin keyring secrets, but use passed": {
-			"ubuntu",
-			[]v1.Secret{{Type: v1.SecretTypeDockercfg, Data: map[string][]byte{v1.DockerConfigKey: dockercfgContent}}},
-			credentialprovider.DockerConfig(map[string]credentialprovider.DockerConfigEntry{
-				"index.docker.io/v1/": {Username: "built-in", Password: "password", Email: "email", Provider: nil},
-			}),
-			[]string{`ubuntu using {"username":"passed-user","password":"passed-password","email":"passed-email"}`},
-		},
-		"builtin keyring secrets, but use passed with new docker config": {
-			"ubuntu",
-			[]v1.Secret{{Type: v1.SecretTypeDockerConfigJson, Data: map[string][]byte{v1.DockerConfigJsonKey: dockerConfigJsonContent}}},
-			credentialprovider.DockerConfig(map[string]credentialprovider.DockerConfigEntry{
-				"index.docker.io/v1/": {Username: "built-in", Password: "password", Email: "email", Provider: nil},
-			}),
-			[]string{`ubuntu using {"username":"passed-user","password":"passed-password","email":"passed-email"}`},
-		},
-	}
-	for i, test := range tests {
-		builtInKeyRing := &credentialprovider.BasicDockerKeyring{}
-		builtInKeyRing.Add(test.builtInDockerConfig)
-
-		fakeClient := libdocker.NewFakeDockerClient()
-
-		dp := dockerPuller{
-			client:  fakeClient,
-			keyring: builtInKeyRing,
-		}
-
-		err := dp.Pull(test.imageName, test.passedSecrets)
-		if err != nil {
-			t.Errorf("%s: unexpected non-nil err: %s", i, err)
-			continue
-		}
-		if err := fakeClient.AssertImagesPulledMsgs(test.expectedPulls); err != nil {
-			t.Errorf("images pulled do not match the expected: %v", err)
-		}
-	}
-}

pkg/kubelet/kuberuntime/fake_kuberuntime_manager.go

@@ -56,7 +56,7 @@ func (f *fakePodGetter) GetPodByUID(uid types.UID) (*v1.Pod, bool) {
 	return pod, found
 }
 
-func NewFakeKubeRuntimeManager(runtimeService internalapi.RuntimeService, imageService internalapi.ImageManagerService, machineInfo *cadvisorapi.MachineInfo, osInterface kubecontainer.OSInterface, runtimeHelper kubecontainer.RuntimeHelper) (*kubeGenericRuntimeManager, error) {
+func NewFakeKubeRuntimeManager(runtimeService internalapi.RuntimeService, imageService internalapi.ImageManagerService, machineInfo *cadvisorapi.MachineInfo, osInterface kubecontainer.OSInterface, runtimeHelper kubecontainer.RuntimeHelper, keyring credentialprovider.DockerKeyring) (*kubeGenericRuntimeManager, error) {
 	recorder := &record.FakeRecorder{}
 	kubeRuntimeManager := &kubeGenericRuntimeManager{
 		recorder:            recorder,
@@ -68,7 +68,7 @@ func NewFakeKubeRuntimeManager(runtimeService internalapi.RuntimeService, imageS
 		runtimeHelper:       runtimeHelper,
 		runtimeService:      runtimeService,
 		imageService:        imageService,
-		keyring:             credentialprovider.NewDockerKeyring(),
+		keyring:             keyring,
 	}
 
 	typedVersion, err := runtimeService.Version(kubeRuntimeAPIVersion)

pkg/kubelet/kuberuntime/kuberuntime_image_test.go

@@ -17,10 +17,16 @@ limitations under the License.
 package kuberuntime
 
 import (
+	"encoding/json"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
 	"k8s.io/apimachinery/pkg/util/sets"
+	"k8s.io/kubernetes/pkg/api/v1"
+	"k8s.io/kubernetes/pkg/credentialprovider"
+	runtimeapi "k8s.io/kubernetes/pkg/kubelet/api/v1alpha1/runtime"
 	kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
 )
 
@@ -94,3 +100,74 @@ func TestImageStats(t *testing.T) {
 	expectedStats := &kubecontainer.ImageStats{TotalStorageBytes: imageSize * uint64(len(images))}
 	assert.Equal(t, expectedStats, actualStats)
 }
+
+func TestPullWithSecrets(t *testing.T) {
+	// auth value is equivalent to: "username":"passed-user","password":"passed-password"
+	dockerCfg := map[string]map[string]string{"index.docker.io/v1/": {"email": "passed-email", "auth": "cGFzc2VkLXVzZXI6cGFzc2VkLXBhc3N3b3Jk"}}
+	dockercfgContent, err := json.Marshal(dockerCfg)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+
+	dockerConfigJson := map[string]map[string]map[string]string{"auths": dockerCfg}
+	dockerConfigJsonContent, err := json.Marshal(dockerConfigJson)
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+
+	tests := map[string]struct {
+		imageName           string
+		passedSecrets       []v1.Secret
+		builtInDockerConfig credentialprovider.DockerConfig
+		expectedAuth        *runtimeapi.AuthConfig
+	}{
+		"no matching secrets": {
+			"ubuntu",
+			[]v1.Secret{},
+			credentialprovider.DockerConfig(map[string]credentialprovider.DockerConfigEntry{}),
+			nil,
+		},
+		"default keyring secrets": {
+			"ubuntu",
+			[]v1.Secret{},
+			credentialprovider.DockerConfig(map[string]credentialprovider.DockerConfigEntry{
+				"index.docker.io/v1/": {Username: "built-in", Password: "password", Provider: nil},
+			}),
+			&runtimeapi.AuthConfig{Username: "built-in", Password: "password"},
+		},
+		"default keyring secrets unused": {
+			"ubuntu",
+			[]v1.Secret{},
+			credentialprovider.DockerConfig(map[string]credentialprovider.DockerConfigEntry{
+				"extraneous": {Username: "built-in", Password: "password", Provider: nil},
+			}),
+			nil,
+		},
+		"builtin keyring secrets, but use passed": {
+			"ubuntu",
+			[]v1.Secret{{Type: v1.SecretTypeDockercfg, Data: map[string][]byte{v1.DockerConfigKey: dockercfgContent}}},
+			credentialprovider.DockerConfig(map[string]credentialprovider.DockerConfigEntry{
+				"index.docker.io/v1/": {Username: "built-in", Password: "password", Provider: nil},
+			}),
+			&runtimeapi.AuthConfig{Username: "passed-user", Password: "passed-password"},
+		},
+		"builtin keyring secrets, but use passed with new docker config": {
+			"ubuntu",
+			[]v1.Secret{{Type: v1.SecretTypeDockerConfigJson, Data: map[string][]byte{v1.DockerConfigJsonKey: dockerConfigJsonContent}}},
+			credentialprovider.DockerConfig(map[string]credentialprovider.DockerConfigEntry{
+				"index.docker.io/v1/": {Username: "built-in", Password: "password", Provider: nil},
+			}),
+			&runtimeapi.AuthConfig{Username: "passed-user", Password: "passed-password"},
+		},
+	}
+	for description, test := range tests {
+		builtInKeyRing := &credentialprovider.BasicDockerKeyring{}
+		builtInKeyRing.Add(test.builtInDockerConfig)
+		_, fakeImageService, fakeManager, err := customTestRuntimeManager(builtInKeyRing)
+		require.NoError(t, err)
+
+		_, err = fakeManager.PullImage(kubecontainer.ImageSpec{Image: test.imageName}, test.passedSecrets)
+		require.NoError(t, err)
+		fakeImageService.AssertImagePulledWithAuth(t, &runtimeapi.ImageSpec{Image: test.imageName}, test.expectedAuth, description)
+	}
+}

pkg/kubelet/kuberuntime/kuberuntime_manager_test.go

@@ -24,11 +24,13 @@ import (
 
 	cadvisorapi "github.com/google/cadvisor/info/v1"
 	"github.com/stretchr/testify/assert"
+
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	kubetypes "k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/util/flowcontrol"
 	"k8s.io/kubernetes/pkg/api/v1"
+	"k8s.io/kubernetes/pkg/credentialprovider"
 	apitest "k8s.io/kubernetes/pkg/kubelet/api/testing"
 	runtimeapi "k8s.io/kubernetes/pkg/kubelet/api/v1alpha1/runtime"
 	kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
@@ -40,6 +42,10 @@ var (
 )
 
 func createTestRuntimeManager() (*apitest.FakeRuntimeService, *apitest.FakeImageService, *kubeGenericRuntimeManager, error) {
+	return customTestRuntimeManager(&credentialprovider.BasicDockerKeyring{})
+}
+
+func customTestRuntimeManager(keyring *credentialprovider.BasicDockerKeyring) (*apitest.FakeRuntimeService, *apitest.FakeImageService, *kubeGenericRuntimeManager, error) {
 	fakeRuntimeService := apitest.NewFakeRuntimeService()
 	fakeImageService := apitest.NewFakeImageService()
 	// Only an empty machineInfo is needed here, because in unit test all containers are besteffort,
@@ -47,7 +53,7 @@ func createTestRuntimeManager() (*apitest.FakeRuntimeService, *apitest.FakeImage
 	// we may want to set memory capacity.
 	machineInfo := &cadvisorapi.MachineInfo{}
 	osInterface := &containertest.FakeOS{}
-	manager, err := NewFakeKubeRuntimeManager(fakeRuntimeService, fakeImageService, machineInfo, osInterface, &containertest.FakeRuntimeHelper{})
+	manager, err := NewFakeKubeRuntimeManager(fakeRuntimeService, fakeImageService, machineInfo, osInterface, &containertest.FakeRuntimeHelper{}, keyring)
 	return fakeRuntimeService, fakeImageService, manager, err
 }