diff --git a/api/v1alpha1/types.go b/api/v1alpha1/types.go index 8a44415..40a04a7 100644 --- a/api/v1alpha1/types.go +++ b/api/v1alpha1/types.go @@ -2,13 +2,17 @@ package v1alpha1 // Definitions to manage status condition types const ( - // ConditionReconciled represents the status of the resource reconciliation - ConditionReconciled = "Reconciled" + ConditionAvailable = "Available" // The resource is in sync with S3 + ConditionProgressing = "Progressing" // The resource is being synced with S3 + ConditionDegraded = "Degraded" // The sync with S3 has failed, resource state is unknown + ConditionRejected = "Rejected" // The resource cannot be sync with S3 and does not exists on S3 ) // Definitions to manage status condition reasons const ( Reconciling = "Reconciling" + InternalError = "OperatorError" + K8sApiError = "KubernetesAPIError" Unreachable = "Unreachable" CreationFailure = "CreationFailure" Reconciled = "Reconciled" diff --git a/internal/controller/bucket/finalizer.go b/internal/controller/bucket/finalizer.go index 3517ac1..4511f32 100644 --- a/internal/controller/bucket/finalizer.go +++ b/internal/controller/bucket/finalizer.go @@ -18,12 +18,13 @@ package bucket_controller import ( "context" + "fmt" s3v1alpha1 "github.com/InseeFrLab/s3-operator/api/v1alpha1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" - "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/reconcile" ) @@ -32,36 +33,29 @@ func (r *BucketReconciler) handleDeletion( req reconcile.Request, bucketResource *s3v1alpha1.Bucket, ) (reconcile.Result, error) { - logger := log.FromContext(ctx) if controllerutil.ContainsFinalizer(bucketResource, bucketFinalizer) { - if err := r.finalizeBucket(ctx, bucketResource); err != nil { - logger.Error( - err, - "An error occurred while attempting to finalize the bucket", - "bucketName", - bucketResource.Spec.Name, - "NamespacedName", - req.NamespacedName.String(), - ) - return r.SetReconciledCondition( + if err := r.finalizeBucket(ctx, req, bucketResource); err != nil { + return r.SetDegradedCondition( ctx, req, bucketResource, + metav1.ConditionFalse, s3v1alpha1.DeletionFailure, - "Bucket deletion has failed", + fmt.Sprintf("Bucket %s deletion has failed", bucketResource.Spec.Name), err, ) } if ok := controllerutil.RemoveFinalizer(bucketResource, bucketFinalizer); !ok { - logger.Info( - "Failed to remove finalizer for bucketResource", - "bucketName", - bucketResource.Spec.Name, - "NamespacedName", - req.NamespacedName.String(), + r.SetProgressingCondition( + ctx, + req, + bucketResource, + metav1.ConditionFalse, + s3v1alpha1.InternalError, + fmt.Sprintf("Failed to remove finalizer for bucket %s", bucketResource.Spec.Name), ) return ctrl.Result{Requeue: true}, nil } @@ -72,14 +66,16 @@ func (r *BucketReconciler) handleDeletion( // your changes to the latest version and try again" which would re-trigger the reconciliation // if we try to update it again in the following operations if err := r.Update(ctx, bucketResource); err != nil { - logger.Error( + r.SetDegradedCondition( + ctx, + req, + bucketResource, + metav1.ConditionFalse, + s3v1alpha1.K8sApiError, + fmt.Sprintf("An error occured when removing finalizer from bucket %s", bucketResource.Spec.Name), err, - "Failed to remove finalizer for bucketResource", - "bucketName", - bucketResource.Spec.Name, - "NamespacedName", - req.NamespacedName.String(), ) + return ctrl.Result{}, err } @@ -89,9 +85,9 @@ func (r *BucketReconciler) handleDeletion( func (r *BucketReconciler) finalizeBucket( ctx context.Context, + req reconcile.Request, bucketResource *s3v1alpha1.Bucket, ) error { - logger := log.FromContext(ctx) s3Client, err := r.S3Instancehelper.GetS3ClientForRessource( ctx, @@ -102,7 +98,15 @@ func (r *BucketReconciler) finalizeBucket( bucketResource.Spec.S3InstanceRef, ) if err != nil { - logger.Error(err, "an error occurred while getting s3Client") + r.SetDegradedCondition( + ctx, + req, + bucketResource, + metav1.ConditionUnknown, + s3v1alpha1.Unreachable, + "Failed to generate s3client from instance", + err, + ) return err } if s3Client.GetConfig().BucketDeletionEnabled { diff --git a/internal/controller/bucket/reconcile.go b/internal/controller/bucket/reconcile.go index df7ba0e..52db48d 100644 --- a/internal/controller/bucket/reconcile.go +++ b/internal/controller/bucket/reconcile.go @@ -21,10 +21,9 @@ import ( "fmt" s3v1alpha1 "github.com/InseeFrLab/s3-operator/api/v1alpha1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" k8sapierrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/api/meta" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/log" @@ -60,28 +59,15 @@ func (r *BucketReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr // Let's just set the status as Unknown when no status are available if len(bucketResource.Status.Conditions) == 0 { - meta.SetStatusCondition( - &bucketResource.Status.Conditions, - metav1.Condition{ - Type: s3v1alpha1.ConditionReconciled, - Status: metav1.ConditionUnknown, - ObservedGeneration: bucketResource.Generation, - Reason: s3v1alpha1.Reconciling, - Message: "Starting reconciliation", - }, - ) - if err = r.Status().Update(ctx, bucketResource); err != nil { - logger.Error( - err, - "Failed to update bucketRessource status", - "bucketName", - bucketResource.Spec.Name, - "NamespacedName", - req.NamespacedName.String(), - ) + _, err = r.SetProgressingCondition(ctx, + req, + bucketResource, + metav1.ConditionUnknown, + s3v1alpha1.Reconciling, + fmt.Sprintf("Newly discovered resource %s", bucketResource.Name)) + if err != nil { return ctrl.Result{}, err } - // Let's re-fetch the bucketResource Custom Resource after update the status // so that we have the latest state of the resource on the cluster and we will avoid // raise the issue "the object has been modified, please apply @@ -102,41 +88,44 @@ func (r *BucketReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr // Add finalizer for this CR if !controllerutil.ContainsFinalizer(bucketResource, bucketFinalizer) { - logger.Info("Adding finalizer to bucket resource", "bucketName", - bucketResource.Spec.Name, "NamespacedName", req.NamespacedName.String()) + r.SetProgressingCondition(ctx, + req, + bucketResource, + metav1.ConditionTrue, + s3v1alpha1.Reconciling, + fmt.Sprintf("Adding finalizer to bucket resource %s", bucketResource.Spec.Name)) if ok := controllerutil.AddFinalizer(bucketResource, bucketFinalizer); !ok { - logger.Error( - err, - "Failed to add finalizer into bucket resource", - "bucketName", - bucketResource.Spec.Name, - "NamespacedName", - req.NamespacedName.String(), - ) + r.SetDegradedCondition(ctx, + req, + bucketResource, + metav1.ConditionFalse, + s3v1alpha1.InternalError, + fmt.Sprintf("Failed to add finalizer to bucket resource %s", bucketResource.Spec.Name), + err) return ctrl.Result{Requeue: true}, nil } if err := r.Update(ctx, bucketResource); err != nil { - logger.Error( - err, - "An error occurred when adding finalizer on bucketResource", - "bucketName", - bucketResource.Spec.Name, - "NamespacedName", - req.NamespacedName.String(), - ) + r.SetDegradedCondition(ctx, + req, + bucketResource, + metav1.ConditionFalse, + s3v1alpha1.K8sApiError, + fmt.Sprintf("An error occurred when adding finalizer on bucket resource %s", bucketResource.Spec.Name), + err) + return ctrl.Result{}, err } if err := r.Get(ctx, req.NamespacedName, bucketResource); err != nil { - logger.Error( - err, - "Failed to re-fetch bucketResource", - "bucketName", - bucketResource.Spec.Name, - "NamespacedName", - req.NamespacedName.String(), - ) + r.SetDegradedCondition(ctx, + req, + bucketResource, + metav1.ConditionFalse, + s3v1alpha1.K8sApiError, + fmt.Sprintf("Failed to re-fetch bucket resource %s", bucketResource.Spec.Name), + err) + return ctrl.Result{}, err } } @@ -144,13 +133,16 @@ func (r *BucketReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr // // Managing bucket deletion with a finalizer // // REF : https://sdk.operatorframework.io/docs/building-operators/golang/advanced-topics/#external-resources if bucketResource.GetDeletionTimestamp() != nil { - logger.Info("bucketResource have been marked for deletion", "bucketName", - bucketResource.Spec.Name, - "NamespacedName", - req.NamespacedName.String()) + r.SetProgressingCondition(ctx, + req, + bucketResource, + metav1.ConditionTrue, + s3v1alpha1.Reconciling, + fmt.Sprintf("Bucket resource have been marked for deletion %s", bucketResource.Spec.Name)) return r.handleDeletion(ctx, req, bucketResource) } + r.SetProgressingCondition(ctx, req, bucketResource, metav1.ConditionTrue, s3v1alpha1.Reconciling, fmt.Sprintf("Starting reconciliation of bucket %s", bucketResource.Name)) return r.handleReconciliation(ctx, req, bucketResource) } @@ -160,7 +152,6 @@ func (r *BucketReconciler) handleReconciliation( req reconcile.Request, bucketResource *s3v1alpha1.Bucket, ) (reconcile.Result, error) { - logger := log.FromContext(ctx) s3Client, err := r.S3Instancehelper.GetS3ClientForRessource( ctx, @@ -171,11 +162,11 @@ func (r *BucketReconciler) handleReconciliation( bucketResource.Spec.S3InstanceRef, ) if err != nil { - logger.Error(err, "an error occurred while getting s3Client") - return r.SetReconciledCondition( + return r.SetDegradedCondition( ctx, req, bucketResource, + metav1.ConditionUnknown, s3v1alpha1.Unreachable, "Failed to generate s3client from instance", err, @@ -186,20 +177,13 @@ func (r *BucketReconciler) handleReconciliation( // Check bucket existence on the S3 server found, err := s3Client.BucketExists(bucketResource.Spec.Name) if err != nil { - logger.Error( - err, - "An error occurred while checking the existence of a bucket", - "bucketName", - bucketResource.Spec.Name, - "NamespacedName", - req.NamespacedName.String(), - ) - return r.SetReconciledCondition( + return r.SetDegradedCondition( ctx, req, bucketResource, + metav1.ConditionUnknown, s3v1alpha1.Unreachable, - "Error while checking if bucket already exist", + fmt.Sprintf("Error while checking if bucket %s already exist", bucketResource.Spec.Name), err, ) @@ -219,7 +203,6 @@ func (r *BucketReconciler) handleUpdate( req reconcile.Request, bucketResource *s3v1alpha1.Bucket, ) (reconcile.Result, error) { - logger := log.FromContext(ctx) s3Client, err := r.S3Instancehelper.GetS3ClientForRessource( ctx, @@ -230,18 +213,11 @@ func (r *BucketReconciler) handleUpdate( bucketResource.Spec.S3InstanceRef, ) if err != nil { - logger.Error( - err, - "An error occurred while getting s3Client for bucket ressource", - "bucketName", - bucketResource.Spec.Name, - "NamespacedName", - req.NamespacedName.String(), - ) - return r.SetReconciledCondition( + return r.SetDegradedCondition( ctx, req, bucketResource, + metav1.ConditionUnknown, s3v1alpha1.Unreachable, "Failed to generate s3client from instance", err, @@ -254,20 +230,13 @@ func (r *BucketReconciler) handleUpdate( // Checking effectiveQuota existence on the bucket effectiveQuota, err := s3Client.GetQuota(bucketResource.Spec.Name) if err != nil { - logger.Error( - err, - "An error occurred while checking the quota for bucket ressource", - "bucketName", - bucketResource.Spec.Name, - "NamespacedName", - req.NamespacedName.String(), - ) - return r.SetReconciledCondition( + return r.SetDegradedCondition( ctx, req, bucketResource, + metav1.ConditionUnknown, s3v1alpha1.Unreachable, - "Checking quota has failed", + fmt.Sprintf("Checking quota of bucket %s has failed", bucketResource.Spec.Name), err, ) } @@ -278,40 +247,26 @@ func (r *BucketReconciler) handleUpdate( // Choosing between override / default quotaToResetTo, convertionSucceed := bucketResource.Spec.Quota.Override.AsInt64() if !convertionSucceed { - logger.Error( - err, - "An error occurred while getting quotas override as int64 for ressource", - "bucketName", - bucketResource.Spec.Name, - "NamespacedName", - req.NamespacedName.String(), - ) - return r.SetReconciledCondition( + return r.SetDegradedCondition( ctx, req, bucketResource, - s3v1alpha1.Unreachable, - "An error occurred while creating bucket", + metav1.ConditionFalse, + s3v1alpha1.InternalError, + fmt.Sprintf("An error occurred while getting overrided quotas as int64 for ressource %s", bucketResource.Spec.Name), err, ) } if quotaToResetTo == 0 { quotaToResetTo, convertionSucceed = bucketResource.Spec.Quota.Default.AsInt64() if !convertionSucceed { - logger.Error( - err, - "An error occurred while getting default quotas as int64 for ressource", - "bucketName", - bucketResource.Spec.Name, - "NamespacedName", - req.NamespacedName.String(), - ) - return r.SetReconciledCondition( + return r.SetDegradedCondition( ctx, req, bucketResource, - s3v1alpha1.Unreachable, - "An error occurred while creating bucket", + metav1.ConditionFalse, + s3v1alpha1.InternalError, + fmt.Sprintf("An error occurred while getting default quotas as int64 for ressource %s", bucketResource.Spec.Name), err, ) } @@ -320,23 +275,18 @@ func (r *BucketReconciler) handleUpdate( if effectiveQuota != quotaToResetTo { err = s3Client.SetQuota(bucketResource.Spec.Name, quotaToResetTo) if err != nil { - logger.Error( - err, - "An error occurred while resetting the quota for bucket ressource", - "bucketName", - bucketResource.Spec.Name, - "NamespacedName", - req.NamespacedName.String(), - ) - return r.SetReconciledCondition( + + return r.SetDegradedCondition( ctx, req, bucketResource, + metav1.ConditionFalse, s3v1alpha1.Unreachable, fmt.Sprintf( - "The quota update (%v => %v) has failed", + "The quota update (%v => %v) has failed for bucket %s", effectiveQuota, quotaToResetTo, + bucketResource.Spec.Name, ), err, ) @@ -353,22 +303,14 @@ func (r *BucketReconciler) handleUpdate( for _, pathInCr := range bucketResource.Spec.Paths { pathExists, err := s3Client.PathExists(bucketResource.Spec.Name, pathInCr) if err != nil { - logger.Error( - err, - "An error occurred while checking a path's existence for bucket ressource", - "path", - pathInCr, - "bucketName", - bucketResource.Spec.Name, - "NamespacedName", - req.NamespacedName.String(), - ) - return r.SetReconciledCondition( + + return r.SetDegradedCondition( ctx, req, bucketResource, + metav1.ConditionFalse, s3v1alpha1.Unreachable, - fmt.Sprintf("The check for path [%s] in bucket has failed", pathInCr), + fmt.Sprintf("The check for path [%s] in bucket %s has failed", pathInCr, bucketResource.Spec.Name), err, ) } @@ -376,35 +318,25 @@ func (r *BucketReconciler) handleUpdate( if !pathExists { err = s3Client.CreatePath(bucketResource.Spec.Name, pathInCr) if err != nil { - logger.Error( - err, - "An error occurred while creating a path for bucket ressource", - "path", - pathInCr, - "bucketName", - bucketResource.Spec.Name, - "NamespacedName", - req.NamespacedName.String(), - ) - return r.SetReconciledCondition( + return r.SetDegradedCondition( ctx, req, bucketResource, + metav1.ConditionFalse, s3v1alpha1.Unreachable, - fmt.Sprintf("The creation of path [%s] in bucket has failed", pathInCr), + fmt.Sprintf("The creation of path [%s] in bucket %s has failed", pathInCr, bucketResource.Spec.Name), err, ) } } } - return r.SetReconciledCondition( + return r.SetAvailableCondition( ctx, req, bucketResource, s3v1alpha1.Reconciled, "Bucket reconciled", - nil, ) } @@ -413,7 +345,6 @@ func (r *BucketReconciler) handleCreation( req reconcile.Request, bucketResource *s3v1alpha1.Bucket, ) (reconcile.Result, error) { - logger := log.FromContext(ctx) s3Client, err := r.S3Instancehelper.GetS3ClientForRessource( ctx, @@ -424,15 +355,7 @@ func (r *BucketReconciler) handleCreation( bucketResource.Spec.S3InstanceRef, ) if err != nil { - logger.Error( - err, - "An error occurred while getting s3Client for bucket ressource", - "bucketName", - bucketResource.Spec.Name, - "NamespacedName", - req.NamespacedName.String(), - ) - return r.SetReconciledCondition( + return r.SetRejectedCondition( ctx, req, bucketResource, @@ -445,20 +368,13 @@ func (r *BucketReconciler) handleCreation( // Bucket creation err = s3Client.CreateBucket(bucketResource.Spec.Name) if err != nil { - logger.Error( - err, - "An error occurred while creating bucket ressource", - "bucketName", - bucketResource.Spec.Name, - "NamespacedName", - req.NamespacedName.String(), - ) - return r.SetReconciledCondition( + + return r.SetRejectedCondition( ctx, req, bucketResource, s3v1alpha1.CreationFailure, - "An error occurred while creating bucket", + fmt.Sprintf("An error occurred while creating bucket %s", bucketResource.Spec.Name), err, ) } @@ -466,37 +382,23 @@ func (r *BucketReconciler) handleCreation( // Setting quotas quotas, convertionSucceed := bucketResource.Spec.Quota.Default.AsInt64() if !convertionSucceed { - logger.Error( - err, - "An error occurred while getting quotas as int64 for ressource", - "bucketName", - bucketResource.Spec.Name, - "NamespacedName", - req.NamespacedName.String(), - ) - return r.SetReconciledCondition( + return r.SetDegradedCondition( ctx, req, bucketResource, - s3v1alpha1.CreationFailure, - "An error occurred while creating bucket", + metav1.ConditionFalse, + s3v1alpha1.InternalError, + fmt.Sprintf("An error occurred while getting default quotas as int64 for ressource %s", bucketResource.Spec.Name), err, ) } err = s3Client.SetQuota(bucketResource.Spec.Name, quotas) if err != nil { - logger.Error( - err, - "An error occurred while setting quota for bucket ressource", - "bucketName", - bucketResource.Spec.Name, - "NamespacedName", - req.NamespacedName.String(), - ) - return r.SetReconciledCondition( + return r.SetDegradedCondition( ctx, req, bucketResource, + metav1.ConditionFalse, s3v1alpha1.Unreachable, fmt.Sprintf( "Setting a quota of [%v] on bucket [%s] has failed", @@ -511,33 +413,23 @@ func (r *BucketReconciler) handleCreation( for _, pathInCr := range bucketResource.Spec.Paths { err = s3Client.CreatePath(bucketResource.Spec.Name, pathInCr) if err != nil { - logger.Error( - err, - "An error occurred while creating path for bucket ressource", - "path", - pathInCr, - "bucketName", - bucketResource.Spec.Name, - "NamespacedName", - req.NamespacedName.String(), - ) - return r.SetReconciledCondition( + return r.SetDegradedCondition( ctx, req, bucketResource, + metav1.ConditionFalse, s3v1alpha1.Unreachable, - fmt.Sprintf("Creation for path [%s] in bucket has failed", pathInCr), + fmt.Sprintf("Creation for path [%s] in bucket %s has failed", pathInCr, bucketResource.Spec.Name), err, ) } } - return r.SetReconciledCondition( + return r.SetAvailableCondition( ctx, req, bucketResource, s3v1alpha1.Reconciled, "Bucket reconciled", - nil, ) } diff --git a/internal/controller/bucket/status.go b/internal/controller/bucket/status.go index e4ff414..0820bac 100644 --- a/internal/controller/bucket/status.go +++ b/internal/controller/bucket/status.go @@ -19,12 +19,79 @@ package bucket_controller import ( "context" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" s3v1alpha1 "github.com/InseeFrLab/s3-operator/api/v1alpha1" ctrl "sigs.k8s.io/controller-runtime" ) -func (r *BucketReconciler) SetReconciledCondition( +func (r *BucketReconciler) SetProgressingCondition( + ctx context.Context, + req ctrl.Request, + bucketResource *s3v1alpha1.Bucket, + status metav1.ConditionStatus, + reason string, + message string, +) (ctrl.Result, error) { + return r.ControllerHelper.SetReconciledCondition( + ctx, + r.Status(), + req, + bucketResource, + &bucketResource.Status.Conditions, + s3v1alpha1.ConditionProgressing, + status, + reason, + message, + nil, + r.ReconcilePeriod, + ) +} +func (r *BucketReconciler) SetAvailableCondition( + ctx context.Context, + req ctrl.Request, + bucketResource *s3v1alpha1.Bucket, + reason string, + message string, +) (ctrl.Result, error) { + return r.ControllerHelper.SetReconciledCondition( + ctx, + r.Status(), + req, + bucketResource, + &bucketResource.Status.Conditions, + s3v1alpha1.ConditionAvailable, + metav1.ConditionTrue, + reason, + message, + nil, + r.ReconcilePeriod, + ) +} +func (r *BucketReconciler) SetDegradedCondition( + ctx context.Context, + req ctrl.Request, + bucketResource *s3v1alpha1.Bucket, + status metav1.ConditionStatus, + reason string, + message string, + err error, +) (ctrl.Result, error) { + return r.ControllerHelper.SetReconciledCondition( + ctx, + r.Status(), + req, + bucketResource, + &bucketResource.Status.Conditions, + s3v1alpha1.ConditionDegraded, + status, + reason, + message, + err, + r.ReconcilePeriod, + ) +} +func (r *BucketReconciler) SetRejectedCondition( ctx context.Context, req ctrl.Request, bucketResource *s3v1alpha1.Bucket, @@ -38,7 +105,8 @@ func (r *BucketReconciler) SetReconciledCondition( req, bucketResource, &bucketResource.Status.Conditions, - s3v1alpha1.ConditionReconciled, + s3v1alpha1.ConditionRejected, + metav1.ConditionFalse, reason, message, err, diff --git a/internal/controller/path/finalizer.go b/internal/controller/path/finalizer.go index 1f60c84..1cd5b21 100644 --- a/internal/controller/path/finalizer.go +++ b/internal/controller/path/finalizer.go @@ -20,9 +20,9 @@ import ( "context" "fmt" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" - "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/reconcile" s3v1alpha1 "github.com/InseeFrLab/s3-operator/api/v1alpha1" @@ -33,24 +33,16 @@ func (r *PathReconciler) handleDeletion( req reconcile.Request, pathResource *s3v1alpha1.Path, ) (reconcile.Result, error) { - logger := log.FromContext(ctx) if controllerutil.ContainsFinalizer(pathResource, pathFinalizer) { - if err := r.finalizePath(ctx, pathResource); err != nil { - logger.Error( - err, - "An error occurred when attempting to finalize the path", - "path", - pathResource.Name, - "NamespacedName", - req.NamespacedName.String(), - ) - return r.SetReconciledCondition( + if err := r.finalizePath(ctx, req, pathResource); err != nil { + return r.SetDegradedCondition( ctx, req, pathResource, + metav1.ConditionFalse, s3v1alpha1.DeletionFailure, - "Path deletion has failed", + fmt.Sprintf("Path %s deletion has failed", pathResource.Name), err, ) } @@ -59,12 +51,13 @@ func (r *PathReconciler) handleDeletion( // removed, the object will be deleted. if ok := controllerutil.RemoveFinalizer(pathResource, pathFinalizer); !ok { - logger.Info( - "Failed to remove finalizer for pathResource", - "pathResource", - pathResource.Name, - "NamespacedName", - req.NamespacedName.String(), + r.SetProgressingCondition( + ctx, + req, + pathResource, + metav1.ConditionFalse, + s3v1alpha1.InternalError, + fmt.Sprintf("Failed to remove finalizer for path %s", pathResource.Name), ) return ctrl.Result{Requeue: true}, nil } @@ -75,14 +68,16 @@ func (r *PathReconciler) handleDeletion( // your changes to the latest version and try again" which would re-trigger the reconciliation // if we try to update it again in the following operations if err := r.Update(ctx, pathResource); err != nil { - logger.Error( + r.SetDegradedCondition( + ctx, + req, + pathResource, + metav1.ConditionFalse, + s3v1alpha1.K8sApiError, + fmt.Sprintf("An error occured when removing finalizer from path %s", pathResource.Name), err, - "An error occurred when removing finalizer from pathResource", - "pathResource", - pathResource.Name, - "NamespacedName", - req.NamespacedName.String(), ) + return ctrl.Result{}, err } @@ -90,8 +85,7 @@ func (r *PathReconciler) handleDeletion( return ctrl.Result{}, nil } -func (r *PathReconciler) finalizePath(ctx context.Context, pathResource *s3v1alpha1.Path) error { - logger := log.FromContext(ctx) +func (r *PathReconciler) finalizePath(ctx context.Context, req reconcile.Request, pathResource *s3v1alpha1.Path) error { s3Client, err := r.S3Instancehelper.GetS3ClientForRessource( ctx, @@ -103,7 +97,15 @@ func (r *PathReconciler) finalizePath(ctx context.Context, pathResource *s3v1alp ) if err != nil { - logger.Error(err, "An error occurred while getting s3Client") + r.SetDegradedCondition( + ctx, + req, + pathResource, + metav1.ConditionUnknown, + s3v1alpha1.Unreachable, + "Failed to generate s3client from instance", + err, + ) return err } @@ -113,14 +115,16 @@ func (r *PathReconciler) finalizePath(ctx context.Context, pathResource *s3v1alp pathExists, err := s3Client.PathExists(pathResource.Spec.BucketName, path) if err != nil { - logger.Error( + r.SetDegradedCondition( + ctx, + req, + pathResource, + metav1.ConditionFalse, + s3v1alpha1.DeletionFailure, + fmt.Sprintf("An error occured while finalizing path %s, failed to check path's existence on bucket %s", pathResource.Name, pathResource.Spec.BucketName), err, - "finalize : an error occurred while checking a path's existence on a bucket", - "bucket", - pathResource.Spec.BucketName, - "path", - path, ) + } if pathExists { diff --git a/internal/controller/path/reconcile.go b/internal/controller/path/reconcile.go index e082e5f..baff6c0 100644 --- a/internal/controller/path/reconcile.go +++ b/internal/controller/path/reconcile.go @@ -21,7 +21,6 @@ import ( "fmt" k8sapierrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -60,25 +59,13 @@ func (r *PathReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl. // Let's just set the status as Unknown when no status are available if len(pathResource.Status.Conditions) == 0 { - meta.SetStatusCondition( - &pathResource.Status.Conditions, - metav1.Condition{ - Type: s3v1alpha1.ConditionReconciled, - Status: metav1.ConditionUnknown, - ObservedGeneration: pathResource.Generation, - Reason: s3v1alpha1.Reconciling, - Message: "Starting reconciliation", - }, - ) - if err = r.Status().Update(ctx, pathResource); err != nil { - logger.Error( - err, - "Failed to update pathResource status", - "pathResourceName", - pathResource.Name, - "NamespacedName", - req.NamespacedName.String(), - ) + _, err = r.SetProgressingCondition(ctx, + req, + pathResource, + metav1.ConditionUnknown, + s3v1alpha1.Reconciling, + fmt.Sprintf("Newly discovered resource %s", pathResource.Name)) + if err != nil { return ctrl.Result{}, err } @@ -88,48 +75,55 @@ func (r *PathReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl. // your changes to the latest version and try again" which would re-trigger the reconciliation // if we try to update it again in the following operations if err := r.Get(ctx, req.NamespacedName, pathResource); err != nil { - logger.Error( - err, - "Failed to re-fetch pathResource", - "pathResourceName", - pathResource.Name, - "NamespacedName", - req.NamespacedName.String(), - ) + r.SetDegradedCondition(ctx, + req, + pathResource, + metav1.ConditionFalse, + s3v1alpha1.K8sApiError, + fmt.Sprintf("Failed to re-fetch path resource %s", pathResource.Name), + err) return ctrl.Result{}, err } } // Add finalizer for this CR if !controllerutil.ContainsFinalizer(pathResource, pathFinalizer) { - logger.Info("Adding finalizer to pathResource", "pathResourceName", - pathResource.Name, "NamespacedName", req.NamespacedName.String()) + r.SetProgressingCondition(ctx, + req, + pathResource, + metav1.ConditionTrue, + s3v1alpha1.Reconciling, + fmt.Sprintf("Adding finalizer to path resource %s", pathResource.Name)) if ok := controllerutil.AddFinalizer(pathResource, pathFinalizer); !ok { - logger.Error( - err, - "Failed to add finalizer into pathResource", - "pathResourceName", - pathResource.Name, "NamespacedName", req.NamespacedName.String()) + r.SetDegradedCondition(ctx, + req, + pathResource, + metav1.ConditionFalse, + s3v1alpha1.InternalError, + fmt.Sprintf("Failed to add finalizer to path resource %s", pathResource.Name), + err) return ctrl.Result{Requeue: true}, nil } if err := r.Update(ctx, pathResource); err != nil { - logger.Error( - err, - "an error occurred when adding finalizer on pathResource", - "pathResourceName", - pathResource.Name, "NamespacedName", req.NamespacedName.String(), - ) + r.SetDegradedCondition(ctx, + req, + pathResource, + metav1.ConditionFalse, + s3v1alpha1.K8sApiError, + fmt.Sprintf("An error occurred when adding finalizer on path resource %s", pathResource.Name), + err) return ctrl.Result{}, err } if err := r.Get(ctx, req.NamespacedName, pathResource); err != nil { - logger.Error( - err, - "Failed to re-fetch pathResource", - "pathResourceName", - pathResource.Name, "NamespacedName", req.NamespacedName.String(), - ) + r.SetDegradedCondition(ctx, + req, + pathResource, + metav1.ConditionFalse, + s3v1alpha1.K8sApiError, + fmt.Sprintf("Failed to re-fetch path resource %s", pathResource.Name), + err) return ctrl.Result{}, err } } @@ -137,13 +131,16 @@ func (r *PathReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl. // Managing path deletion with a finalizer // REF : https://sdk.operatorframework.io/docs/building-operators/golang/advanced-topics/#external-resources if pathResource.GetDeletionTimestamp() != nil { - logger.Info("pathResource have been marked for deletion", "pathResourceName", - pathResource.Name, - "NamespacedName", - req.NamespacedName.String()) + r.SetProgressingCondition(ctx, + req, + pathResource, + metav1.ConditionTrue, + s3v1alpha1.Reconciling, + fmt.Sprintf("Path resource have been marked for deletion %s", pathResource.Name)) return r.handleDeletion(ctx, req, pathResource) } + r.SetProgressingCondition(ctx, req, pathResource, metav1.ConditionTrue, s3v1alpha1.Reconciling, fmt.Sprintf("Starting reconciliation of path %s", pathResource.Name)) return r.handleReconciliation(ctx, req, pathResource) } @@ -154,7 +151,6 @@ func (r *PathReconciler) handleReconciliation( pathResource *s3v1alpha1.Path, ) (reconcile.Result, error) { - logger := log.FromContext(ctx) // Create S3Client s3Client, err := r.S3Instancehelper.GetS3ClientForRessource( @@ -166,11 +162,11 @@ func (r *PathReconciler) handleReconciliation( pathResource.Spec.S3InstanceRef, ) if err != nil { - logger.Error(err, "an error occurred while getting s3Client") - return r.SetReconciledCondition( + return r.SetDegradedCondition( ctx, req, pathResource, + metav1.ConditionUnknown, s3v1alpha1.Unreachable, "Failed to generate s3client from instance", err, @@ -182,36 +178,20 @@ func (r *PathReconciler) handleReconciliation( // Check bucket existence on the S3 server bucketFound, err := s3Client.BucketExists(pathResource.Spec.BucketName) if err != nil { - logger.Error( - err, - "an error occurred while checking the existence of a bucket", - "bucketName", - pathResource.Spec.BucketName, - "NamespacedName", - req.NamespacedName.String(), - ) - return r.SetReconciledCondition( + return r.SetDegradedCondition( ctx, req, pathResource, + metav1.ConditionUnknown, s3v1alpha1.Unreachable, - "Error while checking if bucket already exist", + fmt.Sprintf("Error while checking if bucket %s already exist", pathResource.Spec.BucketName), err, ) } // If bucket does not exist, the Path CR should be in a failing state if !bucketFound { - errorBucketNotFound := fmt.Errorf( - "the path CR %s references a non-existing bucket : %s", - pathResource.Name, - pathResource.Spec.BucketName, - ) - logger.Error(errorBucketNotFound, errorBucketNotFound.Error(), "pathResourceName", - pathResource.Name, - "NamespacedName", - req.NamespacedName.String()) - return r.SetReconciledCondition( + return r.SetRejectedCondition( ctx, req, pathResource, @@ -236,22 +216,13 @@ func (r *PathReconciler) handleReconciliation( for _, pathInCr := range pathResource.Spec.Paths { pathExists, err := s3Client.PathExists(pathResource.Spec.BucketName, pathInCr) if err != nil { - logger.Error( - err, - "An error occurred while checking a path's existence for bucket ressource", - "path", - pathInCr, - "bucketName", - pathResource.Spec.BucketName, - "NamespacedName", - req.NamespacedName.String(), - ) - return r.SetReconciledCondition( + return r.SetDegradedCondition( ctx, req, pathResource, + metav1.ConditionFalse, s3v1alpha1.Unreachable, - fmt.Sprintf("The check for path [%s] in bucket has failed", pathInCr), + fmt.Sprintf("The check for path [%s] in bucket %s has failed", pathInCr, pathResource.Spec.BucketName), err, ) } @@ -259,34 +230,23 @@ func (r *PathReconciler) handleReconciliation( if !pathExists { err = s3Client.CreatePath(pathResource.Spec.BucketName, pathInCr) if err != nil { - logger.Error( - err, - "An error occurred while creating a path for bucket ressource", - "path", - pathInCr, - "bucketName", - pathResource.Spec.BucketName, - "NamespacedName", - req.NamespacedName.String(), - ) - return r.SetReconciledCondition( + return r.SetRejectedCondition( ctx, req, pathResource, s3v1alpha1.Unreachable, - fmt.Sprintf("The creation of path [%s] in bucket has failed", pathInCr), + fmt.Sprintf("The creation of path [%s] in bucket %s has failed", pathInCr, pathResource.Spec.BucketName), err, ) } } } - return r.SetReconciledCondition( + return r.SetAvailableCondition( ctx, req, pathResource, s3v1alpha1.Reconciled, "Path reconciled", - nil, ) } diff --git a/internal/controller/path/status.go b/internal/controller/path/status.go index 2456a8c..19d4262 100644 --- a/internal/controller/path/status.go +++ b/internal/controller/path/status.go @@ -19,15 +19,60 @@ package path_controller import ( "context" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ctrl "sigs.k8s.io/controller-runtime" s3v1alpha1 "github.com/InseeFrLab/s3-operator/api/v1alpha1" ) -func (r *PathReconciler) SetReconciledCondition( +func (r *PathReconciler) SetProgressingCondition( ctx context.Context, req ctrl.Request, pathResource *s3v1alpha1.Path, + status metav1.ConditionStatus, + reason string, + message string, +) (ctrl.Result, error) { + return r.ControllerHelper.SetReconciledCondition( + ctx, + r.Status(), + req, + pathResource, + &pathResource.Status.Conditions, + s3v1alpha1.ConditionProgressing, + status, + reason, + message, + nil, + r.ReconcilePeriod, + ) +} +func (r *PathReconciler) SetAvailableCondition( + ctx context.Context, + req ctrl.Request, + pathResource *s3v1alpha1.Path, + reason string, + message string, +) (ctrl.Result, error) { + return r.ControllerHelper.SetReconciledCondition( + ctx, + r.Status(), + req, + pathResource, + &pathResource.Status.Conditions, + s3v1alpha1.ConditionAvailable, + metav1.ConditionTrue, + reason, + message, + nil, + r.ReconcilePeriod, + ) +} +func (r *PathReconciler) SetDegradedCondition( + ctx context.Context, + req ctrl.Request, + pathResource *s3v1alpha1.Path, + status metav1.ConditionStatus, reason string, message string, err error, @@ -38,11 +83,33 @@ func (r *PathReconciler) SetReconciledCondition( req, pathResource, &pathResource.Status.Conditions, - s3v1alpha1.ConditionReconciled, + s3v1alpha1.ConditionDegraded, + status, + reason, + message, + err, + r.ReconcilePeriod, + ) +} +func (r *PathReconciler) SetRejectedCondition( + ctx context.Context, + req ctrl.Request, + pathResource *s3v1alpha1.Path, + reason string, + message string, + err error, +) (ctrl.Result, error) { + return r.ControllerHelper.SetReconciledCondition( + ctx, + r.Status(), + req, + pathResource, + &pathResource.Status.Conditions, + s3v1alpha1.ConditionRejected, + metav1.ConditionFalse, reason, message, err, r.ReconcilePeriod, ) - } diff --git a/internal/controller/policy/finalizer.go b/internal/controller/policy/finalizer.go index 2adb995..6bee2ff 100644 --- a/internal/controller/policy/finalizer.go +++ b/internal/controller/policy/finalizer.go @@ -18,10 +18,11 @@ package policy_controller import ( "context" + "fmt" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" - "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/reconcile" s3v1alpha1 "github.com/InseeFrLab/s3-operator/api/v1alpha1" @@ -29,9 +30,9 @@ import ( func (r *PolicyReconciler) finalizePolicy( ctx context.Context, + req reconcile.Request, policyResource *s3v1alpha1.Policy, ) error { - logger := log.FromContext(ctx) s3Client, err := r.S3Instancehelper.GetS3ClientForRessource( ctx, r.Client, @@ -41,7 +42,15 @@ func (r *PolicyReconciler) finalizePolicy( policyResource.Spec.S3InstanceRef, ) if err != nil { - logger.Error(err, "an error occurred while getting s3Client") + r.SetDegradedCondition( + ctx, + req, + policyResource, + metav1.ConditionUnknown, + s3v1alpha1.Unreachable, + "Failed to generate s3client from instance", + err, + ) return err } if s3Client.GetConfig().PolicyDeletionEnabled { @@ -55,49 +64,43 @@ func (r *PolicyReconciler) handleDeletion( req reconcile.Request, policyResource *s3v1alpha1.Policy, ) (reconcile.Result, error) { - logger := log.FromContext(ctx) if controllerutil.ContainsFinalizer(policyResource, policyFinalizer) { // Run finalization logic for policyFinalizer. If the // finalization logic fails, don't remove the finalizer so // that we can retry during the next reconciliation. - if err := r.finalizePolicy(ctx, policyResource); err != nil { - logger.Error( - err, - "An error occurred when attempting to finalize the policy", - "policyName", - policyResource.Spec.Name, - "NamespacedName", - req.NamespacedName.String(), - ) - return r.SetReconciledCondition( + if err := r.finalizePolicy(ctx, req, policyResource); err != nil { + return r.SetDegradedCondition( ctx, req, policyResource, + metav1.ConditionFalse, s3v1alpha1.DeletionFailure, - "Policy deletion has failed", + fmt.Sprintf("Policy %s deletion has failed", policyResource.Spec.Name), err, ) } if ok := controllerutil.RemoveFinalizer(policyResource, policyFinalizer); !ok { - logger.Info( - "Failed to remove finalizer for policyResource", - "policyName", - policyResource.Spec.Name, - "NamespacedName", - req.NamespacedName.String(), + r.SetProgressingCondition( + ctx, + req, + policyResource, + metav1.ConditionFalse, + s3v1alpha1.InternalError, + fmt.Sprintf("Failed to remove finalizer for policy %s", policyResource.Spec.Name), ) return ctrl.Result{Requeue: true}, nil } if err := r.Update(ctx, policyResource); err != nil { - logger.Error( + r.SetDegradedCondition( + ctx, + req, + policyResource, + metav1.ConditionFalse, + s3v1alpha1.K8sApiError, + fmt.Sprintf("An error occured when removing finalizer from policy %s", policyResource.Spec.Name), err, - "an error occurred when removing finalizer from policy", - "policyName", - policyResource.Spec.Name, - "NamespacedName", - req.NamespacedName.String(), ) return ctrl.Result{}, err } diff --git a/internal/controller/policy/reconcile.go b/internal/controller/policy/reconcile.go index b01644c..a43ef12 100644 --- a/internal/controller/policy/reconcile.go +++ b/internal/controller/policy/reconcile.go @@ -23,7 +23,6 @@ import ( "fmt" k8sapierrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -63,25 +62,13 @@ func (r *PolicyReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr // Let's just set the status as Unknown when no status are available if len(policyResource.Status.Conditions) == 0 { - meta.SetStatusCondition( - &policyResource.Status.Conditions, - metav1.Condition{ - Type: s3v1alpha1.ConditionReconciled, - Status: metav1.ConditionUnknown, - ObservedGeneration: policyResource.Generation, - Reason: s3v1alpha1.Reconciling, - Message: "Starting reconciliation", - }, - ) - if err = r.Status().Update(ctx, policyResource); err != nil { - logger.Error( - err, - "Failed to update bucketRessource status", - "bucketName", - policyResource.Spec.Name, - "NamespacedName", - req.NamespacedName.String(), - ) + _, err = r.SetProgressingCondition(ctx, + req, + policyResource, + metav1.ConditionUnknown, + s3v1alpha1.Reconciling, + fmt.Sprintf("Newly discovered resource %s", policyResource.Name)) + if err != nil { return ctrl.Result{}, err } @@ -91,61 +78,60 @@ func (r *PolicyReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr // your changes to the latest version and try again" which would re-trigger the reconciliation // if we try to update it again in the following operations if err := r.Get(ctx, req.NamespacedName, policyResource); err != nil { - logger.Error( - err, - "Failed to re-fetch policyResource", - "policyName", - policyResource.Spec.Name, - "NamespacedName", - req.NamespacedName.String(), - ) + + r.SetDegradedCondition(ctx, + req, + policyResource, + metav1.ConditionFalse, + s3v1alpha1.K8sApiError, + fmt.Sprintf("Failed to re-fetch policy resource %s", policyResource.Name), + err) return ctrl.Result{}, err } } // Add finalizer for this CR if !controllerutil.ContainsFinalizer(policyResource, policyFinalizer) { - logger.Info("Adding finalizer to policy resource", "PolicyName", - policyResource.Spec.Name, "NamespacedName", req.NamespacedName.String()) + r.SetProgressingCondition(ctx, + req, + policyResource, + metav1.ConditionTrue, + s3v1alpha1.Reconciling, + fmt.Sprintf("Adding finalizer to policy resource %s", policyResource.Name)) if ok := controllerutil.AddFinalizer(policyResource, policyFinalizer); !ok { - logger.Error( - err, - "Failed to add finalizer into policy resource", - "policyName", - policyResource.Spec.Name, - "NamespacedName", - req.NamespacedName.String(), - ) + r.SetDegradedCondition(ctx, + req, + policyResource, + metav1.ConditionFalse, + s3v1alpha1.InternalError, + fmt.Sprintf("Failed to add finalizer to policy resource %s", policyResource.Name), + err) return ctrl.Result{Requeue: true}, nil } - err = r.Update(ctx, policyResource) - if err != nil { - logger.Error( - err, - "An error occurred when adding finalizer from policyResource", - "policyResource", - policyResource.Spec.Name, - "NamespacedName", - req.NamespacedName.String(), - ) + if err := r.Update(ctx, policyResource); err != nil { + r.SetDegradedCondition(ctx, + req, + policyResource, + metav1.ConditionFalse, + s3v1alpha1.K8sApiError, + fmt.Sprintf("An error occurred when adding finalizer on policy resource %s", policyResource.Name), + err) return ctrl.Result{}, err } - // Let's re-fetch the policy Custom Resource after adding the finalizer // so that we have the latest state of the resource on the cluster and we will avoid // raise the issue "the object has been modified, please apply // your changes to the latest version and try again" which would re-trigger the reconciliation // if we try to update it again in the following operations if err := r.Get(ctx, req.NamespacedName, policyResource); err != nil { - logger.Error( - err, - "Failed to re-fetch policyResource", - "policyName", - policyResource.Spec.Name, - "NamespacedName", - req.NamespacedName.String(), - ) + r.SetDegradedCondition(ctx, + req, + policyResource, + metav1.ConditionFalse, + s3v1alpha1.K8sApiError, + fmt.Sprintf("Failed to re-fetch policy resource %s", policyResource.Name), + err) return ctrl.Result{}, err } } @@ -153,16 +139,18 @@ func (r *PolicyReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr // Managing policy deletion with a finalizer // REF : https://sdk.operatorframework.io/docs/building-operators/golang/advanced-topics/#external-resources if policyResource.GetDeletionTimestamp() != nil { - logger.Info("policyResource have been marked for deletion", "policyName", - policyResource.Spec.Name, - "NamespacedName", - req.NamespacedName.String()) + r.SetProgressingCondition(ctx, + req, + policyResource, + metav1.ConditionTrue, + s3v1alpha1.Reconciling, + fmt.Sprintf("policy resource have been marked for deletion %s", policyResource.Name)) return r.handleDeletion(ctx, req, policyResource) } // Policy lifecycle management (other than deletion) starts here + r.SetProgressingCondition(ctx, req, policyResource, metav1.ConditionTrue, s3v1alpha1.Reconciling, fmt.Sprintf("Starting reconciliation of policy %s", policyResource.Name)) return r.handleReconciliation(ctx, req, policyResource) - } func (r *PolicyReconciler) handleReconciliation( @@ -170,8 +158,9 @@ func (r *PolicyReconciler) handleReconciliation( req reconcile.Request, policyResource *s3v1alpha1.Policy, ) (reconcile.Result, error) { - logger := log.FromContext(ctx) + + // Create S3Client s3Client, err := r.S3Instancehelper.GetS3ClientForRessource( ctx, r.Client, @@ -181,34 +170,27 @@ func (r *PolicyReconciler) handleReconciliation( policyResource.Spec.S3InstanceRef, ) if err != nil { - logger.Error(err, "an error occurred while getting s3Client") - return r.SetReconciledCondition( + return r.SetDegradedCondition( ctx, req, policyResource, + metav1.ConditionUnknown, s3v1alpha1.Unreachable, "Failed to generate s3client from instance", err, ) } - // Check policy existence on the S3 server + // check policy existence on the s3 server effectivePolicy, err := s3Client.GetPolicyInfo(policyResource.Spec.Name) if err != nil { - logger.Error( - err, - "An error occurred while checking the existence of a policy", - "policyName", - policyResource.Spec.Name, - "NamespacedName", - req.NamespacedName.String(), - ) - return r.SetReconciledCondition( + return r.SetDegradedCondition( ctx, req, policyResource, + metav1.ConditionUnknown, s3v1alpha1.Unreachable, - "Error while checking if policy already exist", + fmt.Sprintf("Error while checking if policy %s already exist", policyResource.Spec.Name), err, ) } @@ -226,7 +208,6 @@ func (r *PolicyReconciler) handleUpdate( req reconcile.Request, policyResource *s3v1alpha1.Policy, ) (reconcile.Result, error) { - logger := log.FromContext(ctx) s3Client, err := r.S3Instancehelper.GetS3ClientForRessource( ctx, @@ -238,11 +219,11 @@ func (r *PolicyReconciler) handleUpdate( ) if err != nil { - logger.Error(err, "an error occurred while getting s3Client") - return r.SetReconciledCondition( + return r.SetDegradedCondition( ctx, req, policyResource, + metav1.ConditionUnknown, s3v1alpha1.Unreachable, "Failed to generate s3client from instance", err, @@ -252,42 +233,29 @@ func (r *PolicyReconciler) handleUpdate( // Check policy existence on the S3 server effectivePolicy, err := s3Client.GetPolicyInfo(policyResource.Spec.Name) if err != nil { - logger.Error( - err, - "An error occurred while checking the existence of a policy", - "policyName", - policyResource.Spec.Name, - "NamespacedName", - req.NamespacedName.String(), - ) - return r.SetReconciledCondition( + return r.SetDegradedCondition( ctx, req, policyResource, + metav1.ConditionUnknown, s3v1alpha1.Unreachable, - "Error while checking if policy already exist", + fmt.Sprintf("Error while checking if policy %s already exist", policyResource.Spec.Name), err, ) } matching, err := r.isPolicyMatchingWithCustomResource(policyResource, effectivePolicy) if err != nil { - logger.Error( - err, - "An error occurred while comparing actual and expected configuration for the policy", - "policyName", - policyResource.Spec.Name, - "NamespacedName", - req.NamespacedName.String(), - ) - return r.SetReconciledCondition( + return r.SetDegradedCondition( ctx, req, policyResource, + metav1.ConditionFalse, s3v1alpha1.Unreachable, fmt.Sprintf( - "The comparison between the effective policy [%s] on S3 and its corresponding custom resource on K8S has failed", + "The comparison between the effective policy [%s] on S3 and its corresponding custom resource %s on K8S has failed", policyResource.Spec.Name, + policyResource.Name, ), err, ) @@ -300,41 +268,33 @@ func (r *PolicyReconciler) handleUpdate( policyResource.Spec.PolicyContent, ) if err != nil { - logger.Error( - err, - "An error occurred while updating the policy", - "policyName", - policyResource.Spec.Name, - "NamespacedName", - req.NamespacedName.String(), - ) - r.SetReconciledCondition( + r.SetDegradedCondition( ctx, req, policyResource, + metav1.ConditionFalse, s3v1alpha1.Unreachable, fmt.Sprintf( - "The comparison between the effective policy [%s] on S3 and its corresponding custom resource on K8S has failed", + "The comparison between the effective policy [%s] on S3 and its corresponding custom resource %s on K8S has failed", policyResource.Spec.Name, + policyResource.Name, ), err, ) } } - return r.SetReconciledCondition( + return r.SetAvailableCondition( ctx, req, policyResource, s3v1alpha1.Reconciled, "Policy reconciled", - nil, ) } func (r *PolicyReconciler) handleCreation(ctx context.Context, req reconcile.Request, policyResource *s3v1alpha1.Policy) (reconcile.Result, error) { - logger := log.FromContext(ctx) s3Client, err := r.S3Instancehelper.GetS3ClientForRessource( ctx, @@ -346,8 +306,7 @@ func (r *PolicyReconciler) handleCreation(ctx context.Context, req reconcile.Req ) if err != nil { - logger.Error(err, "An error occurred while getting s3Client") - return r.SetReconciledCondition( + return r.SetRejectedCondition( ctx, req, policyResource, @@ -363,15 +322,8 @@ func (r *PolicyReconciler) handleCreation(ctx context.Context, req reconcile.Req ) if err != nil { - logger.Error( - err, - "An error occurred while creating the policy", - "policyName", - policyResource.Spec.Name, - "NamespacedName", - req.NamespacedName.String(), - ) - return r.SetReconciledCondition( + + return r.SetRejectedCondition( ctx, req, policyResource, @@ -381,13 +333,12 @@ func (r *PolicyReconciler) handleCreation(ctx context.Context, req reconcile.Req ) } - return r.SetReconciledCondition( + return r.SetAvailableCondition( ctx, req, policyResource, s3v1alpha1.Reconciled, "Policy reconciled", - err, ) } diff --git a/internal/controller/policy/status.go b/internal/controller/policy/status.go index ba780b7..8bf8b93 100644 --- a/internal/controller/policy/status.go +++ b/internal/controller/policy/status.go @@ -19,12 +19,79 @@ package policy_controller import ( "context" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ctrl "sigs.k8s.io/controller-runtime" s3v1alpha1 "github.com/InseeFrLab/s3-operator/api/v1alpha1" ) -func (r *PolicyReconciler) SetReconciledCondition( +func (r *PolicyReconciler) SetProgressingCondition( + ctx context.Context, + req ctrl.Request, + policyResource *s3v1alpha1.Policy, + status metav1.ConditionStatus, + reason string, + message string, +) (ctrl.Result, error) { + return r.ControllerHelper.SetReconciledCondition( + ctx, + r.Status(), + req, + policyResource, + &policyResource.Status.Conditions, + s3v1alpha1.ConditionProgressing, + status, + reason, + message, + nil, + r.ReconcilePeriod, + ) +} +func (r *PolicyReconciler) SetAvailableCondition( + ctx context.Context, + req ctrl.Request, + policyResource *s3v1alpha1.Policy, + reason string, + message string, +) (ctrl.Result, error) { + return r.ControllerHelper.SetReconciledCondition( + ctx, + r.Status(), + req, + policyResource, + &policyResource.Status.Conditions, + s3v1alpha1.ConditionAvailable, + metav1.ConditionTrue, + reason, + message, + nil, + r.ReconcilePeriod, + ) +} +func (r *PolicyReconciler) SetDegradedCondition( + ctx context.Context, + req ctrl.Request, + policyResource *s3v1alpha1.Policy, + status metav1.ConditionStatus, + reason string, + message string, + err error, +) (ctrl.Result, error) { + return r.ControllerHelper.SetReconciledCondition( + ctx, + r.Status(), + req, + policyResource, + &policyResource.Status.Conditions, + s3v1alpha1.ConditionDegraded, + status, + reason, + message, + err, + r.ReconcilePeriod, + ) +} +func (r *PolicyReconciler) SetRejectedCondition( ctx context.Context, req ctrl.Request, policyResource *s3v1alpha1.Policy, @@ -38,7 +105,8 @@ func (r *PolicyReconciler) SetReconciledCondition( req, policyResource, &policyResource.Status.Conditions, - s3v1alpha1.ConditionReconciled, + s3v1alpha1.ConditionRejected, + metav1.ConditionFalse, reason, message, err, diff --git a/internal/controller/s3instance/finalizer.go b/internal/controller/s3instance/finalizer.go index 2a5dae4..619c35b 100644 --- a/internal/controller/s3instance/finalizer.go +++ b/internal/controller/s3instance/finalizer.go @@ -18,12 +18,13 @@ package s3instance_controller import ( "context" + "strings" "fmt" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" - "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/reconcile" s3v1alpha1 "github.com/InseeFrLab/s3-operator/api/v1alpha1" @@ -34,28 +35,40 @@ func (r *S3InstanceReconciler) handleS3InstanceDeletion( req ctrl.Request, s3InstanceResource *s3v1alpha1.S3Instance, ) (reconcile.Result, error) { - logger := log.FromContext(ctx) if controllerutil.ContainsFinalizer(s3InstanceResource, s3InstanceFinalizer) { - logger.Info( - "Performing Finalizer Operations for S3Instance before delete CR", - "Namespace", - s3InstanceResource.GetNamespace(), - "Name", - s3InstanceResource.GetName(), + r.SetProgressingCondition( + ctx, + req, + s3InstanceResource, + metav1.ConditionTrue, + s3v1alpha1.Reconciling, + fmt.Sprintf("Performing Finalizer Operations for S3Instance %s before delete CR", s3InstanceResource.GetName()), ) // Vérifier les références existantes if err := r.checkS3InstanceReferences(ctx, s3InstanceResource); err != nil { + r.SetDegradedCondition( + ctx, + req, + s3InstanceResource, + metav1.ConditionFalse, + s3v1alpha1.DeletionBlocked, + fmt.Sprintf("S3Instance %s is still referenced by other resources", s3InstanceResource.Name), + err, + ) return ctrl.Result{}, err } //Remove s3InstanceFinalizer. Once all finalizers have been removed, the object will be deleted. if ok := controllerutil.RemoveFinalizer(s3InstanceResource, s3InstanceFinalizer); !ok { - logger.Info( - "Failed to remove finalizer for S3Instance", - "NamespacedName", - req.NamespacedName.String(), + r.SetProgressingCondition( + ctx, + req, + s3InstanceResource, + metav1.ConditionFalse, + s3v1alpha1.InternalError, + fmt.Sprintf("Failed to remove finalizer for S3Instance %s", s3InstanceResource.Name), ) return ctrl.Result{Requeue: true}, nil } @@ -66,11 +79,14 @@ func (r *S3InstanceReconciler) handleS3InstanceDeletion( // your changes to the latest version and try again" which would re-trigger the reconciliation // if we try to update it again in the following operations if err := r.Update(ctx, s3InstanceResource); err != nil { - logger.Error( + r.SetDegradedCondition( + ctx, + req, + s3InstanceResource, + metav1.ConditionFalse, + s3v1alpha1.K8sApiError, + fmt.Sprintf("An error occured when removing finalizer from S3Instance %s", s3InstanceResource.Name), err, - "Failed to remove finalizer for S3Instance", - "NamespacedName", - req.NamespacedName.String(), ) return ctrl.Result{}, err } @@ -78,25 +94,28 @@ func (r *S3InstanceReconciler) handleS3InstanceDeletion( return ctrl.Result{}, nil } -// checkS3InstanceReferences vérifie si l'instance S3 est encore utilisée +// checkS3InstanceReferences make sure the instance is not used anymore func (r *S3InstanceReconciler) checkS3InstanceReferences(ctx context.Context, s3Instance *s3v1alpha1.S3Instance) error { - // Liste des types de ressources à vérifier + // CR to be checked for existence references := map[string]client.ObjectList{ "Buckets": &s3v1alpha1.BucketList{}, "Policies": &s3v1alpha1.PolicyList{}, "Paths": &s3v1alpha1.PathList{}, "S3Users": &s3v1alpha1.S3UserList{}, } - + var errors []string for name, list := range references { if err := r.List(ctx, list); err != nil { - return fmt.Errorf("échec de la récupération des %s : %w", name, err) + return fmt.Errorf("Failed to retrieve %s : %w", name, err) } if found := r.countReferences(list, s3Instance); found > 0 { - return fmt.Errorf("impossible de supprimer S3Instance, %d %s utilisent cette instance", found, name) + errors = append(errors, fmt.Sprintf("Cannot delete s3Instance as %d %s are used on this instance", found, name)) } } + if len(errors) >0 { + return fmt.Errorf(strings.Join(errors, "\n")) + } return nil } diff --git a/internal/controller/s3instance/finalizer_test.go b/internal/controller/s3instance/finalizer_test.go index 3ef63d0..87a1676 100644 --- a/internal/controller/s3instance/finalizer_test.go +++ b/internal/controller/s3instance/finalizer_test.go @@ -147,7 +147,7 @@ func TestHandleDelete(t *testing.T) { req := ctrl.Request{NamespacedName: types.NamespacedName{Name: s3instanceResource.Name, Namespace: s3instanceResource.Namespace}} _, err := reconciler.Reconcile(context.TODO(), req) assert.Error(t, err) - assert.EqualErrorf(t, err, "impossible de supprimer S3Instance, 1 Buckets utilisent cette instance", err.Error()) + assert.EqualErrorf(t, err, "Cannot delete s3Instance as 1 Buckets are used on this instance", err.Error()) }) t.Run("error if one policy ressource still use it", func(t *testing.T) { @@ -208,7 +208,7 @@ func TestHandleDelete(t *testing.T) { req := ctrl.Request{NamespacedName: types.NamespacedName{Name: s3instanceResource.Name, Namespace: s3instanceResource.Namespace}} _, err := reconciler.Reconcile(context.TODO(), req) assert.Error(t, err) - assert.EqualErrorf(t, err, "impossible de supprimer S3Instance, 1 Policies utilisent cette instance", err.Error()) + assert.EqualErrorf(t, err, "Cannot delete s3Instance as 1 Policies are used on this instance", err.Error()) }) t.Run("error if one path ressource still use it", func(t *testing.T) { @@ -269,7 +269,7 @@ func TestHandleDelete(t *testing.T) { req := ctrl.Request{NamespacedName: types.NamespacedName{Name: s3instanceResource.Name, Namespace: s3instanceResource.Namespace}} _, err := reconciler.Reconcile(context.TODO(), req) assert.Error(t, err) - assert.EqualErrorf(t, err, "impossible de supprimer S3Instance, 1 Paths utilisent cette instance", err.Error()) + assert.EqualErrorf(t, err, "Cannot delete s3Instance as 1 Paths are used on this instance", err.Error()) }) t.Run("error if one user ressource still use it", func(t *testing.T) { @@ -330,6 +330,6 @@ func TestHandleDelete(t *testing.T) { req := ctrl.Request{NamespacedName: types.NamespacedName{Name: s3instanceResource.Name, Namespace: s3instanceResource.Namespace}} _, err := reconciler.Reconcile(context.TODO(), req) assert.Error(t, err) - assert.EqualErrorf(t, err, "impossible de supprimer S3Instance, 1 S3Users utilisent cette instance", err.Error()) + assert.EqualErrorf(t, err, "Cannot delete s3Instance as 1 S3Users are used on this instance", err.Error()) }) } diff --git a/internal/controller/s3instance/reconcile.go b/internal/controller/s3instance/reconcile.go index 548cf3e..cefdafb 100644 --- a/internal/controller/s3instance/reconcile.go +++ b/internal/controller/s3instance/reconcile.go @@ -21,7 +21,6 @@ import ( "fmt" k8sapierrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -63,94 +62,62 @@ func (r *S3InstanceReconciler) Reconcile( return ctrl.Result{}, err } - // Let's just set the status as Unknown when no status are available - if len(s3InstanceResource.Status.Conditions) == 0 { - meta.SetStatusCondition( - &s3InstanceResource.Status.Conditions, - metav1.Condition{ - Type: s3v1alpha1.ConditionReconciled, - Status: metav1.ConditionUnknown, - ObservedGeneration: s3InstanceResource.Generation, - Reason: s3v1alpha1.Reconciling, - Message: "Starting reconciliation", - }, - ) - if err = r.Status().Update(ctx, s3InstanceResource); err != nil { - logger.Error( - err, - "Failed to update s3InstanceResource status", - "NamespacedName", - req.NamespacedName.String(), - ) - return ctrl.Result{}, err - } - - // Let's re-fetch the s3InstanceResource Custom Resource after update the status - // so that we have the latest state of the resource on the cluster and we will avoid - // raise the issue "the object has been modified, please apply - // your changes to the latest version and try again" which would re-trigger the reconciliation - // if we try to update it again in the following operations - if err := r.Get(ctx, req.NamespacedName, s3InstanceResource); err != nil { - logger.Error( - err, - "Failed to re-fetch s3Instance", - "NamespacedName", - req.NamespacedName.String(), - ) - return ctrl.Result{}, err - } - } - // Add finalizer for this CR if !controllerutil.ContainsFinalizer(s3InstanceResource, s3InstanceFinalizer) { - logger.Info("Adding finalizer to s3Instance", "NamespacedName", req.NamespacedName.String()) + r.SetProgressingCondition(ctx, + req, + s3InstanceResource, + metav1.ConditionTrue, + s3v1alpha1.Reconciling, + fmt.Sprintf("Adding finalizer to s3Instance resource %s", s3InstanceResource.Name)) if ok := controllerutil.AddFinalizer(s3InstanceResource, s3InstanceFinalizer); !ok { - logger.Error( - err, - "Failed to add finalizer into the s3Instance", - "NamespacedName", - req.NamespacedName.String(), - ) + r.SetDegradedCondition(ctx, + req, + s3InstanceResource, + metav1.ConditionFalse, + s3v1alpha1.InternalError, + fmt.Sprintf("Failed to add finalizer to s3Instance resource %s", s3InstanceResource.Name), + err) return ctrl.Result{Requeue: true}, nil } - if err = r.Update(ctx, s3InstanceResource); err != nil { - logger.Error( - err, - "an error occurred when adding finalizer on s3Instance", - "s3Instance", - s3InstanceResource.Name, - ) + if err := r.Update(ctx, s3InstanceResource); err != nil { + r.SetDegradedCondition(ctx, + req, + s3InstanceResource, + metav1.ConditionFalse, + s3v1alpha1.K8sApiError, + fmt.Sprintf("An error occurred when adding finalizer on s3Instance resource %s", s3InstanceResource.Name), + err) return ctrl.Result{}, err } - // Let's re-fetch the S3Instance Custom Resource after adding the finalizer - // so that we have the latest state of the resource on the cluster and we will avoid - // raise the issue "the object has been modified, please apply - // your changes to the latest version and try again" which would re-trigger the reconciliation - // if we try to update it again in the following operations if err := r.Get(ctx, req.NamespacedName, s3InstanceResource); err != nil { - logger.Error( - err, - "Failed to re-fetch s3Instance", - "NamespacedName", - req.NamespacedName.String(), - ) + r.SetDegradedCondition(ctx, + req, + s3InstanceResource, + metav1.ConditionFalse, + s3v1alpha1.K8sApiError, + fmt.Sprintf("Failed to re-fetch s3Instance resource %s", s3InstanceResource.Name), + err) return ctrl.Result{}, err } - } - // Check if the s3InstanceResource instance is marked to be deleted, which is // indicated by the deletion timestamp being set. The object will be deleted. if s3InstanceResource.GetDeletionTimestamp() != nil { - logger.Info("s3InstanceResource have been marked for deletion") + r.SetProgressingCondition(ctx, + req, + s3InstanceResource, + metav1.ConditionTrue, + s3v1alpha1.Reconciling, + fmt.Sprintf("S3Instance resource have been marked for deletion %s", s3InstanceResource.Name)) return r.handleS3InstanceDeletion(ctx, req, s3InstanceResource) } // Reconciliation starts here + r.SetProgressingCondition(ctx, req, s3InstanceResource, metav1.ConditionTrue, s3v1alpha1.Reconciling, fmt.Sprintf("Starting reconciliation of s3Instance %s", s3InstanceResource.Name)) return r.handleReconciliation(ctx, req, s3InstanceResource) - } func (r *S3InstanceReconciler) handleReconciliation( @@ -158,44 +125,26 @@ func (r *S3InstanceReconciler) handleReconciliation( req reconcile.Request, s3InstanceResource *s3v1alpha1.S3Instance, ) (reconcile.Result, error) { - logger := log.FromContext(ctx) s3Client, err := r.S3Instancehelper.GetS3ClientFromS3Instance(ctx, r.Client, r.S3factory, s3InstanceResource) if err != nil { - logger.Error( - err, - "Could not generate s3Instance", - "s3InstanceSecretRefName", - s3InstanceResource.Spec.SecretRef, - "NamespacedName", - req.NamespacedName.String(), - ) - return r.SetReconciledCondition(ctx, req, s3InstanceResource, s3v1alpha1.Unreachable, - "Failed to generate S3Instance ", err) + return r.SetDegradedCondition(ctx, req, s3InstanceResource, metav1.ConditionFalse, s3v1alpha1.Unreachable, + fmt.Sprintf("Failed to generate S3Instance %s using secret %s", s3InstanceResource.Name, s3InstanceResource.Spec.SecretRef), err) } _, err = s3Client.ListBuckets() if err != nil { - logger.Error( - err, - "Could not generate s3Instance", - "s3InstanceName", - s3InstanceResource.Name, - "NamespacedName", - req.NamespacedName.String(), - ) - return r.SetReconciledCondition(ctx, req, s3InstanceResource, s3v1alpha1.CreationFailure, - "Failed to generate S3Instance ", err) + return r.SetDegradedCondition(ctx, req, s3InstanceResource, metav1.ConditionFalse, s3v1alpha1.CreationFailure, + fmt.Sprintf("Failed to generate S3Instance %s, listing buckets failed, using secret %s", s3InstanceResource.Name, s3InstanceResource.Spec.SecretRef), err) } - return r.SetReconciledCondition( + return r.SetAvailableCondition( ctx, req, s3InstanceResource, s3v1alpha1.Reconciled, "S3Instance instance reconciled", - nil, ) } diff --git a/internal/controller/s3instance/reconcile_test.go b/internal/controller/s3instance/reconcile_test.go index 94ee103..a7df4bd 100644 --- a/internal/controller/s3instance/reconcile_test.go +++ b/internal/controller/s3instance/reconcile_test.go @@ -134,8 +134,10 @@ func TestHandleCreate(t *testing.T) { Namespace: "s3-operator", Name: "default", }, reconciledInstance) - - assert.Equal(t, "Reconciled", reconciledInstance.Status.Conditions[0].Reason) + // Conditions[0] = Progressing ; Conditions[1] = Reconciled + var conditions = reconciledInstance.Status.Conditions + var lastCondition = conditions[len(conditions) - 1] + assert.Equal(t, s3v1alpha1.Reconciled, lastCondition.Reason) }) t.Run("reason is creation failure because of invalid client", func(t *testing.T) { @@ -150,7 +152,9 @@ func TestHandleCreate(t *testing.T) { Namespace: "s3-operator", Name: "invalid-instance", }, reconciledInstance) - - assert.Equal(t, "CreationFailure", reconciledInstance.Status.Conditions[0].Reason) + // Conditions[0] = Progressing ; Conditions[1] = CreationFailure + var conditions = reconciledInstance.Status.Conditions + var lastCondition = conditions[len(conditions) - 1] + assert.Equal(t, s3v1alpha1.CreationFailure, lastCondition.Reason) }) } diff --git a/internal/controller/s3instance/status.go b/internal/controller/s3instance/status.go index ad82e68..a5b1560 100644 --- a/internal/controller/s3instance/status.go +++ b/internal/controller/s3instance/status.go @@ -19,12 +19,79 @@ package s3instance_controller import ( "context" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ctrl "sigs.k8s.io/controller-runtime" s3v1alpha1 "github.com/InseeFrLab/s3-operator/api/v1alpha1" ) -func (r *S3InstanceReconciler) SetReconciledCondition( +func (r *S3InstanceReconciler) SetProgressingCondition( + ctx context.Context, + req ctrl.Request, + s3InstanceResource *s3v1alpha1.S3Instance, + status metav1.ConditionStatus, + reason string, + message string, +) (ctrl.Result, error) { + return r.ControllerHelper.SetReconciledCondition( + ctx, + r.Status(), + req, + s3InstanceResource, + &s3InstanceResource.Status.Conditions, + s3v1alpha1.ConditionProgressing, + status, + reason, + message, + nil, + r.ReconcilePeriod, + ) +} +func (r *S3InstanceReconciler) SetAvailableCondition( + ctx context.Context, + req ctrl.Request, + s3InstanceResource *s3v1alpha1.S3Instance, + reason string, + message string, +) (ctrl.Result, error) { + return r.ControllerHelper.SetReconciledCondition( + ctx, + r.Status(), + req, + s3InstanceResource, + &s3InstanceResource.Status.Conditions, + s3v1alpha1.ConditionAvailable, + metav1.ConditionTrue, + reason, + message, + nil, + r.ReconcilePeriod, + ) +} +func (r *S3InstanceReconciler) SetDegradedCondition( + ctx context.Context, + req ctrl.Request, + s3InstanceResource *s3v1alpha1.S3Instance, + status metav1.ConditionStatus, + reason string, + message string, + err error, +) (ctrl.Result, error) { + return r.ControllerHelper.SetReconciledCondition( + ctx, + r.Status(), + req, + s3InstanceResource, + &s3InstanceResource.Status.Conditions, + s3v1alpha1.ConditionDegraded, + status, + reason, + message, + err, + r.ReconcilePeriod, + ) +} +func (r *S3InstanceReconciler) SetRejectedCondition( ctx context.Context, req ctrl.Request, s3InstanceResource *s3v1alpha1.S3Instance, @@ -38,7 +105,8 @@ func (r *S3InstanceReconciler) SetReconciledCondition( req, s3InstanceResource, &s3InstanceResource.Status.Conditions, - s3v1alpha1.ConditionReconciled, + s3v1alpha1.ConditionRejected, + metav1.ConditionFalse, reason, message, err, diff --git a/internal/controller/user/finalizer.go b/internal/controller/user/finalizer.go index d9389e4..9f55690 100644 --- a/internal/controller/user/finalizer.go +++ b/internal/controller/user/finalizer.go @@ -18,10 +18,11 @@ package user_controller import ( "context" + "fmt" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" - "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/reconcile" s3v1alpha1 "github.com/InseeFrLab/s3-operator/api/v1alpha1" @@ -29,9 +30,9 @@ import ( func (r *S3UserReconciler) finalizeS3User( ctx context.Context, + req reconcile.Request, userResource *s3v1alpha1.S3User, ) error { - logger := log.FromContext(ctx) s3Client, err := r.S3Instancehelper.GetS3ClientForRessource( ctx, r.Client, @@ -41,7 +42,15 @@ func (r *S3UserReconciler) finalizeS3User( userResource.Spec.S3InstanceRef, ) if err != nil { - logger.Error(err, "An error occurred while getting s3Client") + r.SetDegradedCondition( + ctx, + req, + userResource, + metav1.ConditionUnknown, + s3v1alpha1.Unreachable, + "Failed to generate s3client from instance", + err, + ) return err } if s3Client.GetConfig().S3UserDeletionEnabled { @@ -55,66 +64,43 @@ func (r *S3UserReconciler) handleDeletion( req reconcile.Request, userResource *s3v1alpha1.S3User, ) (reconcile.Result, error) { - logger := log.FromContext(ctx) if controllerutil.ContainsFinalizer(userResource, userFinalizer) { // Run finalization logic for S3UserFinalizer. If the finalization logic fails, don't remove the finalizer so that we can retry during the next reconciliation. - if err := r.finalizeS3User(ctx, userResource); err != nil { - logger.Error( - err, - "An error occurred when attempting to finalize the user", - "userResource", - userResource.Name, - "NamespacedName", - req.NamespacedName.String(), - ) - return r.SetReconciledCondition( + if err := r.finalizeS3User(ctx, req, userResource); err != nil { + return r.SetDegradedCondition( ctx, req, userResource, + metav1.ConditionFalse, s3v1alpha1.DeletionFailure, - "user deletion has failed", + fmt.Sprintf("User %s deletion has failed", userResource.Name), err, ) } userOwnedlinkedSecrets, _, err := r.getUserLinkedSecrets(ctx, userResource) if err != nil { - logger.Error( - err, - "An error occurred when trying to list secret linked to user", - "userResourceName", - userResource.Name, - "NamespacedName", - req.NamespacedName.String(), - ) - return r.SetReconciledCondition( + + return r.SetDegradedCondition( ctx, req, userResource, + metav1.ConditionFalse, s3v1alpha1.DeletionFailure, - "An error occurred when trying to list secret linked to user", + fmt.Sprintf("An error occurred when trying to list secret linked to user %s", userResource.Name), err, ) } for _, linkedSecret := range userOwnedlinkedSecrets { if err := r.deleteSecret(ctx, &linkedSecret); err != nil { - logger.Error( - err, - "An error occurred when trying to list secret linked to user", - "userResourceName", - userResource.Name, - "NamespacedName", - req.NamespacedName.String(), - "secretName", - linkedSecret.Name, - ) - return r.SetReconciledCondition( + return r.SetDegradedCondition( ctx, req, userResource, + metav1.ConditionFalse, s3v1alpha1.DeletionFailure, - "Deletion of secret associated to user have failed", + fmt.Sprintf("Deletion of secret %s associated to user %s have failed", linkedSecret.Name, userResource.Name), err, ) } @@ -123,12 +109,13 @@ func (r *S3UserReconciler) handleDeletion( //Remove userFinalizer. Once all finalizers have been removed, the object will be deleted. if ok := controllerutil.RemoveFinalizer(userResource, userFinalizer); !ok { - logger.Info( - "Failed to remove finalizer for user resource", - "userResource", - userResource.Name, - "NamespacedName", - req.NamespacedName.String(), + r.SetProgressingCondition( + ctx, + req, + userResource, + metav1.ConditionFalse, + s3v1alpha1.InternalError, + fmt.Sprintf("Failed to remove finalizer for user %s", userResource.Name), ) return ctrl.Result{Requeue: true}, nil } @@ -139,13 +126,14 @@ func (r *S3UserReconciler) handleDeletion( // "the object has been modified; please apply your changes to the latest version and try again" error) err = r.Update(ctx, userResource) if err != nil { - logger.Error( + r.SetDegradedCondition( + ctx, + req, + userResource, + metav1.ConditionFalse, + s3v1alpha1.K8sApiError, + fmt.Sprintf("An error occured when removing finalizer from user %s", userResource.Name), err, - "An error occurred when removing finalizer from policy", - "userResource", - userResource.Name, - "NamespacedName", - req.NamespacedName.String(), ) return ctrl.Result{}, err } diff --git a/internal/controller/user/reconcile.go b/internal/controller/user/reconcile.go index 7538fa1..afff965 100644 --- a/internal/controller/user/reconcile.go +++ b/internal/controller/user/reconcile.go @@ -20,11 +20,11 @@ import ( "cmp" "context" "fmt" + "strings" "slices" corev1 "k8s.io/api/core/v1" k8sapierrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" @@ -62,25 +62,13 @@ func (r *S3UserReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr // Let's just set the status as Unknown when no status are available if len(userResource.Status.Conditions) == 0 { - meta.SetStatusCondition( - &userResource.Status.Conditions, - metav1.Condition{ - Type: s3v1alpha1.ConditionReconciled, - Status: metav1.ConditionUnknown, - ObservedGeneration: userResource.Generation, - Reason: s3v1alpha1.Reconciling, - Message: "Starting reconciliation", - }, - ) - if err = r.Status().Update(ctx, userResource); err != nil { - logger.Error( - err, - "Failed to update userRessource status", - "userResourceName", - userResource.Name, - "NamespacedName", - req.NamespacedName.String(), - ) + _, err = r.SetProgressingCondition(ctx, + req, + userResource, + metav1.ConditionUnknown, + s3v1alpha1.Reconciling, + fmt.Sprintf("Newly discovered resource %s", userResource.Name)) + if err != nil { return ctrl.Result{}, err } @@ -90,47 +78,45 @@ func (r *S3UserReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr // your changes to the latest version and try again" which would re-trigger the reconciliation // if we try to update it again in the following operations if err := r.Get(ctx, req.NamespacedName, userResource); err != nil { - logger.Error( - err, - "Failed to re-fetch userResource", - "userResourceName", - userResource.Name, - "NamespacedName", - req.NamespacedName.String(), - ) + r.SetDegradedCondition(ctx, + req, + userResource, + metav1.ConditionFalse, + s3v1alpha1.K8sApiError, + fmt.Sprintf("Failed to re-fetch user resource %s", userResource.Name), + err) return ctrl.Result{}, err } } // Add finalizer for this CR if !controllerutil.ContainsFinalizer(userResource, userFinalizer) { - logger.Info("Adding finalizer to user resource", - "userResourceName", - userResource.Name, - "NamespacedName", - req.NamespacedName.String()) + r.SetProgressingCondition(ctx, + req, + userResource, + metav1.ConditionTrue, + s3v1alpha1.Reconciling, + fmt.Sprintf("Adding finalizer to user resource %s", userResource.Name)) if ok := controllerutil.AddFinalizer(userResource, userFinalizer); !ok { - logger.Error( - err, - "Failed to add finalizer into user resource", - "userResourceName", - userResource.Name, - "NamespacedName", - req.NamespacedName.String(), - ) + r.SetDegradedCondition(ctx, + req, + userResource, + metav1.ConditionFalse, + s3v1alpha1.InternalError, + fmt.Sprintf("Failed to add finalizer to user resource %s", userResource.Name), + err) return ctrl.Result{Requeue: true}, nil } if err = r.Update(ctx, userResource); err != nil { - logger.Error( - err, - "An error occurred when adding finalizer from user", - "userResourceName", - userResource.Name, - "NamespacedName", - req.NamespacedName.String(), - ) + r.SetDegradedCondition(ctx, + req, + userResource, + metav1.ConditionFalse, + s3v1alpha1.K8sApiError, + fmt.Sprintf("An error occurred when adding finalizer on user resource %s", userResource.Name), + err) return ctrl.Result{}, err } @@ -140,14 +126,13 @@ func (r *S3UserReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr // your changes to the latest version and try again" which would re-trigger the reconciliation // if we try to update it again in the following operations if err := r.Get(ctx, req.NamespacedName, userResource); err != nil { - logger.Error( - err, - "Failed to re-fetch userResource", - "userResourceName", - userResource.Name, - "NamespacedName", - req.NamespacedName.String(), - ) + r.SetDegradedCondition(ctx, + req, + userResource, + metav1.ConditionFalse, + s3v1alpha1.K8sApiError, + fmt.Sprintf("Failed to re-fetch user resource %s", userResource.Name), + err) return ctrl.Result{}, err } } @@ -155,14 +140,16 @@ func (r *S3UserReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr // Check if the userResource instance is marked to be deleted, which is // indicated by the deletion timestamp being set. The object will be deleted. if userResource.GetDeletionTimestamp() != nil { - logger.Info("userResource have been marked for deletion", - "userResource", - userResource.Name, - "NamespacedName", - req.NamespacedName.String()) + r.SetProgressingCondition(ctx, + req, + userResource, + metav1.ConditionTrue, + s3v1alpha1.Reconciling, + fmt.Sprintf("User resource have been marked for deletion %s", userResource.Name)) return r.handleDeletion(ctx, req, userResource) } + r.SetProgressingCondition(ctx, req, userResource, metav1.ConditionTrue, s3v1alpha1.Reconciling, fmt.Sprintf("Starting reconciliation of user %s", userResource.Name)) return r.handleReconciliation(ctx, req, userResource) } @@ -172,7 +159,6 @@ func (r *S3UserReconciler) handleReconciliation( req reconcile.Request, userResource *s3v1alpha1.S3User, ) (reconcile.Result, error) { - logger := log.FromContext(ctx) s3Client, err := r.S3Instancehelper.GetS3ClientForRessource( ctx, @@ -183,11 +169,11 @@ func (r *S3UserReconciler) handleReconciliation( userResource.Spec.S3InstanceRef, ) if err != nil { - logger.Error(err, "An error occurred while getting s3Client") - return r.SetReconciledCondition( + return r.SetDegradedCondition( ctx, req, userResource, + metav1.ConditionUnknown, s3v1alpha1.Unreachable, "Failed to generate s3client from instance", err, @@ -196,20 +182,13 @@ func (r *S3UserReconciler) handleReconciliation( found, err := s3Client.UserExist(userResource.Spec.AccessKey) if err != nil { - logger.Error( - err, - "An error occurred while checking the existence of a user", - "userResource", - userResource.Name, - "NamespacedName", - req.NamespacedName.String(), - ) - return r.SetReconciledCondition( + return r.SetDegradedCondition( ctx, req, userResource, + metav1.ConditionUnknown, s3v1alpha1.Unreachable, - "Fail to check existence", + fmt.Sprintf("Error while checking if user %s already exist", userResource.Name), err, ) } @@ -225,7 +204,6 @@ func (r *S3UserReconciler) handleUpdate( req reconcile.Request, userResource *s3v1alpha1.S3User, ) (reconcile.Result, error) { - logger := log.FromContext(ctx) // Create S3Client s3Client, err := r.S3Instancehelper.GetS3ClientForRessource( @@ -237,11 +215,11 @@ func (r *S3UserReconciler) handleUpdate( userResource.Spec.S3InstanceRef, ) if err != nil { - logger.Error(err, "An error occurred while getting s3Client") - return r.SetReconciledCondition( + return r.SetDegradedCondition( ctx, req, userResource, + metav1.ConditionUnknown, s3v1alpha1.Unreachable, "Failed to generate s3client from instance", err, @@ -250,65 +228,32 @@ func (r *S3UserReconciler) handleUpdate( ownedSecret := true userOwnedlinkedSecrets, userUnlinkedSecret, err := r.getUserLinkedSecrets(ctx, userResource) if err != nil { - logger.Error( - err, - "An error occurred while listing the user's secret", - "userResourceName", - userResource.Name, - "NamespacedName", - req.NamespacedName.String(), - ) - return r.SetReconciledCondition( + return r.SetDegradedCondition( ctx, req, userResource, + metav1.ConditionUnknown, s3v1alpha1.Unreachable, - "Impossible to list the user's secret", - err, - ) - } - if err != nil { - logger.Error( - err, - "An error occurred while listing the user's secret", - "userResourceName", - userResource.Name, - "NamespacedName", - req.NamespacedName.String(), - ) - return r.SetReconciledCondition( - ctx, - req, - userResource, - s3v1alpha1.Unreachable, - "Impossible to list the user's secret", + fmt.Sprintf("Impossible to list the secrets of user %s", userResource.Name), err, ) } currentUserSecret := corev1.Secret{} if len(userOwnedlinkedSecrets) == 0 && userUnlinkedSecret == nil { - logger.Info( - "No Secret associated to user found, user will be deleted from the S3 backend, then recreated with a secret", - "userResourceSpecSecretName", - userResource.Spec.SecretName, - "userResourceName", - userResource.Name, - "NamespacedName", - req.NamespacedName.String(), - ) + r.SetProgressingCondition(ctx, + req, + userResource, + metav1.ConditionTrue, + s3v1alpha1.Reconciling, + fmt.Sprintf("No Secret associated to user %s found, user will be deleted from the S3 backend, then recreated with a secret", userResource.Name)) err = s3Client.DeleteUser(userResource.Spec.AccessKey) if err != nil { - logger.Error(err, "Could not delete user on S3 server", "userResource", - userResource.Name, - "userResourceName", - userResource.Name, - "NamespacedName", - req.NamespacedName.String()) - return r.SetReconciledCondition( + return r.SetDegradedCondition( ctx, req, userResource, + metav1.ConditionFalse, s3v1alpha1.Unreachable, fmt.Sprintf( "Deletion of S3user %s on S3 server has failed", @@ -326,13 +271,13 @@ func (r *S3UserReconciler) handleUpdate( for _, linkedsecret := range userOwnedlinkedSecrets { if linkedsecret.Name != cmp.Or(userResource.Spec.SecretName, userResource.Name) { if err := r.deleteSecret(ctx, &linkedsecret); err != nil { - logger.Info("Failed to delete unused secret", "secretName", linkedsecret.Name) - return r.SetReconciledCondition( + return r.SetDegradedCondition( ctx, req, userResource, + metav1.ConditionFalse, s3v1alpha1.Unreachable, - "Failed to delete old linkedSecret", + fmt.Sprintf("Failed to delete old linkedSecret %s for user %s", linkedsecret.Name, userResource.Name), err, ) } @@ -342,26 +287,21 @@ func (r *S3UserReconciler) handleUpdate( } } if !foundSecret { - logger.Info( - "No Secret associated to user found, user will be deleted from the S3 backend, then recreated with a secret", - "userResourceName", - userResource.Name, - "NamespacedName", - req.NamespacedName.String(), - ) + r.SetProgressingCondition(ctx, + req, + userResource, + metav1.ConditionTrue, + s3v1alpha1.Reconciling, + "No Secret associated to user found, user will be deleted from the S3 backend, then recreated with a secret") + err = s3Client.DeleteUser(userResource.Spec.AccessKey) if err != nil { - logger.Error(err, "Could not delete user on S3 server", "userResource", - userResource.Name, - "userResourceName", - userResource.Name, - "NamespacedName", - req.NamespacedName.String()) - return r.SetReconciledCondition( + return r.SetDegradedCondition( ctx, req, userResource, + metav1.ConditionFalse, s3v1alpha1.Unreachable, fmt.Sprintf( "Deletion of S3user %s on S3 server has failed", @@ -373,23 +313,23 @@ func (r *S3UserReconciler) handleUpdate( return r.handleCreate(ctx, req, userResource) } } + r.SetProgressingCondition(ctx, + req, + userResource, + metav1.ConditionTrue, + s3v1alpha1.Reconciling, + fmt.Sprintf("Checking user %s policies", userResource.Name)) + - logger.Info("Checking user policies", "userResource", - userResource.Name, - "NamespacedName", - req.NamespacedName.String()) userPolicies, err := s3Client.GetUserPolicies(userResource.Spec.AccessKey) if err != nil { - logger.Error(err, "Could not check the user's policies", "userResource", - userResource.Name, - "NamespacedName", - req.NamespacedName.String()) - return r.SetReconciledCondition( + return r.SetDegradedCondition( ctx, req, userResource, + metav1.ConditionUnknown, s3v1alpha1.Unreachable, - "Checking the S3user policies has failed", + fmt.Sprintf("Checking the S3user policies has failed for user %s", userResource.Name), err, ) } @@ -399,13 +339,6 @@ func (r *S3UserReconciler) handleUpdate( for _, policy := range userPolicies { policyFound := slices.Contains(userResource.Spec.Policies, policy) if !policyFound { - logger.Info( - fmt.Sprintf("S3User has unexpected policy not in definition: %s", policy), - "userResource", - userResource.Name, - "NamespacedName", - req.NamespacedName.String(), - ) policyToDelete = append(policyToDelete, policy) } } @@ -413,60 +346,55 @@ func (r *S3UserReconciler) handleUpdate( for _, policy := range userResource.Spec.Policies { policyFound := slices.Contains(userPolicies, policy) if !policyFound { - logger.Info( - fmt.Sprintf("S3User is missing policy from definition: %s", policy), - "userResource", - userResource.Name, - "NamespacedName", - req.NamespacedName.String(), - ) policyToAdd = append(policyToAdd, policy) } } - if len(policyToDelete) > 0 { - err = s3Client.RemovePoliciesFromUser(userResource.Spec.AccessKey, policyToDelete) - if err != nil { - logger.Error( - err, - "An error occurred while removing policy to user", - "userResource", - userResource.Name, - "NamespacedName", - req.NamespacedName.String(), - ) - return r.SetReconciledCondition( - ctx, - req, - userResource, - s3v1alpha1.Unreachable, - "Error while updating policies of user", - err, - ) + if len(policyToDelete) > 0 || len(policyToAdd) > 0 { + var message string = "" + if len(policyToDelete) > 0 { + message = fmt.Sprintf("S3User %s has unexpected policy not in definition: %s", userResource.Name, strings.Join(policyToDelete, ", ")) + } + if len(policyToAdd) > 0 { + message += fmt.Sprintf("S3User %s is missing policy from definition: %s", userResource.Name, strings.Join(policyToAdd, ", ")) + } + // Update the condition (+ prints a log) before doing anything + r.SetProgressingCondition(ctx, + req, + userResource, + metav1.ConditionTrue, + s3v1alpha1.Reconciling, + message) + + if len(policyToDelete) > 0 { + err = s3Client.RemovePoliciesFromUser(userResource.Spec.AccessKey, policyToDelete) + if err != nil { + return r.SetDegradedCondition( + ctx, + req, + userResource, + metav1.ConditionFalse, + s3v1alpha1.Unreachable, + fmt.Sprintf("Error while removing policies of user %s", userResource.Name), + err, + ) + } } - } - - if len(policyToAdd) > 0 { - err := s3Client.AddPoliciesToUser(userResource.Spec.AccessKey, policyToAdd) - if err != nil { - logger.Error( - err, - "An error occurred while adding policy to user", - "userResource", - userResource.Name, - "NamespacedName", - req.NamespacedName.String(), - ) - return r.SetReconciledCondition( - ctx, - req, - userResource, - s3v1alpha1.Unreachable, - "Error while updating policies of user", - err, - ) + if len(policyToAdd) > 0 { + err := s3Client.AddPoliciesToUser(userResource.Spec.AccessKey, policyToAdd) + if err != nil { + return r.SetDegradedCondition( + ctx, + req, + userResource, + metav1.ConditionFalse, + s3v1alpha1.Unreachable, + fmt.Sprintf("Error while adding policies of user %s", userResource.Name), + err, + ) + } } } @@ -477,73 +405,56 @@ func (r *S3UserReconciler) handleUpdate( ) if err != nil { - logger.Error( - err, - "An error occurred when checking if user credentials were valid", - "userResource", - userResource.Name, - "NamespacedName", - req.NamespacedName.String(), - ) - return r.SetReconciledCondition( + return r.SetDegradedCondition( ctx, req, userResource, + metav1.ConditionFalse, s3v1alpha1.Unreachable, - "Checking credentials on S3 server has failed", + fmt.Sprintf("Checking credentials on S3 server has failed for user %s", userResource.Name), err, ) } if !credentialsValid { if ownedSecret { - logger.Info( - "The secret containing the credentials will be deleted, and the user will be deleted from the S3 backend, then recreated (through another reconcile)", - "userResource", - userResource.Name, - "NamespacedName", - req.NamespacedName.String(), - ) + r.SetProgressingCondition(ctx, + req, + userResource, + metav1.ConditionTrue, + s3v1alpha1.Reconciling, + fmt.Sprintf("The secret %s containing the credentials will be deleted, and the user %s will be deleted from the S3 backend, then recreated (through another reconcile)", currentUserSecret.Name, userResource.Name)) + err = r.deleteSecret(ctx, ¤tUserSecret) if err != nil { - logger.Error(err, "Deletion of secret associated to user have failed", "userResource", - userResource.Name, - "userResourceName", - userResource.Name, - "NamespacedName", - req.NamespacedName.String()) - return r.SetReconciledCondition( + return r.SetDegradedCondition( ctx, req, userResource, + metav1.ConditionFalse, s3v1alpha1.Unreachable, - "Deletion of secret associated to user have failed", + fmt.Sprintf("Deletion of secret associated to user %s have failed", userResource.Name), err, ) } } else { - logger.Info( - "The user will be deleted from the S3 backend, then recreated (through another reconcile), the secret will be kept.", - "userResource", - userResource.Name, - "NamespacedName", - req.NamespacedName.String(), - ) + r.SetProgressingCondition(ctx, + req, + userResource, + metav1.ConditionTrue, + s3v1alpha1.Reconciling, + fmt.Sprintf("The user %s will be deleted from the S3 backend, then recreated (through another reconcile), the secret %s will be kept.", userResource.Name, currentUserSecret.Name)) + } err = s3Client.DeleteUser(userResource.Spec.AccessKey) if err != nil { - logger.Error(err, "Could not delete user on S3 server", "userResource", - userResource.Name, - "userResourceName", - userResource.Name, - "NamespacedName", - req.NamespacedName.String()) - return r.SetReconciledCondition( + return r.SetDegradedCondition( ctx, req, userResource, + metav1.ConditionFalse, s3v1alpha1.Unreachable, fmt.Sprintf( "Deletion of S3user %s on S3 server has failed", @@ -556,20 +467,12 @@ func (r *S3UserReconciler) handleUpdate( return r.handleCreate(ctx, req, userResource) } - logger.Info("User was reconciled without error", - "userResource", - userResource.Name, - "NamespacedName", - req.NamespacedName.String(), - ) - - return r.SetReconciledCondition( + return r.SetAvailableCondition( ctx, req, userResource, s3v1alpha1.Reconciled, - "User reconciled", - err, + fmt.Sprintf("User %s reconciled", userResource.Name), ) } @@ -578,7 +481,6 @@ func (r *S3UserReconciler) handleCreate( req reconcile.Request, userResource *s3v1alpha1.S3User, ) (reconcile.Result, error) { - logger := log.FromContext(ctx) // Create S3Client s3Client, err := r.S3Instancehelper.GetS3ClientForRessource( @@ -590,12 +492,12 @@ func (r *S3UserReconciler) handleCreate( userResource.Spec.S3InstanceRef, ) if err != nil { - logger.Error(err, "An error occurred while getting s3Client") - return r.SetReconciledCondition( + return r.SetDegradedCondition( ctx, req, userResource, - s3v1alpha1.Unreachable, + metav1.ConditionUnknown, + s3v1alpha1.InternalError, "Failed to generate s3client from instance", err, ) @@ -605,19 +507,12 @@ func (r *S3UserReconciler) handleCreate( secretKey, err := r.PasswordGeneratorHelper.Generate(20, true, false, true) if err != nil { - logger.Error(err, fmt.Sprintf("Fail to generate password for user %s", userResource.Name), - "userResource", - userResource.Name, - "NamespacedName", - req.NamespacedName.String(), - ) - - return r.SetReconciledCondition( + return r.SetRejectedCondition( ctx, req, userResource, - s3v1alpha1.Unreachable, - "An error occurred when attempting to generate password for user", + s3v1alpha1.InternalError, + fmt.Sprintf("Fail to generate password for user %s", userResource.Name), err, ) } @@ -632,17 +527,11 @@ func (r *S3UserReconciler) handleCreate( ) if err != nil { // Error while creating the Kubernetes secret - requeue the request. - logger.Error(err, "Could not generate Kubernetes secret", - "userResource", - userResource.Name, - "NamespacedName", - req.NamespacedName.String(), - ) - return r.SetReconciledCondition( + return r.SetRejectedCondition( ctx, req, userResource, - s3v1alpha1.Unreachable, + s3v1alpha1.K8sApiError, "Generation of associated k8s Secret has failed", err, ) @@ -659,62 +548,45 @@ func (r *S3UserReconciler) handleCreate( // If none exist : we create the user, then the secret if err != nil && k8sapierrors.IsNotFound(err) { - logger.Info( - "No secret found ; creating a new Secret", - "Secret.Namespace", - secret.Namespace, - "Secret.Name", - secret.Name, - ) + r.SetProgressingCondition(ctx, + req, + userResource, + metav1.ConditionTrue, + s3v1alpha1.Reconciling, + fmt.Sprintf("No secret found for user %s creating a new Secret", userResource.Name)) // Creating the user err = s3Client.CreateUser(userResource.Spec.AccessKey, secretKey) if err != nil { - logger.Error( - err, - "An error occurred while creating user on S3 server", - "userResource", - userResource.Name, - "NamespacedName", - req.NamespacedName.String(), - ) - return r.SetReconciledCondition( + + return r.SetRejectedCondition( ctx, req, userResource, s3v1alpha1.Unreachable, - "Creation of user on S3 instance has failed", + fmt.Sprintf("Creation of user %s on S3 instance has failed", userResource.Name), err, ) } // Creating the secret - logger.Info( - "Creating a new secret to store the user's credentials", - "secretName", - secret.Name, - "userResource", - userResource.Name, - "NamespacedName", - req.NamespacedName.String(), - ) + r.SetProgressingCondition(ctx, + req, + userResource, + metav1.ConditionTrue, + s3v1alpha1.Reconciling, + fmt.Sprintf("Creating a new secret (%s) to store the credentials of user %s", secret.Name, userResource.Name)) + err = r.Create(ctx, secret) if err != nil { - logger.Error(err, "Could not create secret for user", - "secretName", - secret.Name, - "userResource", - userResource.Name, - "NamespacedName", - req.NamespacedName.String(), - ) - return r.SetReconciledCondition( + return r.SetDegradedCondition( ctx, req, userResource, + metav1.ConditionFalse, s3v1alpha1.Unreachable, - "Creation of secret for user has failed", + fmt.Sprintf("Creation of secret %s for user %s has failed", secret.Name, userResource.Name), err, ) } @@ -722,40 +594,32 @@ func (r *S3UserReconciler) handleCreate( // Add policies err = r.addPoliciesToUser(ctx, userResource) if err != nil { - return r.SetReconciledCondition( + return r.SetDegradedCondition( ctx, req, userResource, + metav1.ConditionFalse, s3v1alpha1.Unreachable, "Error while updating policies of user on S3 instance", err, ) } - return r.SetReconciledCondition( + return r.SetAvailableCondition( ctx, req, userResource, s3v1alpha1.Reconciled, - "User reconciled", - err, + fmt.Sprintf("User %s reconciled", userResource.Name), ) } else if err != nil { - logger.Error(err, "Couldn't check secret existence", - "secretName", - secret.Name, - "userResource", - userResource.Name, - "NamespacedName", - req.NamespacedName.String(), - ) - return r.SetReconciledCondition( + return r.SetRejectedCondition( ctx, req, userResource, - s3v1alpha1.Unreachable, - "Fail to check if an existing secret already exist", + s3v1alpha1.K8sApiError, + fmt.Sprintf("Fail to check if an existing secret %s already exist for user %s", secret.Name, userResource.Name), err, ) } else { @@ -765,17 +629,7 @@ func (r *S3UserReconciler) handleCreate( if ref.Kind == "S3User" { if ref.UID != userResource.UID { err = fmt.Errorf("The secret matching the new S3User's spec is owned by a different S3User.") - logger.Error(err, - "S3User could not be created because of existing secret", - "conflictingUser", - ref.Name, - "secretName", - secret.Name, - "userResource", - userResource.Name, - "NamespacedName", - req.NamespacedName.String()) - return r.SetReconciledCondition( + return r.SetRejectedCondition( ctx, req, userResource, @@ -790,12 +644,13 @@ func (r *S3UserReconciler) handleCreate( if r.OverrideExistingSecret || r.ReadExistingSecret { if r.ReadExistingSecret { // Case 3.2a : read existing secret instead of updating it - logger.Info(fmt.Sprintf("The secret key will be retrieved from the secret named %s.", secret.Name), "secretName", - secret.Name, - "userResource", - userResource.Name, - "NamespacedName", - req.NamespacedName.String()) + r.SetProgressingCondition(ctx, + req, + userResource, + metav1.ConditionTrue, + s3v1alpha1.Reconciling, + fmt.Sprintf("The secret key of user %s will be retrieved from the secret named %s.", userResource.Name, secret.Name)) + var cpData = *&existingK8sSecret.Data for k, v := range cpData { if k == userResource.Spec.SecretFieldNameSecretKey { @@ -804,82 +659,69 @@ func (r *S3UserReconciler) handleCreate( } } else { // Case 3.2b : they are not valid, but the operator is configured to overwrite it - logger.Info(fmt.Sprintf("A secret with the name %s already exists ; it will be overwritten because of operator configuration", secret.Name), "secretName", - secret.Name, - "userResource", - userResource.Name, - "NamespacedName", - req.NamespacedName.String()) + r.SetProgressingCondition(ctx, + req, + userResource, + metav1.ConditionTrue, + s3v1alpha1.Reconciling, + fmt.Sprintf("A secret with the name %s already exists ; it will be overwritten for user %s because of operator configuration", secret.Name, userResource.Name)) } // Creating the user err = s3Client.CreateUser(userResource.Spec.AccessKey, secretKey) if err != nil { - logger.Error( - err, - "An error occurred while creating user on S3 server", - "userResource", - userResource.Name, - "NamespacedName", - req.NamespacedName.String(), - ) - return r.SetReconciledCondition( + return r.SetRejectedCondition( ctx, req, userResource, s3v1alpha1.Unreachable, - "Creation of user on S3 instance has failed", + fmt.Sprintf("Creation of user %s on S3 instance has failed", userResource.Name), err, ) } if r.OverrideExistingSecret { // Updating the secret - logger.Info("Updating the pre-existing secret with new credentials", - "secretName", - secret.Name, - "userResource", - userResource.Name, - "NamespacedName", - req.NamespacedName.String(), - ) + r.SetProgressingCondition(ctx, + req, + userResource, + metav1.ConditionTrue, + s3v1alpha1.Reconciling, + fmt.Sprintf("Updating the pre-existing secret %s with new credentials for user %s", secret.Name, userResource.Name)) + err = r.Update(ctx, secret) if err != nil { - logger.Error(err, "Could not update secret", "secretName", - secret.Name, - "userResource", - userResource.Name, - "NamespacedName", - req.NamespacedName.String()) - return r.SetReconciledCondition( + return r.SetDegradedCondition( ctx, req, userResource, + metav1.ConditionFalse, s3v1alpha1.Unreachable, - "Update of secret have failed", + fmt.Sprintf("Update of secret %s have failed for user %s", secret.Name, userResource.Name), err, ) + } } // Add policies err = r.addPoliciesToUser(ctx, userResource) if err != nil { - return r.SetReconciledCondition( + return r.SetDegradedCondition( ctx, req, userResource, + metav1.ConditionFalse, s3v1alpha1.Unreachable, "Error while updating associated policy", err, ) } - return r.SetReconciledCondition( + return r.SetAvailableCondition( ctx, req, userResource, s3v1alpha1.Reconciled, - "User reconciled", - err, + fmt.Sprintf("User %s reconciled", userResource.Name), ) } @@ -887,20 +729,13 @@ func (r *S3UserReconciler) handleCreate( // The user will not be created, with no requeue and with two possible ways out : either toggle // OverrideExistingSecret on, or delete the S3User whose credentials are not working anyway. err = fmt.Errorf("A secret with the same name already exists ; as the operator is configured to NOT override nor read any pre-existing secrets, this user will not be created on S3 backend until spec change (to target new secret), or until the operator configuration is changed to override existing secrets") - logger.Error(err, - "S3User could not be created because of existing secret", - "secretName", - secret.Name, - "userResource", - userResource.Name, - "NamespacedName", - req.NamespacedName.String()) - return r.SetReconciledCondition( + + return r.SetRejectedCondition( ctx, req, userResource, s3v1alpha1.CreationFailure, - "Creation of user on S3 instance has failed because secret contains invalid credentials. The user's spec should be changed to target a different secret", + fmt.Sprintf("Creation of user %s on S3 instance has failed because a secret named %s already exists. The user's spec should be changed to target a different secret", userResource.Name, secret.Name), err, ) } diff --git a/internal/controller/user/reconcile_test.go b/internal/controller/user/reconcile_test.go index ff05232..0d4e8b5 100644 --- a/internal/controller/user/reconcile_test.go +++ b/internal/controller/user/reconcile_test.go @@ -128,9 +128,11 @@ func TestExistingSecret(t *testing.T) { Name: "example-user", }, s3UserResourceUpdated) assert.NoError(t, err) - assert.Equal(t, s3v1alpha1.CreationFailure, s3UserResourceUpdated.Status.Conditions[0].Reason) - assert.Equal(t, metav1.ConditionFalse, s3UserResourceUpdated.Status.Conditions[0].Status) - assert.Contains(t, s3UserResourceUpdated.Status.Conditions[0].Message, "Creation of user on S3 instance has failed because secret contains invalid credentials. The user's spec should be changed to target a different secret") + var conditions = s3UserResourceUpdated.Status.Conditions + var lastCondition = conditions[len(conditions) - 1] + assert.Equal(t, s3v1alpha1.CreationFailure, lastCondition.Reason) + assert.Equal(t, metav1.ConditionFalse, lastCondition.Status) + assert.Contains(t, lastCondition.Message, "Creation of user example-user on S3 instance has failed because a secret named example-user-secret already exists") }) reconciler.ReadExistingSecret = false @@ -155,9 +157,11 @@ func TestExistingSecret(t *testing.T) { Name: "example-user", }, s3UserResourceUpdated) assert.NoError(t, err) - assert.Equal(t, s3v1alpha1.Reconciled, s3UserResourceUpdated.Status.Conditions[0].Reason) - assert.Equal(t, metav1.ConditionTrue, s3UserResourceUpdated.Status.Conditions[0].Status) - assert.Contains(t, s3UserResourceUpdated.Status.Conditions[0].Message, "User reconciled") + var conditions = s3UserResourceUpdated.Status.Conditions + var lastCondition = conditions[len(conditions) - 1] + assert.Equal(t, s3v1alpha1.Reconciled, lastCondition.Reason) + assert.Equal(t, metav1.ConditionTrue, lastCondition.Status) + assert.Contains(t, lastCondition.Message, "User example-user reconciled") newSecret := &corev1.Secret{} @@ -190,9 +194,11 @@ func TestExistingSecret(t *testing.T) { Name: "example-user", }, s3UserResourceUpdated) assert.NoError(t, err) - assert.Equal(t, s3v1alpha1.Reconciled, s3UserResourceUpdated.Status.Conditions[0].Reason) - assert.Equal(t, metav1.ConditionTrue, s3UserResourceUpdated.Status.Conditions[0].Status) - assert.Contains(t, s3UserResourceUpdated.Status.Conditions[0].Message, "User reconciled") + var conditions = s3UserResourceUpdated.Status.Conditions + var lastCondition = conditions[len(conditions) - 1] + assert.Equal(t, s3v1alpha1.Reconciled, lastCondition.Reason) + assert.Equal(t, metav1.ConditionTrue, lastCondition.Status) + assert.Contains(t, lastCondition.Message, "User example-user reconciled") newSecret := &corev1.Secret{} err = testUtils.Client.Get(context.TODO(), client.ObjectKey{ @@ -224,9 +230,11 @@ func TestExistingSecret(t *testing.T) { Name: "example-thief-user", }, s3UserResourceUpdated) assert.NoError(t, err) - assert.Equal(t, s3v1alpha1.CreationFailure, s3UserResourceUpdated.Status.Conditions[0].Reason) - assert.Equal(t, metav1.ConditionFalse, s3UserResourceUpdated.Status.Conditions[0].Status) - assert.Contains(t, s3UserResourceUpdated.Status.Conditions[0].Message, "The secret matching the new S3User's spec is owned by a different, pre-existing S3User") + var conditions = s3UserResourceUpdated.Status.Conditions + var lastCondition = conditions[len(conditions) - 1] + assert.Equal(t, s3v1alpha1.CreationFailure, lastCondition.Reason) + assert.Equal(t, metav1.ConditionFalse, lastCondition.Status) + assert.Contains(t, lastCondition.Message, "The secret matching the new S3User's spec is owned by a different, pre-existing S3User") }) } @@ -292,9 +300,11 @@ func TestHandleCreate(t *testing.T) { Name: "example-user", }, s3UserResourceUpdated) assert.NoError(t, err) - assert.Equal(t, s3v1alpha1.Reconciled, s3UserResourceUpdated.Status.Conditions[0].Reason) - assert.Equal(t, metav1.ConditionTrue, s3UserResourceUpdated.Status.Conditions[0].Status) - assert.Contains(t, s3UserResourceUpdated.Status.Conditions[0].Message, "User reconciled") + var conditions = s3UserResourceUpdated.Status.Conditions + var lastCondition = conditions[len(conditions) - 1] + assert.Equal(t, s3v1alpha1.Reconciled, lastCondition.Reason) + assert.Equal(t, metav1.ConditionTrue, lastCondition.Status) + assert.Contains(t, lastCondition.Message, "User example-user reconciled") }) t.Run("error if using invalidS3Instance", func(t *testing.T) { @@ -390,9 +400,11 @@ func TestHandleUpdate(t *testing.T) { Name: "existing-valid-user", }, s3UserResourceUpdated) assert.NoError(t, err) - assert.Equal(t, s3v1alpha1.Reconciled, s3UserResourceUpdated.Status.Conditions[0].Reason) - assert.Equal(t, metav1.ConditionTrue, s3UserResourceUpdated.Status.Conditions[0].Status) - assert.Contains(t, s3UserResourceUpdated.Status.Conditions[0].Message, "User reconciled") + var conditions = s3UserResourceUpdated.Status.Conditions + var lastCondition = conditions[len(conditions) - 1] + assert.Equal(t, s3v1alpha1.Reconciled, lastCondition.Reason) + assert.Equal(t, metav1.ConditionTrue, lastCondition.Status) + assert.Contains(t, lastCondition.Message, "User existing-valid-user reconciled") }) }) @@ -451,9 +463,11 @@ func TestHandleUpdate(t *testing.T) { Name: "existing-valid-user", }, s3UserResourceUpdated) assert.NoError(t, err) - assert.Equal(t, s3v1alpha1.Reconciled, s3UserResourceUpdated.Status.Conditions[0].Reason) - assert.Equal(t, metav1.ConditionTrue, s3UserResourceUpdated.Status.Conditions[0].Status) - assert.Contains(t, s3UserResourceUpdated.Status.Conditions[0].Message, "User reconciled") + var conditions = s3UserResourceUpdated.Status.Conditions + var lastCondition = conditions[len(conditions) - 1] + assert.Equal(t, s3v1alpha1.Reconciled, lastCondition.Reason) + assert.Equal(t, metav1.ConditionTrue, lastCondition.Status) + assert.Contains(t, lastCondition.Message, "User existing-valid-user reconciled") }) }) diff --git a/internal/controller/user/status.go b/internal/controller/user/status.go index 990a159..0911daf 100644 --- a/internal/controller/user/status.go +++ b/internal/controller/user/status.go @@ -19,12 +19,79 @@ package user_controller import ( "context" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ctrl "sigs.k8s.io/controller-runtime" s3v1alpha1 "github.com/InseeFrLab/s3-operator/api/v1alpha1" ) -func (r *S3UserReconciler) SetReconciledCondition( +func (r *S3UserReconciler) SetProgressingCondition( + ctx context.Context, + req ctrl.Request, + userResource *s3v1alpha1.S3User, + status metav1.ConditionStatus, + reason string, + message string, +) (ctrl.Result, error) { + return r.ControllerHelper.SetReconciledCondition( + ctx, + r.Status(), + req, + userResource, + &userResource.Status.Conditions, + s3v1alpha1.ConditionProgressing, + status, + reason, + message, + nil, + r.ReconcilePeriod, + ) +} +func (r *S3UserReconciler) SetAvailableCondition( + ctx context.Context, + req ctrl.Request, + userResource *s3v1alpha1.S3User, + reason string, + message string, +) (ctrl.Result, error) { + return r.ControllerHelper.SetReconciledCondition( + ctx, + r.Status(), + req, + userResource, + &userResource.Status.Conditions, + s3v1alpha1.ConditionAvailable, + metav1.ConditionTrue, + reason, + message, + nil, + r.ReconcilePeriod, + ) +} +func (r *S3UserReconciler) SetDegradedCondition( + ctx context.Context, + req ctrl.Request, + userResource *s3v1alpha1.S3User, + status metav1.ConditionStatus, + reason string, + message string, + err error, +) (ctrl.Result, error) { + return r.ControllerHelper.SetReconciledCondition( + ctx, + r.Status(), + req, + userResource, + &userResource.Status.Conditions, + s3v1alpha1.ConditionDegraded, + status, + reason, + message, + err, + r.ReconcilePeriod, + ) +} +func (r *S3UserReconciler) SetRejectedCondition( ctx context.Context, req ctrl.Request, userResource *s3v1alpha1.S3User, @@ -38,7 +105,8 @@ func (r *S3UserReconciler) SetReconciledCondition( req, userResource, &userResource.Status.Conditions, - s3v1alpha1.ConditionReconciled, + s3v1alpha1.ConditionRejected, + metav1.ConditionFalse, reason, message, err, diff --git a/internal/helpers/S3instance_test.go b/internal/helpers/S3instance_test.go index 678416a..7de612d 100644 --- a/internal/helpers/S3instance_test.go +++ b/internal/helpers/S3instance_test.go @@ -60,7 +60,7 @@ func TestGetS3ClientForRessource(t *testing.T) { Name: "default", Namespace: "s3-operator", }, - Status: s3v1alpha1.S3InstanceStatus{Conditions: []metav1.Condition{{Reason: s3v1alpha1.Reconciled}}}, + Status: s3v1alpha1.S3InstanceStatus{Conditions: []metav1.Condition{{Type: s3v1alpha1.ConditionAvailable, Reason: s3v1alpha1.Reconciled}}}, } s3Instance_not_ready := &s3v1alpha1.S3Instance{ @@ -112,7 +112,7 @@ func TestGetS3ClientForRessource(t *testing.T) { t.Run("error because instance not ready", func(t *testing.T) { s3instanceHelper := helpers.NewS3InstanceHelper() _, err := s3instanceHelper.GetS3ClientForRessource(context.TODO(), client, testUtils.S3Factory, "bucket-example", "default", "s3-operator/not-ready") - assert.Equal(t, "S3instance is not in a ready state", err.Error()) + assert.Equal(t, "S3instance is not in a ready state: ", err.Error()) }) } diff --git a/internal/helpers/controller.go b/internal/helpers/controller.go index a243ada..80d922a 100644 --- a/internal/helpers/controller.go +++ b/internal/helpers/controller.go @@ -44,6 +44,7 @@ func (c *ControllerHelper) SetReconciledCondition( resource client.Object, // Accepts any Kubernetes object with conditions conditions *[]metav1.Condition, // Conditions field reference (must be a pointer) conditionType string, // The type of condition to set + status metav1.ConditionStatus, reason string, message string, err error, @@ -55,30 +56,21 @@ func (c *ControllerHelper) SetReconciledCondition( if err != nil { logger.Error(err, message, "NamespacedName", req.NamespacedName.String()) - changed = meta.SetStatusCondition( - conditions, - metav1.Condition{ - Type: conditionType, - Status: metav1.ConditionFalse, - ObservedGeneration: resource.GetGeneration(), - Reason: reason, - Message: fmt.Sprintf("%s: %s", message, err), - }, - ) + message = fmt.Sprintf("%s: %s", message, err) + } else { logger.Info(message, "NamespacedName", req.NamespacedName.String()) - changed = meta.SetStatusCondition( - conditions, - metav1.Condition{ - Type: conditionType, - Status: metav1.ConditionTrue, - ObservedGeneration: resource.GetGeneration(), - Reason: reason, - Message: message, - }, - ) } - + changed = meta.SetStatusCondition( + conditions, + metav1.Condition{ + Type: conditionType, + Status: status, + ObservedGeneration: resource.GetGeneration(), + Reason: reason, + Message: message, + }, + ) if changed { if errStatusUpdate := statusWriter.Update(ctx, resource); errStatusUpdate != nil { logger.Error(errStatusUpdate, "Failed to update resource status", "ObjectKind", resource.GetObjectKind(), "NamespacedName", req.NamespacedName.String()) diff --git a/internal/helpers/controller_test.go b/internal/helpers/controller_test.go index a71c4db..86b5f7b 100644 --- a/internal/helpers/controller_test.go +++ b/internal/helpers/controller_test.go @@ -70,9 +70,10 @@ func TestSetReconciledCondition(t *testing.T) { ctrl.Request{NamespacedName: types.NamespacedName{Name: s3instanceResource.Name, Namespace: s3instanceResource.Namespace}}, s3instanceResource, &s3instanceResource.Status.Conditions, + s3v1alpha1.ConditionAvailable, + metav1.ConditionTrue, s3v1alpha1.Reconciled, "s3Instance reconciled", - "s3Instance reconciled", nil, time.Duration(10), ) assert.NoError(t, err) @@ -85,7 +86,10 @@ func TestSetReconciledCondition(t *testing.T) { Name: "default", }, s3instanceResourceUpdated) assert.NoError(t, err) - assert.Equal(t, s3v1alpha1.Reconciled, s3instanceResourceUpdated.Status.Conditions[0].Type) + var conditions = s3instanceResourceUpdated.Status.Conditions + var lastCondition = conditions[len(conditions) - 1] + assert.Equal(t, s3v1alpha1.ConditionAvailable, lastCondition.Type) + assert.Equal(t, s3v1alpha1.Reconciled, lastCondition.Reason) assert.Equal(t, "s3Instance reconciled", s3instanceResourceUpdated.Status.Conditions[0].Message) }) @@ -96,9 +100,10 @@ func TestSetReconciledCondition(t *testing.T) { ctrl.Request{NamespacedName: types.NamespacedName{Name: s3instanceResource.Name, Namespace: s3instanceResource.Namespace}}, s3instanceResource, &s3instanceResource.Status.Conditions, + s3v1alpha1.ConditionDegraded, + metav1.ConditionFalse, s3v1alpha1.CreationFailure, "s3Instance reconciled", - "s3Instance reconciled", fmt.Errorf("Something wrong have happened"), time.Duration(10), ) @@ -113,7 +118,10 @@ func TestSetReconciledCondition(t *testing.T) { Name: "default", }, s3instanceResourceUpdated) assert.NoError(t, err) - assert.Equal(t, s3v1alpha1.CreationFailure, s3instanceResourceUpdated.Status.Conditions[1].Type) + var conditions = s3instanceResourceUpdated.Status.Conditions + var lastCondition = conditions[len(conditions) - 1] + assert.Equal(t, s3v1alpha1.ConditionDegraded, lastCondition.Type) + assert.Equal(t, s3v1alpha1.CreationFailure, lastCondition.Reason) assert.Contains(t, s3instanceResourceUpdated.Status.Conditions[1].Message, "Something wrong have happened") }) } diff --git a/internal/helpers/s3instance.go b/internal/helpers/s3instance.go index c74ebe7..672827f 100644 --- a/internal/helpers/s3instance.go +++ b/internal/helpers/s3instance.go @@ -69,8 +69,10 @@ func (s3InstanceHelper *S3InstanceHelper) GetS3ClientForRessource( return nil, fmt.Errorf("S3Instance %s not found", s3InstanceInfo.name) } - if s3Instance.Status.Conditions[0].Reason != s3v1alpha1.Reconciled { - return nil, fmt.Errorf("S3instance is not in a ready state") + var conditions = s3Instance.Status.Conditions + var lastCondition = conditions[len(conditions) - 1] + if lastCondition.Type != s3v1alpha1.ConditionAvailable { + return nil, fmt.Errorf("S3instance is not in a ready state: %s", lastCondition.Type) } return s3InstanceHelper.GetS3ClientFromS3Instance(ctx, client, s3factory, s3Instance) diff --git a/test/utils/testUtils.go b/test/utils/testUtils.go index b899496..c70cfb8 100644 --- a/test/utils/testUtils.go +++ b/test/utils/testUtils.go @@ -132,7 +132,7 @@ func (t *TestUtils) SetupDefaultS3instance() *s3v1alpha1.S3Instance { Name: "default", Namespace: "s3-operator", }, - Status: s3v1alpha1.S3InstanceStatus{Conditions: []metav1.Condition{{Reason: s3v1alpha1.Reconciled}}}, + Status: s3v1alpha1.S3InstanceStatus{Conditions: []metav1.Condition{{Type: s3v1alpha1.ConditionAvailable, Reason: s3v1alpha1.Reconciled}}}, } return s3Instance @@ -155,7 +155,7 @@ func (t *TestUtils) GenerateBasicS3InstanceAndSecret() (*s3v1alpha1.S3Instance, Name: "default", Namespace: "s3-operator", }, - Status: s3v1alpha1.S3InstanceStatus{Conditions: []metav1.Condition{{Reason: s3v1alpha1.Reconciled}}}, + Status: s3v1alpha1.S3InstanceStatus{Conditions: []metav1.Condition{{Type: s3v1alpha1.ConditionAvailable, Reason: s3v1alpha1.Reconciled}}}, } secretResource := &corev1.Secret{