*: use a trimmed, unique cluster name for ELB names

data/data/aws/main.tf

@@ -78,11 +78,12 @@ module "dns" {
 module "vpc" {
   source = "./vpc"
 
-  base_domain  = "${var.base_domain}"
-  cidr_block   = "${var.machine_cidr}"
-  cluster_id   = "${var.cluster_id}"
-  cluster_name = "${var.cluster_name}"
-  region       = "${var.aws_region}"
+  base_domain         = "${var.base_domain}"
+  cidr_block          = "${var.machine_cidr}"
+  cluster_id          = "${var.cluster_id}"
+  cluster_name        = "${var.cluster_name}"
+  region              = "${var.aws_region}"
+  unique_cluster_name = "${var.aws_unique_cluster_name}"
 
   tags = "${merge(map(
       "kubernetes.io/cluster/${var.cluster_name}", "owned",

data/data/aws/variables-aws.tf

@@ -53,3 +53,8 @@ variable "aws_region" {
   type        = "string"
   description = "The target AWS region for the cluster."
 }
+
+variable "aws_unique_cluster_name" {
+  type        = "string"
+  description = "The cluster name trimmed down and made unique"
+}

data/data/aws/vpc/master-elb.tf

@@ -1,5 +1,5 @@
 resource "aws_lb" "api_internal" {
-  name                             = "${var.cluster_name}-int"
+  name                             = "${var.unique_cluster_name}-int"
   load_balancer_type               = "network"
   subnets                          = ["${local.private_subnet_ids}"]
   internal                         = true
@@ -12,7 +12,7 @@ resource "aws_lb" "api_internal" {
 }
 
 resource "aws_lb" "api_external" {
-  name                             = "${var.cluster_name}-ext"
+  name                             = "${var.unique_cluster_name}-ext"
   load_balancer_type               = "network"
   subnets                          = ["${local.public_subnet_ids}"]
   internal                         = false

data/data/aws/vpc/variables.tf

@@ -34,3 +34,8 @@ variable "tags" {
   default     = {}
   description = "AWS tags to be applied to created resources."
 }
+
+variable "unique_cluster_name" {
+  type        = "string"
+  description = "cluster name that has been trimmed and made unique"
+}

pkg/asset/cluster/tfvars.go

@@ -56,6 +56,7 @@ func (t *TerraformVariables) Dependencies() []asset.Asset {
 	return []asset.Asset{
 		&installconfig.ClusterID{},
 		&installconfig.InstallConfig{},
+		&installconfig.UniqueClusterName{},
 		new(rhcos.Image),
 		&bootstrap.Bootstrap{},
 		&machine.Master{},
@@ -67,11 +68,12 @@ func (t *TerraformVariables) Dependencies() []asset.Asset {
 func (t *TerraformVariables) Generate(parents asset.Parents) error {
 	clusterID := &installconfig.ClusterID{}
 	installConfig := &installconfig.InstallConfig{}
+	uniqueClusterName := &installconfig.UniqueClusterName{}
 	bootstrapIgnAsset := &bootstrap.Bootstrap{}
 	masterIgnAsset := &machine.Master{}
 	mastersAsset := &machines.Master{}
 	rhcosImage := new(rhcos.Image)
-	parents.Get(clusterID, installConfig, bootstrapIgnAsset, masterIgnAsset, mastersAsset, rhcosImage)
+	parents.Get(clusterID, installConfig, uniqueClusterName, bootstrapIgnAsset, masterIgnAsset, mastersAsset, rhcosImage)
 
 	bootstrapIgn := string(bootstrapIgnAsset.Files()[0].Data)
 	masterIgn := string(masterIgnAsset.Files()[0].Data)
@@ -109,6 +111,7 @@ func (t *TerraformVariables) Generate(parents asset.Parents) error {
 		}
 		data, err = awstfvars.TFVars(
 			masters[0].Spec.ProviderSpec.Value.Object.(*awsprovider.AWSMachineProviderConfig),
+			uniqueClusterName.ClusterName,
 		)
 		if err != nil {
 			return errors.Wrapf(err, "failed to get %s Terraform variables", platform)

pkg/asset/installconfig/uniqueclustername.go

@@ -0,0 +1,54 @@
+package installconfig
+
+import (
+	"fmt"
+
+	utilrand "k8s.io/apimachinery/pkg/util/rand"
+
+	"github.com/openshift/installer/pkg/asset"
+)
+
+const (
+	// AWS load balancers have a maximum name length of 32. The load balancers
+	// have suffixes of "-int" and "-ext", which are 4 characters.
+	maxNameLen = 32 - 4
+	randomLen  = 5
+	maxBaseLen = maxNameLen - randomLen - 1
+)
+
+// UniqueClusterName is the unique name of the cluster. This combines the name of
+// the cluster supplied by the user with random characters.
+type UniqueClusterName struct {
+	ClusterName string
+}
+
+var _ asset.Asset = (*UniqueClusterName)(nil)
+
+// Dependencies returns no dependencies.
+func (a *UniqueClusterName) Dependencies() []asset.Asset {
+	return []asset.Asset{
+		&InstallConfig{},
+	}
+}
+
+// Generate generates a random, unique cluster name
+func (a *UniqueClusterName) Generate(parents asset.Parents) error {
+	ic := &InstallConfig{}
+	parents.Get(ic)
+
+	a.ClusterName = generateName(ic.Config.ObjectMeta.Name)
+
+	return nil
+}
+
+// Name returns the human-friendly name of the asset.
+func (a *UniqueClusterName) Name() string {
+	return "Unique Cluster Name"
+}
+
+func generateName(base string) string {
+	if len(base) > maxBaseLen {
+		base = base[:maxBaseLen]
+	}
+	return fmt.Sprintf("%s-%s", base, utilrand.String(randomLen))
+}

pkg/asset/installconfig/uniqueclustername_test.go

@@ -0,0 +1,36 @@
+package installconfig
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestGenerateName(t *testing.T) {
+	cases := []struct {
+		name     string
+		base     string
+		expected string
+	}{
+		{
+			name:     "empty",
+			base:     "",
+			expected: "^-[[:alnum:]]{5}$",
+		},
+		{
+			name:     "short",
+			base:     "test-name",
+			expected: "^test-name-[[:alnum:]]{5}$",
+		},
+		{
+			name:     "long",
+			base:     "012345678901234567890123456789",
+			expected: "^0123456789012345678901-[[:alnum:]]{5}$",
+		},
+	}
+	for _, tc := range cases {
+		actual := generateName(tc.base)
+		assert.Regexp(t, tc.expected, actual)
+		assert.True(t, len(actual) <= maxNameLen)
+	}
+}

pkg/asset/machines/aws/machines.go

@@ -130,17 +130,17 @@ func tagsFromUserTags(clusterID, clusterName string, usertags map[string]string)
 }
 
 // ConfigMasters sets the PublicIP flag and assigns a set of load balancers to the given machines
-func ConfigMasters(machines []machineapi.Machine, clusterName string) {
+func ConfigMasters(machines []machineapi.Machine, uniqueClusterName string) {
 	for _, machine := range machines {
 		providerSpec := machine.Spec.ProviderSpec.Value.Object.(*awsprovider.AWSMachineProviderConfig)
 		providerSpec.PublicIP = pointer.BoolPtr(true)
 		providerSpec.LoadBalancers = []awsprovider.LoadBalancerReference{
 			{
-				Name: fmt.Sprintf("%s-ext", clusterName),
+				Name: fmt.Sprintf("%s-ext", uniqueClusterName),
 				Type: awsprovider.NetworkLoadBalancerType,
 			},
 			{
-				Name: fmt.Sprintf("%s-int", clusterName),
+				Name: fmt.Sprintf("%s-int", uniqueClusterName),
 				Type: awsprovider.NetworkLoadBalancerType,
 			},
 		}

pkg/asset/machines/master.go

@@ -61,6 +61,7 @@ func (m *Master) Dependencies() []asset.Asset {
 		// it is put in the dependencies but not fetched in Generate
 		&installconfig.PlatformCredsCheck{},
 		&installconfig.InstallConfig{},
+		&installconfig.UniqueClusterName{},
 		new(rhcos.Image),
 		&machine.Master{},
 	}
@@ -69,14 +70,15 @@ func (m *Master) Dependencies() []asset.Asset {
 // Generate generates the Master asset.
 func (m *Master) Generate(dependencies asset.Parents) error {
 	clusterID := &installconfig.ClusterID{}
-	installconfig := &installconfig.InstallConfig{}
+	installConfig := &installconfig.InstallConfig{}
+	uniqueClusterName := &installconfig.UniqueClusterName{}
 	rhcosImage := new(rhcos.Image)
 	mign := &machine.Master{}
-	dependencies.Get(clusterID, installconfig, rhcosImage, mign)
+	dependencies.Get(clusterID, installConfig, uniqueClusterName, rhcosImage, mign)
 
 	var err error
 	machines := []machineapi.Machine{}
-	ic := installconfig.Config
+	ic := installConfig.Config
 	pool := ic.ControlPlane
 	switch ic.Platform.Name() {
 	case awstypes.Name:
@@ -96,7 +98,7 @@ func (m *Master) Generate(dependencies asset.Parents) error {
 		if err != nil {
 			return errors.Wrap(err, "failed to create master machine objects")
 		}
-		aws.ConfigMasters(machines, ic.ObjectMeta.Name)
+		aws.ConfigMasters(machines, uniqueClusterName.ClusterName)
 	case libvirttypes.Name:
 		mpool := defaultLibvirtMachinePoolPlatform()
 		mpool.Set(ic.Platform.Libvirt.DefaultMachinePlatform)

pkg/tfvars/aws/aws.go

@@ -9,17 +9,18 @@ import (
 )
 
 type config struct {
-	EC2AMIOverride string            `json:"aws_ec2_ami_override,omitempty"`
-	ExtraTags      map[string]string `json:"aws_extra_tags,omitempty"`
-	EC2Type        string            `json:"aws_master_ec2_type,omitempty"`
-	IOPS           int64             `json:"aws_master_root_volume_iops"`
-	Size           int64             `json:"aws_master_root_volume_size,omitempty"`
-	Type           string            `json:"aws_master_root_volume_type,omitempty"`
-	Region         string            `json:"aws_region,omitempty"`
+	EC2AMIOverride    string            `json:"aws_ec2_ami_override,omitempty"`
+	ExtraTags         map[string]string `json:"aws_extra_tags,omitempty"`
+	EC2Type           string            `json:"aws_master_ec2_type,omitempty"`
+	IOPS              int64             `json:"aws_master_root_volume_iops"`
+	Size              int64             `json:"aws_master_root_volume_size,omitempty"`
+	Type              string            `json:"aws_master_root_volume_type,omitempty"`
+	Region            string            `json:"aws_region,omitempty"`
+	UniqueClusterName string            `json:"aws_unique_cluster_name"`
 }
 
 // TFVars generates AWS-specific Terraform variables launching the cluster.
-func TFVars(masterConfig *v1beta1.AWSMachineProviderConfig) ([]byte, error) {
+func TFVars(masterConfig *v1beta1.AWSMachineProviderConfig, uniqueClusterName string) ([]byte, error) {
 	tags := make(map[string]string, len(masterConfig.Tags))
 	for _, tag := range masterConfig.Tags {
 		tags[tag.Name] = tag.Value
@@ -47,12 +48,13 @@ func TFVars(masterConfig *v1beta1.AWSMachineProviderConfig) ([]byte, error) {
 	}
 
 	cfg := &config{
-		Region:         masterConfig.Placement.Region,
-		ExtraTags:      tags,
-		EC2AMIOverride: *masterConfig.AMI.ID,
-		EC2Type:        masterConfig.InstanceType,
-		Size:           *rootVolume.EBS.VolumeSize,
-		Type:           *rootVolume.EBS.VolumeType,
+		Region:            masterConfig.Placement.Region,
+		ExtraTags:         tags,
+		EC2AMIOverride:    *masterConfig.AMI.ID,
+		EC2Type:           masterConfig.InstanceType,
+		Size:              *rootVolume.EBS.VolumeSize,
+		Type:              *rootVolume.EBS.VolumeType,
+		UniqueClusterName: uniqueClusterName,
 	}
 
 	if rootVolume.EBS.Iops != nil {