From fe2c3c351a262d7f26104327de245b768cf90525 Mon Sep 17 00:00:00 2001 From: Yifan Gu Date: Fri, 11 Mar 2016 17:29:25 -0800 Subject: [PATCH] rkt: Split checkVersion() to two parts: get version, and check version. This enables rkt runtime to setup versions during creation, this fixes a kubelet nil pointer panic when kubelet tries to get the rkt versions but it's not set. --- pkg/kubelet/rkt/rkt.go | 18 +++++--- pkg/kubelet/rkt/rkt_test.go | 6 +-- pkg/kubelet/rkt/version.go | 90 ++++++++++++++++++++++++------------- 3 files changed, 74 insertions(+), 40 deletions(-) diff --git a/pkg/kubelet/rkt/rkt.go b/pkg/kubelet/rkt/rkt.go index 4d1119a9991..4360c20f1cb 100644 --- a/pkg/kubelet/rkt/rkt.go +++ b/pkg/kubelet/rkt/rkt.go @@ -117,11 +117,7 @@ type Runtime struct { volumeGetter volumeGetter imagePuller kubecontainer.ImagePuller - // Versions - binVersion rktVersion - apiVersion rktVersion - appcVersion rktVersion - systemdVersion systemdVersion + versions versions } var _ kubecontainer.Runtime = &Runtime{} @@ -184,6 +180,10 @@ func New(config *Config, rkt.imagePuller = kubecontainer.NewImagePuller(recorder, rkt, imageBackOff) } + if err := rkt.getVersions(); err != nil { + return nil, fmt.Errorf("rkt: error getting version info: %v", err) + } + return rkt, nil } @@ -1069,11 +1069,15 @@ func (r *Runtime) Type() string { } func (r *Runtime) Version() (kubecontainer.Version, error) { - return r.binVersion, nil + r.versions.RLock() + defer r.versions.RUnlock() + return r.versions.binVersion, nil } func (r *Runtime) APIVersion() (kubecontainer.Version, error) { - return r.apiVersion, nil + r.versions.RLock() + defer r.versions.RUnlock() + return r.versions.apiVersion, nil } // Status returns error if rkt is unhealthy, nil otherwise. diff --git a/pkg/kubelet/rkt/rkt_test.go b/pkg/kubelet/rkt/rkt_test.go index 901f2ec0de4..2c954900e35 100644 --- a/pkg/kubelet/rkt/rkt_test.go +++ b/pkg/kubelet/rkt/rkt_test.go @@ -260,9 +260,9 @@ func TestCheckVersion(t *testing.T) { assert.Equal(t, fs.called, []string{"Version"}, testCaseHint) } if err == nil { - assert.Equal(t, fr.info.RktVersion, r.binVersion.String(), testCaseHint) - assert.Equal(t, fr.info.AppcVersion, r.appcVersion.String(), testCaseHint) - assert.Equal(t, fr.info.ApiVersion, r.apiVersion.String(), testCaseHint) + assert.Equal(t, fr.info.RktVersion, r.versions.binVersion.String(), testCaseHint) + assert.Equal(t, fr.info.AppcVersion, r.versions.appcVersion.String(), testCaseHint) + assert.Equal(t, fr.info.ApiVersion, r.versions.apiVersion.String(), testCaseHint) } fr.CleanCalls() fs.CleanCalls() diff --git a/pkg/kubelet/rkt/version.go b/pkg/kubelet/rkt/version.go index 4a6aaed7d77..32b66b29bcd 100644 --- a/pkg/kubelet/rkt/version.go +++ b/pkg/kubelet/rkt/version.go @@ -18,6 +18,7 @@ package rkt import ( "fmt" + "sync" "github.com/coreos/go-semver/semver" rktapi "github.com/coreos/rkt/api/v1alpha" @@ -25,6 +26,14 @@ import ( "golang.org/x/net/context" ) +type versions struct { + sync.RWMutex + binVersion rktVersion + apiVersion rktVersion + appcVersion rktVersion + systemdVersion systemdVersion +} + // rktVersion implementes kubecontainer.Version interface by implementing // Compare() and String() (which is implemented by the underlying semver.Version) type rktVersion struct { @@ -54,22 +63,16 @@ func (r rktVersion) Compare(other string) (int, error) { return 0, nil } -// 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. +func (r *Runtime) getVersions() error { + r.versions.Lock() + defer r.versions.Unlock() + + // Get systemd version. var err error - r.systemdVersion, err = r.systemd.Version() + r.versions.systemdVersion, err = r.systemd.Version() if err != nil { return err } - result, err := r.systemdVersion.Compare(minimumSystemdVersion) - if err != nil { - return err - } - 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" @@ -78,51 +81,78 @@ func (r *Runtime) checkVersion(minimumRktBinVersion, recommendedRktBinVersion, m return err } - // Check rkt binary version. - r.binVersion, err = newRktVersion(resp.Info.RktVersion) + // Get rkt binary version. + r.versions.binVersion, err = newRktVersion(resp.Info.RktVersion) if err != nil { return err } - result, err = r.binVersion.Compare(minimumRktBinVersion) + + // Get Appc version. + r.versions.appcVersion, err = newRktVersion(resp.Info.AppcVersion) + if err != nil { + return err + } + + // Get rkt API version. + r.versions.apiVersion, err = newRktVersion(resp.Info.ApiVersion) + if err != nil { + return err + } + return nil +} + +// 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 { + if err := r.getVersions(); err != nil { + return err + } + + r.versions.RLock() + defer r.versions.RUnlock() + + // Check systemd version. + result, err := r.versions.systemdVersion.Compare(minimumSystemdVersion) 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) + return fmt.Errorf("rkt: systemd version(%v) is too old, requires at least %v", r.versions.systemdVersion, minimumSystemdVersion) } - result, err = r.binVersion.Compare(recommendedRktBinVersion) + + // Check rkt binary version. + result, err = r.versions.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", r.versions.binVersion, minimumRktBinVersion) + } + result, err = r.versions.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) + glog.Warningf("rkt: current binary version %q is not recommended (recommended version %q)", r.versions.binVersion, recommendedRktBinVersion) } // Check Appc version. - r.appcVersion, err = newRktVersion(resp.Info.AppcVersion) - if err != nil { - return err - } - result, err = r.appcVersion.Compare(minimumAppcVersion) + result, err = r.versions.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) + return fmt.Errorf("rkt: appc version is too old(%v), requires at least %v", r.versions.appcVersion, minimumAppcVersion) } // Check rkt API version. - r.apiVersion, err = newRktVersion(resp.Info.ApiVersion) - if err != nil { - return err - } - result, err = r.apiVersion.Compare(minimumRktApiVersion) + result, err = r.versions.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 fmt.Errorf("rkt: API version is too old(%v), requires at least %v", r.versions.apiVersion, minimumRktApiVersion) } return nil }