From ce82931cab91949672fe547e2d624c1efb3e2572 Mon Sep 17 00:00:00 2001 From: Bohdan Siryk Date: Mon, 4 Mar 2024 11:19:45 +0200 Subject: [PATCH] issue-711, avoiding cluster creation if cluster with provided name exists was implemented --- .secrets.baseline | 10 +++++----- controllers/clusters/cassandra_controller.go | 18 ++++++++++++++++-- controllers/clusters/helpers.go | 18 ++++++++++++++++++ controllers/clusters/kafka_controller.go | 13 +++++++++++++ .../clusters/kafkaconnect_controller.go | 15 ++++++++++++++- controllers/clusters/opensearch_controller.go | 14 +++++++++++++- controllers/clusters/postgresql_controller.go | 14 +++++++++++++- controllers/clusters/redis_controller.go | 14 +++++++++++++- controllers/clusters/zookeeper_controller.go | 13 +++++++++++++ pkg/instaclustr/client.go | 9 ++++++--- pkg/instaclustr/interfaces.go | 2 +- pkg/instaclustr/mock/client.go | 4 ++-- .../go/api_account_cluster_list_v2_service.go | 10 ++++------ 13 files changed, 131 insertions(+), 23 deletions(-) diff --git a/.secrets.baseline b/.secrets.baseline index c71078562..364427620 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -535,14 +535,14 @@ "filename": "controllers/clusters/helpers.go", "hashed_secret": "5ffe533b830f08a0326348a9160afafc8ada44db", "is_verified": false, - "line_number": 119 + "line_number": 120 }, { "type": "Secret Keyword", "filename": "controllers/clusters/helpers.go", "hashed_secret": "5baa61e4c9b93f3f0682250b6cf8331b7ee68fd8", "is_verified": false, - "line_number": 124 + "line_number": 125 } ], "controllers/clusters/kafkaconnect_controller_test.go": [ @@ -560,7 +560,7 @@ "filename": "controllers/clusters/postgresql_controller.go", "hashed_secret": "5ffe533b830f08a0326348a9160afafc8ada44db", "is_verified": false, - "line_number": 1183 + "line_number": 1195 } ], "controllers/clusters/zookeeper_controller_test.go": [ @@ -725,7 +725,7 @@ "filename": "pkg/instaclustr/client.go", "hashed_secret": "5baa61e4c9b93f3f0682250b6cf8331b7ee68fd8", "is_verified": false, - "line_number": 2084 + "line_number": 2085 } ], "pkg/instaclustr/mock/client.go": [ @@ -1132,5 +1132,5 @@ } ] }, - "generated_at": "2024-02-29T10:48:32Z" + "generated_at": "2024-03-04T15:12:08Z" } diff --git a/controllers/clusters/cassandra_controller.go b/controllers/clusters/cassandra_controller.go index 6f7f02487..6426979f4 100644 --- a/controllers/clusters/cassandra_controller.go +++ b/controllers/clusters/cassandra_controller.go @@ -217,9 +217,23 @@ func (r *CassandraReconciler) createCluster(ctx context.Context, c *v1beta1.Cass var instModel *models.CassandraCluster var err error - if c.Spec.HasRestore() { + id, err := getClusterIDByName(r.API, models.CassandraAppType, c.Spec.Name) + if err != nil { + return err + } + + if id != "" { + l.Info("Cluster with provided name already exists", "name", c.Spec.Name, "clusterID", id) + r.EventRecorder.Eventf(c, models.Warning, models.CreationFailed, + "Failed to create cluster. Cluster %s already exists", c.Spec.Name, + ) + return fmt.Errorf("cluster %s already exists", c.Spec.Name) + } + + switch { + case c.Spec.HasRestore(): instModel, err = r.createCassandraFromRestore(c, l) - } else { + default: instModel, err = r.createCassandra(c, l) } if err != nil { diff --git a/controllers/clusters/helpers.go b/controllers/clusters/helpers.go index a92068507..a12ef44ff 100644 --- a/controllers/clusters/helpers.go +++ b/controllers/clusters/helpers.go @@ -34,6 +34,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/reconcile" + "github.com/instaclustr/operator/pkg/instaclustr" "github.com/instaclustr/operator/pkg/models" "github.com/instaclustr/operator/pkg/utils/dcomparison" ) @@ -238,3 +239,20 @@ func reconcileExternalChanges(c client.Client, r record.EventRecorder, obj Objec return nil } + +func getClusterIDByName(api instaclustr.API, appType string, name string) (string, error) { + clusters, err := api.ListClustersByName(name) + if err != nil { + return "", fmt.Errorf("failed to list clusters by name, err: %w", err) + } + + if len(clusters) == 0 { + return "", nil + } + + if clusters[0].Application != appType { + return "", fmt.Errorf("the cluster %s already exists, but it has other application type %s", name, clusters[0].Application) + } + + return clusters[0].ID, nil +} diff --git a/controllers/clusters/kafka_controller.go b/controllers/clusters/kafka_controller.go index 037a38108..77d5dcec6 100644 --- a/controllers/clusters/kafka_controller.go +++ b/controllers/clusters/kafka_controller.go @@ -107,6 +107,19 @@ func (r *KafkaReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl } func (r *KafkaReconciler) createCluster(ctx context.Context, k *v1beta1.Kafka, l logr.Logger) error { + id, err := getClusterIDByName(r.API, models.KafkaAppType, k.Spec.Name) + if err != nil { + return err + } + + if id != "" { + l.Info("Cluster with provided name already exists", "name", k.Spec.Name, "clusterID", id) + r.EventRecorder.Eventf(k, models.Warning, models.CreationFailed, + "Failed to create cluster. Cluster %s already exists", k.Spec.Name, + ) + return fmt.Errorf("cluster %s already exists", k.Spec.Name) + } + l.Info("Creating cluster", "cluster name", k.Spec.Name, "data centres", k.Spec.DataCentres) diff --git a/controllers/clusters/kafkaconnect_controller.go b/controllers/clusters/kafkaconnect_controller.go index 029b8aea6..46e48c50c 100644 --- a/controllers/clusters/kafkaconnect_controller.go +++ b/controllers/clusters/kafkaconnect_controller.go @@ -123,7 +123,20 @@ func (r *KafkaConnectReconciler) mergeManagedClusterFromRef(ctx context.Context, } func (r *KafkaConnectReconciler) createCluster(ctx context.Context, kc *v1beta1.KafkaConnect, l logr.Logger) error { - err := r.mergeManagedClusterFromRef(ctx, kc) + id, err := getClusterIDByName(r.API, models.KafkaConnectAppType, kc.Spec.Name) + if err != nil { + return err + } + + if id != "" { + l.Info("Cluster with provided name already exists", "name", kc.Spec.Name, "clusterID", id) + r.EventRecorder.Eventf(kc, models.Warning, models.CreationFailed, + "Failed to create cluster. Cluster %s already exists", kc.Spec.Name, + ) + return fmt.Errorf("cluster %s already exists", kc.Spec.Name) + } + + err = r.mergeManagedClusterFromRef(ctx, kc) if err != nil { return err } diff --git a/controllers/clusters/opensearch_controller.go b/controllers/clusters/opensearch_controller.go index c7fa5a41e..c189d5e78 100644 --- a/controllers/clusters/opensearch_controller.go +++ b/controllers/clusters/opensearch_controller.go @@ -182,8 +182,20 @@ func (r *OpenSearchReconciler) createOpenSearch(o *v1beta1.OpenSearch, logger lo } func (r *OpenSearchReconciler) createCluster(ctx context.Context, o *v1beta1.OpenSearch, logger logr.Logger) error { + id, err := getClusterIDByName(r.API, models.OpenSearchAppType, o.Spec.Name) + if err != nil { + return err + } + + if id != "" { + logger.Info("Cluster with provided name already exists", "name", o.Spec.Name, "clusterID", id) + r.EventRecorder.Eventf(o, models.Warning, models.CreationFailed, + "Failed to create cluster. Cluster %s already exists", o.Spec.Name, + ) + return fmt.Errorf("cluster %s already exists", o.Spec.Name) + } + var instaModel *models.OpenSearchCluster - var err error if o.Spec.HasRestore() { instaModel, err = r.createOpenSearchFromRestore(o, logger) diff --git a/controllers/clusters/postgresql_controller.go b/controllers/clusters/postgresql_controller.go index a5087fc60..c8a7da2ae 100644 --- a/controllers/clusters/postgresql_controller.go +++ b/controllers/clusters/postgresql_controller.go @@ -181,8 +181,20 @@ func (r *PostgreSQLReconciler) createPostgreSQL(pg *v1beta1.PostgreSQL, l logr.L } func (r *PostgreSQLReconciler) createCluster(ctx context.Context, pg *v1beta1.PostgreSQL, l logr.Logger) error { + id, err := getClusterIDByName(r.API, models.PgAppType, pg.Spec.Name) + if err != nil { + return err + } + + if id != "" { + l.Info("Cluster with provided name already exists", "name", pg.Spec.Name, "clusterID", id) + r.EventRecorder.Eventf(pg, models.Warning, models.CreationFailed, + "Failed to create cluster. Cluster %s already exists", pg.Spec.Name, + ) + return fmt.Errorf("cluster %s already exists", pg.Spec.Name) + } + var instaModel *models.PGCluster - var err error if pg.Spec.HasRestore() { instaModel, err = r.createFromRestore(pg, l) diff --git a/controllers/clusters/redis_controller.go b/controllers/clusters/redis_controller.go index 68b451d8c..99643d66d 100644 --- a/controllers/clusters/redis_controller.go +++ b/controllers/clusters/redis_controller.go @@ -181,8 +181,20 @@ func (r *RedisReconciler) createRedis(redis *v1beta1.Redis, l logr.Logger) (*mod } func (r *RedisReconciler) createCluster(ctx context.Context, redis *v1beta1.Redis, l logr.Logger) error { + id, err := getClusterIDByName(r.API, models.RedisAppType, redis.Spec.Name) + if err != nil { + return err + } + + if id != "" { + l.Info("Cluster with provided name already exists", "name", redis.Spec.Name, "clusterID", id) + r.EventRecorder.Eventf(redis, models.Warning, models.CreationFailed, + "Failed to create cluster. Cluster %s already exists", redis.Spec.Name, + ) + return fmt.Errorf("cluster %s already exists", redis.Spec.Name) + } + var instaModel *models.RedisCluster - var err error if redis.Spec.HasRestore() { instaModel, err = r.createFromRestore(redis, l) diff --git a/controllers/clusters/zookeeper_controller.go b/controllers/clusters/zookeeper_controller.go index c64d0bb14..12d22804a 100644 --- a/controllers/clusters/zookeeper_controller.go +++ b/controllers/clusters/zookeeper_controller.go @@ -104,6 +104,19 @@ func (r *ZookeeperReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( } func (r *ZookeeperReconciler) createCluster(ctx context.Context, zook *v1beta1.Zookeeper, l logr.Logger) error { + id, err := getClusterIDByName(r.API, models.ZookeeperAppType, zook.Spec.Name) + if err != nil { + return err + } + + if id != "" { + l.Info("Cluster with provided name already exists", "name", zook.Spec.Name, "clusterID", id) + r.EventRecorder.Eventf(zook, models.Warning, models.CreationFailed, + "Failed to create cluster. Cluster %s already exists", zook.Spec.Name, + ) + return fmt.Errorf("cluster %s already exists", zook.Spec.Name) + } + l.Info("Creating zookeeper cluster", "cluster name", zook.Spec.Name, "data centres", zook.Spec.DataCentres) diff --git a/pkg/instaclustr/client.go b/pkg/instaclustr/client.go index 8ad362ee0..d0648137c 100644 --- a/pkg/instaclustr/client.go +++ b/pkg/instaclustr/client.go @@ -22,6 +22,7 @@ import ( "fmt" "io" "net/http" + "strings" "time" clusterresourcesv1beta1 "github.com/instaclustr/operator/apis/clusterresources/v1beta1" @@ -2112,8 +2113,10 @@ func (c *Client) UpdatePostgreSQLDefaultUserPassword(id, password string) error return nil } -func (c *Client) ListClusters() ([]*models.ActiveClusters, error) { - url := c.serverHostname + ClustersEndpoint +func (c *Client) ListClustersByName(name string) ([]*models.ActiveCluster, error) { + url := c.serverHostname + ClustersEndpoint + fmt.Sprintf("?search=name:%s", name) + url = strings.ReplaceAll(url, " ", "%20") + resp, err := c.DoRequest(url, http.MethodGet, nil) if err != nil { return nil, err @@ -2135,7 +2138,7 @@ func (c *Client) ListClusters() ([]*models.ActiveClusters, error) { return nil, err } - return response, nil + return response[0].Clusters, nil } func (c *Client) CreateEncryptionKey( diff --git a/pkg/instaclustr/interfaces.go b/pkg/instaclustr/interfaces.go index 72b679645..013caebb5 100644 --- a/pkg/instaclustr/interfaces.go +++ b/pkg/instaclustr/interfaces.go @@ -94,7 +94,7 @@ type API interface { ResetPostgreSQLConfiguration(id, name string) error GetCadence(id string) (*models.CadenceCluster, error) UpdatePostgreSQLDefaultUserPassword(id, password string) error - ListClusters() ([]*models.ActiveClusters, error) + ListClustersByName(name string) ([]*models.ActiveCluster, error) CreateEncryptionKey(encryptionKeySpec any) (*clusterresourcesv1beta1.AWSEncryptionKeyStatus, error) GetEncryptionKeyStatus(encryptionKeyID string, encryptionKeyEndpoint string) (*clusterresourcesv1beta1.AWSEncryptionKeyStatus, error) DeleteEncryptionKey(encryptionKeyID string) error diff --git a/pkg/instaclustr/mock/client.go b/pkg/instaclustr/mock/client.go index fb6da34f1..dd9c6b8db 100644 --- a/pkg/instaclustr/mock/client.go +++ b/pkg/instaclustr/mock/client.go @@ -358,8 +358,8 @@ func (c *mockClient) UpdateKafkaConnect(id string, kc models.KafkaConnectAPIUpda panic("UpdateKafkaConnect: is not implemented") } -func (c *mockClient) ListClusters() ([]*models.ActiveClusters, error) { - panic("ListClusters: is not implemented") +func (c *mockClient) ListClustersByName(name string) ([]*models.ActiveCluster, error) { + panic("ListClustersByName: is not implemented") } func (c *mockClient) CreateRedisUser(user *models.RedisUser) (string, error) { diff --git a/pkg/instaclustr/mock/server/go/api_account_cluster_list_v2_service.go b/pkg/instaclustr/mock/server/go/api_account_cluster_list_v2_service.go index c5c28b109..c9185634b 100644 --- a/pkg/instaclustr/mock/server/go/api_account_cluster_list_v2_service.go +++ b/pkg/instaclustr/mock/server/go/api_account_cluster_list_v2_service.go @@ -11,8 +11,6 @@ package openapi import ( "context" - "errors" - "net/http" ) // AccountClusterListV2APIService is a service that implements the logic for the AccountClusterListV2APIServicer @@ -31,8 +29,8 @@ func (s *AccountClusterListV2APIService) ClusterManagementV2DataSourcesClustersV // TODO - update ClusterManagementV2DataSourcesClustersV2Get with the required logic for this service method. // Add api_account_cluster_list_v2_service.go to the .openapi-generator-ignore to avoid overwriting this service implementation when updating open api generation. - // TODO: Uncomment the next line to return response Response(200, []AccountClustersV2{}) or use other options such as http.Ok ... - // return Response(200, []AccountClustersV2{}), nil - - return Response(http.StatusNotImplemented, nil), errors.New("ClusterManagementV2DataSourcesClustersV2Get method not implemented") + return Response(200, []AccountClustersV2{{ + AccountId: "test-account-id", + Clusters: []ClusterSummaryV2{}, + }}), nil }