Fix Cluster Name in all situations

Signed-off-by: Mikkel Oscar Lyderik Larsen <[email protected]>
zalando-incubator · Nov 1, 2024 · f675c9b · f675c9b
1 parent 0366db4
commit f675c9b
Show file tree

Hide file tree

Showing 16 changed files with 109 additions and 92 deletions.
diff --git a/api/cluster.go b/api/cluster.go
@@ -12,8 +12,16 @@ import (
 	"github.com/zalando-incubator/cluster-lifecycle-manager/channel"
 )
 
+// A provider ID is a string that identifies a cluster provider.
+type ProviderID string
+
 const (
 	overrideChannelConfigItem = "override_channel"
+
+	// ZalandoAWSProvider is the provider ID for Zalando managed AWS clusters.
+	ZalandoAWSProvider ProviderID = "zalando-aws"
+	// ZalandoEKSProvider is the provider ID for AWS EKS clusters.
+	ZalandoEKSProvider ProviderID = "zalando-eks"
 )
 
 // Cluster describes a kubernetes cluster and related configuration.
@@ -29,7 +37,7 @@ type Cluster struct {
 	LifecycleStatus       string            `json:"lifecycle_status"       yaml:"lifecycle_status"`
 	LocalID               string            `json:"local_id"               yaml:"local_id"`
 	NodePools             []*NodePool       `json:"node_pools"             yaml:"node_pools"`
-	Provider              string            `json:"provider"               yaml:"provider"`
+	Provider              ProviderID        `json:"provider"               yaml:"provider"`
 	Region                string            `json:"region"                 yaml:"region"`
 	Status                *ClusterStatus    `json:"status"                 yaml:"status"`
 	Owner                 string            `json:"owner"                  yaml:"owner"`
@@ -73,7 +81,7 @@ func (cluster *Cluster) Version(channelVersion channel.ConfigVersion) (*ClusterV
 	if err != nil {
 		return nil, err
 	}
-	_, err = state.WriteString(cluster.Provider)
+	_, err = state.WriteString(string(cluster.Provider))
 	if err != nil {
 		return nil, err
 	}
@@ -195,3 +203,10 @@ func (cluster *Cluster) ASGBackedPools() []*NodePool {
 	}
 	return cp
 }
+
+func (cluster Cluster) Name() string {
+	if cluster.Provider == ZalandoEKSProvider {
+		return cluster.LocalID
+	}
+	return cluster.ID
+}
diff --git a/api/cluster_test.go b/api/cluster_test.go
@@ -116,3 +116,21 @@ func TestVersion(t *testing.T) {
 		require.NotEqual(t, version, newVersion, "cluster field: %s", field)
 	}
 }
