Skip to content

Commit

Permalink
Add a cluster method Name to get canonical clusterName based on provi…
Browse files Browse the repository at this point in the history
…der (#829)

* Add a template function to get canonical clusterName based on provider

Signed-off-by: Mikkel Oscar Lyderik Larsen <[email protected]>

* Fix Cluster Name in all situations

Signed-off-by: Mikkel Oscar Lyderik Larsen <[email protected]>

---------

Signed-off-by: Mikkel Oscar Lyderik Larsen <[email protected]>
  • Loading branch information
mikkeloscar authored Nov 1, 2024
1 parent 8fa036f commit 0a7121e
Show file tree
Hide file tree
Showing 15 changed files with 138 additions and 76 deletions.
19 changes: 17 additions & 2 deletions api/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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"`
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
18 changes: 18 additions & 0 deletions api/cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
9 changes: 5 additions & 4 deletions cmd/clm/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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,
Expand All @@ -130,7 +131,7 @@ func main() {
ManageEtcdStack: cfg.ManageEtcdStack,
},
),
provisioner.ZalandoEKSProvider: provisioner.NewZalandoEKSProvisioner(
api.ZalandoEKSProvider: provisioner.NewZalandoEKSProvisioner(
execManager,
secretDecrypter,
cfg.AssumedRole,
Expand Down Expand Up @@ -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",
Expand Down
6 changes: 3 additions & 3 deletions controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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",
Expand Down
56 changes: 28 additions & 28 deletions controller/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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}
}
Expand Down Expand Up @@ -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{},
},

Expand All @@ -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{},
},

Expand All @@ -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),
Expand All @@ -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),
Expand All @@ -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),
Expand All @@ -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),
Expand All @@ -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),
Expand All @@ -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),
Expand All @@ -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),
Expand Down Expand Up @@ -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),
Expand All @@ -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{},
},
},
Expand All @@ -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),
Expand Down Expand Up @@ -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),
Expand Down
10 changes: 5 additions & 5 deletions pkg/updatestrategy/aws_asg.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}

Expand Down Expand Up @@ -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),
},
{
Expand Down
Loading

0 comments on commit 0a7121e

Please sign in to comment.