Add parameter to filter cloud provider. (#765)

* Add parameter to filter cloud provider. Signed-off-by: Rodrigo Reis <[email protected]> * goimports. Signed-off-by: Rodrigo Reis <[email protected]> * typo Signed-off-by: Rodrigo Reis <[email protected]> * Use enum for parameter, improve provider naming. Signed-off-by: Rodrigo Reis <[email protected]> * Test clamped retryer. Signed-off-by: Rodrigo Reis <[email protected]> * Test copy values. Signed-off-by: Rodrigo Reis <[email protected]> * Test cluster version. Signed-off-by: Rodrigo Reis <[email protected]> * goimports. Signed-off-by: Rodrigo Reis <[email protected]> --------- Signed-off-by: Rodrigo Reis <[email protected]>
zalando-incubator · May 15, 2024 · ec5b971 · ec5b971
1 parent de834d6
commit ec5b971
Show file tree

Hide file tree

Showing 12 changed files with 341 additions and 26 deletions.
diff --git a/api/cluster_version_test.go b/api/cluster_version_test.go
@@ -0,0 +1,42 @@
+package api
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestParseVersion(t *testing.T) {
+	for _, tc := range []struct {
+		name     string
+		version  string
+		expected string
+	}{
+		{
+			name:     "empty",
+			version:  "",
+			expected: "#",
+		},
+		{
+			name:     "simple",
+			version:  "foo#bar",
+			expected: "foo#bar",
+		},
+		{
+			name:     "missing hash",
+			version:  "foo",
+			expected: "#",
+		},
+		{
+			name:     "missing version",
+			version:  "#bar",
+			expected: "#bar",
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			version := ParseVersion(tc.version)
+			result := version.String()
+			require.Equal(t, tc.expected, result)
+		})
+	}
+}
diff --git a/cmd/clm/main.go b/cmd/clm/main.go
@@ -137,6 +137,7 @@ func main() {
 			AccountFilter:     cfg.AccountFilter,
 			Interval:          cfg.Interval,
 			DryRun:            cfg.DryRun,
+			Providers:         cfg.Providers,
 			ConcurrentUpdates: cfg.ConcurrentUpdates,
 		}
 

diff --git a/config/config.go b/config/config.go
@@ -27,6 +27,7 @@ const (
 	defaultDrainForceEvictInterval          = "5m"
 	defaultDrainPollInterval                = "30s"
 	defaultUpdateStrategy                   = "clc"
+	defaultProvider                         = "zalando-aws"
 )
 
 var defaultWorkdir = path.Join(os.TempDir(), "clm-workdir")