+
+func TestName(t *testing.T) {
+	cluster := &Cluster{
+		ID:       "aws:123456789012:eu-central-1:test-cluster",
+		LocalID:  "test-cluster",
+		Provider: ZalandoAWSProvider,
+	}
+
+	require.Equal(t, cluster.ID, cluster.Name())
+
+	cluster = &Cluster{
+		ID:       "aws:123456789012:eu-central-1:test-cluster",
+		LocalID:  "test-cluster",
+		Provider: ZalandoEKSProvider,
+	}
+
+	require.Equal(t, cluster.LocalID, cluster.Name())
+}
diff --git a/cmd/clm/main.go b/cmd/clm/main.go
@@ -12,6 +12,7 @@ import (
 
 	kingpin "github.com/alecthomas/kingpin/v2"
 	log "github.com/sirupsen/logrus"
+	"github.com/zalando-incubator/cluster-lifecycle-manager/api"
 	"github.com/zalando-incubator/cluster-lifecycle-manager/channel"
 	"github.com/zalando-incubator/cluster-lifecycle-manager/config"
 	"github.com/zalando-incubator/cluster-lifecycle-manager/controller"
@@ -115,8 +116,8 @@ func main() {
 
 	execManager := command.NewExecManager(cfg.ConcurrentExternalProcesses)
 
-	provisioners := map[provisioner.ProviderID]provisioner.Provisioner{
-		provisioner.ZalandoAWSProvider: provisioner.NewZalandoAWSProvisioner(
+	provisioners := map[api.ProviderID]provisioner.Provisioner{
+		api.ZalandoAWSProvider: provisioner.NewZalandoAWSProvisioner(
 			execManager,
 			clusterTokenSource,
 			secretDecrypter,
@@ -130,7 +131,7 @@ func main() {
 				ManageEtcdStack: cfg.ManageEtcdStack,
 			},
 		),
-		provisioner.ZalandoEKSProvider: provisioner.NewZalandoEKSProvisioner(
+		api.ZalandoEKSProvider: provisioner.NewZalandoEKSProvisioner(
 			execManager,
 			secretDecrypter,
 			cfg.AssumedRole,
@@ -218,7 +219,7 @@ func main() {
 			cluster.ConfigItems[key] = decryptedValue
 		}
 
-		p, ok := provisioners[provisioner.ProviderID(cluster.Provider)]
+		p, ok := provisioners[cluster.Provider]
 		if !ok {
 			log.Fatalf(
 				"Cluster %s: unknown provider %q",

diff --git a/controller/controller.go b/controller/controller.go
@@ -44,7 +44,7 @@ type Controller struct {
 	logger               *log.Entry
 	execManager          *command.ExecManager
 	registry             registry.Registry
-	provisioners         map[provisioner.ProviderID]provisioner.Provisioner
+	provisioners         map[api.ProviderID]provisioner.Provisioner
 	providers            []string
 	channelConfigSourcer channel.ConfigSource
 	interval             time.Duration
@@ -58,7 +58,7 @@ func New(
 	logger *log.Entry,
 	execManager *command.ExecManager,
 	registry registry.Registry,
-	provisioners map[provisioner.ProviderID]provisioner.Provisioner,
+	provisioners map[api.ProviderID]provisioner.Provisioner,
 	channelConfigSourcer channel.ConfigSource,
 	options *Options,
 ) *Controller {
@@ -184,7 +184,7 @@ func (c *Controller) doProcessCluster(ctx context.Context, logger *log.Entry, cl
 		}
 	}()
 
-	provisioner, ok := c.provisioners[provisioner.ProviderID(cluster.Provider)]
+	provisioner, ok := c.provisioners[api.ProviderID(cluster.Provider)]
 	if !ok {
 		return fmt.Errorf(
 			"cluster %s: unknown provider %q",

diff --git a/controller/controller_test.go b/controller/controller_test.go
@@ -18,17 +18,17 @@ import (
 )
 
 const (
-	nextVersion                              = "version"
-	mockProvider      provisioner.ProviderID = "<mock>"
-	otherMockProvider provisioner.ProviderID = "<othermock>"
+	nextVersion                      = "version"
+	mockProvider      api.ProviderID = "<mock>"
+	otherMockProvider api.ProviderID = "<othermock>"
 )
 
 var defaultLogger = log.WithFields(map[string]interface{}{})
 
 type mockProvisioner struct{}
 
 func (p *mockProvisioner) Supports(cluster *api.Cluster) bool {
-	return cluster.Provider == string(mockProvider)
+	return cluster.Provider == mockProvider
 }
 
 func (p *mockProvisioner) Provision(
@@ -116,7 +116,7 @@ func createMockRegistry(lifecycleStatus string, status *api.ClusterStatus) *mock
 		Channel:               "alpha",
 		LifecycleStatus:       lifecycleStatus,
 		Status:                status,
-		Provider:              string(mockProvider),
+		Provider:              mockProvider,
 	}
 	return &mockRegistry{theCluster: cluster}
 }
@@ -221,15 +221,15 @@ func TestProcessCluster(t *testing.T) {
 	for _, ti := range []struct {
 		testcase      string
 		registry      registry.Registry
-		provisioners  map[provisioner.ProviderID]provisioner.Provisioner
+		provisioners  map[api.ProviderID]provisioner.Provisioner
 		channelSource channel.ConfigSource
 		options       *Options
 		success       bool
 	}{
 		{
 			testcase: "lifecycle status requested",
 			registry: createMockRegistry(statusRequested, nil),
-			provisioners: map[provisioner.ProviderID]provisioner.Provisioner{
+			provisioners: map[api.ProviderID]provisioner.Provisioner{
 				mockProvider: &mockProvisioner{},
 			},
 
@@ -240,7 +240,7 @@ func TestProcessCluster(t *testing.T) {
 		{
 			testcase: "lifecycle status requested, invalid provider",
 			registry: createMockRegistry(statusRequested, nil),
-			provisioners: map[provisioner.ProviderID]provisioner.Provisioner{
+			provisioners: map[api.ProviderID]provisioner.Provisioner{
 				otherMockProvider: &mockProvisioner{},
 			},
 
@@ -251,7 +251,7 @@ func TestProcessCluster(t *testing.T) {
 		{
 			testcase: "lifecycle status ready",
 			registry: createMockRegistry(statusReady, nil),
-			provisioners: map[provisioner.ProviderID]provisioner.Provisioner{
+			provisioners: map[api.ProviderID]provisioner.Provisioner{
 				mockProvider: &mockProvisioner{},
 			},
 			channelSource: MockChannelSource(false, false),
@@ -261,7 +261,7 @@ func TestProcessCluster(t *testing.T) {
 		{
 			testcase: "lifecycle status decommission-requested",
 			registry: createMockRegistry(statusDecommissionRequested, nil),
-			provisioners: map[provisioner.ProviderID]provisioner.Provisioner{
+			provisioners: map[api.ProviderID]provisioner.Provisioner{
 				mockProvider: &mockProvisioner{},
 			},
 			channelSource: MockChannelSource(false, false),
@@ -271,7 +271,7 @@ func TestProcessCluster(t *testing.T) {
 		{
 			testcase: "lifecycle status requested, provisioner.Create fails",
 			registry: createMockRegistry(statusRequested, &api.ClusterStatus{CurrentVersion: nextVersion}),
-			provisioners: map[provisioner.ProviderID]provisioner.Provisioner{
+			provisioners: map[api.ProviderID]provisioner.Provisioner{
 				mockProvider: &mockErrCreateProvisioner{},
 			},
 			channelSource: MockChannelSource(false, false),
@@ -281,7 +281,7 @@ func TestProcessCluster(t *testing.T) {
 		{
 			testcase: "lifecycle status ready, version up to date fails",
 			registry: createMockRegistry(statusReady, &api.ClusterStatus{CurrentVersion: nextVersion}),
-			provisioners: map[provisioner.ProviderID]provisioner.Provisioner{
+			provisioners: map[api.ProviderID]provisioner.Provisioner{
 				mockProvider: &mockProvisioner{},
 			},
 			channelSource: MockChannelSource(false, false),
@@ -291,7 +291,7 @@ func TestProcessCluster(t *testing.T) {
 		{
 			testcase: "lifecycle status ready, provisioner.Version failing",
 			registry: createMockRegistry(statusReady, nil),
-			provisioners: map[provisioner.ProviderID]provisioner.Provisioner{
+			provisioners: map[api.ProviderID]provisioner.Provisioner{
 				mockProvider: &mockErrProvisioner{},
 			},
 			channelSource: MockChannelSource(false, false),
@@ -301,7 +301,7 @@ func TestProcessCluster(t *testing.T) {
 		{
 			testcase: "lifecycle status ready, channelSource.Version() fails",
 			registry: createMockRegistry(statusReady, nil),
-			provisioners: map[provisioner.ProviderID]provisioner.Provisioner{
+			provisioners: map[api.ProviderID]provisioner.Provisioner{
 				mockProvider: &mockErrProvisioner{},
 			},
 			channelSource: MockChannelSource(true, false),
@@ -310,7 +310,7 @@ func TestProcessCluster(t *testing.T) {
 		}, {
 			testcase: "lifecycle status ready, channelVersion.Get() fails",
 			registry: createMockRegistry(statusReady, nil),
-			provisioners: map[provisioner.ProviderID]provisioner.Provisioner{
+			provisioners: map[api.ProviderID]provisioner.Provisioner{
 				mockProvider: &mockErrProvisioner{},
 			},
 			channelSource: MockChannelSource(false, true),
@@ -352,7 +352,7 @@ func TestIgnoreUnsupportedProvider(t *testing.T) {
 		defaultLogger,
 		command.NewExecManager(1),
 		registry,
-		map[provisioner.ProviderID]provisioner.Provisioner{
+		map[api.ProviderID]provisioner.Provisioner{
 			mockProvider: &mockProvisioner{},
 		},
 		MockChannelSource(false, false),
@@ -371,36 +371,36 @@ func TestDropUnsupportedClusters(t *testing.T) {
 		testcase     string
 		clusters     []*api.Cluster
 		result       []*api.Cluster
-		provisioners map[provisioner.ProviderID]provisioner.Provisioner
+		provisioners map[api.ProviderID]provisioner.Provisioner
 	}{
 		{
 			testcase:     "empty result on empty input",
 			clusters:     []*api.Cluster{},
 			result:       []*api.Cluster{},
-			provisioners: map[provisioner.ProviderID]provisioner.Provisioner{},
+			provisioners: map[api.ProviderID]provisioner.Provisioner{},
 		},
 		{
 			testcase:     "empty result on nil input",
 			clusters:     nil,
 			result:       []*api.Cluster{},
-			provisioners: map[provisioner.ProviderID]provisioner.Provisioner{},
+			provisioners: map[api.ProviderID]provisioner.Provisioner{},
 		},
 		{
 			testcase: "same result when all clusters supported",
-			clusters: []*api.Cluster{{Provider: string(mockProvider)}},
-			result:   []*api.Cluster{{Provider: string(mockProvider)}},
-			provisioners: map[provisioner.ProviderID]provisioner.Provisioner{
+			clusters: []*api.Cluster{{Provider: mockProvider}},
+			result:   []*api.Cluster{{Provider: mockProvider}},
+			provisioners: map[api.ProviderID]provisioner.Provisioner{
 				mockProvider: &mockProvisioner{},
 			},
 		},
 		{
 			testcase: "filters unsupported clusters",
 			clusters: []*api.Cluster{
-				{Provider: string(mockProvider)},
-				{Provider: string(otherMockProvider)},
+				{Provider: mockProvider},
+				{Provider: otherMockProvider},
 			},
-			result: []*api.Cluster{{Provider: string(mockProvider)}},
-			provisioners: map[provisioner.ProviderID]provisioner.Provisioner{
+			result: []*api.Cluster{{Provider: mockProvider}},
+			provisioners: map[api.ProviderID]provisioner.Provisioner{
 				mockProvider: &mockProvisioner{},
 			},
 		},
@@ -426,7 +426,7 @@ func TestCoalesceFailures(t *testing.T) {
 			defaultLogger,
 			command.NewExecManager(1),
 			registry,
-			map[provisioner.ProviderID]provisioner.Provisioner{
+			map[api.ProviderID]provisioner.Provisioner{
 				mockProvider: &mockCountingErrProvisioner{},
 			},
 			MockChannelSource(false, false),
@@ -454,7 +454,7 @@ func TestCoalesceFailures(t *testing.T) {
 			defaultLogger,
 			command.NewExecManager(1),
 			registry,
-			map[provisioner.ProviderID]provisioner.Provisioner{
+			map[api.ProviderID]provisioner.Provisioner{
 				mockProvider: &mockErrProvisioner{},
 			},
 			MockChannelSource(false, false),

diff --git a/pkg/updatestrategy/aws_asg.go b/pkg/updatestrategy/aws_asg.go
@@ -50,17 +50,17 @@ type ASGNodePoolsBackend struct {
 	asgClient autoscalingiface.AutoScalingAPI
 	ec2Client ec2iface.EC2API
 	elbClient elbiface.ELBAPI
-	clusterID string
+	cluster   *api.Cluster
 }
 
-// NewASGNodePoolsBackend initializes a new ASGNodePoolsBackend for the given clusterID and AWS
+// NewASGNodePoolsBackend initializes a new ASGNodePoolsBackend for the given cluster and AWS
 // session and.
-func NewASGNodePoolsBackend(clusterID string, sess *session.Session) *ASGNodePoolsBackend {
+func NewASGNodePoolsBackend(cluster *api.Cluster, sess *session.Session) *ASGNodePoolsBackend {
 	return &ASGNodePoolsBackend{
 		asgClient: autoscaling.New(sess),
 		ec2Client: ec2.New(sess),
 		elbClient: elb.New(sess),
-		clusterID: clusterID,
+		cluster:   cluster,
 	}
 }
 
@@ -432,7 +432,7 @@ func (n *ASGNodePoolsBackend) getNodePoolASGs(nodePool *api.NodePool) ([]*autosc
 
 	expectedTags := []*autoscaling.TagDescription{
 		{
-			Key:   aws.String(clusterIDTagPrefix + n.clusterID),
+			Key:   aws.String(clusterIDTagPrefix + n.cluster.Name()),
 			Value: aws.String(resourceLifecycleOwned),
 		},
 		{