diff --git a/cmd/kops/create.go b/cmd/kops/create.go index 8f3548e699..fbaaad274b 100644 --- a/cmd/kops/create.go +++ b/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) } diff --git a/cmd/kops/create_cluster.go b/cmd/kops/create_cluster.go index f23d952fd8..db0c757809 100644 --- a/cmd/kops/create_cluster.go +++ b/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) } diff --git a/cmd/kops/edit_cluster.go b/cmd/kops/edit_cluster.go index 565800c870..de4fa2451a 100644 --- a/cmd/kops/edit_cluster.go +++ b/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{ diff --git a/cmd/kops/edit_instancegroup.go b/cmd/kops/edit_instancegroup.go index 0edb7e0c7b..f4e22eaad3 100644 --- a/cmd/kops/edit_instancegroup.go +++ b/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 diff --git a/examples/kops-api-example/up.go b/examples/kops-api-example/up.go index 6253795770..d42912cdf5 100644 --- a/examples/kops-api-example/up.go +++ b/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 } diff --git a/nodeup/pkg/model/kubelet_test.go b/nodeup/pkg/model/kubelet_test.go index 590b2c2fee..920502ff61 100644 --- a/nodeup/pkg/model/kubelet_test.go +++ b/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) } diff --git a/pkg/commands/helpers_readwrite.go b/pkg/commands/helpers_readwrite.go index 4a91e20caf..2e45e11bb1 100644 --- a/pkg/commands/helpers_readwrite.go +++ b/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) } diff --git a/upup/pkg/fi/cloudup/bootstrapchannelbuilder_test.go b/upup/pkg/fi/cloudup/bootstrapchannelbuilder_test.go index 5c0a84086e..b094369a1a 100644 --- a/upup/pkg/fi/cloudup/bootstrapchannelbuilder_test.go +++ b/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) } diff --git a/upup/pkg/fi/cloudup/defaults.go b/upup/pkg/fi/cloudup/defaults.go index fe9097263e..535a0a9e12 100644 --- a/upup/pkg/fi/cloudup/defaults.go +++ b/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) } diff --git a/upup/pkg/fi/cloudup/populate_cluster_spec.go b/upup/pkg/fi/cloudup/populate_cluster_spec.go index 7553d7102d..8a0653d44c 100644 --- a/upup/pkg/fi/cloudup/populate_cluster_spec.go +++ b/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 { diff --git a/upup/pkg/fi/cloudup/populatecluster_test.go b/upup/pkg/fi/cloudup/populatecluster_test.go index cb33c8f811..0318b78132 100644 --- a/upup/pkg/fi/cloudup/populatecluster_test.go +++ b/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) } diff --git a/upup/pkg/fi/cloudup/validation_test.go b/upup/pkg/fi/cloudup/validation_test.go index db9190dc89..a0270d1ad8 100644 --- a/upup/pkg/fi/cloudup/validation_test.go +++ b/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) } diff --git a/upup/pkg/kutil/convert_kubeup_cluster.go b/upup/pkg/kutil/convert_kubeup_cluster.go index 8386c9b6c0..85827f0e82 100644 --- a/upup/pkg/kutil/convert_kubeup_cluster.go +++ b/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) }