Merge pull request #42139 from Random-Liu/unify-fake-runtime-helper

Automatic merge from submit-queue (batch tested with PRs 41921, 41695, 42139, 42090, 41949) Unify fake runtime helper in kuberuntime, rkt and dockertools. Addresses https://github.com/kubernetes/kubernetes/pull/42081#issuecomment-282429775. Add `pkg/kubelet/container/testing/fake_runtime_helper.go`, and change `kuberuntime`, `rkt` and `dockertools` to use it. @yujuhong This is a small unit test refactoring PR. Could you help me review it?
2025-10-31 02:08:13 +00:00 · 2017-03-01 04:10:04 -08:00
parent c6d11c778f 0deec63d1a
commit f68c824f95
11 changed files with 81 additions and 132 deletions
--- a/pkg/kubelet/container/testing/BUILD
+++ b/pkg/kubelet/container/testing/BUILD
@@ -12,6 +12,7 @@ go_library(
    srcs = [
        "fake_cache.go",
        "fake_runtime.go",
+        "fake_runtime_helper.go",
        "mockfileinfo.go",
        "os.go",
        "runtime_mock.go",
@@ -19,6 +20,7 @@ go_library(
    tags = ["automanaged"],
    deps = [
        "//pkg/api/v1:go_default_library",
+        "//pkg/kubelet/cm:go_default_library",
        "//pkg/kubelet/container:go_default_library",
        "//pkg/util/term:go_default_library",
        "//pkg/volume:go_default_library",
--- a/pkg/kubelet/container/testing/fake_runtime_helper.go
+++ b/pkg/kubelet/container/testing/fake_runtime_helper.go
@@ -0,0 +1,63 @@
+/*
+Copyright 2017 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 testing
+
+import (
+	kubetypes "k8s.io/apimachinery/pkg/types"
+	"k8s.io/kubernetes/pkg/api/v1"
+	"k8s.io/kubernetes/pkg/kubelet/cm"
+	kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
+)
+
+// FakeRuntimeHelper implements RuntimeHelper interfaces for testing purposes.
+type FakeRuntimeHelper struct {
+	DNSServers      []string
+	DNSSearches     []string
+	HostName        string
+	HostDomain      string
+	PodContainerDir string
+	Err             error
+}
+
+func (f *FakeRuntimeHelper) GenerateRunContainerOptions(pod *v1.Pod, container *v1.Container, podIP string) (*kubecontainer.RunContainerOptions, error) {
+	var opts kubecontainer.RunContainerOptions
+	if len(container.TerminationMessagePath) != 0 {
+		opts.PodContainerDir = f.PodContainerDir
+	}
+	return &opts, nil
+}
+
+func (f *FakeRuntimeHelper) GetPodCgroupParent(pod *v1.Pod) (cm.CgroupName, string) {
+	return "", ""
+}
+
+func (f *FakeRuntimeHelper) GetClusterDNS(pod *v1.Pod) ([]string, []string, error) {
+	return f.DNSServers, f.DNSSearches, f.Err
+}
+
+// This is not used by docker runtime.
+func (f *FakeRuntimeHelper) GeneratePodHostNameAndDomain(pod *v1.Pod) (string, string, error) {
+	return f.HostName, f.HostDomain, f.Err
+}
+
+func (f *FakeRuntimeHelper) GetPodDir(podUID kubetypes.UID) string {
+	return "/poddir/" + string(podUID)
+}
+
+func (f *FakeRuntimeHelper) GetExtraSupplementalGroupsForPod(pod *v1.Pod) []int64 {
+	return nil
+}
--- a/pkg/kubelet/dockertools/BUILD
+++ b/pkg/kubelet/dockertools/BUILD
@@ -107,7 +107,6 @@ go_test(
        "//pkg/api/v1:go_default_library",
        "//pkg/apis/componentconfig:go_default_library",
        "//pkg/credentialprovider:go_default_library",
-        "//pkg/kubelet/cm:go_default_library",
        "//pkg/kubelet/container:go_default_library",
        "//pkg/kubelet/container/testing:go_default_library",
        "//pkg/kubelet/events:go_default_library",
--- a/pkg/kubelet/dockertools/docker_manager.go
+++ b/pkg/kubelet/dockertools/docker_manager.go
@@ -696,7 +696,7 @@ func (dm *DockerManager) runContainer(
 		// here we just add a unique container id to make the path unique for different instances
 		// of the same container.
 		containerLogPath := path.Join(opts.PodContainerDir, cid)
-		fs, err := os.Create(containerLogPath)
+		fs, err := dm.os.Create(containerLogPath)
 		if err != nil {
 			// TODO: Clean up the previously created dir? return the error?
 			utilruntime.HandleError(fmt.Errorf("error creating termination-log file %q: %v", containerLogPath, err))
@@ -707,7 +707,7 @@ func (dm *DockerManager) runContainer(
 			// open(2) to create the file, so the final mode used is "mode &
 			// ~umask". But we want to make sure the specified mode is used
 			// in the file no matter what the umask is.
-			if err := os.Chmod(containerLogPath, 0666); err != nil {
+			if err := dm.os.Chmod(containerLogPath, 0666); err != nil {
 				utilruntime.HandleError(fmt.Errorf("unable to set termination-log file permissions %q: %v", containerLogPath, err))
 			}

--- a/pkg/kubelet/dockertools/docker_manager_test.go
+++ b/pkg/kubelet/dockertools/docker_manager_test.go
@@ -50,7 +50,6 @@ import (
 	"k8s.io/kubernetes/pkg/api/testapi"
 	"k8s.io/kubernetes/pkg/api/v1"
 	"k8s.io/kubernetes/pkg/apis/componentconfig"
-	"k8s.io/kubernetes/pkg/kubelet/cm"
 	kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
 	containertest "k8s.io/kubernetes/pkg/kubelet/container/testing"
 	"k8s.io/kubernetes/pkg/kubelet/images"
@@ -86,49 +85,6 @@ func (f *fakeHTTP) Get(url string) (*http.Response, error) {
 	return nil, f.err
 }

-// fakeRuntimeHelper implementes kubecontainer.RuntimeHelper inter
-// faces for testing purposes.
-// TODO(random-liu): Move this into pkg/kubelet/container/testing
-type fakeRuntimeHelper struct{}
-
-var _ kubecontainer.RuntimeHelper = &fakeRuntimeHelper{}
-
-var testPodContainerDir string
-
-func (f *fakeRuntimeHelper) GenerateRunContainerOptions(pod *v1.Pod, container *v1.Container, podIP string) (*kubecontainer.RunContainerOptions, error) {
-	var opts kubecontainer.RunContainerOptions
-	var err error
-	if len(container.TerminationMessagePath) != 0 {
-		testPodContainerDir, err = ioutil.TempDir(testTempDir, "fooPodContainerDir")
-		if err != nil {
-			return nil, err
-		}
-		opts.PodContainerDir = testPodContainerDir
-	}
-	return &opts, nil
-}
-
-func (f *fakeRuntimeHelper) GetPodCgroupParent(pod *v1.Pod) (cm.CgroupName, string) {
-	return "", ""
-}
-
-func (f *fakeRuntimeHelper) GetClusterDNS(pod *v1.Pod) ([]string, []string, error) {
-	return nil, nil, fmt.Errorf("not implemented")
-}
-
-// This is not used by docker runtime.
-func (f *fakeRuntimeHelper) GeneratePodHostNameAndDomain(pod *v1.Pod) (string, string, error) {
-	return "", "", nil
-}
-
-func (f *fakeRuntimeHelper) GetPodDir(kubetypes.UID) string {
-	return ""
-}
-
-func (f *fakeRuntimeHelper) GetExtraSupplementalGroupsForPod(pod *v1.Pod) []int64 {
-	return nil
-}
-
 type fakeImageManager struct{}

 func newFakeImageManager() images.ImageManager {
@@ -166,7 +122,7 @@ func createTestDockerManager(fakeHTTPClient *fakeHTTP, fakeDocker *FakeDockerCli
 		0, 0, "",
 		&containertest.FakeOS{},
 		networkPlugin,
-		&fakeRuntimeHelper{},
+		&containertest.FakeRuntimeHelper{},
 		fakeHTTPClient,
 		flowcontrol.NewBackOff(time.Second, 300*time.Second))

@@ -1275,6 +1231,9 @@ func TestPortForwardNoSuchContainer(t *testing.T) {

 func TestSyncPodWithTerminationLog(t *testing.T) {
 	dm, fakeDocker := newTestDockerManager()
+	// Set test pod container directory.
+	testPodContainerDir := "test/pod/container/dir"
+	dm.runtimeHelper.(*containertest.FakeRuntimeHelper).PodContainerDir = testPodContainerDir
 	container := v1.Container{
 		Name: "bar",
 		TerminationMessagePath: "/dev/somepath",
@@ -1293,8 +1252,6 @@ func TestSyncPodWithTerminationLog(t *testing.T) {
 		"create", "start", "inspect_container",
 	})

-	defer os.Remove(testPodContainerDir)
-
 	fakeDocker.Lock()
 	if len(fakeDocker.Created) != 2 ||
 		!matchString(t, "/k8s_POD\\.[a-f0-9]+_foo_new_", fakeDocker.Created[0]) ||
--- a/pkg/kubelet/kuberuntime/BUILD
+++ b/pkg/kubelet/kuberuntime/BUILD
@@ -32,7 +32,6 @@ go_library(
        "//pkg/credentialprovider:go_default_library",
        "//pkg/kubelet/api:go_default_library",
        "//pkg/kubelet/api/v1alpha1/runtime:go_default_library",
-        "//pkg/kubelet/cm:go_default_library",
        "//pkg/kubelet/container:go_default_library",
        "//pkg/kubelet/dockertools:go_default_library",
        "//pkg/kubelet/events:go_default_library",
--- a/pkg/kubelet/kuberuntime/fake_kuberuntime_manager.go
+++ b/pkg/kubelet/kuberuntime/fake_kuberuntime_manager.go
@@ -17,19 +17,16 @@ limitations under the License.
 package kuberuntime

 import (
-	"io/ioutil"
 	"net/http"
 	"time"

 	cadvisorapi "github.com/google/cadvisor/info/v1"
 	"k8s.io/apimachinery/pkg/types"
-	kubetypes "k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/tools/record"
 	"k8s.io/client-go/util/flowcontrol"
 	"k8s.io/kubernetes/pkg/api/v1"
 	"k8s.io/kubernetes/pkg/credentialprovider"
 	internalapi "k8s.io/kubernetes/pkg/kubelet/api"
-	"k8s.io/kubernetes/pkg/kubelet/cm"
 	kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
 	"k8s.io/kubernetes/pkg/kubelet/images"
 	"k8s.io/kubernetes/pkg/kubelet/lifecycle"
@@ -47,42 +44,6 @@ func (f *fakeHTTP) Get(url string) (*http.Response, error) {
 	return nil, f.err
 }

-// fakeRuntimeHelper implements kubecontainer.RuntimeHelper interfaces for testing purposes.
-type fakeRuntimeHelper struct{}
-
-func (f *fakeRuntimeHelper) GetPodCgroupParent(pod *v1.Pod) (cm.CgroupName, string) {
-	return "", ""
-}
-
-func (f *fakeRuntimeHelper) GenerateRunContainerOptions(pod *v1.Pod, container *v1.Container, podIP string) (*kubecontainer.RunContainerOptions, error) {
-	var opts kubecontainer.RunContainerOptions
-	if len(container.TerminationMessagePath) != 0 {
-		testPodContainerDir, err := ioutil.TempDir("", "fooPodContainerDir")
-		if err != nil {
-			return nil, err
-		}
-		opts.PodContainerDir = testPodContainerDir
-	}
-	return &opts, nil
-}
-
-func (f *fakeRuntimeHelper) GetClusterDNS(pod *v1.Pod) ([]string, []string, error) {
-	return nil, nil, nil
-}
-
-// This is not used by docker runtime.
-func (f *fakeRuntimeHelper) GeneratePodHostNameAndDomain(pod *v1.Pod) (string, string, error) {
-	return "", "", nil
-}
-
-func (f *fakeRuntimeHelper) GetPodDir(kubetypes.UID) string {
-	return ""
-}
-
-func (f *fakeRuntimeHelper) GetExtraSupplementalGroupsForPod(pod *v1.Pod) []int64 {
-	return nil
-}
-
 type fakePodGetter struct {
 	pods map[types.UID]*v1.Pod
 }
@@ -96,7 +57,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, networkPlugin network.NetworkPlugin, osInterface kubecontainer.OSInterface) (*kubeGenericRuntimeManager, error) {
+func NewFakeKubeRuntimeManager(runtimeService internalapi.RuntimeService, imageService internalapi.ImageManagerService, machineInfo *cadvisorapi.MachineInfo, networkPlugin network.NetworkPlugin, osInterface kubecontainer.OSInterface, runtimeHelper kubecontainer.RuntimeHelper) (*kubeGenericRuntimeManager, error) {
 	recorder := &record.FakeRecorder{}
 	kubeRuntimeManager := &kubeGenericRuntimeManager{
 		recorder:            recorder,
@@ -106,7 +67,7 @@ func NewFakeKubeRuntimeManager(runtimeService internalapi.RuntimeService, imageS
 		machineInfo:         machineInfo,
 		osInterface:         osInterface,
 		networkPlugin:       networkPlugin,
-		runtimeHelper:       &fakeRuntimeHelper{},
+		runtimeHelper:       runtimeHelper,
 		runtimeService:      runtimeService,
 		imageService:        imageService,
 		keyring:             credentialprovider.NewDockerKeyring(),
--- a/pkg/kubelet/kuberuntime/kuberuntime_manager_test.go
+++ b/pkg/kubelet/kuberuntime/kuberuntime_manager_test.go
@@ -58,7 +58,7 @@ func createTestRuntimeManager() (*apitest.FakeRuntimeService, *apitest.FakeImage
 		network.UseDefaultMTU,
 	)
 	osInterface := &containertest.FakeOS{}
-	manager, err := NewFakeKubeRuntimeManager(fakeRuntimeService, fakeImageService, machineInfo, networkPlugin, osInterface)
+	manager, err := NewFakeKubeRuntimeManager(fakeRuntimeService, fakeImageService, machineInfo, networkPlugin, osInterface, &containertest.FakeRuntimeHelper{})
 	return fakeRuntimeService, fakeImageService, manager, err
 }

--- a/pkg/kubelet/rkt/BUILD
+++ b/pkg/kubelet/rkt/BUILD
@@ -71,7 +71,6 @@ go_test(
    tags = ["automanaged"],
    deps = [
        "//pkg/api/v1:go_default_library",
-        "//pkg/kubelet/cm:go_default_library",
        "//pkg/kubelet/container:go_default_library",
        "//pkg/kubelet/container/testing:go_default_library",
        "//pkg/kubelet/lifecycle:go_default_library",
--- a/pkg/kubelet/rkt/fake_rkt_interface_test.go
+++ b/pkg/kubelet/rkt/fake_rkt_interface_test.go
@@ -28,8 +28,6 @@ import (
 	"google.golang.org/grpc"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/kubernetes/pkg/api/v1"
-	"k8s.io/kubernetes/pkg/kubelet/cm"
-	kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
 )

 // fakeRktInterface mocks the rktapi.PublicAPIClient interface for testing purpose.
@@ -151,40 +149,6 @@ func (f *fakeSystemd) ResetFailedUnit(name string) error {
 	return f.err
 }

-// fakeRuntimeHelper implementes kubecontainer.RuntimeHelper interfaces for testing purpose.
-// TODO(random-liu): Move this into pkg/kubelet/container/testing
-type fakeRuntimeHelper struct {
-	dnsServers  []string
-	dnsSearches []string
-	hostName    string
-	hostDomain  string
-	err         error
-}
-
-func (f *fakeRuntimeHelper) GenerateRunContainerOptions(pod *v1.Pod, container *v1.Container, podIP string) (*kubecontainer.RunContainerOptions, error) {
-	return nil, fmt.Errorf("Not implemented")
-}
-
-func (f *fakeRuntimeHelper) GetPodCgroupParent(pod *v1.Pod) (cm.CgroupName, string) {
-	return "", ""
-}
-
-func (f *fakeRuntimeHelper) GetClusterDNS(pod *v1.Pod) ([]string, []string, error) {
-	return f.dnsServers, f.dnsSearches, f.err
-}
-
-func (f *fakeRuntimeHelper) GeneratePodHostNameAndDomain(pod *v1.Pod) (string, string, error) {
-	return f.hostName, f.hostDomain, nil
-}
-
-func (f *fakeRuntimeHelper) GetPodDir(podUID types.UID) string {
-	return "/poddir/" + string(podUID)
-}
-
-func (f *fakeRuntimeHelper) GetExtraSupplementalGroupsForPod(pod *v1.Pod) []int64 {
-	return nil
-}
-
 type fakeRktCli struct {
 	sync.Mutex
 	cmds   []string
--- a/pkg/kubelet/rkt/rkt_test.go
+++ b/pkg/kubelet/rkt/rkt_test.go
@@ -585,7 +585,7 @@ func TestGetPodStatus(t *testing.T) {
 	fs := newFakeSystemd()
 	fnp := nettest.NewMockNetworkPlugin(ctrl)
 	fos := &containertesting.FakeOS{}
-	frh := &fakeRuntimeHelper{}
+	frh := &containertesting.FakeRuntimeHelper{}
 	r := &Runtime{
 		apisvc:        fr,
 		systemd:       fs,
@@ -1391,7 +1391,12 @@ func TestGenerateRunCommand(t *testing.T) {
 	for i, tt := range tests {
 		testCaseHint := fmt.Sprintf("test case #%d", i)
 		rkt.network = network.NewPluginManager(tt.networkPlugin)
-		rkt.runtimeHelper = &fakeRuntimeHelper{tt.dnsServers, tt.dnsSearches, tt.hostName, "", tt.err}
+		rkt.runtimeHelper = &containertesting.FakeRuntimeHelper{
+			DNSServers:  tt.dnsServers,
+			DNSSearches: tt.dnsSearches,
+			HostName:    tt.hostName,
+			Err:         tt.err,
+		}
 		rkt.execer = &utilexec.FakeExec{CommandScript: []utilexec.FakeCommandAction{func(cmd string, args ...string) utilexec.Cmd {
 			return utilexec.InitFakeCmd(&utilexec.FakeCmd{}, cmd, args...)
 		}}}