Merge pull request #54405 from resouer/clean-docker-dep

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

[Part 1] Remove docker dep in kubelet startup

**What this PR does / why we need it**:

Remove dependency of docker during kubelet start up.

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: 

Part 1 of #54090 

**Special notes for your reviewer**:
Changes include:

1. Move docker client initialization into dockershim pkg.
2. Pass a docker `ClientConfig` from kubelet to dockershim
3. Pass parameters needed by `FakeDockerClient` thru `ClientConfig` to dockershim

(TODO, the second part) Make dockershim tolerate when dockerd is down, otherwise it will still fail kubelet

Please note after this PR, kubelet will still fail if dockerd is down, this will be fixed in the subsequent PR by making dockershim tolerate dockerd failure (initializing docker client in a separate goroutine), and refactoring cgroup and log driver detection. 

**Release note**:

```release-note
Remove docker dependency during kubelet start up 
```

cmd/kubelet/app/BUILD

@@ -52,7 +52,6 @@ go_library(
         "//pkg/kubelet/config:go_default_library",
         "//pkg/kubelet/container:go_default_library",
         "//pkg/kubelet/dockershim:go_default_library",
-        "//pkg/kubelet/dockershim/libdocker:go_default_library",
         "//pkg/kubelet/dockershim/remote:go_default_library",
         "//pkg/kubelet/eviction:go_default_library",
         "//pkg/kubelet/eviction/api:go_default_library",

cmd/kubelet/app/server.go

@@ -72,7 +72,6 @@ import (
 	"k8s.io/kubernetes/pkg/kubelet/config"
 	kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
 	"k8s.io/kubernetes/pkg/kubelet/dockershim"
-	"k8s.io/kubernetes/pkg/kubelet/dockershim/libdocker"
 	dockerremote "k8s.io/kubernetes/pkg/kubelet/dockershim/remote"
 	"k8s.io/kubernetes/pkg/kubelet/eviction"
 	evictionapi "k8s.io/kubernetes/pkg/kubelet/eviction/api"
@@ -145,12 +144,13 @@ func UnsecuredDependencies(s *options.KubeletServer) (*kubelet.Dependencies, err
 		writer = &kubeio.NsenterWriter{}
 	}
 
-	var dockerClient libdocker.Interface
+	var dockerClientConfig *dockershim.ClientConfig
 	if s.ContainerRuntime == kubetypes.DockerContainerRuntime {
-		dockerClient = libdocker.ConnectToDockerOrDie(s.DockerEndpoint, s.RuntimeRequestTimeout.Duration,
-			s.ImagePullProgressDeadline.Duration)
-	} else {
-		dockerClient = nil
+		dockerClientConfig = &dockershim.ClientConfig{
+			DockerEndpoint:            s.DockerEndpoint,
+			RuntimeRequestTimeout:     s.RuntimeRequestTimeout.Duration,
+			ImagePullProgressDeadline: s.ImagePullProgressDeadline.Duration,
+		}
 	}
 
 	return &kubelet.Dependencies{
@@ -158,7 +158,7 @@ func UnsecuredDependencies(s *options.KubeletServer) (*kubelet.Dependencies, err
 		CAdvisorInterface:   nil, // cadvisor.New launches background processes (bg http.ListenAndServe, and some bg cleaners), not set here
 		Cloud:               nil, // cloud provider might start background processes
 		ContainerManager:    nil,
-		DockerClient:        dockerClient,
+		DockerClientConfig:  dockerClientConfig,
 		KubeClient:          nil,
 		HeartbeatClient:     nil,
 		ExternalKubeClient:  nil,
@@ -898,9 +898,13 @@ func BootstrapKubeletConfigController(defaultConfig *kubeletconfiginternal.Kubel
 // TODO(random-liu): Move this to a separate binary.
 func RunDockershim(f *options.KubeletFlags, c *kubeletconfiginternal.KubeletConfiguration) error {
 	r := &f.ContainerRuntimeOptions
-	// Create docker client.
-	dockerClient := libdocker.ConnectToDockerOrDie(r.DockerEndpoint, c.RuntimeRequestTimeout.Duration,
-		r.ImagePullProgressDeadline.Duration)
+
+	// Initialize docker client configuration.
+	dockerClientConfig := &dockershim.ClientConfig{
+		DockerEndpoint:            r.DockerEndpoint,
+		RuntimeRequestTimeout:     c.RuntimeRequestTimeout.Duration,
+		ImagePullProgressDeadline: r.ImagePullProgressDeadline.Duration,
+	}
 
 	// Initialize network plugin settings.
 	nh := &kubelet.NoOpLegacyHost{}
@@ -924,7 +928,7 @@ func RunDockershim(f *options.KubeletFlags, c *kubeletconfiginternal.KubeletConf
 		SupportedPortForwardProtocols:   streaming.DefaultConfig.SupportedPortForwardProtocols,
 	}
 
-	ds, err := dockershim.NewDockerService(dockerClient, r.PodSandboxImage, streamingConfig, &pluginSettings,
+	ds, err := dockershim.NewDockerService(dockerClientConfig, r.PodSandboxImage, streamingConfig, &pluginSettings,
 		f.RuntimeCgroups, c.CgroupDriver, r.DockershimRootDirectory, r.DockerDisableSharedPID)
 	if err != nil {
 		return err

cmd/kubemark/BUILD

@@ -22,6 +22,7 @@ go_library(
         "//pkg/client/metrics/prometheus:go_default_library",
         "//pkg/kubelet/cadvisor/testing:go_default_library",
         "//pkg/kubelet/cm:go_default_library",
+        "//pkg/kubelet/dockershim:go_default_library",
         "//pkg/kubelet/dockershim/libdocker:go_default_library",
         "//pkg/kubemark:go_default_library",
         "//pkg/util/iptables/testing:go_default_library",

cmd/kubemark/hollow-node.go

@@ -32,6 +32,7 @@ import (
 	_ "k8s.io/kubernetes/pkg/client/metrics/prometheus" // for client metric registration
 	cadvisortest "k8s.io/kubernetes/pkg/kubelet/cadvisor/testing"
 	"k8s.io/kubernetes/pkg/kubelet/cm"
+	"k8s.io/kubernetes/pkg/kubelet/dockershim"
 	"k8s.io/kubernetes/pkg/kubelet/dockershim/libdocker"
 	"k8s.io/kubernetes/pkg/kubemark"
 	fakeiptables "k8s.io/kubernetes/pkg/util/iptables/testing"
@@ -116,14 +117,18 @@ func main() {
 			NodeName: config.NodeName,
 		}
 		containerManager := cm.NewStubContainerManager()
-		fakeDockerClient := libdocker.NewFakeDockerClient().WithTraceDisabled()
-		fakeDockerClient.EnableSleep = true
+
+		fakeDockerClientConfig := &dockershim.ClientConfig{
+			DockerEndpoint:    libdocker.FakeDockerEndpoint,
+			EnableSleep:       true,
+			WithTraceDisabled: true,
+		}
 
 		hollowKubelet := kubemark.NewHollowKubelet(
 			config.NodeName,
 			clientset,
 			cadvisorInterface,
-			fakeDockerClient,
+			fakeDockerClientConfig,
 			config.KubeletPort,
 			config.KubeletReadOnlyPort,
 			containerManager,

pkg/kubelet/BUILD

@@ -53,7 +53,6 @@ go_library(
         "//pkg/kubelet/configmap:go_default_library",
         "//pkg/kubelet/container:go_default_library",
         "//pkg/kubelet/dockershim:go_default_library",
-        "//pkg/kubelet/dockershim/libdocker:go_default_library",
         "//pkg/kubelet/dockershim/remote:go_default_library",
         "//pkg/kubelet/envvars:go_default_library",
         "//pkg/kubelet/events:go_default_library",

pkg/kubelet/dockershim/docker_container.go

@@ -197,7 +197,7 @@ func (ds *dockerService) createContainerLogSymlink(containerID string) error {
 				path, realPath, containerID, err)
 		}
 	} else {
-		supported, err := IsCRISupportedLogDriver(ds.client)
+		supported, err := ds.IsCRISupportedLogDriver()
 		if err != nil {
 			glog.Warningf("Failed to check supported logging driver by CRI: %v", err)
 			return nil

pkg/kubelet/dockershim/docker_service.go

@@ -148,9 +148,41 @@ type dockerNetworkHost struct {
 
 var internalLabelKeys []string = []string{containerTypeLabelKey, containerLogPathLabelKey, sandboxIDLabelKey}
 
+// ClientConfig is parameters used to initialize docker client
+type ClientConfig struct {
+	DockerEndpoint            string
+	RuntimeRequestTimeout     time.Duration
+	ImagePullProgressDeadline time.Duration
+
+	// Configuration for fake docker client
+	EnableSleep       bool
+	WithTraceDisabled bool
+}
+
+// NewDockerClientFromConfig create a docker client from given configure
+// return nil if nil configure is given.
+func NewDockerClientFromConfig(config *ClientConfig) libdocker.Interface {
+	if config != nil {
+		// Create docker client.
+		client := libdocker.ConnectToDockerOrDie(
+			config.DockerEndpoint,
+			config.RuntimeRequestTimeout,
+			config.ImagePullProgressDeadline,
+			config.WithTraceDisabled,
+			config.EnableSleep,
+		)
+		return client
+	}
+
+	return nil
+}
+
 // NOTE: Anything passed to DockerService should be eventually handled in another way when we switch to running the shim as a different process.
-func NewDockerService(client libdocker.Interface, podSandboxImage string, streamingConfig *streaming.Config,
+func NewDockerService(config *ClientConfig, podSandboxImage string, streamingConfig *streaming.Config,
 	pluginSettings *NetworkPluginSettings, cgroupsName string, kubeCgroupDriver string, dockershimRootDir string, disableSharedPID bool) (DockerService, error) {
+
+	client := NewDockerClientFromConfig(config)
+
 	c := libdocker.NewInstrumentedInterface(client)
 	checkpointHandler, err := NewPersistentCheckpointHandler(dockershimRootDir)
 	if err != nil {
@@ -238,6 +270,15 @@ type DockerService interface {
 	Start() error
 	// For serving streaming calls.
 	http.Handler
+
+	// IsCRISupportedLogDriver checks whether the logging driver used by docker is
+	// suppoted by native CRI integration.
+	// TODO(resouer): remove this when deprecating unsupported log driver
+	IsCRISupportedLogDriver() (bool, error)
+
+	// NewDockerLegacyService created docker legacy service when log driver is not supported.
+	// TODO(resouer): remove this when deprecating unsupported log driver
+	NewDockerLegacyService() DockerLegacyService
 }
 
 type dockerService struct {
@@ -480,8 +521,10 @@ type dockerLegacyService struct {
 	client libdocker.Interface
 }
 
-func NewDockerLegacyService(client libdocker.Interface) DockerLegacyService {
-	return &dockerLegacyService{client: client}
+// NewDockerLegacyService created docker legacy service when log driver is not supported.
+// TODO(resouer): remove this when deprecating unsupported log driver
+func (d *dockerService) NewDockerLegacyService() DockerLegacyService {
+	return &dockerLegacyService{client: d.client}
 }
 
 // GetContainerLogs get container logs directly from docker daemon.
@@ -553,8 +596,8 @@ var criSupportedLogDrivers = []string{"json-file"}
 
 // IsCRISupportedLogDriver checks whether the logging driver used by docker is
 // suppoted by native CRI integration.
-func IsCRISupportedLogDriver(client libdocker.Interface) (bool, error) {
-	info, err := client.Info()
+func (d *dockerService) IsCRISupportedLogDriver() (bool, error) {
+	info, err := d.client.Info()
 	if err != nil {
 		return false, fmt.Errorf("failed to get docker info: %v", err)
 	}

pkg/kubelet/dockershim/libdocker/client.go

@@ -40,6 +40,9 @@ const (
 	// This is only used by GetKubeletDockerContainers(), and should be removed
 	// along with the function.
 	containerNamePrefix = "k8s"
+
+	// Fake docker endpoint
+	FakeDockerEndpoint = "fake://"
 )
 
 // Interface is an abstract interface for testability.  It abstracts the interface of docker client.
@@ -86,9 +89,18 @@ func getDockerClient(dockerEndpoint string) (*dockerapi.Client, error) {
 // is the timeout for docker requests. If timeout is exceeded, the request
 // will be cancelled and throw out an error. If requestTimeout is 0, a default
 // value will be applied.
-func ConnectToDockerOrDie(dockerEndpoint string, requestTimeout, imagePullProgressDeadline time.Duration) Interface {
-	if dockerEndpoint == "fake://" {
-		return NewFakeDockerClient()
+func ConnectToDockerOrDie(dockerEndpoint string, requestTimeout, imagePullProgressDeadline time.Duration,
+	withTraceDisabled bool, enableSleep bool) Interface {
+	if dockerEndpoint == FakeDockerEndpoint {
+		fakeClient := NewFakeDockerClient()
+		if withTraceDisabled {
+			fakeClient = fakeClient.WithTraceDisabled()
+		}
+
+		if enableSleep {
+			fakeClient.EnableSleep = true
+		}
+		return fakeClient
 	}
 	client, err := getDockerClient(dockerEndpoint)
 	if err != nil {

pkg/kubelet/gpu/nvidia/BUILD

@@ -14,6 +14,7 @@ go_library(
     ],
     importpath = "k8s.io/kubernetes/pkg/kubelet/gpu/nvidia",
     deps = [
+        "//pkg/kubelet/dockershim:go_default_library",
         "//pkg/kubelet/dockershim/libdocker:go_default_library",
         "//pkg/kubelet/gpu:go_default_library",
         "//vendor/github.com/golang/glog:go_default_library",
@@ -42,6 +43,7 @@ go_test(
     importpath = "k8s.io/kubernetes/pkg/kubelet/gpu/nvidia",
     library = ":go_default_library",
     deps = [
+        "//pkg/kubelet/dockershim:go_default_library",
         "//pkg/kubelet/dockershim/libdocker:go_default_library",
         "//vendor/github.com/stretchr/testify/assert:go_default_library",
         "//vendor/k8s.io/api/core/v1:go_default_library",

pkg/kubelet/gpu/nvidia/nvidia_gpu_manager.go

@@ -30,6 +30,7 @@ import (
 	"k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	"k8s.io/apimachinery/pkg/util/sets"
+	"k8s.io/kubernetes/pkg/kubelet/dockershim"
 	"k8s.io/kubernetes/pkg/kubelet/dockershim/libdocker"
 	"k8s.io/kubernetes/pkg/kubelet/gpu"
 )
@@ -67,10 +68,12 @@ type nvidiaGPUManager struct {
 
 // NewNvidiaGPUManager returns a GPUManager that manages local Nvidia GPUs.
 // TODO: Migrate to use pod level cgroups and make it generic to all runtimes.
-func NewNvidiaGPUManager(activePodsLister activePodsLister, dockerClient libdocker.Interface) (gpu.GPUManager, error) {
+func NewNvidiaGPUManager(activePodsLister activePodsLister, config *dockershim.ClientConfig) (gpu.GPUManager, error) {
+	dockerClient := dockershim.NewDockerClientFromConfig(config)
 	if dockerClient == nil {
-		return nil, fmt.Errorf("invalid docker client specified")
+		return nil, fmt.Errorf("invalid docker client configure specified")
 	}
+
 	return &nvidiaGPUManager{
 		allGPUs:          sets.NewString(),
 		dockerClient:     dockerClient,

pkg/kubelet/gpu/nvidia/nvidia_gpu_manager_test.go

@@ -28,6 +28,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/uuid"
+	"k8s.io/kubernetes/pkg/kubelet/dockershim"
 	"k8s.io/kubernetes/pkg/kubelet/dockershim/libdocker"
 )
 
@@ -73,8 +74,9 @@ func TestNewNvidiaGPUManager(t *testing.T) {
 	as.NotNil(err)
 
 	// Expects a GPUManager to be created with non-nil dockerClient.
-	fakeDocker := libdocker.NewFakeDockerClient()
-	testGpuManager2, err := NewNvidiaGPUManager(podLister, fakeDocker)
+	testGpuManager2, err := NewNvidiaGPUManager(podLister, &dockershim.ClientConfig{
+		DockerEndpoint: libdocker.FakeDockerEndpoint,
+	})
 	as.NotNil(testGpuManager2)
 	as.Nil(err)
 

pkg/kubelet/kubelet.go

@@ -68,7 +68,6 @@ import (
 	"k8s.io/kubernetes/pkg/kubelet/configmap"
 	kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
 	"k8s.io/kubernetes/pkg/kubelet/dockershim"
-	"k8s.io/kubernetes/pkg/kubelet/dockershim/libdocker"
 	dockerremote "k8s.io/kubernetes/pkg/kubelet/dockershim/remote"
 	"k8s.io/kubernetes/pkg/kubelet/events"
 	"k8s.io/kubernetes/pkg/kubelet/eviction"
@@ -250,7 +249,7 @@ type Dependencies struct {
 	CAdvisorInterface       cadvisor.Interface
 	Cloud                   cloudprovider.Interface
 	ContainerManager        cm.ContainerManager
-	DockerClient            libdocker.Interface
+	DockerClientConfig      *dockershim.ClientConfig
 	EventClient             v1core.EventsGetter
 	HeartbeatClient         v1core.CoreV1Interface
 	KubeClient              clientset.Interface
@@ -608,7 +607,7 @@ func NewMainKubelet(kubeCfg *kubeletconfiginternal.KubeletConfiguration,
 		case kubetypes.DockerContainerRuntime:
 			// Create and start the CRI shim running as a grpc server.
 			streamingConfig := getStreamingConfig(kubeCfg, kubeDeps)
-			ds, err := dockershim.NewDockerService(kubeDeps.DockerClient, crOptions.PodSandboxImage, streamingConfig,
+			ds, err := dockershim.NewDockerService(kubeDeps.DockerClientConfig, crOptions.PodSandboxImage, streamingConfig,
 				&pluginSettings, runtimeCgroups, kubeCfg.CgroupDriver, crOptions.DockershimRootDirectory,
 				crOptions.DockerDisableSharedPID)
 			if err != nil {
@@ -632,12 +631,12 @@ func NewMainKubelet(kubeCfg *kubeletconfiginternal.KubeletConfiguration,
 			}
 
 			// Create dockerLegacyService when the logging driver is not supported.
-			supported, err := dockershim.IsCRISupportedLogDriver(kubeDeps.DockerClient)
+			supported, err := ds.IsCRISupportedLogDriver()
 			if err != nil {
 				return nil, err
 			}
 			if !supported {
-				klet.dockerLegacyService = dockershim.NewDockerLegacyService(kubeDeps.DockerClient)
+				klet.dockerLegacyService = ds.NewDockerLegacyService()
 				legacyLogProvider = dockershim.NewLegacyLogProvider(klet.dockerLegacyService)
 			}
 		case kubetypes.RemoteContainerRuntime:
@@ -886,7 +885,7 @@ func NewMainKubelet(kubeCfg *kubeletconfiginternal.KubeletConfiguration,
 	klet.softAdmitHandlers.AddPodAdmitHandler(lifecycle.NewNoNewPrivsAdmitHandler(klet.containerRuntime))
 	if utilfeature.DefaultFeatureGate.Enabled(features.Accelerators) {
 		if containerRuntime == kubetypes.DockerContainerRuntime {
-			if klet.gpuManager, err = nvidia.NewNvidiaGPUManager(klet, kubeDeps.DockerClient); err != nil {
+			if klet.gpuManager, err = nvidia.NewNvidiaGPUManager(klet, kubeDeps.DockerClientConfig); err != nil {
 				return nil, err
 			}
 		} else {

pkg/kubemark/BUILD

@@ -25,7 +25,7 @@ go_library(
         "//pkg/kubelet/cadvisor:go_default_library",
         "//pkg/kubelet/cm:go_default_library",
         "//pkg/kubelet/container/testing:go_default_library",
-        "//pkg/kubelet/dockershim/libdocker:go_default_library",
+        "//pkg/kubelet/dockershim:go_default_library",
         "//pkg/kubelet/types:go_default_library",
         "//pkg/proxy:go_default_library",
         "//pkg/proxy/config:go_default_library",

pkg/kubemark/hollow_kubelet.go

@@ -28,7 +28,7 @@ import (
 	"k8s.io/kubernetes/pkg/kubelet/cadvisor"
 	"k8s.io/kubernetes/pkg/kubelet/cm"
 	containertest "k8s.io/kubernetes/pkg/kubelet/container/testing"
-	"k8s.io/kubernetes/pkg/kubelet/dockershim/libdocker"
+	"k8s.io/kubernetes/pkg/kubelet/dockershim"
 	kubetypes "k8s.io/kubernetes/pkg/kubelet/types"
 	kubeio "k8s.io/kubernetes/pkg/util/io"
 	"k8s.io/kubernetes/pkg/util/mount"
@@ -50,7 +50,7 @@ func NewHollowKubelet(
 	nodeName string,
 	client *clientset.Clientset,
 	cadvisorInterface cadvisor.Interface,
-	dockerClient libdocker.Interface,
+	dockerClientConfig *dockershim.ClientConfig,
 	kubeletPort, kubeletReadOnlyPort int,
 	containerManager cm.ContainerManager,
 	maxPods int, podsPerCore int,
@@ -66,18 +66,18 @@ func NewHollowKubelet(
 	volumePlugins := empty_dir.ProbeVolumePlugins()
 	volumePlugins = append(volumePlugins, secret.ProbeVolumePlugins()...)
 	d := &kubelet.Dependencies{
-		KubeClient:        client,
-		HeartbeatClient:   client.CoreV1(),
-		DockerClient:      dockerClient,
-		CAdvisorInterface: cadvisorInterface,
-		Cloud:             nil,
-		OSInterface:       &containertest.FakeOS{},
-		ContainerManager:  containerManager,
-		VolumePlugins:     volumePlugins,
-		TLSOptions:        nil,
-		OOMAdjuster:       oom.NewFakeOOMAdjuster(),
-		Writer:            &kubeio.StdWriter{},
-		Mounter:           mount.New("" /* default mount path */),
+		KubeClient:         client,
+		HeartbeatClient:    client.CoreV1(),
+		DockerClientConfig: dockerClientConfig,
+		CAdvisorInterface:  cadvisorInterface,
+		Cloud:              nil,
+		OSInterface:        &containertest.FakeOS{},
+		ContainerManager:   containerManager,
+		VolumePlugins:      volumePlugins,
+		TLSOptions:         nil,
+		OOMAdjuster:        oom.NewFakeOOMAdjuster(),
+		Writer:             &kubeio.StdWriter{},
+		Mounter:            mount.New("" /* default mount path */),
 	}
 
 	return &HollowKubelet{

test/e2e_node/garbage_collector_test.go

@@ -249,7 +249,13 @@ func containerGCTest(f *framework.Framework, test testRun) {
 func dockerContainerGCTest(f *framework.Framework, test testRun) {
 	var runtime libdocker.Interface
 	BeforeEach(func() {
-		runtime = libdocker.ConnectToDockerOrDie(defaultDockerEndpoint, defaultRuntimeRequestTimeoutDuration, defaultImagePullProgressDeadline)
+		runtime = libdocker.ConnectToDockerOrDie(
+			defaultDockerEndpoint,
+			defaultRuntimeRequestTimeoutDuration,
+			defaultImagePullProgressDeadline,
+			false,
+			false,
+		)
 	})
 	for _, pod := range test.testPods {
 		// Initialize the getContainerNames function to use the libdocker api