From 3cce79d4e43aa4bb681dd1f34eca7d3beff2c3a3 Mon Sep 17 00:00:00 2001 From: justinsb Date: Fri, 28 Jul 2023 17:06:19 -0400 Subject: [PATCH] gce: Refactor resource labeling Create a more strongly-typed label object and use it when labeling cluster resources. --- pkg/model/components/etcdmanager/model.go | 3 ++- pkg/model/gcemodel/api_loadbalancer.go | 20 ++++++++++++-------- pkg/model/gcemodel/autoscalinggroup.go | 7 ++++--- pkg/model/master_volumes.go | 4 +++- pkg/resources/gce/gce.go | 6 +++--- upup/pkg/fi/cloudup/gce/labels.go | 3 ++- upup/pkg/fi/cloudup/gce/utils.go | 11 +++++++++++ upup/pkg/fi/cloudup/utils.go | 3 ++- 8 files changed, 39 insertions(+), 18 deletions(-) diff --git a/pkg/model/components/etcdmanager/model.go b/pkg/model/components/etcdmanager/model.go index 451870320a..758a16e4bf 100644 --- a/pkg/model/components/etcdmanager/model.go +++ b/pkg/model/components/etcdmanager/model.go @@ -476,8 +476,9 @@ func (b *EtcdManagerBuilder) buildPod(etcdCluster kops.EtcdClusterSpec, instance case kops.CloudProviderGCE: config.VolumeProvider = "gce" + clusterLabel := gce.LabelForCluster(b.Cluster.Name) config.VolumeTag = []string{ - gce.GceLabelNameKubernetesCluster + "=" + gce.SafeClusterName(b.Cluster.Name), + clusterLabel.Key + "=" + clusterLabel.Value, gce.GceLabelNameEtcdClusterPrefix + etcdCluster.Name, gce.GceLabelNameRolePrefix + "master=master", } diff --git a/pkg/model/gcemodel/api_loadbalancer.go b/pkg/model/gcemodel/api_loadbalancer.go index acb942e13e..10fe65e4e7 100644 --- a/pkg/model/gcemodel/api_loadbalancer.go +++ b/pkg/model/gcemodel/api_loadbalancer.go @@ -70,6 +70,8 @@ func (b *APILoadBalancerBuilder) createPublicLB(c *fi.CloudupModelBuilderContext } c.AddTask(ipAddress) + clusterLabel := gce.LabelForCluster(b.ClusterName()) + c.AddTask(&gcetasks.ForwardingRule{ Name: s(b.NameForForwardingRule("api")), Lifecycle: b.Lifecycle, @@ -78,8 +80,8 @@ func (b *APILoadBalancerBuilder) createPublicLB(c *fi.CloudupModelBuilderContext IPAddress: ipAddress, IPProtocol: "TCP", Labels: map[string]string{ - gce.GceLabelNameKubernetesCluster: gce.SafeClusterName(b.ClusterName()), - "name": "api", + clusterLabel.Key: clusterLabel.Value, + "name": "api", }, }) if b.Cluster.UsesNoneDNS() { @@ -91,8 +93,8 @@ func (b *APILoadBalancerBuilder) createPublicLB(c *fi.CloudupModelBuilderContext IPAddress: ipAddress, IPProtocol: "TCP", Labels: map[string]string{ - gce.GceLabelNameKubernetesCluster: gce.SafeClusterName(b.ClusterName()), - "name": "kops-controller", + clusterLabel.Key: clusterLabel.Value, + "name": "kops-controller", }, }) } @@ -145,6 +147,8 @@ func (b *APILoadBalancerBuilder) addFirewallRules(c *fi.CloudupModelBuilderConte // GCP this entails creating a health check, backend service, and one forwarding rule // per specified subnet pointing to that backend service. func (b *APILoadBalancerBuilder) createInternalLB(c *fi.CloudupModelBuilderContext) error { + clusterLabel := gce.LabelForCluster(b.ClusterName()) + hc := &gcetasks.HealthCheck{ Name: s(b.NameForHealthCheck("api")), Port: wellknownports.KubeAPIServer, @@ -213,8 +217,8 @@ func (b *APILoadBalancerBuilder) createInternalLB(c *fi.CloudupModelBuilderConte Network: network, Subnetwork: subnet, Labels: map[string]string{ - gce.GceLabelNameKubernetesCluster: gce.SafeClusterName(b.ClusterName()), - "name": "api-" + sn.Name, + clusterLabel.Key: clusterLabel.Value, + "name": "api-" + sn.Name, }, }) if b.Cluster.UsesNoneDNS() { @@ -229,8 +233,8 @@ func (b *APILoadBalancerBuilder) createInternalLB(c *fi.CloudupModelBuilderConte Network: network, Subnetwork: subnet, Labels: map[string]string{ - gce.GceLabelNameKubernetesCluster: gce.SafeClusterName(b.ClusterName()), - "name": "kops-controller-" + sn.Name, + clusterLabel.Key: clusterLabel.Value, + "name": "kops-controller-" + sn.Name, }, }) } diff --git a/pkg/model/gcemodel/autoscalinggroup.go b/pkg/model/gcemodel/autoscalinggroup.go index 8995372151..dbf44a20df 100644 --- a/pkg/model/gcemodel/autoscalinggroup.go +++ b/pkg/model/gcemodel/autoscalinggroup.go @@ -175,11 +175,12 @@ func (b *AutoscalingGroupModelBuilder) buildInstanceTemplate(c *fi.CloudupModelB case kops.InstanceGroupRoleBastion: t.Tags = append(t.Tags, b.GCETagForRole(kops.InstanceGroupRoleBastion)) } + clusterLabel := gce.LabelForCluster(b.ClusterName()) roleLabel := gce.GceLabelNameRolePrefix + ig.Spec.Role.ToLowerString() t.Labels = map[string]string{ - gce.GceLabelNameKubernetesCluster: gce.SafeClusterName(b.ClusterName()), - roleLabel: "", - gce.GceLabelNameInstanceGroup: ig.ObjectMeta.Name, + clusterLabel.Key: clusterLabel.Value, + roleLabel: "", + gce.GceLabelNameInstanceGroup: ig.ObjectMeta.Name, } if ig.Spec.Role == kops.InstanceGroupRoleControlPlane { t.Labels[gce.GceLabelNameRolePrefix+"master"] = "" diff --git a/pkg/model/master_volumes.go b/pkg/model/master_volumes.go index fc15763cd1..802af8f795 100644 --- a/pkg/model/master_volumes.go +++ b/pkg/model/master_volumes.go @@ -272,9 +272,11 @@ func (b *MasterVolumeBuilder) addGCEVolume(c *fi.CloudupModelBuilderContext, pre // This is the configuration of the etcd cluster clusterSpec := m.Name + "/" + strings.Join(allMembers, ",") + clusterLabel := gce.LabelForCluster(b.ClusterName()) + // The tags are how protokube knows to mount the volume and use it for etcd tags := make(map[string]string) - tags[gce.GceLabelNameKubernetesCluster] = gce.SafeClusterName(b.ClusterName()) + tags[clusterLabel.Key] = clusterLabel.Value tags[gce.GceLabelNameRolePrefix+"master"] = "master" // Can't start with a number tags[gce.GceLabelNameEtcdClusterPrefix+etcd.Name] = gce.EncodeGCELabel(clusterSpec) diff --git a/pkg/resources/gce/gce.go b/pkg/resources/gce/gce.go index 81475fccf0..cefb0890dd 100644 --- a/pkg/resources/gce/gce.go +++ b/pkg/resources/gce/gce.go @@ -299,7 +299,7 @@ func (d *clusterDiscoveryGCE) listManagedInstances(igm *compute.InstanceGroupMan func (d *clusterDiscoveryGCE) findGCEDisks() ([]*compute.Disk, error) { c := d.gceCloud - clusterTag := gce.SafeClusterName(d.clusterName) + clusterLabel := gce.LabelForCluster(d.clusterName) var matches []*compute.Disk @@ -316,8 +316,8 @@ func (d *clusterDiscoveryGCE) findGCEDisks() ([]*compute.Disk, error) { for _, d := range list.Disks { match := false for k, v := range d.Labels { - if k == gce.GceLabelNameKubernetesCluster { - if v == clusterTag { + if k == clusterLabel.Key { + if v == clusterLabel.Value { match = true } else { match = false diff --git a/upup/pkg/fi/cloudup/gce/labels.go b/upup/pkg/fi/cloudup/gce/labels.go index 823eeb1474..c0926b1952 100644 --- a/upup/pkg/fi/cloudup/gce/labels.go +++ b/upup/pkg/fi/cloudup/gce/labels.go @@ -27,7 +27,8 @@ import ( const ( // The tag name we use to differentiate multiple logically independent clusters running in the same region - GceLabelNameKubernetesCluster = "k8s-io-cluster-name" + gceLabelNameKubernetesCluster = "k8s-io-cluster-name" + GceLabelNameInstanceGroup = "k8s-io-instance-group" GceLabelNameRolePrefix = "k8s-io-role-" GceLabelNameEtcdClusterPrefix = "k8s-io-etcd-" diff --git a/upup/pkg/fi/cloudup/gce/utils.go b/upup/pkg/fi/cloudup/gce/utils.go index 64bfb3582c..c59cb797b7 100644 --- a/upup/pkg/fi/cloudup/gce/utils.go +++ b/upup/pkg/fi/cloudup/gce/utils.go @@ -99,6 +99,17 @@ func SafeClusterName(clusterName string) string { return safeClusterName } +type Label struct { + Key string + Value string +} + +// LabelForCluster returns the GCE label we use for resources for this cluster. +func LabelForCluster(clusterName string) Label { + v := SafeClusterName(clusterName) + return Label{Key: gceLabelNameKubernetesCluster, Value: v} +} + // SafeTruncatedClusterName returns a safe and truncated cluster name func SafeTruncatedClusterName(clusterName string, maxLength int) string { // GCE does not support . in tags / names diff --git a/upup/pkg/fi/cloudup/utils.go b/upup/pkg/fi/cloudup/utils.go index d29cb89db8..a0ec5be66d 100644 --- a/upup/pkg/fi/cloudup/utils.go +++ b/upup/pkg/fi/cloudup/utils.go @@ -57,7 +57,8 @@ func BuildCloud(cluster *kops.Cluster) (fi.Cloud, error) { return nil, fmt.Errorf("project is required for GCE - try gcloud config get-value project") } - labels := map[string]string{gce.GceLabelNameKubernetesCluster: gce.SafeClusterName(cluster.ObjectMeta.Name)} + clusterLabel := gce.LabelForCluster(cluster.ObjectMeta.Name) + labels := map[string]string{clusterLabel.Key: clusterLabel.Value} gceCloud, err := gce.NewGCECloud(region, project, labels) if err != nil {