From 32356ddc99c5bdd3a45644542ce6b0a3f2812804 Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Mon, 7 Oct 2019 16:59:40 -0700 Subject: [PATCH 1/4] pkg/types/aws/platform: Add Subnets property This allows users to feed in prexisting subnets. I've also added classification logic copied from the upstream Kubernetes AWS cloud provider for categorizing private vs. public subnets. There's no explicit install-config field for the VPC; we'll extract that from the given subnets. populateSubnets is a bit heavy if all you need is the VPC, but Abhinav prefers it [1], it would save future public/private subnet lookup by warming those caches, and we'll only call Metadata.AWS late in pkg/asset/cluster/tfvars.go (via a future commit), so the VPC cache-warming logic doesn't get called at the moment anyway. There's no verification yet; we'll get to that in follow-up work. More on the DescribeSubnetsPagesWithContext FIXME in 37a7f49c77 (pkg/destroy/aws: Delete subnets by VPC, 2019-08-13, #2214). [1]: https://github.com/openshift/installer/pull/2477#discussion_r334701272 --- pkg/asset/installconfig/aws/metadata.go | 75 ++++++++++- pkg/asset/installconfig/aws/subnet.go | 151 +++++++++++++++++++++++ pkg/asset/installconfig/installconfig.go | 2 +- pkg/types/aws/platform.go | 5 + 4 files changed, 230 insertions(+), 3 deletions(-) create mode 100644 pkg/asset/installconfig/aws/subnet.go diff --git a/pkg/asset/installconfig/aws/metadata.go b/pkg/asset/installconfig/aws/metadata.go index 1e273e144e2..ca41650e833 100644 --- a/pkg/asset/installconfig/aws/metadata.go +++ b/pkg/asset/installconfig/aws/metadata.go @@ -14,13 +14,17 @@ import ( type Metadata struct { session *session.Session availabilityZones []string + privateSubnets map[string]Subnet + publicSubnets map[string]Subnet region string + subnets []string + vpc string mutex sync.Mutex } // NewMetadata initializes a new Metadata object. -func NewMetadata(region string) *Metadata { - return &Metadata{region: region} +func NewMetadata(region string, subnets []string) *Metadata { + return &Metadata{region: region, subnets: subnets} } // Session holds an AWS session which can be used for AWS API calls @@ -63,3 +67,70 @@ func (m *Metadata) AvailabilityZones(ctx context.Context) ([]string, error) { return m.availabilityZones, nil } + +// PrivateSubnets retrieves subnet metadata indexed by subnet ID, for +// subnets that the cloud-provider logic considers to be private +// (i.e. not public). +func (m *Metadata) PrivateSubnets(ctx context.Context) (map[string]Subnet, error) { + m.mutex.Lock() + defer m.mutex.Unlock() + + err := m.populateSubnets(ctx) + if err != nil { + return nil, err + } + + return m.privateSubnets, nil +} + +// PublicSubnets retrieves subnet metadata indexed by subnet ID, for +// subnets that the cloud-provider logic considers to be public +// (e.g. with suitable routing for hosting public load balancers). +func (m *Metadata) PublicSubnets(ctx context.Context) (map[string]Subnet, error) { + m.mutex.Lock() + defer m.mutex.Unlock() + + err := m.populateSubnets(ctx) + if err != nil { + return nil, err + } + + return m.publicSubnets, nil +} + +func (m *Metadata) populateSubnets(ctx context.Context) error { + if len(m.publicSubnets) > 0 || len(m.privateSubnets) > 0 { + return nil + } + + if len(m.subnets) == 0 { + return errors.New("no subnets configured") + } + + session, err := m.unlockedSession(ctx) + if err != nil { + return err + } + + m.vpc, m.privateSubnets, m.publicSubnets, err = subnets(ctx, session, m.subnets) + return err +} + +// VPC retrieves the VPC ID containing PublicSubnets and PrivateSubnets. +func (m *Metadata) VPC(ctx context.Context) (string, error) { + m.mutex.Lock() + defer m.mutex.Unlock() + + if m.vpc == "" { + if len(m.subnets) == 0 { + return "", errors.New("cannot calculate VPC without configured subnets") + } + + err := m.populateSubnets(ctx) + if err != nil { + return "", err + } + } + + return m.vpc, nil +} diff --git a/pkg/asset/installconfig/aws/subnet.go b/pkg/asset/installconfig/aws/subnet.go new file mode 100644 index 00000000000..a9356e25bc8 --- /dev/null +++ b/pkg/asset/installconfig/aws/subnet.go @@ -0,0 +1,151 @@ +package aws + +import ( + "context" + "fmt" + "strings" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/aws/session" + "github.com/aws/aws-sdk-go/service/ec2" + "github.com/pkg/errors" + "github.com/sirupsen/logrus" +) + +// Subnet holds metadata for a subnet. +type Subnet struct { + // ARN is the subnet's Amazon Resource Name. + ARN string + + // Zone is the subnet's availability zone. + Zone string +} + +// subnets retrieves metadata for the given subnet(s). +func subnets(ctx context.Context, session *session.Session, ids []string) (vpc string, private map[string]Subnet, public map[string]Subnet, err error) { + metas := make(map[string]Subnet, len(ids)) + private = map[string]Subnet{} + public = map[string]Subnet{} + var vpcFromSubnet string + client := ec2.New(session) + + idPointers := make([]*string, len(ids)) + for _, id := range ids { + idPointers = append(idPointers, aws.String(id)) + } + results, err := client.DescribeSubnetsWithContext( // FIXME: port to DescribeSubnetsPagesWithContext once we bump our vendored AWS package past v1.19.30 + ctx, + &ec2.DescribeSubnetsInput{SubnetIds: idPointers}, + ) + if err != nil { + return vpc, nil, nil, errors.Wrap(err, "describing subnets") + } + for _, subnet := range results.Subnets { + if subnet.SubnetId == nil { + continue + } + if subnet.SubnetArn == nil { + return vpc, nil, nil, errors.Errorf("%s has no ARN", *subnet.SubnetId) + } + if subnet.VpcId == nil { + return vpc, nil, nil, errors.Errorf("%s has no VPC", *subnet.SubnetId) + } + if subnet.AvailabilityZone == nil { + return vpc, nil, nil, errors.Errorf("%s has no availability zone", *subnet.SubnetId) + } + + if vpc == "" { + vpc = *subnet.VpcId + vpcFromSubnet = *subnet.SubnetId + } else if *subnet.VpcId != vpc { + return vpc, nil, nil, errors.Errorf("all subnets must belong to the same VPC: %s is from %s, but %s is from %s", *subnet.SubnetId, *subnet.VpcId, vpcFromSubnet, vpc) + } + + metas[*subnet.SubnetId] = Subnet{ + ARN: *subnet.SubnetArn, + Zone: *subnet.AvailabilityZone, + } + } + + var routeTables []*ec2.RouteTable + err = client.DescribeRouteTablesPagesWithContext( + ctx, + &ec2.DescribeRouteTablesInput{ + Filters: []*ec2.Filter{{ + Name: aws.String("vpc-id"), + Values: []*string{aws.String(vpc)}, + }}, + }, + func(results *ec2.DescribeRouteTablesOutput, lastPage bool) bool { + routeTables = append(routeTables, results.RouteTables...) + return !lastPage + }, + ) + if err != nil { + return vpc, nil, nil, errors.Wrap(err, "describing route tables") + } + + for _, id := range ids { + meta, ok := metas[id] + if !ok { + return vpc, nil, nil, errors.Errorf("failed to find %s", id) + } + isPublic, err := isSubnetPublic(routeTables, id) + if err != nil { + return vpc, nil, nil, err + } + if isPublic { + public[id] = meta + } else { + private[id] = meta + } + } + + return vpc, private, public, nil +} + +// https://github.com/kubernetes/kubernetes/blob/9f036cd43d35a9c41d7ac4ca82398a6d0bef957b/staging/src/k8s.io/legacy-cloud-providers/aws/aws.go#L3376-L3419 +func isSubnetPublic(rt []*ec2.RouteTable, subnetID string) (bool, error) { + var subnetTable *ec2.RouteTable + for _, table := range rt { + for _, assoc := range table.Associations { + if aws.StringValue(assoc.SubnetId) == subnetID { + subnetTable = table + break + } + } + } + + if subnetTable == nil { + // If there is no explicit association, the subnet will be implicitly + // associated with the VPC's main routing table. + for _, table := range rt { + for _, assoc := range table.Associations { + if aws.BoolValue(assoc.Main) == true { + logrus.Debugf("Assuming implicit use of main routing table %s for %s", + aws.StringValue(table.RouteTableId), subnetID) + subnetTable = table + break + } + } + } + } + + if subnetTable == nil { + return false, fmt.Errorf("could not locate routing table for %s", subnetID) + } + + for _, route := range subnetTable.Routes { + // There is no direct way in the AWS API to determine if a subnet is public or private. + // A public subnet is one which has an internet gateway route + // we look for the gatewayId and make sure it has the prefix of igw to differentiate + // from the default in-subnet route which is called "local" + // or other virtual gateway (starting with vgv) + // or vpc peering connections (starting with pcx). + if strings.HasPrefix(aws.StringValue(route.GatewayId), "igw") { + return true, nil + } + } + + return false, nil +} diff --git a/pkg/asset/installconfig/installconfig.go b/pkg/asset/installconfig/installconfig.go index 997df70fcac..62dda256cc1 100644 --- a/pkg/asset/installconfig/installconfig.go +++ b/pkg/asset/installconfig/installconfig.go @@ -128,7 +128,7 @@ func (a *InstallConfig) finish(filename string) error { defaults.SetInstallConfigDefaults(a.Config) if a.Config.AWS != nil { - a.AWS = aws.NewMetadata(a.Config.Platform.AWS.Region) + a.AWS = aws.NewMetadata(a.Config.Platform.AWS.Region, a.Config.Platform.AWS.Subnets) } if err := validation.ValidateInstallConfig(a.Config, openstackvalidation.NewValidValuesFetcher()).ToAggregate(); err != nil { diff --git a/pkg/types/aws/platform.go b/pkg/types/aws/platform.go index 93153f4b8c7..45d0911ec51 100644 --- a/pkg/types/aws/platform.go +++ b/pkg/types/aws/platform.go @@ -10,6 +10,11 @@ type Platform struct { // Region specifies the AWS region where the cluster will be created. Region string `json:"region"` + // Subnets specifies existing subnets (by ID) where cluster + // resources will be created. Leave unset to have the installer + // create subnets in a new VPC on your behalf. + Subnets []string `json:"subnets,omitempty"` + // UserTags additional keys and values that the installer will add // as tags to all resources that it creates. Resources created by the // cluster itself may not include these tags. From 7c18912ba0b7352da84d456319abf5d1d8bb8036 Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Sun, 6 Oct 2019 00:23:01 -0700 Subject: [PATCH 2/4] pkg/asset/machines/aws/machines: Select user-provided subnet by ID User-provided VPCs may not have our convenient {clusterID}-private-{zone} tag to search on. We can get close with: $ aws ec2 describe-subnets --filters Name=tag-key,Values=kubernetes.io/cluster/${clusterID} Name=availability-zone,Values=us-west-2a or similar, but that still returns both the private and public subnets in that zone. I couldn't figure out a generic way to find user-provided subnets by tags, so with this commit I'm switching on "are our subnets user-provided?", and if they are I'm attaching MachineSets to them by subnet ID. While I was adjusting the call signatures, I also exploded the types in the Machines, MachineSets, and provider (e.g. to take a region string, etc. instead of an InstallConfig type). This more clearly separates information that is being applied at a higher level (e.g. AMI lookup for osImage, vs. InstallConfig.Platform.AWS.AMIID) and decouples folks who are calling MachineSets externally (like Hive [1]) from some of our internal data types. [1]: https://github.com/openshift/hive/blob/8ec56c103aba24ed216b432fa3d3e592d677d4e2/pkg/controller/remotemachineset/remotemachineset_controller.go#L374 --- pkg/asset/machines/aws/machines.go | 63 +++++++++++++++++---------- pkg/asset/machines/aws/machinesets.go | 25 ++++++++--- pkg/asset/machines/master.go | 46 +++++++++++++++---- pkg/asset/machines/worker.go | 45 +++++++++++++++---- 4 files changed, 130 insertions(+), 49 deletions(-) diff --git a/pkg/asset/machines/aws/machines.go b/pkg/asset/machines/aws/machines.go index e0874d06520..5bdbbf13b5c 100644 --- a/pkg/asset/machines/aws/machines.go +++ b/pkg/asset/machines/aws/machines.go @@ -18,16 +18,11 @@ import ( ) // Machines returns a list of machines for a machinepool. -func Machines(clusterID string, config *types.InstallConfig, pool *types.MachinePool, osImage, role, userDataSecret string) ([]machineapi.Machine, error) { - if configPlatform := config.Platform.Name(); configPlatform != aws.Name { - return nil, fmt.Errorf("non-AWS configuration: %q", configPlatform) - } +func Machines(clusterID string, region string, subnets map[string]string, pool *types.MachinePool, osImage, role, userDataSecret string, userTags map[string]string) ([]machineapi.Machine, error) { if poolPlatform := pool.Platform.Name(); poolPlatform != aws.Name { return nil, fmt.Errorf("non-AWS machine-pool: %q", poolPlatform) } - platform := config.Platform.AWS mpool := pool.Platform.AWS - azs := mpool.Zones total := int64(1) if pool.Replicas != nil { @@ -35,8 +30,23 @@ func Machines(clusterID string, config *types.InstallConfig, pool *types.Machine } var machines []machineapi.Machine for idx := int64(0); idx < total; idx++ { - azIndex := int(idx) % len(azs) - provider, err := provider(clusterID, platform, mpool, osImage, azIndex, role, userDataSecret) + zone := mpool.Zones[int(idx)%len(mpool.Zones)] + subnet, ok := subnets[zone] + if len(subnets) > 0 && !ok { + return nil, errors.Errorf("no subnet for zone %s", zone) + } + provider, err := provider( + clusterID, + region, + subnet, + mpool.InstanceType, + &mpool.EC2RootVolume, + osImage, + zone, + role, + userDataSecret, + userTags, + ) if err != nil { return nil, errors.Wrap(err, "failed to create provider") } @@ -68,25 +78,25 @@ func Machines(clusterID string, config *types.InstallConfig, pool *types.Machine return machines, nil } -func provider(clusterID string, platform *aws.Platform, mpool *aws.MachinePool, osImage string, azIdx int, role, userDataSecret string) (*awsprovider.AWSMachineProviderConfig, error) { - az := mpool.Zones[azIdx] +func provider(clusterID string, region string, subnet string, instanceType string, root *aws.EC2RootVolume, osImage string, zone, role, userDataSecret string, userTags map[string]string) (*awsprovider.AWSMachineProviderConfig, error) { amiID := osImage - tags, err := tagsFromUserTags(clusterID, platform.UserTags) + tags, err := tagsFromUserTags(clusterID, userTags) if err != nil { return nil, errors.Wrap(err, "failed to create awsprovider.TagSpecifications from UserTags") } - return &awsprovider.AWSMachineProviderConfig{ + + config := &awsprovider.AWSMachineProviderConfig{ TypeMeta: metav1.TypeMeta{ APIVersion: "awsproviderconfig.openshift.io/v1beta1", Kind: "AWSMachineProviderConfig", }, - InstanceType: mpool.InstanceType, + InstanceType: instanceType, BlockDevices: []awsprovider.BlockDeviceMappingSpec{ { EBS: &awsprovider.EBSBlockDeviceSpec{ - VolumeType: pointer.StringPtr(mpool.Type), - VolumeSize: pointer.Int64Ptr(int64(mpool.Size)), - Iops: pointer.Int64Ptr(int64(mpool.IOPS)), + VolumeType: pointer.StringPtr(root.Type), + VolumeSize: pointer.Int64Ptr(int64(root.Size)), + Iops: pointer.Int64Ptr(int64(root.IOPS)), }, }, }, @@ -95,20 +105,25 @@ func provider(clusterID string, platform *aws.Platform, mpool *aws.MachinePool, IAMInstanceProfile: &awsprovider.AWSResourceReference{ID: pointer.StringPtr(fmt.Sprintf("%s-%s-profile", clusterID, role))}, UserDataSecret: &corev1.LocalObjectReference{Name: userDataSecret}, CredentialsSecret: &corev1.LocalObjectReference{Name: "aws-cloud-credentials"}, - Subnet: awsprovider.AWSResourceReference{ - Filters: []awsprovider.Filter{{ - Name: "tag:Name", - Values: []string{fmt.Sprintf("%s-private-%s", clusterID, az)}, - }}, - }, - Placement: awsprovider.Placement{Region: platform.Region, AvailabilityZone: az}, + Placement: awsprovider.Placement{Region: region, AvailabilityZone: zone}, SecurityGroups: []awsprovider.AWSResourceReference{{ Filters: []awsprovider.Filter{{ Name: "tag:Name", Values: []string{fmt.Sprintf("%s-%s-sg", clusterID, role)}, }}, }}, - }, nil + } + + if subnet == "" { + config.Subnet.Filters = []awsprovider.Filter{{ + Name: "tag:Name", + Values: []string{fmt.Sprintf("%s-private-%s", clusterID, zone)}, + }} + } else { + config.Subnet.ID = pointer.StringPtr(subnet) + } + + return config, nil } func tagsFromUserTags(clusterID string, usertags map[string]string) ([]awsprovider.TagSpecification, error) { diff --git a/pkg/asset/machines/aws/machinesets.go b/pkg/asset/machines/aws/machinesets.go index 3cea271b78e..9032a7b31f5 100644 --- a/pkg/asset/machines/aws/machinesets.go +++ b/pkg/asset/machines/aws/machinesets.go @@ -14,14 +14,10 @@ import ( ) // MachineSets returns a list of machinesets for a machinepool. -func MachineSets(clusterID string, config *types.InstallConfig, pool *types.MachinePool, osImage, role, userDataSecret string) ([]*machineapi.MachineSet, error) { - if configPlatform := config.Platform.Name(); configPlatform != aws.Name { - return nil, fmt.Errorf("non-AWS configuration: %q", configPlatform) - } +func MachineSets(clusterID string, region string, subnets map[string]string, pool *types.MachinePool, osImage, role, userDataSecret string, userTags map[string]string) ([]*machineapi.MachineSet, error) { if poolPlatform := pool.Platform.Name(); poolPlatform != aws.Name { return nil, fmt.Errorf("non-AWS machine-pool: %q", poolPlatform) } - platform := config.Platform.AWS mpool := pool.Platform.AWS azs := mpool.Zones @@ -31,13 +27,28 @@ func MachineSets(clusterID string, config *types.InstallConfig, pool *types.Mach } numOfAZs := int64(len(azs)) var machinesets []*machineapi.MachineSet - for idx, az := range azs { + for idx, az := range mpool.Zones { replicas := int32(total / numOfAZs) if int64(idx) < total%numOfAZs { replicas++ } - provider, err := provider(clusterID, platform, mpool, osImage, idx, role, userDataSecret) + subnet, ok := subnets[az] + if len(subnets) > 0 && !ok { + return nil, errors.Errorf("no subnet for zone %s", az) + } + provider, err := provider( + clusterID, + region, + subnet, + mpool.InstanceType, + &mpool.EC2RootVolume, + osImage, + az, + role, + userDataSecret, + userTags, + ) if err != nil { return nil, errors.Wrap(err, "failed to create provider") } diff --git a/pkg/asset/machines/master.go b/pkg/asset/machines/master.go index 154149544b0..cb4c9b72e12 100644 --- a/pkg/asset/machines/master.go +++ b/pkg/asset/machines/master.go @@ -122,31 +122,59 @@ func awsDefaultMasterMachineType(installconfig *installconfig.InstallConfig) str // Generate generates the Master asset. func (m *Master) Generate(dependencies asset.Parents) error { + ctx := context.TODO() clusterID := &installconfig.ClusterID{} - installconfig := &installconfig.InstallConfig{} + installConfig := &installconfig.InstallConfig{} rhcosImage := new(rhcos.Image) mign := &machine.Master{} - dependencies.Get(clusterID, installconfig, rhcosImage, mign) + dependencies.Get(clusterID, installConfig, rhcosImage, mign) - ic := installconfig.Config + ic := installConfig.Config pool := ic.ControlPlane var err error machines := []machineapi.Machine{} switch ic.Platform.Name() { case awstypes.Name: + subnets := map[string]string{} + if len(ic.Platform.AWS.Subnets) > 0 { + subnetMeta, err := installConfig.AWS.PrivateSubnets(ctx) + if err != nil { + return err + } + for id, subnet := range subnetMeta { + subnets[subnet.Zone] = id + } + } + mpool := defaultAWSMachinePoolPlatform() - mpool.InstanceType = awsDefaultMasterMachineType(installconfig) + mpool.InstanceType = awsDefaultMasterMachineType(installConfig) mpool.Set(ic.Platform.AWS.DefaultMachinePlatform) mpool.Set(pool.Platform.AWS) if len(mpool.Zones) == 0 { - mpool.Zones, err = installconfig.AWS.AvailabilityZones(context.TODO()) - if err != nil { - return err + if len(subnets) > 0 { + for zone := range subnets { + mpool.Zones = append(mpool.Zones, zone) + } + } else { + mpool.Zones, err = installConfig.AWS.AvailabilityZones(ctx) + if err != nil { + return err + } } } + pool.Platform.AWS = &mpool - machines, err = aws.Machines(clusterID.InfraID, ic, pool, string(*rhcosImage), "master", "master-user-data") + machines, err = aws.Machines( + clusterID.InfraID, + installConfig.Config.Platform.AWS.Region, + subnets, + pool, + string(*rhcosImage), + "master", + "master-user-data", + installConfig.Config.Platform.AWS.UserTags, + ) if err != nil { return errors.Wrap(err, "failed to create master machine objects") } @@ -192,7 +220,7 @@ func (m *Master) Generate(dependencies asset.Parents) error { openstack.ConfigMasters(machines, clusterID.InfraID) case azuretypes.Name: mpool := defaultAzureMachinePoolPlatform() - mpool.InstanceType = azuredefaults.ControlPlaneInstanceType(installconfig.Config.Platform.Azure.Region) + mpool.InstanceType = azuredefaults.ControlPlaneInstanceType(installConfig.Config.Platform.Azure.Region) mpool.OSDisk.DiskSizeGB = 1024 mpool.Set(ic.Platform.Azure.DefaultMachinePlatform) mpool.Set(pool.Platform.Azure) diff --git a/pkg/asset/machines/worker.go b/pkg/asset/machines/worker.go index 43d20b03b73..1e18064eabf 100644 --- a/pkg/asset/machines/worker.go +++ b/pkg/asset/machines/worker.go @@ -136,16 +136,17 @@ func awsDefaultWorkerMachineType(installconfig *installconfig.InstallConfig) str // Generate generates the Worker asset. func (w *Worker) Generate(dependencies asset.Parents) error { + ctx := context.TODO() clusterID := &installconfig.ClusterID{} - installconfig := &installconfig.InstallConfig{} + installConfig := &installconfig.InstallConfig{} rhcosImage := new(rhcos.Image) wign := &machine.Worker{} - dependencies.Get(clusterID, installconfig, rhcosImage, wign) + dependencies.Get(clusterID, installConfig, rhcosImage, wign) machineConfigs := []*mcfgv1.MachineConfig{} machineSets := []runtime.Object{} var err error - ic := installconfig.Config + ic := installConfig.Config for _, pool := range ic.Compute { if pool.Hyperthreading == types.HyperthreadingDisabled { machineConfigs = append(machineConfigs, machineconfig.ForHyperthreadingDisabled("worker")) @@ -155,18 +156,44 @@ func (w *Worker) Generate(dependencies asset.Parents) error { } switch ic.Platform.Name() { case awstypes.Name: + subnets := map[string]string{} + if len(ic.Platform.AWS.Subnets) > 0 { + subnetMeta, err := installConfig.AWS.PrivateSubnets(ctx) + if err != nil { + return err + } + for id, subnet := range subnetMeta { + subnets[subnet.Zone] = id + } + } + mpool := defaultAWSMachinePoolPlatform() - mpool.InstanceType = awsDefaultWorkerMachineType(installconfig) + mpool.InstanceType = awsDefaultWorkerMachineType(installConfig) mpool.Set(ic.Platform.AWS.DefaultMachinePlatform) mpool.Set(pool.Platform.AWS) if len(mpool.Zones) == 0 { - mpool.Zones, err = installconfig.AWS.AvailabilityZones(context.TODO()) - if err != nil { - return err + if len(subnets) > 0 { + for zone := range subnets { + mpool.Zones = append(mpool.Zones, zone) + } + } else { + mpool.Zones, err = installConfig.AWS.AvailabilityZones(ctx) + if err != nil { + return err + } } } pool.Platform.AWS = &mpool - sets, err := aws.MachineSets(clusterID.InfraID, ic, &pool, string(*rhcosImage), "worker", "worker-user-data") + sets, err := aws.MachineSets( + clusterID.InfraID, + installConfig.Config.Platform.AWS.Region, + subnets, + &pool, + string(*rhcosImage), + "worker", + "worker-user-data", + installConfig.Config.Platform.AWS.UserTags, + ) if err != nil { return errors.Wrap(err, "failed to create worker machine objects") } @@ -175,7 +202,7 @@ func (w *Worker) Generate(dependencies asset.Parents) error { } case azuretypes.Name: mpool := defaultAzureMachinePoolPlatform() - mpool.InstanceType = azuredefaults.ComputeInstanceType(installconfig.Config.Platform.Azure.Region) + mpool.InstanceType = azuredefaults.ComputeInstanceType(installConfig.Config.Platform.Azure.Region) mpool.Set(ic.Platform.Azure.DefaultMachinePlatform) mpool.Set(pool.Platform.Azure) if len(mpool.Zones) == 0 { From 4db31270f1953f729948159875a9fafc24dbd1c3 Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Thu, 3 Oct 2019 09:41:31 -0700 Subject: [PATCH 3/4] pkg/asset/cluster/aws: Tag shared subnets So the kubelet cloud-provider can place load balancers appropriately. This also helps make it clear to other consumers that we are using these resources for the cluster ("hmm, I wonder if it would be a bad idea to delete this subnet? Ah, yeah, I don't want to break cluster example-123"). But because AWS only allows 50 tags per resource [1], we're not tagging shared resources like the VPC or Route 53 zones because those don't need to be tagged to get a working cluster. Users are free to add 'shared' tags to resources like that as they see fit. I also considered using the EC2-specific API [2], but went with the generic tag API in case we wanted to tag Route 53 zones or other non-EC2 resources as well in the future. Tag before entering Terraform to claim the space before we actually put cluster resources inside the subnets. That keeps folks from unknowingly reaping them before we start adding resources, and it ensures we have space for the new tags. The 20-tag limit that leads to the loop is from [3]. We're unlikely to exceed 20 with just the VPC and subnets, but it seemed reasonable to plan for the future to avoid surprises if we grow this list going forward. [1]: https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/Using_Tags.html#tag-restrictions [2]: https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_CreateTags.html [3]: https://docs.aws.amazon.com/sdk-for-go/api/service/resourcegroupstaggingapi/#TagResourcesInput --- pkg/asset/cluster/aws/aws.go | 61 ++++++++++++++++++++++++++++++++++-- pkg/asset/cluster/cluster.go | 8 +++++ 2 files changed, 66 insertions(+), 3 deletions(-) diff --git a/pkg/asset/cluster/aws/aws.go b/pkg/asset/cluster/aws/aws.go index a5237212069..41f43332093 100644 --- a/pkg/asset/cluster/aws/aws.go +++ b/pkg/asset/cluster/aws/aws.go @@ -2,15 +2,21 @@ package aws import ( + "context" "fmt" + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/resourcegroupstaggingapi" + "github.com/sirupsen/logrus" + + "github.com/openshift/installer/pkg/asset/installconfig" "github.com/openshift/installer/pkg/types" - "github.com/openshift/installer/pkg/types/aws" + awstypes "github.com/openshift/installer/pkg/types/aws" ) // Metadata converts an install configuration to AWS metadata. -func Metadata(clusterID, infraID string, config *types.InstallConfig) *aws.Metadata { - return &aws.Metadata{ +func Metadata(clusterID, infraID string, config *types.InstallConfig) *awstypes.Metadata { + return &awstypes.Metadata{ Region: config.Platform.AWS.Region, Identifier: []map[string]string{{ fmt.Sprintf("kubernetes.io/cluster/%s", infraID): "owned", @@ -19,3 +25,52 @@ func Metadata(clusterID, infraID string, config *types.InstallConfig) *aws.Metad }}, } } + +// PreTerraform performs any infrastructure initialization which must +// happen before Terraform creates the remaining infrastructure. +func PreTerraform(ctx context.Context, clusterID string, installConfig *installconfig.InstallConfig) error { + if len(installConfig.Config.Platform.AWS.Subnets) == 0 { + return nil + } + + privateSubnets, err := installConfig.AWS.PrivateSubnets(ctx) + if err != nil { + return err + } + + publicSubnets, err := installConfig.AWS.PublicSubnets(ctx) + + arns := make([]string, 0, len(privateSubnets)+len(publicSubnets)) + for _, subnet := range privateSubnets { + arns = append(arns, subnet.ARN) + } + for _, subnet := range publicSubnets { + arns = append(arns, subnet.ARN) + } + + session, err := installConfig.AWS.Session(ctx) + if err != nil { + return err + } + + request := &resourcegroupstaggingapi.TagResourcesInput{ + Tags: map[string]*string{ + fmt.Sprintf("kubernetes.io/cluster/%s", clusterID): aws.String("shared"), + }, + } + + tagClient := resourcegroupstaggingapi.New(session) + for i := 0; i < len(arns); i += 20 { + request.ResourceARNList = make([]*string, 0, 20) + for j := 0; i+j < len(arns) && j < 20; j++ { + logrus.Debugf("Tagging %s with kubernetes.io/cluster/%s: shared", arns[i+j], clusterID) + request.ResourceARNList = append(request.ResourceARNList, aws.String(arns[i+j])) + } + _, err = tagClient.TagResourcesWithContext(ctx, request) + if err != nil { + return err + } + } + + return nil +} diff --git a/pkg/asset/cluster/cluster.go b/pkg/asset/cluster/cluster.go index 47458a3aa6e..ab6acc0c699 100644 --- a/pkg/asset/cluster/cluster.go +++ b/pkg/asset/cluster/cluster.go @@ -1,6 +1,7 @@ package cluster import ( + "context" "fmt" "io/ioutil" "os" @@ -10,6 +11,7 @@ import ( "github.com/sirupsen/logrus" "github.com/openshift/installer/pkg/asset" + "github.com/openshift/installer/pkg/asset/cluster/aws" "github.com/openshift/installer/pkg/asset/installconfig" "github.com/openshift/installer/pkg/asset/password" "github.com/openshift/installer/pkg/terraform" @@ -70,6 +72,12 @@ func (c *Cluster) Generate(parents asset.Parents) (err error) { } logrus.Infof("Creating infrastructure resources...") + if installConfig.Config.Platform.AWS != nil { + if err := aws.PreTerraform(context.TODO(), clusterID.InfraID, installConfig); err != nil { + return err + } + } + stateFile, err := terraform.Apply(tmpDir, installConfig.Config.Platform.Name(), extraArgs...) if err != nil { err = errors.Wrap(err, "failed to create cluster") From b27d6333e72cd37f52cf14c7b67053c26e3c2ec3 Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Mon, 7 Oct 2019 22:09:09 -0700 Subject: [PATCH 4/4] pkg/tfvars/aws: Pass VPC/subnets through to Terraform No sense in carrying these around in Go and then creating new subnets in Terraform anyway ;). --- pkg/asset/cluster/tfvars.go | 36 +++++++++++++++++++++++++++++++++--- pkg/tfvars/aws/aws.go | 8 +++++++- 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/pkg/asset/cluster/tfvars.go b/pkg/asset/cluster/tfvars.go index 9945533480f..fe56fb35594 100644 --- a/pkg/asset/cluster/tfvars.go +++ b/pkg/asset/cluster/tfvars.go @@ -85,6 +85,7 @@ func (t *TerraformVariables) Dependencies() []asset.Asset { // Generate generates the terraform.tfvars file. func (t *TerraformVariables) Generate(parents asset.Parents) error { + ctx := context.TODO() clusterID := &installconfig.ClusterID{} installConfig := &installconfig.InstallConfig{} bootstrapIgnAsset := &bootstrap.Bootstrap{} @@ -133,6 +134,35 @@ func (t *TerraformVariables) Generate(parents asset.Parents) error { switch platform { case aws.Name: + var vpc string + var privateSubnets []string + var publicSubnets []string + + if len(installConfig.Config.Platform.AWS.Subnets) > 0 { + subnets, err := installConfig.AWS.PrivateSubnets(ctx) + if err != nil { + return err + } + + for id := range subnets { + privateSubnets = append(privateSubnets, id) + } + + subnets, err = installConfig.AWS.PublicSubnets(ctx) + if err != nil { + return err + } + + for id := range subnets { + publicSubnets = append(publicSubnets, id) + } + + vpc, err = installConfig.AWS.VPC(ctx) + if err != nil { + return err + } + } + masters, err := mastersAsset.Machines() if err != nil { return err @@ -149,7 +179,7 @@ func (t *TerraformVariables) Generate(parents asset.Parents) error { for i, m := range workers { workerConfigs[i] = m.Spec.Template.Spec.ProviderSpec.Value.Object.(*awsprovider.AWSMachineProviderConfig) } - data, err := awstfvars.TFVars(masterConfigs, workerConfigs) + data, err := awstfvars.TFVars(vpc, privateSubnets, publicSubnets, masterConfigs, workerConfigs) if err != nil { return errors.Wrapf(err, "failed to get %s Terraform variables", platform) } @@ -203,7 +233,7 @@ func (t *TerraformVariables) Generate(parents asset.Parents) error { Data: data, }) case gcp.Name: - sess, err := gcpconfig.GetSession(context.TODO()) + sess, err := gcpconfig.GetSession(ctx) if err != nil { return err } @@ -225,7 +255,7 @@ func (t *TerraformVariables) Generate(parents asset.Parents) error { for i, w := range workers { workerConfigs[i] = w.Spec.Template.Spec.ProviderSpec.Value.Object.(*gcpprovider.GCPMachineProviderSpec) } - publicZone, err := gcpconfig.GetPublicZone(context.TODO(), installConfig.Config.GCP.ProjectID, installConfig.Config.BaseDomain) + publicZone, err := gcpconfig.GetPublicZone(ctx, installConfig.Config.GCP.ProjectID, installConfig.Config.BaseDomain) if err != nil { return errors.Wrapf(err, "failed to get GCP public zone") } diff --git a/pkg/tfvars/aws/aws.go b/pkg/tfvars/aws/aws.go index 84e683f0796..20cc161e264 100644 --- a/pkg/tfvars/aws/aws.go +++ b/pkg/tfvars/aws/aws.go @@ -21,10 +21,13 @@ type config struct { Size int64 `json:"aws_master_root_volume_size,omitempty"` Type string `json:"aws_master_root_volume_type,omitempty"` Region string `json:"aws_region,omitempty"` + VPC string `json:"aws_vpc,omitempty"` + PrivateSubnets []string `json:"aws_private_subnets,omitempty"` + PublicSubnets []string `json:"aws_public_subnets,omitempty"` } // TFVars generates AWS-specific Terraform variables launching the cluster. -func TFVars(masterConfigs []*v1beta1.AWSMachineProviderConfig, workerConfigs []*v1beta1.AWSMachineProviderConfig) ([]byte, error) { +func TFVars(vpc string, privateSubnets []string, publicSubnets []string, masterConfigs []*v1beta1.AWSMachineProviderConfig, workerConfigs []*v1beta1.AWSMachineProviderConfig) ([]byte, error) { masterConfig := masterConfigs[0] tags := make(map[string]string, len(masterConfig.Tags)) @@ -80,6 +83,9 @@ func TFVars(masterConfigs []*v1beta1.AWSMachineProviderConfig, workerConfigs []* MasterInstanceType: masterConfig.InstanceType, Size: *rootVolume.EBS.VolumeSize, Type: *rootVolume.EBS.VolumeType, + VPC: vpc, + PrivateSubnets: privateSubnets, + PublicSubnets: publicSubnets, } if rootVolume.EBS.Iops != nil {