Remove some rather long networking nil checks

cmd/kops/create_cluster.go

@@ -1109,7 +1109,7 @@ func RunCreateCluster(ctx context.Context, f *util.Factory, out io.Writer, c *Cr
 
 	case api.TopologyPrivate:
 		if !supportsPrivateTopology(cluster.Spec.Networking) {
-			return fmt.Errorf("Invalid networking option %s. Currently only '--networking kopeio-vxlan (or kopeio)', '--networking weave', '--networking flannel', '--networking calico', '--networking canal', '--networking kube-router', '--networking romana', '--networking amazon-vpc-routed-eni', '--networking cilium', '--networking lyftvpc', '--networking cni' are supported for private topologies", c.Networking)
+			return fmt.Errorf("invalid networking option %s. Kubenet does not support private topology", c.Networking)
 		}
 		cluster.Spec.Topology = &api.TopologySpec{
 			Masters: api.TopologyPrivate,
@@ -1452,11 +1452,7 @@ func RunCreateCluster(ctx context.Context, f *util.Factory, out io.Writer, c *Cr
 }
 
 func supportsPrivateTopology(n *api.NetworkingSpec) bool {
-
+	return n.Kubenet == nil
-	if n.CNI != nil || n.Kopeio != nil || n.Weave != nil || n.Flannel != nil || n.Calico != nil || n.Canal != nil || n.Kuberouter != nil || n.Romana != nil || n.AmazonVPC != nil || n.Cilium != nil || n.LyftVPC != nil || n.GCE != nil {
-		return true
-	}
-	return false
 }
 
 func trimCommonPrefix(names []string) []string {

nodeup/pkg/model/context.go

@@ -343,16 +343,6 @@ func (c *NodeupModelContext) UseEtcdTLSAuth() bool {
 	return false
 }
 
-// UsesCNI checks if the cluster has CNI configured
-func (c *NodeupModelContext) UsesCNI() bool {
-	networking := c.Cluster.Spec.Networking
-	if networking == nil || networking.Classic != nil {
-		return false
-	}
-
-	return true
-}
-
 // UseNodeAuthorization checks if have a node authorization policy
 func (c *NodeupModelContext) UseNodeAuthorization() bool {
 	return c.Cluster.Spec.NodeAuthorization != nil

nodeup/pkg/model/kubelet.go

@@ -25,6 +25,8 @@ import (
 	"strings"
 	"time"
 
+	"k8s.io/kops/pkg/model/components"
+
 	"github.com/aws/aws-sdk-go/aws/ec2metadata"
 	"github.com/aws/aws-sdk-go/aws/session"
 
@@ -134,7 +136,7 @@ func (b *KubeletBuilder) Build(c *fi.ModelBuilderContext) error {
 		}
 	}
 
-	if b.UsesCNI() {
+	if components.UsesCNI(b.Cluster.Spec.Networking) {
 		c.AddTask(&nodetasks.File{
 			Path: b.CNIConfDir(),
 			Type: nodetasks.FileType_Directory,
@@ -212,10 +214,8 @@ func (b *KubeletBuilder) buildSystemdEnvironmentFile(kubeletConfig *kops.Kubelet
 		flags += " --cloud-config=" + CloudConfigFilePath
 	}
 
-	if b.UsesCNI() {
+	flags += " --cni-bin-dir=" + b.CNIBinDir()
-		flags += " --cni-bin-dir=" + b.CNIBinDir()
+	flags += " --cni-conf-dir=" + b.CNIConfDir()
-		flags += " --cni-conf-dir=" + b.CNIConfDir()
-	}
 
 	if b.UsesSecondaryIP() {
 		sess := session.Must(session.NewSession())

pkg/model/components/context.go

@@ -76,26 +76,31 @@ func KubernetesVersion(clusterSpec *kops.ClusterSpec) (*semver.Version, error) {
 }
 
 // UsesKubenet returns true if our networking is derived from kubenet
-func UsesKubenet(clusterSpec *kops.ClusterSpec) (bool, error) {
+func UsesKubenet(networking *kops.NetworkingSpec) bool {
-	networking := clusterSpec.Networking
+	if networking == nil {
-	if networking == nil || networking.Classic != nil {
+		panic("no networking mode set")
-		return false, nil
+	}
-	} else if networking.Kubenet != nil {
+	if networking.Kubenet != nil {
-		return true, nil
+		return true
 	} else if networking.GCE != nil {
 		// GCE IP Alias networking is based on kubenet
-		return true, nil
+		return true
 	} else if networking.External != nil {
 		// external is based on kubenet
-		return true, nil
+		return true
-	} else if networking.CNI != nil || networking.Weave != nil || networking.Flannel != nil || networking.Calico != nil || networking.Canal != nil || networking.Kuberouter != nil || networking.Romana != nil || networking.AmazonVPC != nil || networking.Cilium != nil || networking.LyftVPC != nil {
-		return false, nil
 	} else if networking.Kopeio != nil {
 		// Kopeio is based on kubenet / external
-		return true, nil
+		return true
-	} else {
-		return false, fmt.Errorf("no networking mode set")
 	}
+
+	return false
+
+}
+
+// UsesCNI returns true if the networking provider is a CNI plugin
+func UsesCNI(networking *kops.NetworkingSpec) bool {
+	// Kubenet and CNI are the only kubelet networking plugins right now.
+	return !UsesKubenet(networking)
 }
 
 func WellKnownServiceIP(clusterSpec *kops.ClusterSpec, id int) (net.IP, error) {

pkg/model/components/kubecontrollermanager.go

@@ -20,6 +20,8 @@ import (
 	"fmt"
 	"time"
 
+	"k8s.io/kops/pkg/model/components"
+
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/klog"
 	"k8s.io/kops/pkg/apis/kops"
@@ -140,7 +142,7 @@ func (b *KubeControllerManagerOptionsBuilder) BuildOptions(o interface{}) error
 		}
 	} else if networking.External != nil {
 		kcm.ConfigureCloudRoutes = fi.Bool(false)
-	} else if networking.CNI != nil || networking.Weave != nil || networking.Flannel != nil || networking.Calico != nil || networking.Canal != nil || networking.Kuberouter != nil || networking.Romana != nil || networking.AmazonVPC != nil || networking.Cilium != nil || networking.LyftVPC != nil {
+	} else if components.UsesCNI(networking) {
 		kcm.ConfigureCloudRoutes = fi.Bool(false)
 	} else if networking.Kopeio != nil {
 		// Kopeio is based on kubenet / external

pkg/model/components/kubelet.go

@@ -17,6 +17,7 @@ limitations under the License.
 package components
 
 import (
+	"fmt"
 	"strings"
 
 	"k8s.io/klog"
@@ -173,11 +174,12 @@ func (b *KubeletOptionsBuilder) BuildOptions(o interface{}) error {
 		clusterSpec.Kubelet.CloudProvider = "external"
 	}
 
-	usesKubenet, err := UsesKubenet(clusterSpec)
+	networking := clusterSpec.Networking
-	if err != nil {
+	if networking == nil {
-		return err
+		return fmt.Errorf("no networking mode set")
+
 	}
-	if usesKubenet {
+	if UsesKubenet(networking) {
 		clusterSpec.Kubelet.NetworkPluginName = "kubenet"
 
 		// AWS MTU is 9001

pkg/model/components/kubelet_test.go

@@ -29,6 +29,7 @@ func buildKubeletTestCluster() *kops.Cluster {
 			KubernetesVersion:     "1.6.2",
 			ServiceClusterIPRange: "10.10.0.0/16",
 			Kubelet:               &kops.KubeletConfigSpec{},
+			Networking:            &kops.NetworkingSpec{},
 		},
 	}
 }

pkg/model/components/networking.go

@@ -43,7 +43,7 @@ func (b *NetworkingOptionsBuilder) BuildOptions(o interface{}) error {
 		return fmt.Errorf("networking not set")
 	}
 
-	if networking.CNI != nil || networking.Weave != nil || networking.Flannel != nil || networking.Calico != nil || networking.Canal != nil || networking.Kuberouter != nil || networking.Romana != nil || networking.AmazonVPC != nil || networking.Cilium != nil || networking.LyftVPC != nil {
+	if UsesCNI(networking) {
 		options.Kubelet.NetworkPluginName = "cni"
 
 		// ConfigureCBR0 flag removed from 1.5

upup/pkg/fi/cloudup/apply_cluster.go

@@ -1200,15 +1200,13 @@ func (c *ApplyClusterCmd) AddFileAssets(assetBuilder *assets.AssetBuilder) error
 		c.Assets = append(c.Assets, BuildMirroredAsset(u, hash))
 	}
 
-	if usesCNI(c.Cluster) {
+	cniAsset, cniAssetHash, err := findCNIAssets(c.Cluster, assetBuilder)
-		cniAsset, cniAssetHash, err := findCNIAssets(c.Cluster, assetBuilder)
+	if err != nil {
-		if err != nil {
+		return err
-			return err
-		}
-
-		c.Assets = append(c.Assets, BuildMirroredAsset(cniAsset, cniAssetHash))
 	}
 
+	c.Assets = append(c.Assets, BuildMirroredAsset(cniAsset, cniAssetHash))
+
 	if c.Cluster.Spec.Networking.LyftVPC != nil {
 		var hash *hashing.Hash
 

upup/pkg/fi/cloudup/networking.go

@@ -28,88 +28,6 @@ import (
 	"k8s.io/kops/util/pkg/hashing"
 )
 
-func usesCNI(c *kopsapi.Cluster) bool {
-	networkConfig := c.Spec.Networking
-	if networkConfig == nil || networkConfig.Classic != nil {
-		// classic
-		return false
-	}
-
-	if networkConfig.Kubenet != nil {
-		// kubenet is now configured via CNI
-		return true
-	}
-
-	if networkConfig.GCE != nil {
-		// GCE is kubenet at the node level
-		return true
-	}
-
-	if networkConfig.External != nil {
-		// external: assume uses CNI
-		return true
-	}
-
-	if networkConfig.Kopeio != nil {
-		// Kopeio uses kubenet (and thus CNI)
-		return true
-	}
-
-	if networkConfig.Weave != nil {
-		//  Weave uses CNI
-		return true
-	}
-
-	if networkConfig.Flannel != nil {
-		//  Flannel uses CNI
-		return true
-	}
-
-	if networkConfig.Calico != nil {
-		//  Calico uses CNI
-		return true
-	}
-
-	if networkConfig.Canal != nil {
-		// Canal uses CNI
-		return true
-	}
-
-	if networkConfig.Kuberouter != nil {
-		// Kuberouter uses CNI
-		return true
-	}
-
-	if networkConfig.Romana != nil {
-		//  Romana uses CNI
-		return true
-	}
-
-	if networkConfig.AmazonVPC != nil {
-		//  AmazonVPC uses CNI
-		return true
-	}
-
-	if networkConfig.Cilium != nil {
-		// Cilium uses CNI
-		return true
-	}
-
-	if networkConfig.CNI != nil {
-		// CNI definitely uses CNI!
-		return true
-	}
-
-	if networkConfig.LyftVPC != nil {
-		// LyftVPC uses CNI
-		return true
-	}
-
-	// Assume other modes also use CNI
-	klog.Warningf("Unknown networking mode configured")
-	return true
-}
-
 // TODO: we really need to sort this out:
 // https://github.com/kubernetes/kops/issues/724
 // https://github.com/kubernetes/kops/issues/626