Skip to content

Commit

Permalink
Remove character restriction from CLUSTER_NAME env var
Browse files Browse the repository at this point in the history
- No longer limited to 11 characters and no `-` characters in the
CLUSTER_NAME environment variable.

- This restriction was originally introduced due to naming limitations
in the ALB controller. Going forward, we create a second field to hold
the albNamePrefix. This field still uses the value of CLUSTER_NAME but
strips any `-` characters and limits it to 11 characters.

- Resolves: #198
  • Loading branch information
joshrosso committed Sep 20, 2017
1 parent 8549167 commit 918d034
Show file tree
Hide file tree
Showing 8 changed files with 71 additions and 51 deletions.
2 changes: 1 addition & 1 deletion docs/setup.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ helm registry install quay.io/coreos/alb-ingress-controller-helm
value: us-west-1
```

- `CLUSTER_NAME`: name of the cluster. Must be under 11 characters and cannot contain `-`. If doing auto-detection of subnets (described in prerequisites above) `CLUSTER_NAME` must match the AWS tags associated with the subnets you wish ALBs to be provisioned.
- `CLUSTER_NAME`: name of the cluster. If doing auto-detection of subnets (described in prerequisites above) `CLUSTER_NAME` must match the AWS tags associated with the subnets you wish ALBs to be provisioned.

```yaml
- name: CLUSTER_NAME
Expand Down
2 changes: 1 addition & 1 deletion docs/walkthrough.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ In this example, you'll
value: us-west-1
```

- `CLUSTER_NAME`: name of the cluster. Must be under 11 characters and cannot contain `-`.
- `CLUSTER_NAME`: name of the cluster.

```yaml
- name: CLUSTER_NAME
Expand Down
8 changes: 4 additions & 4 deletions pkg/alb/loadbalancer/loadbalancer.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ const (
)

