From 6f03bf7d2c5d4aa660020032a68f1c4d8439961c Mon Sep 17 00:00:00 2001 From: "mykyta.oleksiienko" Date: Wed, 6 Sep 2023 18:14:37 +0300 Subject: [PATCH] issue-528, handle the user errors --- config/samples/clusters_v1beta1_redis.yaml | 14 +++---- .../clusterresources/redisuser_controller.go | 42 ++++++++++++++++--- controllers/clusters/redis_controller.go | 35 ++++++++-------- pkg/instaclustr/client.go | 31 ++++++++++++++ pkg/instaclustr/interfaces.go | 1 + pkg/instaclustr/mock/client.go | 4 ++ 6 files changed, 96 insertions(+), 31 deletions(-) diff --git a/config/samples/clusters_v1beta1_redis.yaml b/config/samples/clusters_v1beta1_redis.yaml index 93a310590..afb643cf6 100644 --- a/config/samples/clusters_v1beta1_redis.yaml +++ b/config/samples/clusters_v1beta1_redis.yaml @@ -9,19 +9,19 @@ metadata: app.kubernetes.io/created-by: operator name: redis-sample spec: - name: "Username-redis" + name: "Username-test" version: "7.0.12" slaTier: "NON_PRODUCTION" clientEncryption: false passwordAndUserAuth: true privateNetworkCluster: false userRefs: - # - name: redisuser-sample-1 - # namespace: default - # - name: redisuser-sample-2 - # namespace: default - # - name: redisuser-sample-3 - # namespace: default + - name: redisuser-sample-1 + namespace: default + - name: redisuser-sample-2 + namespace: default + - name: redisuser-sample-3 + namespace: default # twoFactorDelete: # - email: "rostyslp@netapp.com" dataCentres: diff --git a/controllers/clusterresources/redisuser_controller.go b/controllers/clusterresources/redisuser_controller.go index 3f3be53a6..af2350493 100644 --- a/controllers/clusterresources/redisuser_controller.go +++ b/controllers/clusterresources/redisuser_controller.go @@ -18,8 +18,8 @@ package clusterresources import ( "context" + "errors" "fmt" - "github.com/go-logr/logr" k8sCore "k8s.io/api/core/v1" k8sErrors "k8s.io/apimachinery/pkg/api/errors" @@ -132,9 +132,12 @@ func (r *RedisUserReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( } for clusterID, event := range user.Status.ClustersEvents { + userID := fmt.Sprintf(instaclustr.RedisUserIDFmt, clusterID, username) + if event == models.CreatingEvent { - _, err = r.API.CreateRedisUser(user.Spec.ToInstAPI(password, clusterID, username)) - if err != nil { + err = r.API.GetRedisUser(userID) + + if err != nil && !errors.Is(err, instaclustr.NotFound) { l.Error(err, "Cannot create a user for the Redis cluster", "cluster ID", clusterID, "username", username) @@ -144,6 +147,19 @@ func (r *RedisUserReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( return models.ReconcileRequeue, nil } + if errors.Is(err, instaclustr.NotFound) { + _, err = r.API.CreateRedisUser(user.Spec.ToInstAPI(password, clusterID, username)) + if err != nil { + l.Error(err, "Cannot create a user for the Redis cluster", + "cluster ID", clusterID, + "username", username) + r.EventRecorder.Eventf(user, models.Warning, models.CreatingEvent, + "Cannot create user. Reason: %v", err) + + return models.ReconcileRequeue, nil + } + } + user.Status.ClustersEvents[clusterID] = models.Created err = r.Status().Patch(ctx, user, patch) if err != nil { @@ -165,7 +181,7 @@ func (r *RedisUserReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( if event == models.DeletingEvent { userID := fmt.Sprintf(instaclustr.RedisUserIDFmt, clusterID, username) err = r.API.DeleteRedisUser(userID) - if err != nil { + if err != nil && !errors.Is(err, instaclustr.NotFound) { l.Error(err, "Cannot delete Redis user from the cluster.", "cluster ID", clusterID, "user ID", userID) r.EventRecorder.Eventf(user, models.Warning, models.DeletingEvent, @@ -205,11 +221,11 @@ func (r *RedisUserReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( return models.ReconcileRequeue, nil } + continue } - userID := fmt.Sprintf(instaclustr.RedisUserIDFmt, clusterID, username) err = r.API.UpdateRedisUser(user.ToInstAPIUpdate(password, userID)) - if err != nil { + if err != nil && !errors.Is(instaclustr.NotFound, err) { l.Error(err, "Cannot update redis user password", "secret name", user.Spec.SecretRef.Name, "secret namespace", user.Spec.SecretRef.Namespace, @@ -226,6 +242,18 @@ func (r *RedisUserReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( return models.ReconcileRequeue, nil } + if errors.Is(instaclustr.NotFound, err) { + l.Info("Cannot update redis user password", + "secret name", user.Spec.SecretRef.Name, + "secret namespace", user.Spec.SecretRef.Namespace, + "username", username, + "cluster ID", clusterID, + "user ID", userID, + ) + + r.EventRecorder.Eventf(user, models.Warning, models.UpdateFailed, + "Given user doesn`t exist on the cluster ID: %v", clusterID) + } l.Info("Redis user has been updated", "secret name", user.Spec.SecretRef.Name, @@ -339,6 +367,8 @@ func (r *RedisUserReconciler) handleDeleteUser( return err } + l.Info("CR Redis User was deleted") + return nil } diff --git a/controllers/clusters/redis_controller.go b/controllers/clusters/redis_controller.go index ec6ad15b3..239bee6e5 100644 --- a/controllers/clusters/redis_controller.go +++ b/controllers/clusters/redis_controller.go @@ -505,23 +505,6 @@ func (r *RedisReconciler) handleDeleteCluster( return models.ReconcileRequeue } - for _, ref := range redis.Spec.UserRefs { - err = r.detachUserResource(ctx, logger, redis, ref) - if err != nil { - logger.Error(err, "Cannot detach Redis user", - "cluster name", redis.Spec.Name, - "cluster status", redis.Status.State, - ) - - r.EventRecorder.Eventf( - redis, models.Warning, models.DeletionFailed, - "Cluster detaching on the Instaclustr is failed. Reason: %v", - err, - ) - return models.ReconcileRequeue - } - } - if !errors.Is(err, instaclustr.NotFound) { logger.Info("Sending cluster deletion to the Instaclustr API", "cluster name", redis.Spec.Name, @@ -571,6 +554,7 @@ func (r *RedisReconciler) handleDeleteCluster( } } + r.Scheduler.RemoveJob(redis.GetJobID(scheduler.StatusChecker)) r.Scheduler.RemoveJob(redis.GetJobID(scheduler.BackupsChecker)) logger.Info("Deleting cluster backup resources", @@ -599,7 +583,22 @@ func (r *RedisReconciler) handleDeleteCluster( "cluster ID", redis.Status.ID, ) - r.Scheduler.RemoveJob(redis.GetJobID(scheduler.StatusChecker)) + for _, ref := range redis.Spec.UserRefs { + err = r.detachUserResource(ctx, logger, redis, ref) + if err != nil { + logger.Error(err, "Cannot detach Redis user", + "cluster name", redis.Spec.Name, + "cluster status", redis.Status.State, + ) + + r.EventRecorder.Eventf( + redis, models.Warning, models.DeletionFailed, + "Cluster detaching on the Instaclustr is failed. Reason: %v", + err, + ) + return models.ReconcileRequeue + } + } patch := redis.NewPatch() controllerutil.RemoveFinalizer(redis, models.DeletionFinalizer) diff --git a/pkg/instaclustr/client.go b/pkg/instaclustr/client.go index e3499eecd..838693df3 100644 --- a/pkg/instaclustr/client.go +++ b/pkg/instaclustr/client.go @@ -177,6 +177,37 @@ func (c *Client) GetRedis(id string) ([]byte, error) { return body, nil } +func (c *Client) GetRedisUser(id string) error { + url := c.serverHostname + RedisUserEndpoint + id + + resp, err := c.DoRequest(url, http.MethodGet, nil) + if err != nil { + return err + } + + defer resp.Body.Close() + body, err := io.ReadAll(resp.Body) + if err != nil { + return err + } + + if resp.StatusCode == http.StatusNotFound { + return NotFound + } + + if resp.StatusCode != http.StatusOK { + return fmt.Errorf("status code: %d, message: %s", resp.StatusCode, body) + } + + userRedis := &models.RedisUser{} + err = json.Unmarshal(body, userRedis) + if err != nil { + return err + } + + return nil +} + func (c *Client) UpdateRedis(id string, r *models.RedisDataCentreUpdate) error { url := c.serverHostname + RedisEndpoint + id data, err := json.Marshal(r) diff --git a/pkg/instaclustr/interfaces.go b/pkg/instaclustr/interfaces.go index c3d733021..4d476efae 100644 --- a/pkg/instaclustr/interfaces.go +++ b/pkg/instaclustr/interfaces.go @@ -73,6 +73,7 @@ type API interface { CreateRedisUser(user *models.RedisUser) (string, error) UpdateRedisUser(user *models.RedisUserUpdate) error DeleteRedisUser(id string) error + GetRedisUser(id string) error CreateKafkaACL(url string, kafkaACL *kafkamanagementv1beta1.KafkaACLSpec) (*kafkamanagementv1beta1.KafkaACLStatus, error) GetKafkaACLStatus(kafkaACLID, kafkaACLEndpoint string) (*kafkamanagementv1beta1.KafkaACLStatus, error) DeleteKafkaACL(kafkaACLID, kafkaACLEndpoint string) error diff --git a/pkg/instaclustr/mock/client.go b/pkg/instaclustr/mock/client.go index 921e3984d..caa18fa81 100644 --- a/pkg/instaclustr/mock/client.go +++ b/pkg/instaclustr/mock/client.go @@ -371,3 +371,7 @@ func (c *mockClient) CreateAWSEndpointServicePrincipal(spec any) ([]byte, error) func (c *mockClient) DeleteAWSEndpointServicePrincipal(principalID string) error { panic("DeleteAWSEndpointServicePrincipal: is not implemented") } + +func (c *mockClient) GetRedisUser(id string) error { + panic("GetRedisUser: is not implemented") +}