Skip to content

Commit

Permalink
Remove some rather long networking nil checks
Browse files Browse the repository at this point in the history
  • Loading branch information
Ole Markus With committed May 21, 2020
1 parent df90d61 commit b39b388
Show file tree
Hide file tree
Showing 9 changed files with 35 additions and 120 deletions.
8 changes: 2 additions & 6 deletions cmd/kops/create_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -1452,11 +1452,7 @@ func RunCreateCluster(ctx context.Context, f *util.Factory, out io.Writer, c *Cr
}

func supportsPrivateTopology(n *api.NetworkingSpec) bool {

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
return n.Kubenet == nil
}

func trimCommonPrefix(names []string) []string {
Expand Down
10 changes: 0 additions & 10 deletions nodeup/pkg/model/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 4 additions & 2 deletions nodeup/pkg/model/kubelet.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -212,7 +214,7 @@ func (b *KubeletBuilder) buildSystemdEnvironmentFile(kubeletConfig *kops.Kubelet
flags += " --cloud-config=" + CloudConfigFilePath
}

if b.UsesCNI() {
if components.UsesCNI(b.Cluster.Spec.Networking) {
flags += " --cni-bin-dir=" + b.CNIBinDir()
flags += " --cni-conf-dir=" + b.CNIConfDir()
}
Expand Down
31 changes: 18 additions & 13 deletions pkg/model/components/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
networking := clusterSpec.Networking
if networking == nil || networking.Classic != nil {
return false, nil
} else if networking.Kubenet != nil {
return true, nil
func UsesKubenet(networking *kops.NetworkingSpec) bool {
if networking == nil {
panic("no networking mode set")
}
if networking.Kubenet != 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
} 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
return true
} else if networking.Kopeio != nil {
// Kopeio is based on kubenet / external
return true, nil
} else {
return false, fmt.Errorf("no networking mode set")
return true
}

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) {
Expand Down
4 changes: 3 additions & 1 deletion pkg/model/components/kubecontrollermanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
10 changes: 6 additions & 4 deletions pkg/model/components/kubelet.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package components

import (
"fmt"
"strings"

"k8s.io/klog"
Expand Down Expand Up @@ -173,11 +174,12 @@ func (b *KubeletOptionsBuilder) BuildOptions(o interface{}) error {
clusterSpec.Kubelet.CloudProvider = "external"
}

usesKubenet, err := UsesKubenet(clusterSpec)
if err != nil {
return err
networking := clusterSpec.Networking
if networking == nil {
return fmt.Errorf("no networking mode set")

}
if usesKubenet {
if UsesKubenet(networking) {
clusterSpec.Kubelet.NetworkPluginName = "kubenet"

// AWS MTU is 9001
Expand Down
2 changes: 1 addition & 1 deletion pkg/model/components/networking.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion upup/pkg/fi/cloudup/apply_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -1200,7 +1200,7 @@ func (c *ApplyClusterCmd) AddFileAssets(assetBuilder *assets.AssetBuilder) error
c.Assets = append(c.Assets, BuildMirroredAsset(u, hash))
}

if usesCNI(c.Cluster) {
if components.UsesCNI(c.Cluster.Spec.Networking) {
cniAsset, cniAssetHash, err := findCNIAssets(c.Cluster, assetBuilder)
if err != nil {
return err
Expand Down
82 changes: 0 additions & 82 deletions upup/pkg/fi/cloudup/networking.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit b39b388

Please sign in to comment.