Use IsKubernetesGTE helper instead of reparsing semver

nodeup/pkg/model/context.go

@@ -20,28 +20,40 @@ import (
 	"fmt"
 	"path/filepath"
 
+	"github.com/blang/semver"
+	"github.com/golang/glog"
 	"k8s.io/kops/nodeup/pkg/distros"
 	"k8s.io/kops/pkg/apis/kops"
 	"k8s.io/kops/pkg/apis/kops/util"
 	"k8s.io/kops/pkg/apis/nodeup"
 	"k8s.io/kops/pkg/kubeconfig"
 	"k8s.io/kops/upup/pkg/fi"
-
-	"github.com/blang/semver"
 )
 
 // NodeupModelContext is the context supplied the nodeup tasks
 type NodeupModelContext struct {
-	Architecture      Architecture
-	Assets            *fi.AssetStore
-	Cluster           *kops.Cluster
-	Distribution      distros.Distribution
-	InstanceGroup     *kops.InstanceGroup
-	IsMaster          bool
-	KeyStore          fi.CAStore
-	KubernetesVersion semver.Version
-	NodeupConfig      *nodeup.Config
-	SecretStore       fi.SecretStore
+	Architecture  Architecture
+	Assets        *fi.AssetStore
+	Cluster       *kops.Cluster
+	Distribution  distros.Distribution
+	InstanceGroup *kops.InstanceGroup
+	IsMaster      bool
+	KeyStore      fi.CAStore
+	NodeupConfig  *nodeup.Config
+	SecretStore   fi.SecretStore
+
+	kubernetesVersion semver.Version
+}
+
+// Init completes initialization of the object, for example pre-parsing the kubernetes version
+func (c *NodeupModelContext) Init() error {
+	k8sVersion, err := util.ParseKubernetesVersion(c.Cluster.Spec.KubernetesVersion)
+	if err != nil || k8sVersion == nil {
+		return fmt.Errorf("unable to parse KubernetesVersion %q", c.Cluster.Spec.KubernetesVersion)
+	}
+	c.kubernetesVersion = *k8sVersion
+
+	return nil
 }
 
 // SSLHostPaths returns the TLS paths for the distribution