@@ -47,6 +48,7 @@ type LifecycleManagerConfig struct {
 	Listen                      string
 	Workdir                     string
 	Directory                   string
+	Providers                   []string
 	ConfigSources               []string
 	SSHPrivateKeyFile           string
 	CredentialsDir              string
@@ -89,6 +91,10 @@ func (cfg *LifecycleManagerConfig) ParseFlags() string {
 	kingpin.Flag("dry-run", "Don't make any changes, just print.").BoolVar(&cfg.DryRun)
 	kingpin.Flag("listen", "Address to listen at, e.g. :9090 or 0.0.0.0:9090").Default(defaultListener).StringVar(&cfg.Listen)
 	kingpin.Flag("workdir", "Path to working directory used for storing channel configurations.").Default(defaultWorkdir).StringVar(&cfg.Workdir)
+	kingpin.Flag(
+		"provider",
+		"Cloud provider. Defaults to single provider \"zalando-aws\".",
+	).Default(defaultProvider).EnumsVar(&cfg.Providers, "zalando-aws")
 	kingpin.Flag("config-source", "Config source specification (NAME:dir:PATH or NAME:git:URL). At least one is required.").StringsVar(&cfg.ConfigSources)
 	kingpin.Flag("directory", "Use a single directory as a config source (for local/development use)").StringVar(&cfg.Directory)
 	kingpin.Flag("concurrent-updates", "Number of updates allowed to run in parallel.").Default(defaultConcurrentUpdates).UintVar(&cfg.ConcurrentUpdates)

diff --git a/controller/controller.go b/controller/controller.go
@@ -33,6 +33,7 @@ var (
 type Options struct {
 	Interval          time.Duration
 	AccountFilter     config.IncludeExcludeFilter
+	Providers         []string
 	DryRun            bool
 	ConcurrentUpdates uint
 	EnvironmentOrder  []string
@@ -44,6 +45,7 @@ type Controller struct {
 	execManager          *command.ExecManager
 	registry             registry.Registry
 	provisioner          provisioner.Provisioner
+	providers            []string
 	channelConfigSourcer channel.ConfigSource
 	interval             time.Duration
 	dryRun               bool
@@ -58,6 +60,7 @@ func New(logger *log.Entry, execManager *command.ExecManager, registry registry.
 		execManager:          execManager,
 		registry:             registry,
 		provisioner:          provisioner,
+		providers:            options.Providers,
 		channelConfigSourcer: channel.NewCachingSource(channelConfigSourcer),
 		interval:             options.Interval,
 		dryRun:               options.DryRun,
@@ -116,7 +119,11 @@ func (c *Controller) refresh() error {
 		return err
 	}
 
-	clusters, err := c.registry.ListClusters(registry.Filter{})
+	clusters, err := c.registry.ListClusters(
+		registry.Filter{
+			Providers: c.providers,
+		},
+	)
 	if err != nil {
 		return err
 	}

diff --git a/pkg/aws/retryer_test.go b/pkg/aws/retryer_test.go
@@ -0,0 +1,88 @@
+package aws
+
+import (
+	"testing"
+	"time"
+
+	"github.com/aws/aws-sdk-go/aws/client/metadata"
+	"github.com/aws/aws-sdk-go/aws/request"
+	"github.com/stretchr/testify/require"
+)
+
+func TestShouldRetry(t *testing.T) {
+	for _, tc := range []struct {
+		caseName         string
+		maxRetries       int
+		maxRetryInterval time.Duration
+		request          *request.Request
+		expected         bool
+	}{
+		{
+			caseName:   "should not retry",
+			maxRetries: 1,
+			request:    &request.Request{},
+			expected:   false,
+		},
+		{
+			caseName:   "should retry with metadata service",
+			maxRetries: 1,
+			request: &request.Request{
+				ClientInfo: metadata.ClientInfo{
+					ServiceName: "ec2metadata",
+				},
+			},
+			expected: true,
+		},
+		{
+			caseName:   "should not retry with metadata service",
+			maxRetries: 1,
+			request: &request.Request{
+				ClientInfo: metadata.ClientInfo{
+					ServiceName: "ec2metadata",
+				},
+				RetryCount: 8,
+			},
+			expected: false,
+		},
+	} {
+		t.Run(tc.caseName, func(t *testing.T) {
+			retryer := NewClampedRetryer(tc.maxRetries, time.Second)
+
+			res := retryer.ShouldRetry(tc.request)
+			require.Equal(t, tc.expected, res)
+		})
+	}
+}
+
+func TestRetryRules(t *testing.T) {
+	for _, tc := range []struct {
+		caseName            string
+		maxRetryInterval    time.Duration
+		request             *request.Request
+		expectedLessOrEqual time.Duration
+	}{
+		{
+			caseName:            "should return max retry interval",
+			maxRetryInterval:    time.Millisecond,
+			request:             &request.Request{},
+			expectedLessOrEqual: time.Millisecond,
+		},
+		{
+			caseName:         "should not return max retry interval",
+			maxRetryInterval: time.Second,
+			request: &request.Request{
+				ClientInfo: metadata.ClientInfo{
+					ServiceName: "ec2metadata",
+				},
+			},
+			expectedLessOrEqual: time.Second / 2,
+		},
+	} {
+		t.Run(tc.caseName, func(t *testing.T) {
+			retryer := NewClampedRetryer(1, tc.maxRetryInterval)
+
+			res := retryer.RetryRules(tc.request)
+			require.LessOrEqual(t, res, tc.expectedLessOrEqual)
+		})
+	}
+}
diff --git a/pkg/util/copy_test.go b/pkg/util/copy_test.go
@@ -0,0 +1,38 @@
+package util
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestCopyValues(t *testing.T) {
+	for _, tc := range []struct {
+		name  string
+		value map[string]interface{}
+	}{
+		{
+			name:  "empty",
+			value: map[string]interface{}{},
+		},
+		{
+			name: "simple",
+			value: map[string]interface{}{
+				"foo": "bar",
+			},
+		},
+		{
+			name: "nested",
+			value: map[string]interface{}{
+				"foo": map[string]interface{}{
+					"bar": "baz",
+				},
+			},
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			result := CopyValues(tc.value)
+			require.Equal(t, tc.value, result)
+		})
+	}
+}
diff --git a/provisioner/clusterpy.go b/provisioner/clusterpy.go
@@ -32,7 +32,6 @@ import (
 )
 
 const (
-	providerID                         = "zalando-aws"
 	etcdStackFileName                  = "stack.yaml"
 	clusterStackFileName               = "cluster.yaml"
 	etcdStackNameDefault               = "etcd-cluster-etcd"
@@ -103,7 +102,7 @@ func NewClusterpyProvisioner(execManager *command.ExecManager, tokenSource oauth
 }
 
 func (p *clusterpyProvisioner) Supports(cluster *api.Cluster) bool {
-	return cluster.Provider == providerID
+	return cluster.Provider == string(ZalandoAWSProvider)
 }
 
 func (p *clusterpyProvisioner) updateDefaults(cluster *api.Cluster, channelConfig channel.Config, adapter *awsAdapter, instanceTypes *awsUtils.InstanceTypes) error {
@@ -176,6 +175,10 @@ func (p *clusterpyProvisioner) propagateConfigItemsToNodePools(cluster *api.Clus
 // Provision provisions/updates a cluster on AWS. Provision is an idempotent
 // operation for the same input.
 func (p *clusterpyProvisioner) Provision(ctx context.Context, logger *log.Entry, cluster *api.Cluster, channelConfig channel.Config) error {
+	if !p.Supports(cluster) {
+		return ErrProviderNotSupported
+	}
+
 	instanceTypes, awsAdapter, updater, err := p.prepareProvision(logger, cluster, channelConfig)
 	if err != nil {
 		return err
@@ -599,7 +602,7 @@ func selectSubnetIDs(subnets []*ec2.Subnet) *AZInfo {
 
 // Decommission decommissions a cluster provisioned in AWS.
 func (p *clusterpyProvisioner) Decommission(ctx context.Context, logger *log.Entry, cluster *api.Cluster) error {
-	if cluster.Provider != providerID {
+	if !p.Supports(cluster) {
 		return ErrProviderNotSupported
 	}
 
@@ -779,10 +782,6 @@ func (p *clusterpyProvisioner) setupAWSAdapter(logger *log.Entry, cluster *api.C
 // prepares to provision a cluster by initializing the aws adapter.
 // TODO: this is doing a lot of things to glue everything together, this should be refactored.
 func (p *clusterpyProvisioner) prepareProvision(logger *log.Entry, cluster *api.Cluster, channelConfig channel.Config) (*awsUtils.InstanceTypes, *awsAdapter, updatestrategy.UpdateStrategy, error) {
-	if cluster.Provider != providerID {
-		return nil, nil, nil, ErrProviderNotSupported
-	}
-
 	logger.Infof("clusterpy: Prepare for provisioning cluster %s (%s)..", cluster.ID, cluster.LifecycleStatus)
 
 	adapter, err := p.setupAWSAdapter(logger, cluster)

diff --git a/provisioner/clusterpy_test.go b/provisioner/clusterpy_test.go
@@ -1,6 +1,7 @@
 package provisioner
 
 import (
+	"context"
 	"errors"
 	"fmt"
 	"net/http"
@@ -517,3 +518,22 @@ func TestWaitForAPIServer(t *testing.T) {
 		})
 	}
 }
+
+func TestProvisionDoesNotSupportProvider(t *testing.T) {
+	cluster := &api.Cluster{
+		Provider: "zalando-eks",
+	}
+
+	p := clusterpyProvisioner{}
+	err := p.Provision(context.TODO(), nil, cluster, nil)
+	assert.Equal(t, ErrProviderNotSupported, err)
+}
+func TestDecommissionDoesNotSupportProvider(t *testing.T) {
+	cluster := &api.Cluster{
+		Provider: "zalando-eks",
+	}
+
+	p := clusterpyProvisioner{}
+	err := p.Decommission(context.TODO(), nil, cluster)
+	assert.Equal(t, ErrProviderNotSupported, err)
+}
diff --git a/provisioner/provisioner.go b/provisioner/provisioner.go
@@ -11,21 +11,31 @@ import (
 	log "github.com/sirupsen/logrus"
 )
 
+type (
+	// A provider ID is a string that identifies a cluster provider.
+	ProviderID string
+
+	// Options is the options that can be passed to a provisioner when initialized.
+	Options struct {
+		DryRun          bool
+		ApplyOnly       bool
+		UpdateStrategy  config.UpdateStrategy
+		RemoveVolumes   bool
+		ManageEtcdStack bool
+	}
+)
+
+const (
+	// ZalandoAWS Provider is the provider ID for Zalando managed AWS clusters.
+	ZalandoAWSProvider ProviderID = "zalando-aws"
+)
+
 var (
 	// ErrProviderNotSupported is the error returned from porvisioners if
 	// they don't support the cluster provider defined.
 	ErrProviderNotSupported = errors.New("unsupported provider type")
 )
 
-// Options is the options that can be passed to a provisioner when initialized.
-type Options struct {
-	DryRun          bool
-	ApplyOnly       bool
-	UpdateStrategy  config.UpdateStrategy
-	RemoveVolumes   bool
-	ManageEtcdStack bool
-}
-
 // Provisioner is an interface describing how to provision or decommission
 // clusters.
 type Provisioner interface {

diff --git a/registry/http.go b/registry/http.go
@@ -63,16 +63,18 @@ func (r *httpRegistry) ListClusters(filter Filter) ([]*api.Cluster, error) {
 	var result []*api.Cluster
 
 	for _, cluster := range resp.Payload.Items {
-		if filter.LifecycleStatus == nil || *cluster.LifecycleStatus == *filter.LifecycleStatus {
-			c, err := convertFromClusterModel(cluster)
-			if err != nil {
-				return nil, err
-			}
-			if account, ok := accounts[c.InfrastructureAccount]; ok {
-				c.Owner = *account.Owner
-			}
-			result = append(result, c)
+		if !filter.Includes(cluster) {
+			continue
 		}
+
+		c, err := convertFromClusterModel(cluster)
+		if err != nil {
+			return nil, err
+		}
+		if account, ok := accounts[c.InfrastructureAccount]; ok {
+			c.Owner = *account.Owner
+		}
+		result = append(result, c)
 	}
 
 	return result, nil