Skip to content

Commit

Permalink
Merge pull request #3593 from justinsb/limit_gce_task_length
Browse files Browse the repository at this point in the history 
Automatic merge from submit-queue.

GCE: Limit length of InstanceTemplate names
  • Loading branch information
Kubernetes Submit Queue committed Oct 11, 2017
2 parents 961a68e + d71bd09 commit 5b6a8ec
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 26 deletions.
14 changes: 11 additions & 3 deletions pkg/model/gcemodel/autoscalinggroup.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,11 @@ func (b *AutoscalingGroupModelBuilder) Build(c *fi.ModelBuilderContext) error {
volumeType = DefaultVolumeType
}

namePrefix := gce.LimitedLengthName(name, gcetasks.InstanceTemplateNamePrefixMaxLength)

t := &gcetasks.InstanceTemplate{
Name: s(name),
NamePrefix: s(namePrefix),
Lifecycle: b.Lifecycle,
Network: b.LinkToNetwork(),
MachineType: s(ig.Spec.MachineType),
Expand Down Expand Up @@ -118,9 +121,6 @@ func (b *AutoscalingGroupModelBuilder) Build(c *fi.ModelBuilderContext) error {
return err
}

targetSizes := make([]int, len(zones), len(zones))
totalSize := 0

// TODO: Duplicated from aws - move to defaults?
minSize := 1
if ig.Spec.MinSize != nil {
Expand All @@ -129,6 +129,14 @@ func (b *AutoscalingGroupModelBuilder) Build(c *fi.ModelBuilderContext) error {
minSize = 2
}

// We have to assign instances to the various zones
// TODO: Switch to regional managed instance group
// But we can't yet use RegionInstanceGroups:
// 1) no support in terraform
// 2) we can't steer to specific zones AFAICT, only to all zones in the region

targetSizes := make([]int, len(zones), len(zones))
totalSize := 0
for i := range zones {
targetSizes[i] = minSize / len(zones)
totalSize += targetSizes[i]
Expand Down
32 changes: 16 additions & 16 deletions tests/integration/update_cluster/ha_gce/kubernetes.tf
View file
Original file line number Diff line number Diff line change
Expand Up @@ -273,48 +273,48 @@ resource "google_compute_firewall" "ssh-external-to-node-ha-gce-example-com" {
target_tags = ["ha-gce-example-com-k8s-io-role-node"]
}

resource "google_compute_instance_group_manager" "us-test1-a-master-us-test1-a-ha-gce-example-com" {
name = "us-test1-a-master-us-test1-a-ha-gce-example-com"
resource "google_compute_instance_group_manager" "a-master-us-test1-a-ha-gce-example-com" {
name = "a-master-us-test1-a-ha-gce-example-com"
zone = "us-test1-a"
base_instance_name = "master-us-test1-a"
instance_template = "${google_compute_instance_template.master-us-test1-a-ha-gce-example-com.self_link}"
target_size = 1
}

resource "google_compute_instance_group_manager" "us-test1-a-nodes-ha-gce-example-com" {
name = "us-test1-a-nodes-ha-gce-example-com"
resource "google_compute_instance_group_manager" "a-nodes-ha-gce-example-com" {
name = "a-nodes-ha-gce-example-com"
zone = "us-test1-a"
base_instance_name = "nodes"
instance_template = "${google_compute_instance_template.nodes-ha-gce-example-com.self_link}"
target_size = 1
}

resource "google_compute_instance_group_manager" "us-test1-b-master-us-test1-b-ha-gce-example-com" {
name = "us-test1-b-master-us-test1-b-ha-gce-example-com"
resource "google_compute_instance_group_manager" "b-master-us-test1-b-ha-gce-example-com" {
name = "b-master-us-test1-b-ha-gce-example-com"
zone = "us-test1-b"
base_instance_name = "master-us-test1-b"
instance_template = "${google_compute_instance_template.master-us-test1-b-ha-gce-example-com.self_link}"
target_size = 1
}

resource "google_compute_instance_group_manager" "us-test1-b-nodes-ha-gce-example-com" {
name = "us-test1-b-nodes-ha-gce-example-com"
resource "google_compute_instance_group_manager" "b-nodes-ha-gce-example-com" {
name = "b-nodes-ha-gce-example-com"
zone = "us-test1-b"
base_instance_name = "nodes"
instance_template = "${google_compute_instance_template.nodes-ha-gce-example-com.self_link}"
target_size = 1
}

resource "google_compute_instance_group_manager" "us-test1-c-master-us-test1-c-ha-gce-example-com" {
name = "us-test1-c-master-us-test1-c-ha-gce-example-com"
resource "google_compute_instance_group_manager" "c-master-us-test1-c-ha-gce-example-com" {
name = "c-master-us-test1-c-ha-gce-example-com"
zone = "us-test1-c"
base_instance_name = "master-us-test1-c"
instance_template = "${google_compute_instance_template.master-us-test1-c-ha-gce-example-com.self_link}"
target_size = 1
}

resource "google_compute_instance_group_manager" "us-test1-c-nodes-ha-gce-example-com" {
name = "us-test1-c-nodes-ha-gce-example-com"
resource "google_compute_instance_group_manager" "c-nodes-ha-gce-example-com" {
name = "c-nodes-ha-gce-example-com"
zone = "us-test1-c"
base_instance_name = "nodes"
instance_template = "${google_compute_instance_template.nodes-ha-gce-example-com.self_link}"
Expand Down Expand Up @@ -357,7 +357,7 @@ resource "google_compute_instance_template" "master-us-test1-a-ha-gce-example-co
}

tags = ["ha-gce-example-com-k8s-io-role-master"]
name_prefix = "master-us-test1-a-ha-gce-example-com"
name_prefix = "master-us-test1-a-ha-gce-example-com-"
}

resource "google_compute_instance_template" "master-us-test1-b-ha-gce-example-com" {
Expand Down Expand Up @@ -396,7 +396,7 @@ resource "google_compute_instance_template" "master-us-test1-b-ha-gce-example-co
}

tags = ["ha-gce-example-com-k8s-io-role-master"]
name_prefix = "master-us-test1-b-ha-gce-example-com"
name_prefix = "master-us-test1-b-ha-gce-example-com-"
}

resource "google_compute_instance_template" "master-us-test1-c-ha-gce-example-com" {
Expand Down Expand Up @@ -435,7 +435,7 @@ resource "google_compute_instance_template" "master-us-test1-c-ha-gce-example-co
}

tags = ["ha-gce-example-com-k8s-io-role-master"]
name_prefix = "master-us-test1-c-ha-gce-example-com"
name_prefix = "master-us-test1-c-ha-gce-example-com-"
}

resource "google_compute_instance_template" "nodes-ha-gce-example-com" {
Expand Down Expand Up @@ -474,7 +474,7 @@ resource "google_compute_instance_template" "nodes-ha-gce-example-com" {
}

tags = ["ha-gce-example-com-k8s-io-role-node"]
name_prefix = "nodes-ha-gce-example-com"
name_prefix = "nodes-ha-gce-example-com-"
}

resource "google_compute_network" "default" {
Expand Down
40 changes: 38 additions & 2 deletions upup/pkg/fi/cloudup/gce/instancegroups.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,18 @@ limitations under the License.
package gce

import (
"encoding/base32"
"fmt"
"hash/fnv"
"strconv"
"strings"

"github.com/golang/glog"
context "golang.org/x/net/context"
compute "google.golang.org/api/compute/v0.beta"
"k8s.io/api/core/v1"
"k8s.io/kops/pkg/apis/kops"
"k8s.io/kops/pkg/cloudinstances"
"strconv"
)

// DeleteGroup deletes a cloud of instances controlled by an Instance Group Manager
Expand Down Expand Up @@ -195,10 +199,42 @@ func getCloudGroups(c GCECloud, cluster *kops.Cluster, instancegroups []*kops.In

// NameForInstanceGroupManager builds a name for an InstanceGroupManager in the specified zone
func NameForInstanceGroupManager(c *kops.Cluster, ig *kops.InstanceGroup, zone string) string {
name := SafeObjectName(zone+"."+ig.ObjectMeta.Name, c.ObjectMeta.Name)
shortZone := zone
lastDash := strings.LastIndex(shortZone, "-")
if lastDash != -1 {
shortZone = shortZone[lastDash+1:]
}
name := SafeObjectName(shortZone+"."+ig.ObjectMeta.Name, c.ObjectMeta.Name)
name = LimitedLengthName(name, 63)
return name
}

// LimitedLengthName returns a string subject to a maximum length
func LimitedLengthName(s string, n int) string {
// We only use the hash if we need to
if len(s) <= n {
return s
}

h := fnv.New32a()
if _, err := h.Write([]byte(s)); err != nil {
glog.Fatalf("error hashing values: %v", err)
}
hashString := base32.HexEncoding.EncodeToString(h.Sum(nil))
hashString = strings.ToLower(hashString)
if len(hashString) > 6 {
hashString = hashString[:6]
}

maxBaseLength := n - len(hashString) - 1
if len(s) > maxBaseLength {
s = s[:maxBaseLength]
}
s = s + "-" + hashString

return s
}

// matchInstanceGroup filters a list of instancegroups for recognized cloud groups
func matchInstanceGroup(mig *compute.InstanceGroupManager, c *kops.Cluster, instancegroups []*kops.InstanceGroup) (*kops.InstanceGroup, error) {
migName := LastComponent(mig.Name)
Expand Down
18 changes: 13 additions & 5 deletions upup/pkg/fi/cloudup/gcetasks/instancetemplate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,18 @@ import (
"k8s.io/kops/upup/pkg/fi/cloudup/terraform"
)

// InstanceTemplateNamePrefixMaxLength is the max length for the NamePrefix of an InstanceTemplate
// 52 = 63 - 10 - 1; 63 is the GCE limit; 10 is the length of seconds since epoch; and one for the dash
const InstanceTemplateNamePrefixMaxLength = 63 - 10 - 1

// InstanceTemplate represents a GCE InstanceTemplate
//go:generate fitask -type=InstanceTemplate
type InstanceTemplate struct {
// Name will be used for as the name prefix
Name *string
Name *string

// NamePrefix is used as the prefix for the names; we add a timestamp. Max = InstanceTemplateNamePrefixMaxLength
NamePrefix *string

Lifecycle *fi.Lifecycle

Network *Network
Expand Down Expand Up @@ -83,7 +90,7 @@ func (e *InstanceTemplate) Find(c *fi.Context) (*InstanceTemplate, error) {
}

for _, r := range response.Items {
if !strings.HasPrefix(r.Name, fi.StringValue(e.Name)) {
if !strings.HasPrefix(r.Name, fi.StringValue(e.NamePrefix)+"-") {
continue
}

Expand Down Expand Up @@ -155,6 +162,7 @@ func (e *InstanceTemplate) Find(c *fi.Context) (*InstanceTemplate, error) {

// Prevent spurious changes
actual.Name = e.Name
actual.NamePrefix = e.NamePrefix

actual.ID = &r.Name
if e.ID == nil {
Expand Down Expand Up @@ -356,7 +364,7 @@ func (_ *InstanceTemplate) RenderGCE(t *gce.GCEAPITarget, a, e, changes *Instanc
if a == nil {
glog.V(4).Infof("Creating InstanceTemplate %v", i)

name := fi.StringValue(e.Name) + "-" + strconv.FormatInt(time.Now().Unix(), 10)
name := fi.StringValue(e.NamePrefix) + "-" + strconv.FormatInt(time.Now().Unix(), 10)
e.ID = &name
i.Name = name

Expand Down Expand Up @@ -508,7 +516,7 @@ func (_ *InstanceTemplate) RenderTerraform(t *terraform.TerraformTarget, a, e, c
name := fi.StringValue(e.Name)

tf := &terraformInstanceTemplate{
NamePrefix: fi.StringValue(e.Name),
NamePrefix: fi.StringValue(e.NamePrefix) + "-",
}

tf.CanIPForward = i.Properties.CanIpForward
Expand Down

0 comments on commit 5b6a8ec

Please sign in to comment.