diff --git a/pkg/kubelet/dockertools/docker.go b/pkg/kubelet/dockertools/docker.go index a03c3e45187..1cf2d91a5ac 100644 --- a/pkg/kubelet/dockertools/docker.go +++ b/pkg/kubelet/dockertools/docker.go @@ -69,8 +69,8 @@ type DockerInterface interface { PullImage(opts docker.PullImageOptions, auth docker.AuthConfiguration) error RemoveImage(image string) error Logs(string, dockertypes.ContainerLogsOptions, StreamOptions) error - Version() (*docker.Env, error) - Info() (*docker.Env, error) + Version() (*dockertypes.Version, error) + Info() (*dockertypes.Info, error) CreateExec(string, dockertypes.ExecConfig) (*dockertypes.ContainerExecCreateResponse, error) StartExec(string, dockertypes.ExecStartCheck, StreamOptions) error InspectExec(id string) (*dockertypes.ContainerExecInspect, error) diff --git a/pkg/kubelet/dockertools/docker_test.go b/pkg/kubelet/dockertools/docker_test.go index c00a5589b01..6c301073a6c 100644 --- a/pkg/kubelet/dockertools/docker_test.go +++ b/pkg/kubelet/dockertools/docker_test.go @@ -157,28 +157,6 @@ func TestContainerNaming(t *testing.T) { } } -func TestVersion(t *testing.T) { - fakeDocker := NewFakeDockerClientWithVersion("1.1.3", "1.15") - manager := &DockerManager{client: fakeDocker} - version, err := manager.Version() - if err != nil { - t.Errorf("got error while getting docker server version - %s", err) - } - expectedVersion, _ := docker.NewAPIVersion("1.1.3") - if e, a := expectedVersion.String(), version.String(); e != a { - t.Errorf("invalid docker server version. expected: %v, got: %v", e, a) - } - - version, err = manager.APIVersion() - if err != nil { - t.Errorf("got error while getting docker server version - %s", err) - } - expectedVersion, _ = docker.NewAPIVersion("1.15") - if e, a := expectedVersion.String(), version.String(); e != a { - t.Errorf("invalid docker server version. expected: %v, got: %v", e, a) - } -} - func TestParseImageName(t *testing.T) { tests := []struct { imageName string diff --git a/pkg/kubelet/dockertools/fake_docker_client.go b/pkg/kubelet/dockertools/fake_docker_client.go index d5412658ee9..963ad9152ca 100644 --- a/pkg/kubelet/dockertools/fake_docker_client.go +++ b/pkg/kubelet/dockertools/fake_docker_client.go @@ -50,8 +50,8 @@ type FakeDockerClient struct { Stopped []string Removed []string RemovedImages sets.String - VersionInfo docker.Env - Information docker.Env + VersionInfo dockertypes.Version + Information dockertypes.Info ExecInspect *dockertypes.ContainerExecInspect execCmd []string EnableSleep bool @@ -67,7 +67,7 @@ func NewFakeDockerClient() *FakeDockerClient { func NewFakeDockerClientWithVersion(version, apiVersion string) *FakeDockerClient { return &FakeDockerClient{ - VersionInfo: docker.Env{fmt.Sprintf("Version=%s", version), fmt.Sprintf("ApiVersion=%s", apiVersion)}, + VersionInfo: dockertypes.Version{Version: version, APIVersion: apiVersion}, Errors: make(map[string]error), RemovedImages: sets.String{}, ContainerMap: make(map[string]*dockertypes.ContainerJSON), @@ -437,13 +437,13 @@ func (f *FakeDockerClient) PullImage(opts docker.PullImageOptions, auth docker.A return err } -func (f *FakeDockerClient) Version() (*docker.Env, error) { +func (f *FakeDockerClient) Version() (*dockertypes.Version, error) { f.Lock() defer f.Unlock() return &f.VersionInfo, f.popError("version") } -func (f *FakeDockerClient) Info() (*docker.Env, error) { +func (f *FakeDockerClient) Info() (*dockertypes.Info, error) { return &f.Information, nil } diff --git a/pkg/kubelet/dockertools/instrumented_docker.go b/pkg/kubelet/dockertools/instrumented_docker.go index fb04f0f6350..e15b557ced0 100644 --- a/pkg/kubelet/dockertools/instrumented_docker.go +++ b/pkg/kubelet/dockertools/instrumented_docker.go @@ -148,7 +148,7 @@ func (in instrumentedDockerInterface) Logs(id string, opts dockertypes.Container return err } -func (in instrumentedDockerInterface) Version() (*docker.Env, error) { +func (in instrumentedDockerInterface) Version() (*dockertypes.Version, error) { const operation = "version" defer recordOperation(operation, time.Now()) @@ -157,7 +157,7 @@ func (in instrumentedDockerInterface) Version() (*docker.Env, error) { return out, err } -func (in instrumentedDockerInterface) Info() (*docker.Env, error) { +func (in instrumentedDockerInterface) Info() (*dockertypes.Info, error) { const operation = "info" defer recordOperation(operation, time.Now()) diff --git a/pkg/kubelet/dockertools/kube_docker_client.go b/pkg/kubelet/dockertools/kube_docker_client.go index c44e5e48571..397bbca216e 100644 --- a/pkg/kubelet/dockertools/kube_docker_client.go +++ b/pkg/kubelet/dockertools/kube_docker_client.go @@ -87,19 +87,6 @@ func convertFilters(filters map[string][]string) dockerfilters.Args { return args } -// convertEnv converts data to a go-dockerclient Env -func convertEnv(src interface{}) (*docker.Env, error) { - m := make(map[string]interface{}) - if err := convertType(&src, &m); err != nil { - return nil, err - } - env := &docker.Env{} - for k, v := range m { - env.SetAuto(k, v) - } - return env, nil -} - func (k *kubeDockerClient) ListContainers(options dockertypes.ContainerListOptions) ([]dockertypes.Container, error) { containers, err := k.client.ContainerList(getDefaultContext(), options) if err != nil { @@ -219,20 +206,20 @@ func (d *kubeDockerClient) Logs(id string, opts dockertypes.ContainerLogsOptions return d.redirectResponseToOutputStream(sopts.RawTerminal, sopts.OutputStream, sopts.ErrorStream, resp) } -func (d *kubeDockerClient) Version() (*docker.Env, error) { +func (d *kubeDockerClient) Version() (*dockertypes.Version, error) { resp, err := d.client.ServerVersion(getDefaultContext()) if err != nil { return nil, err } - return convertEnv(resp) + return &resp, nil } -func (d *kubeDockerClient) Info() (*docker.Env, error) { +func (d *kubeDockerClient) Info() (*dockertypes.Info, error) { resp, err := d.client.Info(getDefaultContext()) if err != nil { return nil, err } - return convertEnv(resp) + return &resp, nil } // TODO(random-liu): Add unit test for exec and attach functions, just like what go-dockerclient did. diff --git a/pkg/kubelet/dockertools/manager.go b/pkg/kubelet/dockertools/manager.go index b79a89f4a8e..320674ac989 100644 --- a/pkg/kubelet/dockertools/manager.go +++ b/pkg/kubelet/dockertools/manager.go @@ -25,13 +25,11 @@ import ( "os" "os/exec" "path" - "regexp" "strconv" "strings" "sync" "time" - "github.com/coreos/go-semver/semver" dockertypes "github.com/docker/engine-api/types" dockercontainer "github.com/docker/engine-api/types/container" dockerstrslice "github.com/docker/engine-api/types/strslice" @@ -220,7 +218,7 @@ func NewDockerManager( glog.Errorf("Failed to execute Info() call to the Docker client: %v", err) glog.Warningf("Using fallback default of /var/lib/docker for location of Docker runtime") } else { - dockerRoot = dockerInfo.Get("DockerRootDir") + dockerRoot = dockerInfo.DockerRootDir glog.Infof("Setting dockerRoot to %s", dockerRoot) } @@ -873,67 +871,48 @@ func getDockerNetworkMode(container *dockertypes.ContainerJSON) string { } // dockerVersion implementes kubecontainer.Version interface by implementing -// Compare() and String() (which is implemented by the underlying semver.Version) -// TODO: this code is the same as rktVersion and may make sense to be moved to -// somewhere shared. -type dockerVersion struct { - *semver.Version +// Compare() and String(). It could contain either server version or api version. +type dockerVersion string + +func (v dockerVersion) String() string { + return string(v) } -// Older versions of Docker could return non-semantically versioned values (distros like Fedora -// included partial values such as 1.8.1.fc21 which is not semver). Force those values to be semver. -var almostSemverRegexp = regexp.MustCompile(`^(\d+\.\d+\.\d+)\.(.*)$`) +func (v dockerVersion) Compare(other string) (int, error) { + return compare(string(v), other), nil +} -// newDockerVersion returns a semantically versioned docker version value -func newDockerVersion(version string) (dockerVersion, error) { - sem, err := semver.NewVersion(version) - if err != nil { - matches := almostSemverRegexp.FindStringSubmatch(version) - if matches == nil { - return dockerVersion{}, err +// compare is copied from engine-api, it compares two version strings, returns -1 if +// v1 < v2, 1 if v1 > v2, 0 otherwise. +// TODO(random-liu): Leveraging the version comparison in engine-api after bumping up +// the engine-api version. See #24076 +func compare(v1, v2 string) int { + var ( + currTab = strings.Split(v1, ".") + otherTab = strings.Split(v2, ".") + ) + + max := len(currTab) + if len(otherTab) > max { + max = len(otherTab) + } + for i := 0; i < max; i++ { + var currInt, otherInt int + + if len(currTab) > i { + currInt, _ = strconv.Atoi(currTab[i]) + } + if len(otherTab) > i { + otherInt, _ = strconv.Atoi(otherTab[i]) + } + if currInt > otherInt { + return 1 + } + if otherInt > currInt { + return -1 } - sem, err = semver.NewVersion(strings.Join(matches[1:], "-")) } - return dockerVersion{sem}, err -} - -func (r dockerVersion) Compare(other string) (int, error) { - v, err := newDockerVersion(other) - if err != nil { - return -1, err - } - - if r.LessThan(*v.Version) { - return -1, nil - } - if v.Version.LessThan(*r.Version) { - return 1, nil - } - return 0, nil -} - -// dockerVersion implementes kubecontainer.Version interface by implementing -// Compare() and String() on top og go-dockerclient's APIVersion. This version -// string doesn't conform to semantic versioning, as it is only "x.y" -type dockerAPIVersion docker.APIVersion - -func (dv dockerAPIVersion) String() string { - return docker.APIVersion(dv).String() -} - -func (dv dockerAPIVersion) Compare(other string) (int, error) { - a := docker.APIVersion(dv) - b, err := docker.NewAPIVersion(other) - if err != nil { - return 0, err - } - if a.LessThan(b) { - return -1, nil - } - if a.GreaterThan(b) { - return 1, nil - } - return 0, nil + return 0 } func (dm *DockerManager) Type() string { @@ -941,33 +920,21 @@ func (dm *DockerManager) Type() string { } func (dm *DockerManager) Version() (kubecontainer.Version, error) { - env, err := dm.client.Version() + v, err := dm.client.Version() if err != nil { return nil, fmt.Errorf("docker: failed to get docker version: %v", err) } - engineVersion := env.Get("Version") - version, err := newDockerVersion(engineVersion) - if err != nil { - glog.Errorf("docker: failed to parse docker server version %q: %v", engineVersion, err) - return nil, fmt.Errorf("docker: failed to parse docker server version %q: %v", engineVersion, err) - } - return version, nil + return dockerVersion(v.Version), nil } func (dm *DockerManager) APIVersion() (kubecontainer.Version, error) { - env, err := dm.client.Version() + v, err := dm.client.Version() if err != nil { return nil, fmt.Errorf("docker: failed to get docker version: %v", err) } - apiVersion := env.Get("ApiVersion") - version, err := docker.NewAPIVersion(apiVersion) - if err != nil { - glog.Errorf("docker: failed to parse docker api version %q: %v", apiVersion, err) - return nil, fmt.Errorf("docker: failed to parse docker api version %q: %v", apiVersion, err) - } - return dockerAPIVersion(version), nil + return dockerVersion(v.APIVersion), nil } // Status returns error if docker daemon is unhealthy, nil otherwise. diff --git a/pkg/kubelet/dockertools/manager_test.go b/pkg/kubelet/dockertools/manager_test.go index 162c37a89f4..c5041dccf46 100644 --- a/pkg/kubelet/dockertools/manager_test.go +++ b/pkg/kubelet/dockertools/manager_test.go @@ -93,8 +93,13 @@ func (f *fakeRuntimeHelper) GeneratePodHostNameAndDomain(pod *api.Pod) (string, return "", "" } -func newTestDockerManagerWithHTTPClientWithVersion(fakeHTTPClient *fakeHTTP, version, apiVersion string) (*DockerManager, *FakeDockerClient) { - fakeDocker := NewFakeDockerClientWithVersion(version, apiVersion) +func createTestDockerManager(fakeHTTPClient *fakeHTTP, fakeDocker *FakeDockerClient) (*DockerManager, *FakeDockerClient) { + if fakeHTTPClient == nil { + fakeHTTPClient = &fakeHTTP{} + } + if fakeDocker == nil { + fakeDocker = NewFakeDockerClient() + } fakeRecorder := &record.FakeRecorder{} containerRefManager := kubecontainer.NewRefManager() networkPlugin, _ := network.InitNetworkPlugin([]network.NetworkPlugin{}, "", nettest.NewFakeHost(nil)) @@ -116,11 +121,16 @@ func newTestDockerManagerWithHTTPClientWithVersion(fakeHTTPClient *fakeHTTP, ver } func newTestDockerManagerWithHTTPClient(fakeHTTPClient *fakeHTTP) (*DockerManager, *FakeDockerClient) { - return newTestDockerManagerWithHTTPClientWithVersion(fakeHTTPClient, "1.8.1", "1.20") + return createTestDockerManager(fakeHTTPClient, nil) +} + +func newTestDockerManagerWithVersion(version, apiVersion string) (*DockerManager, *FakeDockerClient) { + fakeDocker := NewFakeDockerClientWithVersion(version, apiVersion) + return createTestDockerManager(nil, fakeDocker) } func newTestDockerManager() (*DockerManager, *FakeDockerClient) { - return newTestDockerManagerWithHTTPClient(&fakeHTTP{}) + return createTestDockerManager(nil, nil) } func matchString(t *testing.T, pattern, str string) bool { @@ -131,35 +141,6 @@ func matchString(t *testing.T, pattern, str string) bool { return match } -func TestNewDockerVersion(t *testing.T) { - cases := []struct { - value string - out string - err bool - }{ - {value: "1", err: true}, - {value: "1.8", err: true}, - {value: "1.8.1", out: "1.8.1"}, - {value: "1.8.1.fc21", out: "1.8.1-fc21"}, - {value: "1.8.1.fc21.other", out: "1.8.1-fc21.other"}, - {value: "1.8.1-fc21.other", out: "1.8.1-fc21.other"}, - {value: "1.8.1-beta.12", out: "1.8.1-beta.12"}, - } - for _, test := range cases { - v, err := newDockerVersion(test.value) - switch { - case err != nil && test.err: - continue - case (err != nil) != test.err: - t.Errorf("error for %q: expected %t, got %v", test.value, test.err, err) - continue - } - if v.String() != test.out { - t.Errorf("unexpected parsed version %q for %q", v, test.value) - } - } -} - func TestSetEntrypointAndCommand(t *testing.T) { cases := []struct { name string @@ -1730,7 +1711,7 @@ func verifySyncResults(t *testing.T, expectedResults []*kubecontainer.SyncResult } func TestSeccompIsDisabledWithDockerV110(t *testing.T) { - dm, fakeDocker := newTestDockerManagerWithHTTPClientWithVersion(&fakeHTTP{}, "1.10.1", "1.22") + dm, fakeDocker := newTestDockerManagerWithVersion("1.10.1", "1.22") pod := &api.Pod{ ObjectMeta: api.ObjectMeta{ UID: "12345678", @@ -1769,7 +1750,7 @@ func TestSeccompIsDisabledWithDockerV110(t *testing.T) { } func TestSecurityOptsAreNilWithDockerV19(t *testing.T) { - dm, fakeDocker := newTestDockerManagerWithHTTPClientWithVersion(&fakeHTTP{}, "1.9.1", "1.21") + dm, fakeDocker := newTestDockerManagerWithVersion("1.9.1", "1.21") pod := &api.Pod{ ObjectMeta: api.ObjectMeta{ UID: "12345678", @@ -1808,10 +1789,6 @@ func TestSecurityOptsAreNilWithDockerV19(t *testing.T) { } func TestCheckVersionCompatibility(t *testing.T) { - apiVersion, err := docker.NewAPIVersion(minimumDockerAPIVersion) - if err != nil { - t.Fatalf("unexpected error %v", err) - } type test struct { version string compatible bool @@ -1821,22 +1798,17 @@ func TestCheckVersionCompatibility(t *testing.T) { {minimumDockerAPIVersion, true}, // Invalid apiversion {"invalid_api_version", false}, - } - for i := range apiVersion { - apiVersion[i]++ - // Newer apiversion - tests = append(tests, test{apiVersion.String(), true}) - apiVersion[i] -= 2 // Older apiversion - if apiVersion[i] >= 0 { - tests = append(tests, test{apiVersion.String(), false}) - } - apiVersion[i]++ + {"1.0.0", false}, + // Newer apiversion + // NOTE(random-liu): We need to bump up the newer apiversion, + // if docker apiversion really reaches "9.9.9" someday. But I + // really doubt whether the test could live that long. + {"9.9.9", true}, } - for i, tt := range tests { testCase := fmt.Sprintf("test case #%d test version %q", i, tt.version) - dm, fakeDocker := newTestDockerManagerWithHTTPClientWithVersion(&fakeHTTP{}, "", tt.version) + dm, fakeDocker := newTestDockerManagerWithVersion("", tt.version) err := dm.checkVersionCompatibility() assert.Equal(t, tt.compatible, err == nil, testCase) if tt.compatible == true { @@ -1848,6 +1820,27 @@ func TestCheckVersionCompatibility(t *testing.T) { } } +func TestVersion(t *testing.T) { + expectedVersion := "1.8.1" + expectedAPIVersion := "1.20" + dm, _ := newTestDockerManagerWithVersion(expectedVersion, expectedAPIVersion) + version, err := dm.Version() + if err != nil { + t.Errorf("got error while getting docker server version - %v", err) + } + if e, a := expectedVersion, version.String(); e != a { + t.Errorf("expect docker server version %q, got %q", e, a) + } + + apiVersion, err := dm.APIVersion() + if err != nil { + t.Errorf("got error while getting docker api version - %v", err) + } + if e, a := expectedAPIVersion, apiVersion.String(); e != a { + t.Errorf("expect docker api version %q, got %q", e, a) + } +} + func TestGetPodStatusNoSuchContainer(t *testing.T) { const ( noSuchContainerID = "nosuchcontainer"