@@ -181,7 +193,10 @@ func (c *NodeupModelContext) buildPKIKubeconfig(id string) (string, error) {
 
 // IsKubernetesGTE checks if the version is greater-than-or-equal
 func (c *NodeupModelContext) IsKubernetesGTE(version string) bool {
-	return util.IsKubernetesGTE(version, c.KubernetesVersion)
+	if c.kubernetesVersion.Major == 0 {
+		glog.Fatalf("kubernetesVersion not set (%s); Init not called", c.kubernetesVersion)
+	}
+	return util.IsKubernetesGTE(version, c.kubernetesVersion)
 }
 
 // UseEtcdTLS checks if the etcd cluster has TLS enabled bool

nodeup/pkg/model/kubelet.go

@@ -23,14 +23,12 @@ import (
 	"k8s.io/client-go/pkg/api/v1"
 	"k8s.io/kops/nodeup/pkg/distros"
 	"k8s.io/kops/pkg/apis/kops"
-	"k8s.io/kops/pkg/apis/kops/util"
 	"k8s.io/kops/pkg/flagbuilder"
 	"k8s.io/kops/pkg/systemd"
 	"k8s.io/kops/upup/pkg/fi"
 	"k8s.io/kops/upup/pkg/fi/nodeup/nodetasks"
 	"k8s.io/kops/upup/pkg/fi/utils"
 
-	"github.com/blang/semver"
 	"github.com/golang/glog"
 )
 
@@ -259,11 +257,6 @@ const RoleLabelNode16 = "node-role.kubernetes.io/node"
 
 // buildKubeletConfigSpec returns the kubeletconfig for the specified instanceGroup
 func (b *KubeletBuilder) buildKubeletConfigSpec() (*kops.KubeletConfigSpec, error) {
-	sv, err := util.ParseKubernetesVersion(b.Cluster.Spec.KubernetesVersion)
-	if err != nil {
-		return nil, fmt.Errorf("Failed to lookup kubernetes version: %v", err)
-	}
-
 	// Merge KubeletConfig for NodeLabels
 	c := &kops.KubeletConfigSpec{}
 	if b.InstanceGroup.Spec.Role == kops.InstanceGroupRoleMaster {
@@ -303,10 +296,8 @@ func (b *KubeletBuilder) buildKubeletConfigSpec() (*kops.KubeletConfigSpec, erro
 		c.NodeLabels[k] = v
 	}
 
-	// --register-with-taints was available in the first 1.6.0 alpha, no need to rely on semver's pre/build ordering
-	sv.Pre = nil
-	sv.Build = nil
-	if sv.GTE(semver.Version{Major: 1, Minor: 6, Patch: 0, Pre: nil, Build: nil}) {
+	// Use --register-with-taints for k8s 1.6 and on
+	if b.IsKubernetesGTE("1.6") {
 		for _, t := range b.InstanceGroup.Spec.Taints {
 			c.Taints = append(c.Taints, t)
 		}

nodeup/pkg/model/kubelet_test.go

@@ -50,6 +50,10 @@ func Test_InstanceGroupKubeletMerge(t *testing.T) {
 			InstanceGroup: instanceGroup,
 		},
 	}
+	if err := b.Init(); err != nil {
+		t.Error(err)
+	}
+
 	var mergedKubeletSpec, err = b.buildKubeletConfigSpec()
 	if err != nil {
 		t.Error(err)
@@ -108,6 +112,10 @@ func TestTaintsAppliedAfter160(t *testing.T) {
 				InstanceGroup: ig,
 			},
 		}
+		if err := b.Init(); err != nil {
+			t.Error(err)
+		}
+
 		c, err := b.buildKubeletConfigSpec()
 
 		if g.expectError {
@@ -123,7 +131,7 @@ func TestTaintsAppliedAfter160(t *testing.T) {
 		}
 
 		if fi.BoolValue(c.RegisterSchedulable) != g.expectSchedulable {
-			t.Fatalf("Expected RegisterSchedulable == %v, got %v", g.expectSchedulable, fi.BoolValue(c.RegisterSchedulable))
+			t.Fatalf("Expected RegisterSchedulable == %v, got %v (for %v)", g.expectSchedulable, fi.BoolValue(c.RegisterSchedulable), g.version)
 		}
 
 		if !stringSlicesEqual(g.expectTaints, c.Taints) {
@@ -226,6 +234,9 @@ func LoadModel(basedir string) (*NodeupModelContext, error) {
 		Distribution:  distros.DistributionXenial,
 		InstanceGroup: instanceGroup,
 	}
+	if err := nodeUpModelContext.Init(); err != nil {
+		return nil, err
+	}
 
 	return nodeUpModelContext, nil
 }

upup/pkg/fi/nodeup/command.go

@@ -25,11 +25,12 @@ import (
 	"strings"
 	"time"
 
+	"github.com/golang/glog"
+	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/kops/nodeup/pkg/distros"
 	"k8s.io/kops/nodeup/pkg/model"
 	api "k8s.io/kops/pkg/apis/kops"
 	"k8s.io/kops/pkg/apis/kops/registry"
-	"k8s.io/kops/pkg/apis/kops/util"
 	"k8s.io/kops/pkg/apis/nodeup"
 	"k8s.io/kops/pkg/assets"
 	"k8s.io/kops/upup/pkg/fi"
@@ -38,9 +39,6 @@ import (
 	"k8s.io/kops/upup/pkg/fi/nodeup/nodetasks"
 	"k8s.io/kops/upup/pkg/fi/utils"
 	"k8s.io/kops/util/pkg/vfs"
-
-	"github.com/golang/glog"
-	"k8s.io/apimachinery/pkg/util/sets"
 )
 
 // We should probably retry for a long time - there is not really any great fallback
@@ -178,22 +176,19 @@ func (c *NodeUpCommand) Run(out io.Writer) error {
 		return fmt.Errorf("error initializing: %v", err)
 	}
 
-	k8sVersion, err := util.ParseKubernetesVersion(c.cluster.Spec.KubernetesVersion)
-	if err != nil || k8sVersion == nil {
-		return fmt.Errorf("unable to parse KubernetesVersion %q", c.cluster.Spec.KubernetesVersion)
-	}
-
 	modelContext := &model.NodeupModelContext{
-		Architecture:      model.ArchitectureAmd64,
-		Assets:            assetStore,
-		Cluster:           c.cluster,
-		Distribution:      distribution,
-		InstanceGroup:     c.instanceGroup,
-		IsMaster:          nodeTags.Has(TagMaster),
-		KeyStore:          tf.keyStore,
-		KubernetesVersion: *k8sVersion,
-		NodeupConfig:      c.config,
-		SecretStore:       tf.secretStore,
+		Architecture:  model.ArchitectureAmd64,
+		Assets:        assetStore,
+		Cluster:       c.cluster,
+		Distribution:  distribution,
+		InstanceGroup: c.instanceGroup,
+		IsMaster:      nodeTags.Has(TagMaster),
+		KeyStore:      tf.keyStore,
+		NodeupConfig:  c.config,
+		SecretStore:   tf.secretStore,
+	}
+	if err := modelContext.Init(); err != nil {
+		return err
 	}
 
 	loader := NewLoader(c.config, c.cluster, assetStore, nodeTags)