rkt: Split checkVersion() to two parts: get version, and check version.

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.

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()

pkg/kubelet/rkt/version.go

@@ -18,13 +18,22 @@ package rkt
 
 import (
 	"fmt"
+	"sync"
 
 	"github.com/coreos/go-semver/semver"
 	rktapi "github.com/coreos/rkt/api/v1alpha"
 	"github.com/golang/glog"
 	"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,75 +63,96 @@ 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)
+
+	// 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
 	}
-	if result < 0 {
-		return fmt.Errorf("rkt: systemd version(%v) is too old, requires at least %v", r.systemdVersion, minimumSystemdVersion)
+
+	// Get rkt binary version.
+	r.versions.binVersion, err = newRktVersion(resp.Info.RktVersion)
+	if err != nil {
+		return err
 	}
 
-	// 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{})
+	// Get Appc version.
+	r.versions.appcVersion, err = newRktVersion(resp.Info.AppcVersion)
 	if err != nil {
 		return err
 	}
 
-	// Check rkt binary version.
-	r.binVersion, err = newRktVersion(resp.Info.RktVersion)
+	// Get rkt API version.
+	r.versions.apiVersion, err = newRktVersion(resp.Info.ApiVersion)
 	if err != nil {
 		return err
 	}
-	result, err = r.binVersion.Compare(minimumRktBinVersion)
+	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
 }