From 5b423dd458c9a592b3419feaeb4d2d385aa1c75f Mon Sep 17 00:00:00 2001 From: Yifan Gu Date: Wed, 18 Nov 2015 18:35:31 -0800 Subject: [PATCH] rkt: Refactor version check with api-service. Also mocked systemd interfaces for testing purpose. --- pkg/kubelet/rkt/fake_rkt_interface.go | 123 +++++++++++++++++++++ pkg/kubelet/rkt/rkt.go | 98 ++++++----------- pkg/kubelet/rkt/rkt_test.go | 146 +++++++++++++++++++++++++ pkg/kubelet/rkt/systemd.go | 103 ++++++++++++++++++ pkg/kubelet/rkt/version.go | 148 +++++++++++++++----------- 5 files changed, 489 insertions(+), 129 deletions(-) create mode 100644 pkg/kubelet/rkt/fake_rkt_interface.go create mode 100644 pkg/kubelet/rkt/rkt_test.go create mode 100644 pkg/kubelet/rkt/systemd.go diff --git a/pkg/kubelet/rkt/fake_rkt_interface.go b/pkg/kubelet/rkt/fake_rkt_interface.go new file mode 100644 index 00000000000..c7a0a87df04 --- /dev/null +++ b/pkg/kubelet/rkt/fake_rkt_interface.go @@ -0,0 +1,123 @@ +/* +Copyright 2015 The Kubernetes Authors All rights reserved. + +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 rkt + +import ( + "fmt" + "strconv" + "sync" + + "github.com/coreos/go-systemd/dbus" + rktapi "github.com/coreos/rkt/api/v1alpha" + "golang.org/x/net/context" + "google.golang.org/grpc" +) + +// fakeRktInterface mocks the rktapi.PublicAPIClient interface for testing purpose. +type fakeRktInterface struct { + sync.Mutex + info rktapi.Info + called []string + err error +} + +func newFakeRktInterface() *fakeRktInterface { + return &fakeRktInterface{} +} + +func (f *fakeRktInterface) CleanCalls() { + f.Lock() + defer f.Unlock() + f.called = nil +} + +func (f *fakeRktInterface) GetInfo(ctx context.Context, in *rktapi.GetInfoRequest, opts ...grpc.CallOption) (*rktapi.GetInfoResponse, error) { + f.Lock() + defer f.Unlock() + + f.called = append(f.called, "GetInfo") + return &rktapi.GetInfoResponse{&f.info}, f.err +} + +func (f *fakeRktInterface) ListPods(ctx context.Context, in *rktapi.ListPodsRequest, opts ...grpc.CallOption) (*rktapi.ListPodsResponse, error) { + return nil, fmt.Errorf("Not implemented") +} + +func (f *fakeRktInterface) InspectPod(ctx context.Context, in *rktapi.InspectPodRequest, opts ...grpc.CallOption) (*rktapi.InspectPodResponse, error) { + return nil, fmt.Errorf("Not implemented") +} + +func (f *fakeRktInterface) ListImages(ctx context.Context, in *rktapi.ListImagesRequest, opts ...grpc.CallOption) (*rktapi.ListImagesResponse, error) { + return nil, fmt.Errorf("Not implemented") +} + +func (f *fakeRktInterface) InspectImage(ctx context.Context, in *rktapi.InspectImageRequest, opts ...grpc.CallOption) (*rktapi.InspectImageResponse, error) { + return nil, fmt.Errorf("Not implemented") +} + +func (f *fakeRktInterface) ListenEvents(ctx context.Context, in *rktapi.ListenEventsRequest, opts ...grpc.CallOption) (rktapi.PublicAPI_ListenEventsClient, error) { + return nil, fmt.Errorf("Not implemented") +} + +func (f *fakeRktInterface) GetLogs(ctx context.Context, in *rktapi.GetLogsRequest, opts ...grpc.CallOption) (rktapi.PublicAPI_GetLogsClient, error) { + return nil, fmt.Errorf("Not implemented") +} + +// fakeSystemd mocks the systemdInterface for testing purpose. +// TODO(yifan): Remove this once we have a package for launching rkt pods. +// See https://github.com/coreos/rkt/issues/1769. +type fakeSystemd struct { + sync.Mutex + called []string + version string + err error +} + +func newFakeSystemd() *fakeSystemd { + return &fakeSystemd{} +} + +func (f *fakeSystemd) CleanCalls() { + f.Lock() + defer f.Unlock() + f.called = nil +} + +func (f *fakeSystemd) Version() (systemdVersion, error) { + f.Lock() + defer f.Unlock() + + f.called = append(f.called, "Version") + v, _ := strconv.Atoi(f.version) + return systemdVersion(v), f.err +} + +func (f *fakeSystemd) ListUnits() ([]dbus.UnitStatus, error) { + return nil, fmt.Errorf("Not implemented") +} + +func (f *fakeSystemd) StopUnit(name, mode string) (string, error) { + return "", fmt.Errorf("Not implemented") +} + +func (f *fakeSystemd) RestartUnit(name, mode string) (string, error) { + return "", fmt.Errorf("Not implemented") +} + +func (f *fakeSystemd) Reload() error { + return fmt.Errorf("Not implemented") +} diff --git a/pkg/kubelet/rkt/rkt.go b/pkg/kubelet/rkt/rkt.go index f2a3b0280ba..432c33a0ee4 100644 --- a/pkg/kubelet/rkt/rkt.go +++ b/pkg/kubelet/rkt/rkt.go @@ -30,10 +30,12 @@ import ( "syscall" "time" + "google.golang.org/grpc" + appcschema "github.com/appc/spec/schema" appctypes "github.com/appc/spec/schema/types" - "github.com/coreos/go-systemd/dbus" "github.com/coreos/go-systemd/unit" + rktapi "github.com/coreos/rkt/api/v1alpha" "github.com/docker/docker/pkg/parsers" docker "github.com/fsouza/go-dockerclient" "github.com/golang/glog" @@ -53,10 +55,11 @@ import ( const ( RktType = "rkt" - acVersion = "0.7.1" - minimumRktVersion = "0.9.0" - recommendRktVersion = "0.9.0" - systemdMinimumVersion = "219" + minimumAppcVersion = "0.7.1" + minimumRktBinVersion = "0.9.0" + recommendedRktBinVersion = "0.9.0" + minimumRktApiVersion = "1.0.0-alpha" + minimumSystemdVersion = "219" systemdServiceDir = "/run/systemd/system" rktDataDir = "/var/lib/rkt" @@ -73,14 +76,18 @@ const ( authDir = "auth.d" dockerAuthTemplate = `{"rktKind":"dockerAuth","rktVersion":"v1","registries":[%q],"credentials":{"user":%q,"password":%q}}` - defaultImageTag = "latest" + defaultImageTag = "latest" + defaultRktAPIServiceAddr = "localhost:15441" ) // Runtime implements the Containerruntime for rkt. The implementation // uses systemd, so in order to run this runtime, systemd must be installed // on the machine. type Runtime struct { - systemd *dbus.Conn + systemd systemdInterface + // The grpc client for rkt api-service. + apisvcConn *grpc.ClientConn + apisvc rktapi.PublicAPIClient // The absolute path to rkt binary. rktBinAbsPath string config *Config @@ -93,6 +100,12 @@ type Runtime struct { livenessManager proberesults.Manager volumeGetter volumeGetter imagePuller kubecontainer.ImagePuller + + // Versions + binVersion rktVersion + apiVersion rktVersion + appcVersion rktVersion + systemdVersion systemdVersion } var _ kubecontainer.Runtime = &Runtime{} @@ -114,21 +127,16 @@ func New(config *Config, imageBackOff *util.Backoff, serializeImagePulls bool, ) (*Runtime, error) { - systemdVersion, err := getSystemdVersion() + // Create dbus connection. + systemd, err := newSystemd() if err != nil { - return nil, err - } - result, err := systemdVersion.Compare(systemdMinimumVersion) - if err != nil { - return nil, err - } - if result < 0 { - return nil, fmt.Errorf("rkt: systemd version is too old, requires at least %v", systemdMinimumVersion) + return nil, fmt.Errorf("rkt: cannot create systemd interface: %v", err) } - systemd, err := dbus.New() + // TODO(yifan): Use secure connection. + apisvcConn, err := grpc.Dial(defaultRktAPIServiceAddr, grpc.WithInsecure()) if err != nil { - return nil, fmt.Errorf("cannot connect to dbus: %v", err) + return nil, fmt.Errorf("rkt: cannot connect to rkt api service: %v", err) } rktBinAbsPath := config.Path @@ -144,6 +152,8 @@ func New(config *Config, rkt := &Runtime{ systemd: systemd, rktBinAbsPath: rktBinAbsPath, + apisvcConn: apisvcConn, + apisvc: rktapi.NewPublicAPIClient(apisvcConn), config: config, dockerKeyring: credentialprovider.NewDockerKeyring(), containerRefManager: containerRefManager, @@ -158,28 +168,13 @@ func New(config *Config, rkt.imagePuller = kubecontainer.NewImagePuller(recorder, rkt, imageBackOff) } - // Test the rkt version. - version, err := rkt.Version() - if err != nil { + if err := rkt.checkVersion(minimumRktBinVersion, recommendedRktBinVersion, minimumAppcVersion, minimumRktApiVersion, minimumSystemdVersion); err != nil { + // TODO(yifan): Latest go-systemd version have the ability to close the + // dbus connection. However the 'docker/libcontainer' package is using + // the older go-systemd version, so we can't update the go-systemd version. + rkt.apisvcConn.Close() return nil, err } - result, err = version.Compare(minimumRktVersion) - if err != nil { - return nil, err - } - if result < 0 { - return nil, fmt.Errorf("rkt: version is too old, requires at least %v", minimumRktVersion) - } - - result, err = version.Compare(recommendRktVersion) - if err != nil { - return nil, err - } - if result != 0 { - // TODO(yifan): Record an event to expose the information. - glog.Warningf("rkt: current version %q is not recommended (recommended version %q)", version, recommendRktVersion) - } - return rkt, nil } @@ -880,33 +875,8 @@ func (r *Runtime) Type() string { return RktType } -// Version invokes 'rkt version' to get the version information of the rkt -// runtime on the machine. -// The return values are an int array containers the version number. -// -// Example: -// rkt:0.3.2+git --> []int{0, 3, 2}. -// func (r *Runtime) Version() (kubecontainer.Version, error) { - output, err := r.runCommand("version") - if err != nil { - return nil, err - } - - // Example output for 'rkt version': - // rkt version 0.3.2+git - // appc version 0.3.0+git - for _, line := range output { - tuples := strings.Split(strings.TrimSpace(line), " ") - if len(tuples) != 3 { - glog.Warningf("rkt: cannot parse the output: %q.", line) - continue - } - if tuples[0] == "rkt" { - return parseVersion(tuples[2]) - } - } - return nil, fmt.Errorf("rkt: cannot determine the version") + return r.binVersion, nil } // TODO(yifan): This is very racy, unefficient, and unsafe, we need to provide diff --git a/pkg/kubelet/rkt/rkt_test.go b/pkg/kubelet/rkt/rkt_test.go new file mode 100644 index 00000000000..471d795fd85 --- /dev/null +++ b/pkg/kubelet/rkt/rkt_test.go @@ -0,0 +1,146 @@ +/* +Copyright 2015 The Kubernetes Authors All rights reserved. + +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 rkt + +import ( + "fmt" + "testing" + + rktapi "github.com/coreos/rkt/api/v1alpha" + "github.com/stretchr/testify/assert" +) + +func TestCheckVersion(t *testing.T) { + fr := newFakeRktInterface() + fs := newFakeSystemd() + r := &Runtime{apisvc: fr, systemd: fs} + + fr.info = rktapi.Info{ + RktVersion: "1.2.3+git", + AppcVersion: "1.2.4+git", + ApiVersion: "1.2.6-alpha", + } + fs.version = "100" + tests := []struct { + minimumRktBinVersion string + recommendedRktBinVersion string + minimumAppcVersion string + minimumRktApiVersion string + minimumSystemdVersion string + err error + calledGetInfo bool + calledSystemVersion bool + }{ + // Good versions. + { + "1.2.3", + "1.2.3", + "1.2.4", + "1.2.5", + "99", + nil, + true, + true, + }, + // Good versions. + { + "1.2.3+git", + "1.2.3+git", + "1.2.4+git", + "1.2.6-alpha", + "100", + nil, + true, + true, + }, + // Requires greater binary version. + { + "1.2.4", + "1.2.4", + "1.2.4", + "1.2.6-alpha", + "100", + fmt.Errorf("rkt: binary version is too old(%v), requires at least %v", fr.info.RktVersion, "1.2.4"), + true, + true, + }, + // Requires greater Appc version. + { + "1.2.3", + "1.2.3", + "1.2.5", + "1.2.6-alpha", + "100", + fmt.Errorf("rkt: appc version is too old(%v), requires at least %v", fr.info.AppcVersion, "1.2.5"), + true, + true, + }, + // Requires greater API version. + { + "1.2.3", + "1.2.3", + "1.2.4", + "1.2.6", + "100", + fmt.Errorf("rkt: API version is too old(%v), requires at least %v", fr.info.ApiVersion, "1.2.6"), + true, + true, + }, + // Requires greater API version. + { + "1.2.3", + "1.2.3", + "1.2.4", + "1.2.7", + "100", + fmt.Errorf("rkt: API version is too old(%v), requires at least %v", fr.info.ApiVersion, "1.2.7"), + true, + true, + }, + // Requires greater systemd version. + { + "1.2.3", + "1.2.3", + "1.2.4", + "1.2.7", + "101", + fmt.Errorf("rkt: systemd version(%v) is too old, requires at least %v", fs.version, "101"), + false, + true, + }, + } + + for i, tt := range tests { + testCaseHint := fmt.Sprintf("test case #%d", i) + err := r.checkVersion(tt.minimumRktBinVersion, tt.recommendedRktBinVersion, tt.minimumAppcVersion, tt.minimumRktApiVersion, tt.minimumSystemdVersion) + assert.Equal(t, err, tt.err, testCaseHint) + + if tt.calledGetInfo { + assert.Equal(t, fr.called, []string{"GetInfo"}, testCaseHint) + } + if tt.calledSystemVersion { + assert.Equal(t, fs.called, []string{"Version"}, testCaseHint) + } + if err == nil { + assert.Equal(t, r.binVersion.String(), fr.info.RktVersion, testCaseHint) + assert.Equal(t, r.appcVersion.String(), fr.info.AppcVersion, testCaseHint) + assert.Equal(t, r.apiVersion.String(), fr.info.ApiVersion, testCaseHint) + } + fr.CleanCalls() + fs.CleanCalls() + } +} diff --git a/pkg/kubelet/rkt/systemd.go b/pkg/kubelet/rkt/systemd.go new file mode 100644 index 00000000000..0534d2148b4 --- /dev/null +++ b/pkg/kubelet/rkt/systemd.go @@ -0,0 +1,103 @@ +/* +Copyright 2015 The Kubernetes Authors All rights reserved. + +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 rkt + +import ( + "fmt" + "os/exec" + "strconv" + "strings" + + "github.com/coreos/go-systemd/dbus" +) + +// systemdVersion is a type wraps the int to implement kubecontainer.Version interface. +type systemdVersion int + +func (s systemdVersion) String() string { + return fmt.Sprintf("%d", s) +} + +func (s systemdVersion) Compare(other string) (int, error) { + v, err := strconv.Atoi(other) + if err != nil { + return -1, err + } + if int(s) < v { + return -1, nil + } else if int(s) > v { + return 1, nil + } + return 0, nil +} + +// systemdInterface is an abstraction of the go-systemd/dbus to make +// it mockable for testing. +// TODO(yifan): Eventually we should move these functionalities to: +// 1. a package for launching/stopping rkt pods. +// 2. rkt api-service interface for listing pods. +// See https://github.com/coreos/rkt/issues/1769. +type systemdInterface interface { + // Version returns the version of the systemd. + Version() (systemdVersion, error) + // ListUnits lists all the loaded units. + ListUnits() ([]dbus.UnitStatus, error) + // StopUnits stops the unit with the given name. + StopUnit(name, mode string) (string, error) + // StopUnits restarts the unit with the given name. + RestartUnit(name, mode string) (string, error) + // Reload is equivalent to 'systemctl daemon-reload'. + Reload() error +} + +// systemd implements the systemdInterface using dbus and systemctl. +// All the functions other then Version() are already implemented by go-systemd/dbus. +type systemd struct { + *dbus.Conn +} + +// newSystemd creates a systemd object that implements systemdInterface. +func newSystemd() (*systemd, error) { + dbusConn, err := dbus.New() + if err != nil { + return nil, err + } + return &systemd{dbusConn}, nil +} + +// Version returns the version of the systemd. +func (s *systemd) Version() (systemdVersion, error) { + output, err := exec.Command("systemctl", "--version").Output() + if err != nil { + return -1, err + } + // Example output of 'systemctl --version': + // + // systemd 215 + // +PAM +AUDIT +SELINUX +IMA +SYSVINIT +LIBCRYPTSETUP +GCRYPT +ACL +XZ -SECCOMP -APPARMOR + // + lines := strings.Split(string(output), "\n") + tuples := strings.Split(lines[0], " ") + if len(tuples) != 2 { + return -1, fmt.Errorf("rkt: Failed to parse version %v", lines) + } + result, err := strconv.Atoi(string(tuples[1])) + if err != nil { + return -1, err + } + return systemdVersion(result), nil +} diff --git a/pkg/kubelet/rkt/version.go b/pkg/kubelet/rkt/version.go index 593d692c4df..4a6aaed7d77 100644 --- a/pkg/kubelet/rkt/version.go +++ b/pkg/kubelet/rkt/version.go @@ -18,93 +18,111 @@ package rkt import ( "fmt" - "os/exec" - "strconv" - "strings" - appctypes "github.com/appc/spec/schema/types" + "github.com/coreos/go-semver/semver" + rktapi "github.com/coreos/rkt/api/v1alpha" + "github.com/golang/glog" + "golang.org/x/net/context" ) -type rktVersion []int +// rktVersion implementes kubecontainer.Version interface by implementing +// Compare() and String() (which is implemented by the underlying semver.Version) +type rktVersion struct { + *semver.Version +} -func parseVersion(input string) (rktVersion, error) { - nsv, err := appctypes.NewSemVer(input) +func newRktVersion(version string) (rktVersion, error) { + sem, err := semver.NewVersion(version) if err != nil { - return nil, err + return rktVersion{}, err } - return rktVersion{int(nsv.Major), int(nsv.Minor), int(nsv.Patch)}, nil + return rktVersion{sem}, nil } func (r rktVersion) Compare(other string) (int, error) { - v, err := parseVersion(other) + v, err := semver.NewVersion(other) if err != nil { return -1, err } - for i := range r { - if i > len(v)-1 { - return 1, nil - } - if r[i] < v[i] { - return -1, nil - } - if r[i] > v[i] { - return 1, nil - } - } - - // When loop ends, len(r) is <= len(v). - if len(r) < len(v) { + if r.LessThan(*v) { return -1, nil } - return 0, nil -} - -func (r rktVersion) String() string { - var version []string - for _, v := range r { - version = append(version, fmt.Sprintf("%d", v)) - } - return strings.Join(version, ".") -} - -type systemdVersion int - -func (s systemdVersion) String() string { - return fmt.Sprintf("%d", s) -} - -func (s systemdVersion) Compare(other string) (int, error) { - v, err := strconv.Atoi(other) - if err != nil { - return -1, err - } - if int(s) < v { - return -1, nil - } else if int(s) > v { + if v.LessThan(*r.Version) { return 1, nil } return 0, nil } -func getSystemdVersion() (systemdVersion, error) { - output, err := exec.Command("systemctl", "--version").Output() +// checkVersion tests whether the rkt/systemd/rkt-api-service that meet the version requirement. +// If all version requirements are met, it returns nil. +func (r *Runtime) checkVersion(minimumRktBinVersion, recommendedRktBinVersion, minimumAppcVersion, minimumRktApiVersion, minimumSystemdVersion string) error { + // Check systemd version. + var err error + r.systemdVersion, err = r.systemd.Version() if err != nil { - return -1, err + return err } - // Example output of 'systemctl --version': - // - // systemd 215 - // +PAM +AUDIT +SELINUX +IMA +SYSVINIT +LIBCRYPTSETUP +GCRYPT +ACL +XZ -SECCOMP -APPARMOR - // - lines := strings.Split(string(output), "\n") - tuples := strings.Split(lines[0], " ") - if len(tuples) != 2 { - return -1, fmt.Errorf("rkt: Failed to parse version %v", lines) - } - result, err := strconv.Atoi(string(tuples[1])) + result, err := r.systemdVersion.Compare(minimumSystemdVersion) if err != nil { - return -1, err + return err } - return systemdVersion(result), nil + if result < 0 { + return fmt.Errorf("rkt: systemd version(%v) is too old, requires at least %v", r.systemdVersion, minimumSystemdVersion) + } + + // Example for the version strings returned by GetInfo(): + // RktVersion:"0.10.0+gitb7349b1" AppcVersion:"0.7.1" ApiVersion:"1.0.0-alpha" + resp, err := r.apisvc.GetInfo(context.Background(), &rktapi.GetInfoRequest{}) + if err != nil { + return err + } + + // Check rkt binary version. + r.binVersion, err = newRktVersion(resp.Info.RktVersion) + if err != nil { + return err + } + result, err = r.binVersion.Compare(minimumRktBinVersion) + if err != nil { + return err + } + if result < 0 { + return fmt.Errorf("rkt: binary version is too old(%v), requires at least %v", resp.Info.RktVersion, minimumRktBinVersion) + } + result, err = r.binVersion.Compare(recommendedRktBinVersion) + if err != nil { + return err + } + if result != 0 { + // TODO(yifan): Record an event to expose the information. + glog.Warningf("rkt: current binary version %q is not recommended (recommended version %q)", resp.Info.RktVersion, recommendedRktBinVersion) + } + + // Check Appc version. + r.appcVersion, err = newRktVersion(resp.Info.AppcVersion) + if err != nil { + return err + } + result, err = r.appcVersion.Compare(minimumAppcVersion) + if err != nil { + return err + } + if result < 0 { + return fmt.Errorf("rkt: appc version is too old(%v), requires at least %v", resp.Info.AppcVersion, minimumAppcVersion) + } + + // Check rkt API version. + r.apiVersion, err = newRktVersion(resp.Info.ApiVersion) + if err != nil { + return err + } + result, err = r.apiVersion.Compare(minimumRktApiVersion) + if err != nil { + return err + } + if result < 0 { + return fmt.Errorf("rkt: API version is too old(%v), requires at least %v", resp.Info.ApiVersion, minimumRktApiVersion) + } + return nil }