diff --git a/cmd/kops/create_cluster.go b/cmd/kops/create_cluster.go index 64066389dc..e18abb6280 100644 --- a/cmd/kops/create_cluster.go +++ b/cmd/kops/create_cluster.go @@ -510,14 +510,14 @@ func RunCreateCluster(ctx context.Context, f *util.Factory, out io.Writer, c *Cr } assetBuilder := assets.NewAssetBuilder(cluster, "") - fullCluster, err := cloudup.PopulateClusterSpec(clientset, cluster, assetBuilder) + fullCluster, err := cloudup.PopulateClusterSpec(clientset, cluster, cloud, assetBuilder) if err != nil { return err } var fullInstanceGroups []*api.InstanceGroup for _, group := range instanceGroups { - fullGroup, err := cloudup.PopulateInstanceGroupSpec(fullCluster, group, clusterResult.Channel) + fullGroup, err := cloudup.PopulateInstanceGroupSpec(fullCluster, group, cloud, clusterResult.Channel) if err != nil { return err } diff --git a/cmd/kops/create_ig.go b/cmd/kops/create_ig.go index ad79793f51..55fcf1ed40 100644 --- a/cmd/kops/create_ig.go +++ b/cmd/kops/create_ig.go @@ -160,7 +160,12 @@ func RunCreateInstanceGroup(ctx context.Context, f *util.Factory, cmd *cobra.Com ig.Spec.Subnets = options.Subnets - ig, err = cloudup.PopulateInstanceGroupSpec(cluster, ig, channel) + cloud, err := cloudup.BuildCloud(cluster) + if err != nil { + return err + } + + ig, err = cloudup.PopulateInstanceGroupSpec(cluster, ig, cloud, channel) if err != nil { return err } @@ -228,11 +233,6 @@ func RunCreateInstanceGroup(ctx context.Context, f *util.Factory, cmd *cobra.Com return fmt.Errorf("unexpected object type: %T", obj) } - cloud, err := cloudup.BuildCloud(cluster) - if err != nil { - return err - } - err = validation.CrossValidateInstanceGroup(group, cluster, cloud).ToAggregate() if err != nil { return err diff --git a/cmd/kops/edit_cluster.go b/cmd/kops/edit_cluster.go index de4fa2451a..5f9aaff42c 100644 --- a/cmd/kops/edit_cluster.go +++ b/cmd/kops/edit_cluster.go @@ -217,7 +217,7 @@ func RunEditCluster(ctx context.Context, f *util.Factory, cmd *cobra.Command, ar } assetBuilder := assets.NewAssetBuilder(newCluster, "") - fullCluster, err := cloudup.PopulateClusterSpec(clientset, newCluster, assetBuilder) + fullCluster, err := cloudup.PopulateClusterSpec(clientset, newCluster, cloud, assetBuilder) if err != nil { results = editResults{ file: file, diff --git a/cmd/kops/edit_instancegroup.go b/cmd/kops/edit_instancegroup.go index f4e22eaad3..685149c12d 100644 --- a/cmd/kops/edit_instancegroup.go +++ b/cmd/kops/edit_instancegroup.go @@ -155,12 +155,11 @@ func RunEditInstanceGroup(ctx context.Context, f *util.Factory, cmd *cobra.Comma return fmt.Errorf("object was not of expected type: %T", newObj) } - fullGroup, err := cloudup.PopulateInstanceGroupSpec(cluster, newGroup, channel) + cloud, err := cloudup.BuildCloud(cluster) if err != nil { return err } - - cloud, err := cloudup.BuildCloud(cluster) + fullGroup, err := cloudup.PopulateInstanceGroupSpec(cluster, newGroup, cloud, channel) if err != nil { return err } @@ -173,7 +172,7 @@ func RunEditInstanceGroup(ctx context.Context, f *util.Factory, cmd *cobra.Comma } assetBuilder := assets.NewAssetBuilder(cluster, "") - fullCluster, err := cloudup.PopulateClusterSpec(clientset, cluster, assetBuilder) + fullCluster, err := cloudup.PopulateClusterSpec(clientset, cluster, cloud, assetBuilder) if err != nil { return err } diff --git a/cmd/kops/toolbox_instance_selector.go b/cmd/kops/toolbox_instance_selector.go index 3fb32c1f38..cd9dc14510 100644 --- a/cmd/kops/toolbox_instance_selector.go +++ b/cmd/kops/toolbox_instance_selector.go @@ -278,7 +278,7 @@ func RunToolboxInstanceSelector(ctx context.Context, f *util.Factory, args []str if instanceSelectorOpts.ClusterAutoscaler { ig = decorateWithClusterAutoscalerLabels(ig, clusterName) } - ig, err = cloudup.PopulateInstanceGroupSpec(cluster, ig, channel) + ig, err = cloudup.PopulateInstanceGroupSpec(cluster, ig, cloud, channel) if err != nil { return err } diff --git a/cmd/kops/update_cluster.go b/cmd/kops/update_cluster.go index 1049c6278a..869601552b 100644 --- a/cmd/kops/update_cluster.go +++ b/cmd/kops/update_cluster.go @@ -266,7 +266,13 @@ func RunUpdateCluster(ctx context.Context, f *util.Factory, clusterName string, lifecycleOverrideMap[taskName] = lifecycleOverride } + cloud, err := cloudup.BuildCloud(cluster) + if err != nil { + return nil, err + } + applyCmd := &cloudup.ApplyClusterCmd{ + Cloud: cloud, Clientset: clientset, Cluster: cluster, DryRun: isDryrun, diff --git a/nodeup/pkg/model/kubelet_test.go b/nodeup/pkg/model/kubelet_test.go index 920502ff61..06cc956f9e 100644 --- a/nodeup/pkg/model/kubelet_test.go +++ b/nodeup/pkg/model/kubelet_test.go @@ -226,7 +226,7 @@ func BuildNodeupModelContext(basedir string) (*NodeupModelContext, error) { return nodeUpModelContext, nil } -func mockedPopulateClusterSpec(c *kops.Cluster) (*kops.Cluster, error) { +func mockedPopulateClusterSpec(c *kops.Cluster, cloud fi.Cloud) (*kops.Cluster, error) { vfs.Context.ResetMemfsContext(true) assetBuilder := assets.NewAssetBuilder(c, "") @@ -235,7 +235,7 @@ func mockedPopulateClusterSpec(c *kops.Cluster) (*kops.Cluster, error) { return nil, fmt.Errorf("error building vfspath: %v", err) } clientset := vfsclientset.NewVFSClientset(basePath) - return cloudup.PopulateClusterSpec(clientset, c, assetBuilder) + return cloudup.PopulateClusterSpec(clientset, c, cloud, assetBuilder) } // Fixed cert and key, borrowed from the create_kubecfg_test.go test @@ -307,7 +307,7 @@ func RunGoldenTest(t *testing.T, basedir string, key string, builder func(*Nodeu t.Fatalf("error from PerformAssignments: %v", err) } - full, err := mockedPopulateClusterSpec(nodeupModelContext.Cluster) + full, err := mockedPopulateClusterSpec(nodeupModelContext.Cluster, cloud) if err != nil { t.Fatalf("unexpected error from mockedPopulateClusterSpec: %v", err) } diff --git a/pkg/commands/helpers_readwrite.go b/pkg/commands/helpers_readwrite.go index 2e45e11bb1..aacef2869c 100644 --- a/pkg/commands/helpers_readwrite.go +++ b/pkg/commands/helpers_readwrite.go @@ -41,7 +41,7 @@ func UpdateCluster(ctx context.Context, clientset simple.Clientset, cluster *kop } assetBuilder := assets.NewAssetBuilder(cluster, "") - fullCluster, err := cloudup.PopulateClusterSpec(clientset, cluster, assetBuilder) + fullCluster, err := cloudup.PopulateClusterSpec(clientset, cluster, cloud, assetBuilder) if err != nil { return err } diff --git a/upup/pkg/fi/cloudup/apply_cluster.go b/upup/pkg/fi/cloudup/apply_cluster.go index 2537593a5c..8ca371e700 100644 --- a/upup/pkg/fi/cloudup/apply_cluster.go +++ b/upup/pkg/fi/cloudup/apply_cluster.go @@ -84,6 +84,7 @@ var ( ) type ApplyClusterCmd struct { + Cloud fi.Cloud Cluster *kops.Cluster InstanceGroups []*kops.InstanceGroup @@ -258,10 +259,7 @@ func (c *ApplyClusterCmd) Run(ctx context.Context) error { } } - cloud, err := BuildCloud(cluster) - if err != nil { - return err - } + cloud := c.Cloud err = validation.DeepValidate(c.Cluster, c.InstanceGroups, true, cloud) if err != nil { @@ -840,14 +838,14 @@ func (c *ApplyClusterCmd) Run(ctx context.Context) error { // upgradeSpecs ensures that fields are fully populated / defaulted func (c *ApplyClusterCmd) upgradeSpecs(assetBuilder *assets.AssetBuilder) error { - fullCluster, err := PopulateClusterSpec(c.Clientset, c.Cluster, assetBuilder) + fullCluster, err := PopulateClusterSpec(c.Clientset, c.Cluster, c.Cloud, assetBuilder) if err != nil { return err } c.Cluster = fullCluster for i, g := range c.InstanceGroups { - fullGroup, err := PopulateInstanceGroupSpec(fullCluster, g, c.channel) + fullGroup, err := PopulateInstanceGroupSpec(fullCluster, g, c.Cloud, c.channel) if err != nil { return err } diff --git a/upup/pkg/fi/cloudup/bootstrapchannelbuilder_test.go b/upup/pkg/fi/cloudup/bootstrapchannelbuilder_test.go index b094369a1a..189e813086 100644 --- a/upup/pkg/fi/cloudup/bootstrapchannelbuilder_test.go +++ b/upup/pkg/fi/cloudup/bootstrapchannelbuilder_test.go @@ -88,7 +88,7 @@ func runChannelBuilderTest(t *testing.T, key string, addonManifests []string) { t.Fatalf("error from PerformAssignments for %q: %v", key, err) } - fullSpec, err := mockedPopulateClusterSpec(cluster) + fullSpec, err := mockedPopulateClusterSpec(cluster, cloud) if err != nil { t.Fatalf("error from PopulateClusterSpec for %q: %v", key, err) } diff --git a/upup/pkg/fi/cloudup/defaults.go b/upup/pkg/fi/cloudup/defaults.go index 535a0a9e12..150858a7d0 100644 --- a/upup/pkg/fi/cloudup/defaults.go +++ b/upup/pkg/fi/cloudup/defaults.go @@ -46,6 +46,10 @@ func PerformAssignments(c *kops.Cluster, cloud fi.Cloud) error { c.Spec.Topology = &kops.TopologySpec{Masters: kops.TopologyPublic, Nodes: kops.TopologyPublic} } + if cloud == nil { + return fmt.Errorf("cloud cannot be nil") + } + if cloud.ProviderID() == kops.CloudProviderGCE { if err := gce.PerformNetworkAssignments(c, cloud); err != nil { return err @@ -99,7 +103,7 @@ func PerformAssignments(c *kops.Cluster, cloud fi.Cloud) error { pd := cloud.ProviderID() if pd == kops.CloudProviderAWS || pd == kops.CloudProviderOpenstack || pd == kops.CloudProviderALI { // TODO: Use vpcInfo - err := assignCIDRsToSubnets(c) + err := assignCIDRsToSubnets(c, cloud) if err != nil { return err } diff --git a/upup/pkg/fi/cloudup/defaults_test.go b/upup/pkg/fi/cloudup/defaults_test.go index 7fe7de3546..7aa7ec9674 100644 --- a/upup/pkg/fi/cloudup/defaults_test.go +++ b/upup/pkg/fi/cloudup/defaults_test.go @@ -23,7 +23,7 @@ import ( ) func TestPopulateClusterSpec_Proxy(t *testing.T) { - c := buildMinimalCluster() + _, c := buildMinimalCluster() c.Spec.EgressProxy = &kops.EgressProxySpec{ ProxyExcludes: "google.com", diff --git a/upup/pkg/fi/cloudup/populate_cluster_spec.go b/upup/pkg/fi/cloudup/populate_cluster_spec.go index 8a0653d44c..fdb231cb1e 100644 --- a/upup/pkg/fi/cloudup/populate_cluster_spec.go +++ b/upup/pkg/fi/cloudup/populate_cluster_spec.go @@ -43,6 +43,8 @@ import ( var EtcdClusters = []string{"main", "events"} type populateClusterSpec struct { + cloud fi.Cloud + // InputCluster is the api object representing the whole cluster, as input by the user // We build it up into a complete config, but we write the values as input InputCluster *kopsapi.Cluster @@ -56,8 +58,9 @@ type populateClusterSpec struct { // PopulateClusterSpec takes a user-specified cluster spec, and computes the full specification that should be set on the cluster. // We do this so that we don't need any real "brains" on the node side. -func PopulateClusterSpec(clientset simple.Clientset, cluster *kopsapi.Cluster, assetBuilder *assets.AssetBuilder) (*kopsapi.Cluster, error) { +func PopulateClusterSpec(clientset simple.Clientset, cluster *kopsapi.Cluster, cloud fi.Cloud, assetBuilder *assets.AssetBuilder) (*kopsapi.Cluster, error) { c := &populateClusterSpec{ + cloud: cloud, InputCluster: cluster, assetBuilder: assetBuilder, } @@ -83,6 +86,8 @@ func (c *populateClusterSpec) run(clientset simple.Clientset) error { return errs.ToAggregate() } + cloud := c.cloud + // Copy cluster & instance groups, so we can modify them freely cluster := &kopsapi.Cluster{} @@ -98,11 +103,6 @@ func (c *populateClusterSpec) run(clientset simple.Clientset) error { return err } - cloud, err := BuildCloud(cluster) - if err != nil { - return err - } - err = PerformAssignments(cluster, cloud) if err != nil { return err @@ -301,7 +301,7 @@ func (c *populateClusterSpec) run(clientset simple.Clientset) error { fullCluster.Spec = *completed if errs := validation.ValidateCluster(fullCluster, true); len(errs) != 0 { - return fmt.Errorf("Completed cluster failed validation: %v", errs.ToAggregate()) + return fmt.Errorf("completed cluster failed validation: %v", errs.ToAggregate()) } c.fullCluster = fullCluster diff --git a/upup/pkg/fi/cloudup/populate_instancegroup_spec.go b/upup/pkg/fi/cloudup/populate_instancegroup_spec.go index cdb5bc0dd4..8d369933a6 100644 --- a/upup/pkg/fi/cloudup/populate_instancegroup_spec.go +++ b/upup/pkg/fi/cloudup/populate_instancegroup_spec.go @@ -59,7 +59,7 @@ var awsDedicatedInstanceExceptions = map[string]bool{ // PopulateInstanceGroupSpec sets default values in the InstanceGroup // The InstanceGroup is simpler than the cluster spec, so we just populate in place (like the rest of k8s) -func PopulateInstanceGroupSpec(cluster *kops.Cluster, input *kops.InstanceGroup, channel *kops.Channel) (*kops.InstanceGroup, error) { +func PopulateInstanceGroupSpec(cluster *kops.Cluster, input *kops.InstanceGroup, cloud fi.Cloud, channel *kops.Channel) (*kops.InstanceGroup, error) { var err error err = validation.ValidateInstanceGroup(input, nil).ToAggregate() if err != nil { @@ -72,7 +72,7 @@ func PopulateInstanceGroupSpec(cluster *kops.Cluster, input *kops.InstanceGroup, // TODO: Clean up if ig.IsMaster() { if ig.Spec.MachineType == "" { - ig.Spec.MachineType, err = defaultMachineType(cluster, ig) + ig.Spec.MachineType, err = defaultMachineType(cloud, cluster, ig) if err != nil { return nil, fmt.Errorf("error assigning default machine type for masters: %v", err) } @@ -86,7 +86,7 @@ func PopulateInstanceGroupSpec(cluster *kops.Cluster, input *kops.InstanceGroup, } } else if ig.Spec.Role == kops.InstanceGroupRoleBastion { if ig.Spec.MachineType == "" { - ig.Spec.MachineType, err = defaultMachineType(cluster, ig) + ig.Spec.MachineType, err = defaultMachineType(cloud, cluster, ig) if err != nil { return nil, fmt.Errorf("error assigning default machine type for bastions: %v", err) } @@ -99,7 +99,7 @@ func PopulateInstanceGroupSpec(cluster *kops.Cluster, input *kops.InstanceGroup, } } else { if ig.Spec.MachineType == "" { - ig.Spec.MachineType, err = defaultMachineType(cluster, ig) + ig.Spec.MachineType, err = defaultMachineType(cloud, cluster, ig) if err != nil { return nil, fmt.Errorf("error assigning default machine type for nodes: %v", err) } @@ -123,7 +123,7 @@ func PopulateInstanceGroupSpec(cluster *kops.Cluster, input *kops.InstanceGroup, switch kops.CloudProviderID(cluster.Spec.CloudProvider) { case kops.CloudProviderAWS: if _, ok := awsDedicatedInstanceExceptions[ig.Spec.MachineType]; ok { - return nil, fmt.Errorf("Invalid dedicated instance type: %s", ig.Spec.MachineType) + return nil, fmt.Errorf("invalid dedicated instance type: %s", ig.Spec.MachineType) } default: klog.Warning("Trying to set tenancy on non-AWS environment") @@ -132,7 +132,7 @@ func PopulateInstanceGroupSpec(cluster *kops.Cluster, input *kops.InstanceGroup, if ig.IsMaster() { if len(ig.Spec.Subnets) == 0 { - return nil, fmt.Errorf("Master InstanceGroup %s did not specify any Subnets", ig.ObjectMeta.Name) + return nil, fmt.Errorf("master InstanceGroup %s did not specify any Subnets", ig.ObjectMeta.Name) } } else { if len(ig.Spec.Subnets) == 0 { @@ -152,13 +152,9 @@ func PopulateInstanceGroupSpec(cluster *kops.Cluster, input *kops.InstanceGroup, } // defaultMachineType returns the default MachineType for the instance group, based on the cloudprovider -func defaultMachineType(cluster *kops.Cluster, ig *kops.InstanceGroup) (string, error) { +func defaultMachineType(cloud fi.Cloud, cluster *kops.Cluster, ig *kops.InstanceGroup) (string, error) { switch kops.CloudProviderID(cluster.Spec.CloudProvider) { case kops.CloudProviderAWS: - cloud, err := BuildCloud(cluster) - if err != nil { - return "", fmt.Errorf("error building cloud for AWS cluster: %v", err) - } instanceType, err := cloud.(awsup.AWSCloud).DefaultInstanceType(cluster, ig) if err != nil { @@ -189,11 +185,6 @@ func defaultMachineType(cluster *kops.Cluster, ig *kops.InstanceGroup) (string, } case kops.CloudProviderOpenstack: - cloud, err := BuildCloud(cluster) - if err != nil { - return "", fmt.Errorf("error building cloud for Openstack cluster: %v", err) - } - instanceType, err := cloud.(openstack.OpenstackCloud).DefaultInstanceType(cluster, ig) if err != nil { return "", fmt.Errorf("error finding default machine type: %v", err) diff --git a/upup/pkg/fi/cloudup/populatecluster_test.go b/upup/pkg/fi/cloudup/populatecluster_test.go index 0318b78132..1ae64d7416 100644 --- a/upup/pkg/fi/cloudup/populatecluster_test.go +++ b/upup/pkg/fi/cloudup/populatecluster_test.go @@ -31,32 +31,28 @@ import ( "k8s.io/kops/util/pkg/vfs" ) -func buildMinimalCluster() *kopsapi.Cluster { - awsup.InstallMockAWSCloud(MockAWSRegion, "abcd") +func buildMinimalCluster() (*awsup.MockAWSCloud, *kopsapi.Cluster) { + cloud := awsup.InstallMockAWSCloud(MockAWSRegion, "abcd") c := testutils.BuildMinimalCluster("testcluster.test.com") - return c + return cloud, c } func TestPopulateCluster_Default_NoError(t *testing.T) { - c := buildMinimalCluster() - cloud, err := BuildCloud(c) - if err != nil { - t.Fatalf("error from BuildCloud: %v", err) - } + cloud, c := buildMinimalCluster() - err = PerformAssignments(c, cloud) + err := PerformAssignments(c, cloud) if err != nil { t.Fatalf("error from PerformAssignments: %v", err) } - _, err = mockedPopulateClusterSpec(c) + _, err = mockedPopulateClusterSpec(c, cloud) if err != nil { t.Fatalf("Unexpected error from PopulateCluster: %v", err) } } -func mockedPopulateClusterSpec(c *kopsapi.Cluster) (*kopsapi.Cluster, error) { +func mockedPopulateClusterSpec(c *kopsapi.Cluster, cloud fi.Cloud) (*kopsapi.Cluster, error) { vfs.Context.ResetMemfsContext(true) assetBuilder := assets.NewAssetBuilder(c, "") @@ -65,11 +61,11 @@ func mockedPopulateClusterSpec(c *kopsapi.Cluster) (*kopsapi.Cluster, error) { return nil, fmt.Errorf("error building vfspath: %v", err) } clientset := vfsclientset.NewVFSClientset(basePath) - return PopulateClusterSpec(clientset, c, assetBuilder) + return PopulateClusterSpec(clientset, c, cloud, assetBuilder) } func TestPopulateCluster_Docker_Spec(t *testing.T) { - c := buildMinimalCluster() + cloud, c := buildMinimalCluster() c.Spec.Docker = &kopsapi.DockerConfig{ MTU: fi.Int32(5678), InsecureRegistry: fi.String("myregistry.com:1234"), @@ -77,17 +73,13 @@ 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, cloud) + err := PerformAssignments(c, cloud) if err != nil { t.Fatalf("error from PerformAssignments: %v", err) } - full, err := mockedPopulateClusterSpec(c) + full, err := mockedPopulateClusterSpec(c, cloud) if err != nil { t.Fatalf("Unexpected error from PopulateCluster: %v", err) } @@ -114,18 +106,14 @@ func TestPopulateCluster_Docker_Spec(t *testing.T) { } func TestPopulateCluster_StorageDefault(t *testing.T) { - c := buildMinimalCluster() - cloud, err := BuildCloud(c) - if err != nil { - t.Fatalf("error from BuildCloud: %v", err) - } + cloud, c := buildMinimalCluster() - err = PerformAssignments(c, cloud) + err := PerformAssignments(c, cloud) if err != nil { t.Fatalf("error from PerformAssignments: %v", err) } - full, err := mockedPopulateClusterSpec(c) + full, err := mockedPopulateClusterSpec(c, cloud) if err != nil { t.Fatalf("Unexpected error from PopulateCluster: %v", err) } @@ -146,7 +134,7 @@ func build(c *kopsapi.Cluster) (*kopsapi.Cluster, error) { return nil, fmt.Errorf("error from PerformAssignments: %v", err) } - full, err := mockedPopulateClusterSpec(c) + full, err := mockedPopulateClusterSpec(c, cloud) if err != nil { return nil, fmt.Errorf("Unexpected error from PopulateCluster: %v", err) } @@ -154,7 +142,7 @@ func build(c *kopsapi.Cluster) (*kopsapi.Cluster, error) { } func TestPopulateCluster_Kubenet(t *testing.T) { - c := buildMinimalCluster() + _, c := buildMinimalCluster() full, err := build(c) if err != nil { @@ -171,7 +159,7 @@ func TestPopulateCluster_Kubenet(t *testing.T) { } func TestPopulateCluster_CNI(t *testing.T) { - c := buildMinimalCluster() + _, c := buildMinimalCluster() c.Spec.Kubelet = &kopsapi.KubeletConfigSpec{ ConfigureCBR0: fi.Bool(false), @@ -199,7 +187,7 @@ func TestPopulateCluster_CNI(t *testing.T) { } func TestPopulateCluster_Custom_CIDR(t *testing.T) { - c := buildMinimalCluster() + cloud, c := buildMinimalCluster() c.Spec.NetworkCIDR = "172.20.2.0/24" c.Spec.Subnets = []kopsapi.ClusterSubnetSpec{ {Name: "subnet-us-mock-1a", Zone: "us-mock-1a", CIDR: "172.20.2.0/27"}, @@ -207,17 +195,12 @@ func TestPopulateCluster_Custom_CIDR(t *testing.T) { {Name: "subnet-us-mock-1c", Zone: "us-mock-1c", CIDR: "172.20.2.64/27"}, } - cloud, err := BuildCloud(c) - if err != nil { - t.Fatalf("error from BuildCloud: %v", err) - } - - err = PerformAssignments(c, cloud) + err := PerformAssignments(c, cloud) if err != nil { t.Fatalf("error from PerformAssignments: %v", err) } - full, err := mockedPopulateClusterSpec(c) + full, err := mockedPopulateClusterSpec(c, cloud) if err != nil { t.Fatalf("Unexpected error from PopulateCluster: %v", err) } @@ -227,19 +210,15 @@ func TestPopulateCluster_Custom_CIDR(t *testing.T) { } func TestPopulateCluster_IsolateMasters(t *testing.T) { - c := buildMinimalCluster() + cloud, 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, cloud) + err := PerformAssignments(c, cloud) if err != nil { t.Fatalf("error from PerformAssignments: %v", err) } - full, err := mockedPopulateClusterSpec(c) + full, err := mockedPopulateClusterSpec(c, cloud) if err != nil { t.Fatalf("Unexpected error from PopulateCluster: %v", err) } @@ -252,20 +231,15 @@ func TestPopulateCluster_IsolateMasters(t *testing.T) { } func TestPopulateCluster_IsolateMastersFalse(t *testing.T) { - c := buildMinimalCluster() + cloud, c := buildMinimalCluster() // default: c.Spec.IsolateMasters = fi.Bool(false) - cloud, err := BuildCloud(c) - if err != nil { - t.Fatalf("error from BuildCloud: %v", err) - } - - err = PerformAssignments(c, cloud) + err := PerformAssignments(c, cloud) if err != nil { t.Fatalf("error from PerformAssignments: %v", err) } - full, err := mockedPopulateClusterSpec(c) + full, err := mockedPopulateClusterSpec(c, cloud) if err != nil { t.Fatalf("Unexpected error from PopulateCluster: %v", err) } @@ -275,52 +249,52 @@ func TestPopulateCluster_IsolateMastersFalse(t *testing.T) { } func TestPopulateCluster_Name_Required(t *testing.T) { - c := buildMinimalCluster() + cloud, c := buildMinimalCluster() c.ObjectMeta.Name = "" - expectErrorFromPopulateCluster(t, c, "Name") + expectErrorFromPopulateCluster(t, c, cloud, "Name") } func TestPopulateCluster_Zone_Required(t *testing.T) { - c := buildMinimalCluster() + cloud, c := buildMinimalCluster() c.Spec.Subnets = nil - expectErrorFromPopulateCluster(t, c, "subnet") + expectErrorFromPopulateCluster(t, c, cloud, "subnet") } func TestPopulateCluster_NetworkCIDR_Required(t *testing.T) { - c := buildMinimalCluster() + cloud, c := buildMinimalCluster() c.Spec.NetworkCIDR = "" - expectErrorFromPopulateCluster(t, c, "networkCIDR") + expectErrorFromPopulateCluster(t, c, cloud, "networkCIDR") } func TestPopulateCluster_NonMasqueradeCIDR_Required(t *testing.T) { - c := buildMinimalCluster() + cloud, c := buildMinimalCluster() c.Spec.NonMasqueradeCIDR = "" - expectErrorFromPopulateCluster(t, c, "nonMasqueradeCIDR") + expectErrorFromPopulateCluster(t, c, cloud, "nonMasqueradeCIDR") } func TestPopulateCluster_CloudProvider_Required(t *testing.T) { - c := buildMinimalCluster() + cloud, c := buildMinimalCluster() c.Spec.CloudProvider = "" - expectErrorFromPopulateCluster(t, c, "cloudProvider") + expectErrorFromPopulateCluster(t, c, cloud, "cloudProvider") } func TestPopulateCluster_TopologyInvalidNil_Required(t *testing.T) { - c := buildMinimalCluster() + cloud, c := buildMinimalCluster() c.Spec.Topology.Masters = "" c.Spec.Topology.Nodes = "" - expectErrorFromPopulateCluster(t, c, "topology") + expectErrorFromPopulateCluster(t, c, cloud, "topology") } func TestPopulateCluster_TopologyInvalidValue_Required(t *testing.T) { - c := buildMinimalCluster() + cloud, c := buildMinimalCluster() c.Spec.Topology.Masters = "123" c.Spec.Topology.Nodes = "abc" - expectErrorFromPopulateCluster(t, c, "topology") + expectErrorFromPopulateCluster(t, c, cloud, "topology") } //func TestPopulateCluster_TopologyInvalidMatchingValues_Required(t *testing.T) { @@ -333,25 +307,25 @@ func TestPopulateCluster_TopologyInvalidValue_Required(t *testing.T) { func TestPopulateCluster_BastionInvalidMatchingValues_Required(t *testing.T) { // We can't have a bastion with public masters / nodes - c := buildMinimalCluster() + cloud, c := buildMinimalCluster() c.Spec.Topology.Masters = kopsapi.TopologyPublic c.Spec.Topology.Nodes = kopsapi.TopologyPublic c.Spec.Topology.Bastion = &kopsapi.BastionSpec{} - expectErrorFromPopulateCluster(t, c, "bastion") + expectErrorFromPopulateCluster(t, c, cloud, "bastion") } func TestPopulateCluster_BastionIdleTimeoutInvalidNegative_Required(t *testing.T) { - c := buildMinimalCluster() + cloud, c := buildMinimalCluster() c.Spec.Topology.Masters = kopsapi.TopologyPrivate c.Spec.Topology.Nodes = kopsapi.TopologyPrivate c.Spec.Topology.Bastion = &kopsapi.BastionSpec{} c.Spec.Topology.Bastion.IdleTimeoutSeconds = fi.Int64(-1) - expectErrorFromPopulateCluster(t, c, "bastion") + expectErrorFromPopulateCluster(t, c, cloud, "bastion") } -func expectErrorFromPopulateCluster(t *testing.T, c *kopsapi.Cluster, message string) { - _, err := mockedPopulateClusterSpec(c) +func expectErrorFromPopulateCluster(t *testing.T, c *kopsapi.Cluster, cloud fi.Cloud, message string) { + _, err := mockedPopulateClusterSpec(c, cloud) if err == nil { t.Fatalf("Expected error from PopulateCluster") } @@ -362,7 +336,7 @@ func expectErrorFromPopulateCluster(t *testing.T, c *kopsapi.Cluster, message st } func TestPopulateCluster_APIServerCount(t *testing.T) { - c := buildMinimalCluster() + _, c := buildMinimalCluster() full, err := build(c) if err != nil { @@ -375,20 +349,15 @@ func TestPopulateCluster_APIServerCount(t *testing.T) { } func TestPopulateCluster_AnonymousAuth(t *testing.T) { - c := buildMinimalCluster() + cloud, c := buildMinimalCluster() c.Spec.KubernetesVersion = "1.15.0" - cloud, err := BuildCloud(c) - if err != nil { - t.Fatalf("error from BuildCloud: %v", err) - } - - err = PerformAssignments(c, cloud) + err := PerformAssignments(c, cloud) if err != nil { t.Fatalf("error from PerformAssignments: %v", err) } - full, err := mockedPopulateClusterSpec(c) + full, err := mockedPopulateClusterSpec(c, cloud) if err != nil { t.Fatalf("Unexpected error from PopulateCluster: %v", err) } @@ -430,7 +399,7 @@ func TestPopulateCluster_DockerVersion(t *testing.T) { } for _, test := range grid { - c := buildMinimalCluster() + _, c := buildMinimalCluster() c.Spec.KubernetesVersion = test.KubernetesVersion full, err := build(c) @@ -445,20 +414,15 @@ func TestPopulateCluster_DockerVersion(t *testing.T) { } func TestPopulateCluster_KubeController_High_Enough_Version(t *testing.T) { - c := buildMinimalCluster() + cloud, c := buildMinimalCluster() c.Spec.KubernetesVersion = "v1.9.0" - cloud, err := BuildCloud(c) - if err != nil { - t.Fatalf("error from BuildCloud: %v", err) - } - - err = PerformAssignments(c, cloud) + err := PerformAssignments(c, cloud) if err != nil { t.Fatalf("error from PerformAssignments: %v", err) } - full, err := mockedPopulateClusterSpec(c) + full, err := mockedPopulateClusterSpec(c, cloud) if err != nil { t.Fatalf("Unexpected error from PopulateCluster: %v", err) } diff --git a/upup/pkg/fi/cloudup/populateinstancegroup_test.go b/upup/pkg/fi/cloudup/populateinstancegroup_test.go index 079df7a40b..1cdd78fafa 100644 --- a/upup/pkg/fi/cloudup/populateinstancegroup_test.go +++ b/upup/pkg/fi/cloudup/populateinstancegroup_test.go @@ -43,7 +43,7 @@ func buildMinimalMasterInstanceGroup(subnet string) *kopsapi.InstanceGroup { } func TestPopulateInstanceGroup_Name_Required(t *testing.T) { - cluster := buildMinimalCluster() + _, cluster := buildMinimalCluster() g := buildMinimalNodeInstanceGroup() g.ObjectMeta.Name = "" @@ -53,7 +53,7 @@ func TestPopulateInstanceGroup_Name_Required(t *testing.T) { } func TestPopulateInstanceGroup_Role_Required(t *testing.T) { - cluster := buildMinimalCluster() + _, cluster := buildMinimalCluster() g := buildMinimalNodeInstanceGroup() g.Spec.Role = "" @@ -63,7 +63,12 @@ func TestPopulateInstanceGroup_Role_Required(t *testing.T) { } func expectErrorFromPopulateInstanceGroup(t *testing.T, cluster *kopsapi.Cluster, g *kopsapi.InstanceGroup, channel *kopsapi.Channel, message string) { - _, err := PopulateInstanceGroupSpec(cluster, g, channel) + cloud, err := BuildCloud(cluster) + if err != nil { + t.Fatalf("error from BuildCloud: %v", err) + } + + _, err = PopulateInstanceGroupSpec(cluster, g, cloud, channel) if err == nil { t.Fatalf("Expected error from PopulateInstanceGroup") } diff --git a/upup/pkg/fi/cloudup/subnets.go b/upup/pkg/fi/cloudup/subnets.go index d3747864d7..5f918e3de0 100644 --- a/upup/pkg/fi/cloudup/subnets.go +++ b/upup/pkg/fi/cloudup/subnets.go @@ -41,7 +41,7 @@ func (a ByZone) Less(i, j int) bool { return a[i].Zone < a[j].Zone } -func assignCIDRsToSubnets(c *kops.Cluster) error { +func assignCIDRsToSubnets(c *kops.Cluster, cloud fi.Cloud) error { // TODO: We probably could query for the existing subnets & allocate appropriately // for now we'll require users to set CIDRs themselves @@ -51,10 +51,6 @@ func assignCIDRsToSubnets(c *kops.Cluster) error { } if c.Spec.NetworkID != "" { - cloud, err := BuildCloud(c) - if err != nil { - return err - } vpcInfo, err := cloud.FindVPCInfo(c.Spec.NetworkID) if err != nil { diff --git a/upup/pkg/fi/cloudup/subnets_test.go b/upup/pkg/fi/cloudup/subnets_test.go index 28daa6d640..c153347f36 100644 --- a/upup/pkg/fi/cloudup/subnets_test.go +++ b/upup/pkg/fi/cloudup/subnets_test.go @@ -99,7 +99,7 @@ func Test_AssignSubnets(t *testing.T) { c.Spec.NetworkCIDR = "10.0.0.0/8" c.Spec.Subnets = test.subnets - err := assignCIDRsToSubnets(c) + err := assignCIDRsToSubnets(c, nil) if err != nil { t.Fatalf("unexpected error on test %d: %v", i+1, err) } diff --git a/upup/pkg/fi/cloudup/validation_test.go b/upup/pkg/fi/cloudup/validation_test.go index a0270d1ad8..7e9c12e034 100644 --- a/upup/pkg/fi/cloudup/validation_test.go +++ b/upup/pkg/fi/cloudup/validation_test.go @@ -30,19 +30,14 @@ import ( const MockAWSRegion = "us-mock-1" func buildDefaultCluster(t *testing.T) *api.Cluster { - c := buildMinimalCluster() + cloud, c := buildMinimalCluster() - cloud, err := BuildCloud(c) - if err != nil { - t.Fatalf("error from BuildCloud: %v", err) - } - - err = PerformAssignments(c, cloud) + err := PerformAssignments(c, cloud) if err != nil { t.Fatalf("error from PerformAssignments: %v", err) } - fullSpec, err := mockedPopulateClusterSpec(c) + fullSpec, err := mockedPopulateClusterSpec(c, cloud) if err != nil { t.Fatalf("error from PopulateClusterSpec: %v", err) } diff --git a/upup/pkg/kutil/convert_kubeup_cluster.go b/upup/pkg/kutil/convert_kubeup_cluster.go index 85827f0e82..550a621cc9 100644 --- a/upup/pkg/kutil/convert_kubeup_cluster.go +++ b/upup/pkg/kutil/convert_kubeup_cluster.go @@ -108,7 +108,7 @@ func (x *ConvertKubeupCluster) Upgrade(ctx context.Context) error { } assetBuilder := assets.NewAssetBuilder(cluster, "") - fullCluster, err := cloudup.PopulateClusterSpec(x.Clientset, cluster, assetBuilder) + fullCluster, err := cloudup.PopulateClusterSpec(x.Clientset, cluster, x.Cloud, assetBuilder) if err != nil { return err } diff --git a/upup/pkg/kutil/import_cluster.go b/upup/pkg/kutil/import_cluster.go index 7e5a181ac0..e0f68d9b4c 100644 --- a/upup/pkg/kutil/import_cluster.go +++ b/upup/pkg/kutil/import_cluster.go @@ -481,7 +481,7 @@ func (x *ImportCluster) ImportAWSCluster(ctx context.Context) error { var fullInstanceGroups []*kops.InstanceGroup for _, ig := range instanceGroups { - full, err := cloudup.PopulateInstanceGroupSpec(cluster, ig, channel) + full, err := cloudup.PopulateInstanceGroupSpec(cluster, ig, awsCloud, channel) if err != nil { return err }