type NewDesiredLoadBalancerOptions struct {
ClusterName string
ALBNamePrefix string
Namespace string
IngressName string
ExistingLoadBalancer *LoadBalancer
Expand All @@ -70,7 +70,7 @@ func (a portList) Less(i, j int) bool { return a[i] < a[j] }
func NewDesiredLoadBalancer(o *NewDesiredLoadBalancerOptions) *LoadBalancer {
// TODO: LB name must contain only alphanumeric characters or hyphens, and must
// not begin or end with a hyphen.
name := createLBName(o.Namespace, o.IngressName, o.ClusterName)
name := createLBName(o.Namespace, o.IngressName, o.ALBNamePrefix)

newLoadBalancer := &LoadBalancer{
ID: name,
Expand Down Expand Up @@ -107,7 +107,7 @@ func NewDesiredLoadBalancer(o *NewDesiredLoadBalancerOptions) *LoadBalancer {
type NewCurrentLoadBalancerOptions struct {
LoadBalancer *elbv2.LoadBalancer
Tags util.Tags
ClusterName string
ALBNamePrefix string
Logger *log.Logger
ManagedSG *string
ManagedInstanceSG *string
Expand All @@ -126,7 +126,7 @@ func NewCurrentLoadBalancer(o *NewCurrentLoadBalancerOptions) (*LoadBalancer, er
return nil, fmt.Errorf("The LoadBalancer %s does not have a Namespace tag, can't import", *o.LoadBalancer.LoadBalancerName)
}

name := createLBName(namespace, ingressName, o.ClusterName)
name := createLBName(namespace, ingressName, o.ALBNamePrefix)
if name != *o.LoadBalancer.LoadBalancerName {
return nil, fmt.Errorf("Loadbalancer does not have expected (calculated) name. "+
"Expecting %s but was %s.", name, *o.LoadBalancer.LoadBalancerName)
Expand Down
8 changes: 4 additions & 4 deletions pkg/alb/targetgroup/targetgroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ type TargetGroup struct {
type NewDesiredTargetGroupOptions struct {
Annotations *annotations.Annotations
Tags util.Tags
ClusterName string
ALBNamePrefix string
LoadBalancerID string
Port int64
Logger *log.Logger
Expand All @@ -55,7 +55,7 @@ func NewDesiredTargetGroup(o *NewDesiredTargetGroupOptions) *TargetGroup {
hasher.Write([]byte(o.LoadBalancerID))
output := hex.EncodeToString(hasher.Sum(nil))

id := fmt.Sprintf("%.12s-%.5d-%.5s-%.7s", o.ClusterName, o.Port, *o.Annotations.BackendProtocol, output)
id := fmt.Sprintf("%.12s-%.5d-%.5s-%.7s", o.ALBNamePrefix, o.Port, *o.Annotations.BackendProtocol, output)

// Add the service name tag to the Target group as it's needed when reassembling ingresses after
// controller relaunch.
Expand Down Expand Up @@ -104,7 +104,7 @@ func NewDesiredTargetGroup(o *NewDesiredTargetGroupOptions) *TargetGroup {
type NewCurrentTargetGroupOptions struct {
TargetGroup *elbv2.TargetGroup
Tags util.Tags
ClusterName string
ALBNamePrefix string
LoadBalancerID string
Logger *log.Logger
}
Expand All @@ -115,7 +115,7 @@ func NewCurrentTargetGroup(o *NewCurrentTargetGroupOptions) (*TargetGroup, error
hasher.Write([]byte(o.LoadBalancerID))
output := hex.EncodeToString(hasher.Sum(nil))

id := fmt.Sprintf("%.12s-%.5d-%.5s-%.7s", o.ClusterName, *o.TargetGroup.Port, *o.TargetGroup.Protocol, output)
id := fmt.Sprintf("%.12s-%.5d-%.5s-%.7s", o.ALBNamePrefix, *o.TargetGroup.Port, *o.TargetGroup.Protocol, output)

svcName, ok := o.Tags.Get("ServiceName")
if !ok {
Expand Down
8 changes: 4 additions & 4 deletions pkg/alb/targetgroups/targetgroups.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func (t TargetGroups) StripDesiredState() {

type NewCurrentTargetGroupsOptions struct {
TargetGroups []*elbv2.TargetGroup
ClusterName string
ALBNamePrefix string
LoadBalancerID string
Logger *log.Logger
}
Expand All @@ -88,7 +88,7 @@ func NewCurrentTargetGroups(o *NewCurrentTargetGroupsOptions) (TargetGroups, err
tg, err := targetgroup.NewCurrentTargetGroup(&targetgroup.NewCurrentTargetGroupOptions{
TargetGroup: targetGroup,
Tags: tags,
ClusterName: o.ClusterName,
ALBNamePrefix: o.ALBNamePrefix,
LoadBalancerID: o.LoadBalancerID,
Logger: o.Logger,
})
Expand All @@ -114,7 +114,7 @@ type NewDesiredTargetGroupsOptions struct {
LoadBalancerID string
ExistingTargetGroups TargetGroups
Annotations *annotations.Annotations
ClusterName string
ALBNamePrefix string
Namespace string
Tags util.Tags
Logger *log.Logger
Expand All @@ -139,7 +139,7 @@ func NewDesiredTargetGroups(o *NewDesiredTargetGroupsOptions) (TargetGroups, err
targetGroup := targetgroup.NewDesiredTargetGroup(&targetgroup.NewDesiredTargetGroupOptions{
Annotations: o.Annotations,
Tags: o.Tags,
ClusterName: o.ClusterName,
ALBNamePrefix: o.ALBNamePrefix,
LoadBalancerID: o.LoadBalancerID,
Port: *port,
Logger: o.Logger,
Expand Down
59 changes: 32 additions & 27 deletions pkg/albingress/albingress.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ type ALBIngress struct {
namespace string
ingressName string
clusterName string
albNamePrefix string
recorder record.EventRecorder
ingress *extensions.Ingress
lock *sync.Mutex
Expand All @@ -45,11 +46,12 @@ type ALBIngress struct {
}

type NewALBIngressOptions struct {
Namespace string
Name string
ClusterName string
Ingress *extensions.Ingress
Recorder record.EventRecorder
Namespace string
Name string
ClusterName string
ALBNamePrefix string
Ingress *extensions.Ingress
Recorder record.EventRecorder
}

// ID returns an ingress id based off of a namespace and name
Expand All @@ -63,20 +65,22 @@ func ID(namespace, name string) string {
func NewALBIngress(o *NewALBIngressOptions) *ALBIngress {
ingressID := ID(o.Namespace, o.Name)
return &ALBIngress{
ID: ingressID,
namespace: o.Namespace,
clusterName: o.ClusterName,
ingressName: o.Name,
lock: new(sync.Mutex),
logger: log.New(ingressID),
recorder: o.Recorder,
ID: ingressID,
namespace: o.Namespace,
clusterName: o.ClusterName,
albNamePrefix: o.ALBNamePrefix,
ingressName: o.Name,
lock: new(sync.Mutex),
logger: log.New(ingressID),
recorder: o.Recorder,
}
}

type NewALBIngressFromIngressOptions struct {
Ingress *extensions.Ingress
ExistingIngress *ALBIngress
ClusterName string
ALBNamePrefix string
GetServiceNodePort func(string, int32) (*int64, error)
GetNodes func() util.AWSStringSlice
Recorder record.EventRecorder
Expand All @@ -91,11 +95,12 @@ func NewALBIngressFromIngress(o *NewALBIngressFromIngressOptions) *ALBIngress {

// Create newIngress ALBIngress object holding the resource details and some cluster information.
newIngress := NewALBIngress(&NewALBIngressOptions{
Namespace: o.Ingress.GetNamespace(),
Name: o.Ingress.Name,
ClusterName: o.ClusterName,
Recorder: o.Recorder,
Ingress: o.Ingress,
Namespace: o.Ingress.GetNamespace(),
Name: o.Ingress.Name,
ClusterName: o.ClusterName,
ALBNamePrefix: o.ALBNamePrefix,
Recorder: o.Recorder,
Ingress: o.Ingress,
})

if o.ExistingIngress != nil {
Expand All @@ -111,7 +116,7 @@ func NewALBIngressFromIngress(o *NewALBIngressFromIngressOptions) *ALBIngress {
}

// Load up the ingress with our current annotations.
newIngress.annotations, err = annotations.ParseAnnotations(o.Ingress.Annotations, newIngress.clusterName)
newIngress.annotations, err = annotations.ParseAnnotations(o.Ingress.Annotations, o.ClusterName)
if err != nil {
msg := fmt.Sprintf("Error parsing annotations: %s", err.Error())
newIngress.Reconciled = false
Expand All @@ -131,7 +136,7 @@ func NewALBIngressFromIngress(o *NewALBIngressFromIngressOptions) *ALBIngress {

// Assemble the load balancer
newIngress.LoadBalancer = loadbalancer.NewDesiredLoadBalancer(&loadbalancer.NewDesiredLoadBalancerOptions{
ClusterName: o.ClusterName,
ALBNamePrefix: o.ALBNamePrefix,
Namespace: o.Ingress.GetNamespace(),
ExistingLoadBalancer: newIngress.LoadBalancer,
IngressName: o.Ingress.Name,
Expand All @@ -146,7 +151,7 @@ func NewALBIngressFromIngress(o *NewALBIngressFromIngressOptions) *ALBIngress {
LoadBalancerID: newIngress.LoadBalancer.ID,
ExistingTargetGroups: newIngress.LoadBalancer.TargetGroups,
Annotations: newIngress.annotations,
ClusterName: o.ClusterName,
ALBNamePrefix: o.ALBNamePrefix,
Namespace: o.Ingress.GetNamespace(),
Tags: newIngress.Tags(),
Logger: newIngress.logger,
Expand Down Expand Up @@ -182,7 +187,7 @@ func NewALBIngressFromIngress(o *NewALBIngressFromIngressOptions) *ALBIngress {

type NewALBIngressFromAWSLoadBalancerOptions struct {
LoadBalancer *elbv2.LoadBalancer
ClusterName string
ALBNamePrefix string
Recorder record.EventRecorder
ManagedSG *string
ManagedSGPorts []int64
Expand All @@ -209,17 +214,17 @@ func NewALBIngressFromAWSLoadBalancer(o *NewALBIngressFromAWSLoadBalancerOptions

// Assemble ingress
ingress := NewALBIngress(&NewALBIngressOptions{
Namespace: namespace,
Name: ingressName,
ClusterName: o.ClusterName,
Recorder: o.Recorder,
Namespace: namespace,
Name: ingressName,
ALBNamePrefix: o.ALBNamePrefix,
Recorder: o.Recorder,
})

// Assemble load balancer
ingress.LoadBalancer, err = loadbalancer.NewCurrentLoadBalancer(&loadbalancer.NewCurrentLoadBalancerOptions{
LoadBalancer: o.LoadBalancer,
Tags: tags,
ClusterName: o.ClusterName,
ALBNamePrefix: o.ALBNamePrefix,
Logger: ingress.logger,
ManagedSG: o.ManagedSG,
ManagedSGPorts: o.ManagedSGPorts,
Expand All @@ -237,7 +242,7 @@ func NewALBIngressFromAWSLoadBalancer(o *NewALBIngressFromAWSLoadBalancerOptions

ingress.LoadBalancer.TargetGroups, err = targetgroups.NewCurrentTargetGroups(&targetgroups.NewCurrentTargetGroupsOptions{
TargetGroups: targetGroups,
ClusterName: o.ClusterName,
ALBNamePrefix: o.ALBNamePrefix,
LoadBalancerID: ingress.LoadBalancer.ID,
Logger: ingress.logger,
})
Expand Down
12 changes: 8 additions & 4 deletions pkg/albingresses/albingresses.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ func init() {
type NewALBIngressesFromIngressesOptions struct {
Recorder record.EventRecorder
ClusterName string
ALBNamePrefix string
Ingresses []interface{}
ALBIngresses ALBIngresses
IngressClass string
Expand Down Expand Up @@ -61,6 +62,7 @@ func NewALBIngressesFromIngresses(o *NewALBIngressesFromIngressesOptions) ALBIng
Ingress: ingResource,
ExistingIngress: existingIngress,
ClusterName: o.ClusterName,
ALBNamePrefix: o.ALBNamePrefix,
GetServiceNodePort: o.GetServiceNodePort,
GetNodes: o.GetNodes,
Recorder: o.Recorder,
Expand All @@ -74,8 +76,8 @@ func NewALBIngressesFromIngresses(o *NewALBIngressesFromIngressesOptions) ALBIng

// AssembleIngressesFromAWSOptions are the options to AssembleIngressesFromAWS
type AssembleIngressesFromAWSOptions struct {
Recorder record.EventRecorder
ClusterName string
Recorder record.EventRecorder
ALBNamePrefix string
}

// AssembleIngressesFromAWS builds a list of existing ingresses from resources in AWS
Expand All @@ -84,7 +86,7 @@ func AssembleIngressesFromAWS(o *AssembleIngressesFromAWSOptions) ALBIngresses {
var ingresses ALBIngresses

// Fetch a list of load balancers that match this cluser name
loadBalancers, err := albelbv2.ELBV2svc.ClusterLoadBalancers(&o.ClusterName)
loadBalancers, err := albelbv2.ELBV2svc.ClusterLoadBalancers(&o.ALBNamePrefix)
if err != nil {
logger.Fatalf(err.Error())
}
Expand All @@ -107,6 +109,7 @@ func AssembleIngressesFromAWS(o *AssembleIngressesFromAWSOptions) ALBIngresses {
}

for _, tag := range tags {
// If the subnet is labeled as managed by ALB, capture it as the managedSG
if *tag.Key == ec2.ManagedByKey && *tag.Value == ec2.ManagedByValue {
managedSG = loadBalancer.SecurityGroups[0]
ports, err := ec2.EC2svc.DescribeSGPorts(loadBalancer.SecurityGroups[0])
Expand All @@ -117,6 +120,7 @@ func AssembleIngressesFromAWS(o *AssembleIngressesFromAWSOptions) ALBIngresses {
managedSGPorts = ports
}
}
// when a alb-managed SG existed, we must find a correlated instance SG
if managedSG != nil {
instanceSG, err := ec2.EC2svc.DescribeSGByPermissionGroup(managedSG)
if err != nil {
Expand All @@ -128,7 +132,7 @@ func AssembleIngressesFromAWS(o *AssembleIngressesFromAWSOptions) ALBIngresses {

albIngress, err := albingress.NewALBIngressFromAWSLoadBalancer(&albingress.NewALBIngressFromAWSLoadBalancerOptions{
LoadBalancer: loadBalancer,
ClusterName: o.ClusterName,
ALBNamePrefix: o.ALBNamePrefix,
Recorder: o.Recorder,
ManagedSG: managedSG,
ManagedSGPorts: managedSGPorts,
Expand Down
23 changes: 17 additions & 6 deletions pkg/controller/alb-controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ type ALBController struct {
recorder record.EventRecorder
ALBIngresses albingresses.ALBIngresses
clusterName string
albNamePrefix string
IngressClass string
lastUpdate time.Time
albSyncInterval time.Duration
Expand Down Expand Up @@ -67,20 +68,21 @@ func NewALBController(awsconfig *aws.Config, conf *config.Config) *ALBController
// Additionally, it calls the ingress assembly from AWS.
func (ac *ALBController) Configure(ic *controller.GenericController) {
ac.IngressClass = ic.IngressClass()
ac.albNamePrefix = cleanClusterName(ac.clusterName)
if ac.IngressClass != "" {
logger.Infof("Ingress class set to %s", ac.IngressClass)
}

if len(ac.clusterName) > 11 {
logger.Exitf("Cluster name must be 11 characters or less")
if len(ac.albNamePrefix) > 11 {
logger.Exitf("ALB name prefix must be 11 characters or less")
}

if ac.clusterName == "" {
logger.Exitf("A cluster name must be defined")
}

if strings.Contains(ac.clusterName, "-") {
logger.Exitf("Cluster name cannot contain '-'")
if strings.Contains(ac.albNamePrefix, "-") {
logger.Exitf("ALB name prefix cannot contain '-'")
}

ac.recorder = ic.GetRecorder()
Expand Down Expand Up @@ -113,8 +115,8 @@ func (ac *ALBController) syncALBsWithAWS() {
ac.mutex.Lock()
defer ac.mutex.Unlock()
ac.ALBIngresses = albingresses.AssembleIngressesFromAWS(&albingresses.AssembleIngressesFromAWSOptions{
Recorder: ac.recorder,
ClusterName: ac.clusterName,
Recorder: ac.recorder,
ALBNamePrefix: ac.albNamePrefix,
})
}

Expand All @@ -139,6 +141,7 @@ func (ac *ALBController) update() {
newIngresses := albingresses.NewALBIngressesFromIngresses(&albingresses.NewALBIngressesFromIngressesOptions{
Recorder: ac.recorder,
ClusterName: ac.clusterName,
ALBNamePrefix: ac.albNamePrefix,
Ingresses: ac.storeLister.Ingress.List(),
ALBIngresses: ac.ALBIngresses,
IngressClass: ac.IngressClass,
Expand Down Expand Up @@ -308,3 +311,11 @@ func (ac *ALBController) GetNodes() util.AWSStringSlice {
sort.Sort(result)
return result
}

func cleanClusterName(cn string) string {
n := strings.Replace(cn, "-", "", -1)
if len(n) > 11 {
n = n[:11]
}
return n
}

0 comments on commit 918d034

Please sign in to comment.