Merge pull request #9973 from olemarkus/cleanup-cloud

Build cloud outside of PerformAssignments

cmd/kops/create.go

@@ -144,9 +144,14 @@ func RunCreate(ctx context.Context, f *util.Factory, out io.Writer, c *CreateOpt
 
 			switch v := o.(type) {
 			case *kopsapi.Cluster:
+				cloud, err := cloudup.BuildCloud(v)
+				if err != nil {
+					return err
+				}
+
 				// Adding a PerformAssignments() call here as the user might be trying to use
 				// the new `-f` feature, with an old cluster definition.
-				err = cloudup.PerformAssignments(v)
+				err = cloudup.PerformAssignments(v, cloud)
 				if err != nil {
 					return fmt.Errorf("error populating configuration: %v", err)
 				}

cmd/kops/create_cluster.go

@@ -493,7 +493,12 @@ func RunCreateCluster(ctx context.Context, f *util.Factory, out io.Writer, c *Cr
 		return err
 	}
 
-	err = cloudup.PerformAssignments(cluster)
+	cloud, err := cloudup.BuildCloud(cluster)
+	if err != nil {
+		return err
+	}
+
+	err = cloudup.PerformAssignments(cluster, cloud)
 	if err != nil {
 		return fmt.Errorf("error populating configuration: %v", err)
 	}

cmd/kops/edit_cluster.go

@@ -206,7 +206,12 @@ func RunEditCluster(ctx context.Context, f *util.Factory, cmd *cobra.Command, ar
 			continue
 		}
 
-		err = cloudup.PerformAssignments(newCluster)
+		cloud, err := cloudup.BuildCloud(newCluster)
+		if err != nil {
+			return err
+		}
+
+		err = cloudup.PerformAssignments(newCluster, cloud)
 		if err != nil {
 			return preservedFile(fmt.Errorf("error populating configuration: %v", err), file, out)
 		}
@@ -222,11 +227,6 @@ func RunEditCluster(ctx context.Context, f *util.Factory, cmd *cobra.Command, ar
 			continue
 		}
 
-		cloud, err := cloudup.BuildCloud(fullCluster)
-		if err != nil {
-			return err
-		}
-
 		err = validation.DeepValidate(fullCluster, instanceGroups, true, cloud)
 		if err != nil {
 			results = editResults{

cmd/kops/edit_instancegroup.go

@@ -160,9 +160,14 @@ func RunEditInstanceGroup(ctx context.Context, f *util.Factory, cmd *cobra.Comma
 		return err
 	}
 
+	cloud, err := cloudup.BuildCloud(cluster)
+	if err != nil {
+		return err
+	}
+
 	// We need the full cluster spec to perform deep validation
 	// Note that we don't write it back though
-	err = cloudup.PerformAssignments(cluster)
+	err = cloudup.PerformAssignments(cluster, cloud)
 	if err != nil {
 		return fmt.Errorf("error populating configuration: %v", err)
 	}
@@ -173,11 +178,6 @@ func RunEditInstanceGroup(ctx context.Context, f *util.Factory, cmd *cobra.Comma
 		return err
 	}
 
-	cloud, err := cloudup.BuildCloud(fullCluster)
-	if err != nil {
-		return err
-	}
-
 	err = validation.CrossValidateInstanceGroup(fullGroup, fullCluster, cloud).ToAggregate()
 	if err != nil {
 		return err

examples/kops-api-example/up.go

@@ -65,11 +65,16 @@ func up(ctx context.Context) error {
 		cluster.Spec.EtcdClusters = append(cluster.Spec.EtcdClusters, etcdCluster)
 	}
 
-	if err := cloudup.PerformAssignments(cluster); err != nil {
+	cloud, err := cloudup.BuildCloud(cluster)
+	if err != nil {
 		return err
 	}
 
-	_, err := clientset.CreateCluster(ctx, cluster)
+	if err := cloudup.PerformAssignments(cluster, cloud); err != nil {
+		return err
+	}
+
+	_, err = clientset.CreateCluster(ctx, cluster)
 	if err != nil {
 		return err
 	}

nodeup/pkg/model/kubelet_test.go

@@ -296,8 +296,13 @@ func RunGoldenTest(t *testing.T, basedir string, key string, builder func(*Nodeu
 	nodeupModelContext.KeyStore = keystore
 
 	// Populate the cluster
+	cloud, err := cloudup.BuildCloud(nodeupModelContext.Cluster)
+	if err != nil {
+		t.Fatalf("error from BuildCloud: %v", err)
+	}
+
 	{
-		err := cloudup.PerformAssignments(nodeupModelContext.Cluster)
+		err := cloudup.PerformAssignments(nodeupModelContext.Cluster, cloud)
 		if err != nil {
 			t.Fatalf("error from PerformAssignments: %v", err)
 		}

pkg/commands/helpers_readwrite.go

@@ -30,7 +30,12 @@ import (
 
 // UpdateCluster writes the updated cluster to the state store, after performing validation
 func UpdateCluster(ctx context.Context, clientset simple.Clientset, cluster *kops.Cluster, instanceGroups []*kops.InstanceGroup) error {
-	err := cloudup.PerformAssignments(cluster)
+	cloud, err := cloudup.BuildCloud(cluster)
+	if err != nil {
+		return err
+	}
+
+	err = cloudup.PerformAssignments(cluster, cloud)
 	if err != nil {
 		return fmt.Errorf("error populating configuration: %v", err)
 	}

upup/pkg/fi/cloudup/bootstrapchannelbuilder_test.go

@@ -79,7 +79,12 @@ func runChannelBuilderTest(t *testing.T, key string, addonManifests []string) {
 	}
 	cluster := obj.(*kopsapi.Cluster)
 
-	if err := PerformAssignments(cluster); err != nil {
+	cloud, err := BuildCloud(cluster)
+	if err != nil {
+		t.Fatalf("error from BuildCloud: %v", err)
+	}
+
+	if err := PerformAssignments(cluster, cloud); err != nil {
 		t.Fatalf("error from PerformAssignments for %q: %v", key, err)
 	}
 

upup/pkg/fi/cloudup/defaults.go

@@ -23,6 +23,7 @@ import (
 
 	"k8s.io/klog/v2"
 	"k8s.io/kops/pkg/apis/kops"
+	"k8s.io/kops/upup/pkg/fi"
 	"k8s.io/kops/upup/pkg/fi/cloudup/gce"
 	"k8s.io/kops/util/pkg/vfs"
 
@@ -38,12 +39,7 @@ import (
 // any time Run() is called in apply_cluster.go we will reach this function.
 // Please do all after-market logic here.
 //
-func PerformAssignments(c *kops.Cluster) error {
-	cloud, err := BuildCloud(c)
-	if err != nil {
-		return err
-	}
-
+func PerformAssignments(c *kops.Cluster, cloud fi.Cloud) error {
 	// Topology support
 	// TODO Kris: Unsure if this needs to be here, or if the API conversion code will handle it
 	if c.Spec.Topology == nil {
@@ -103,16 +99,17 @@ func PerformAssignments(c *kops.Cluster) error {
 	pd := cloud.ProviderID()
 	if pd == kops.CloudProviderAWS || pd == kops.CloudProviderOpenstack || pd == kops.CloudProviderALI {
 		// TODO: Use vpcInfo
-		err = assignCIDRsToSubnets(c)
+		err := assignCIDRsToSubnets(c)
 		if err != nil {
 			return err
 		}
 	}
 
-	c.Spec.EgressProxy, err = assignProxy(c)
+	proxy, err := assignProxy(c)
 	if err != nil {
 		return err
 	}
+	c.Spec.EgressProxy = proxy
 
 	return ensureKubernetesVersion(c)
 }

upup/pkg/fi/cloudup/populate_cluster_spec.go

@@ -98,7 +98,12 @@ func (c *populateClusterSpec) run(clientset simple.Clientset) error {
 		return err
 	}
 
-	err = PerformAssignments(cluster)
+	cloud, err := BuildCloud(cluster)
+	if err != nil {
+		return err
+	}
+
+	err = PerformAssignments(cluster, cloud)
 	if err != nil {
 		return err
 	}
@@ -219,11 +224,6 @@ func (c *populateClusterSpec) run(clientset simple.Clientset) error {
 		klog.V(2).Infof("Normalizing kubernetes version: %q -> %q", cluster.Spec.KubernetesVersion, versionWithoutV)
 		cluster.Spec.KubernetesVersion = versionWithoutV
 	}
-	cloud, err := BuildCloud(cluster)
-	if err != nil {
-		return err
-	}
-
 	if cluster.Spec.DNSZone == "" && !dns.IsGossipHostname(cluster.ObjectMeta.Name) {
 		dns, err := cloud.DNS()
 		if err != nil {

upup/pkg/fi/cloudup/populatecluster_test.go

@@ -40,8 +40,12 @@ func buildMinimalCluster() *kopsapi.Cluster {
 }
 func TestPopulateCluster_Default_NoError(t *testing.T) {
 	c := buildMinimalCluster()
+	cloud, err := BuildCloud(c)
+	if err != nil {
+		t.Fatalf("error from BuildCloud: %v", err)
+	}
 
-	err := PerformAssignments(c)
+	err = PerformAssignments(c, cloud)
 	if err != nil {
 		t.Fatalf("error from PerformAssignments: %v", err)
 	}
@@ -73,8 +77,12 @@ func TestPopulateCluster_Docker_Spec(t *testing.T) {
 		RegistryMirrors:    []string{"https://registry.example.com"},
 		LogOpt:             []string{"env=FOO"},
 	}
+	cloud, err := BuildCloud(c)
+	if err != nil {
+		t.Fatalf("error from BuildCloud: %v", err)
+	}
 
-	err := PerformAssignments(c)
+	err = PerformAssignments(c, cloud)
 	if err != nil {
 		t.Fatalf("error from PerformAssignments: %v", err)
 	}
@@ -107,7 +115,12 @@ func TestPopulateCluster_Docker_Spec(t *testing.T) {
 
 func TestPopulateCluster_StorageDefault(t *testing.T) {
 	c := buildMinimalCluster()
-	err := PerformAssignments(c)
+	cloud, err := BuildCloud(c)
+	if err != nil {
+		t.Fatalf("error from BuildCloud: %v", err)
+	}
+
+	err = PerformAssignments(c, cloud)
 	if err != nil {
 		t.Fatalf("error from PerformAssignments: %v", err)
 	}
@@ -123,7 +136,12 @@ func TestPopulateCluster_StorageDefault(t *testing.T) {
 }
 
 func build(c *kopsapi.Cluster) (*kopsapi.Cluster, error) {
-	err := PerformAssignments(c)
+	cloud, err := BuildCloud(c)
+	if err != nil {
+		return nil, fmt.Errorf("error from BuildCloud: %v", err)
+	}
+
+	err = PerformAssignments(c, cloud)
 	if err != nil {
 		return nil, fmt.Errorf("error from PerformAssignments: %v", err)
 	}
@@ -189,7 +207,12 @@ func TestPopulateCluster_Custom_CIDR(t *testing.T) {
 		{Name: "subnet-us-mock-1c", Zone: "us-mock-1c", CIDR: "172.20.2.64/27"},
 	}
 
-	err := PerformAssignments(c)
+	cloud, err := BuildCloud(c)
+	if err != nil {
+		t.Fatalf("error from BuildCloud: %v", err)
+	}
+
+	err = PerformAssignments(c, cloud)
 	if err != nil {
 		t.Fatalf("error from PerformAssignments: %v", err)
 	}
@@ -206,8 +229,12 @@ func TestPopulateCluster_Custom_CIDR(t *testing.T) {
 func TestPopulateCluster_IsolateMasters(t *testing.T) {
 	c := buildMinimalCluster()
 	c.Spec.IsolateMasters = fi.Bool(true)
+	cloud, err := BuildCloud(c)
+	if err != nil {
+		t.Fatalf("error from BuildCloud: %v", err)
+	}
 
-	err := PerformAssignments(c)
+	err = PerformAssignments(c, cloud)
 	if err != nil {
 		t.Fatalf("error from PerformAssignments: %v", err)
 	}
@@ -228,7 +255,12 @@ func TestPopulateCluster_IsolateMastersFalse(t *testing.T) {
 	c := buildMinimalCluster()
 	// default: c.Spec.IsolateMasters = fi.Bool(false)
 
-	err := PerformAssignments(c)
+	cloud, err := BuildCloud(c)
+	if err != nil {
+		t.Fatalf("error from BuildCloud: %v", err)
+	}
+
+	err = PerformAssignments(c, cloud)
 	if err != nil {
 		t.Fatalf("error from PerformAssignments: %v", err)
 	}
@@ -346,7 +378,12 @@ func TestPopulateCluster_AnonymousAuth(t *testing.T) {
 	c := buildMinimalCluster()
 	c.Spec.KubernetesVersion = "1.15.0"
 
-	err := PerformAssignments(c)
+	cloud, err := BuildCloud(c)
+	if err != nil {
+		t.Fatalf("error from BuildCloud: %v", err)
+	}
+
+	err = PerformAssignments(c, cloud)
 	if err != nil {
 		t.Fatalf("error from PerformAssignments: %v", err)
 	}
@@ -411,7 +448,12 @@ func TestPopulateCluster_KubeController_High_Enough_Version(t *testing.T) {
 	c := buildMinimalCluster()
 	c.Spec.KubernetesVersion = "v1.9.0"
 
-	err := PerformAssignments(c)
+	cloud, err := BuildCloud(c)
+	if err != nil {
+		t.Fatalf("error from BuildCloud: %v", err)
+	}
+
+	err = PerformAssignments(c, cloud)
 	if err != nil {
 		t.Fatalf("error from PerformAssignments: %v", err)
 	}

upup/pkg/fi/cloudup/validation_test.go

@@ -32,7 +32,12 @@ const MockAWSRegion = "us-mock-1"
 func buildDefaultCluster(t *testing.T) *api.Cluster {
 	c := buildMinimalCluster()
 
-	err := PerformAssignments(c)
+	cloud, err := BuildCloud(c)
+	if err != nil {
+		t.Fatalf("error from BuildCloud: %v", err)
+	}
+
+	err = PerformAssignments(c, cloud)
 	if err != nil {
 		t.Fatalf("error from PerformAssignments: %v", err)
 	}

upup/pkg/kutil/convert_kubeup_cluster.go

@@ -97,7 +97,7 @@ func (x *ConvertKubeupCluster) Upgrade(ctx context.Context) error {
 		}
 	}
 
-	err = cloudup.PerformAssignments(cluster)
+	err = cloudup.PerformAssignments(cluster, awsCloud)
 	if err != nil {
 		return fmt.Errorf("error populating cluster defaults: %v", err)
 	}