From cc1f1626ffabaea77308a8e6721f18a1422999c8 Mon Sep 17 00:00:00 2001 From: Marcin Maciaszczyk Date: Wed, 29 Jan 2025 14:11:03 +0100 Subject: [PATCH] refactor --- .../internal/controller/catalog_controller.go | 2 +- .../controller/clusterrestore_controller.go | 2 +- go/controller/internal/controller/common.go | 10 +++------- .../controller/customstackrun_controller.go | 4 ++-- .../controller/gitrepository_controller.go | 17 +++++++---------- .../controller/globalservice_controller.go | 8 ++++---- .../infrastructurestack_controller.go | 15 ++++++--------- .../controller/managednamespace_controller.go | 4 ++-- .../controller/notificationrouter_controller.go | 2 +- .../controller/notificationsink_controller.go | 2 +- .../internal/controller/observer_controller.go | 2 +- .../internal/controller/pipeline_controller.go | 4 ++-- .../controller/servicedeployment_controller.go | 14 +++++++------- 13 files changed, 38 insertions(+), 48 deletions(-) diff --git a/go/controller/internal/controller/catalog_controller.go b/go/controller/internal/controller/catalog_controller.go index 640f596721..2fac37c460 100644 --- a/go/controller/internal/controller/catalog_controller.go +++ b/go/controller/internal/controller/catalog_controller.go @@ -108,7 +108,7 @@ func (r *CatalogReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ if err != nil { if errors.IsNotFound(err) { utils.MarkCondition(catalog.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, notFoundOrReadyErrorMessage(err)) - return RequeueAfter(requeueWaitForResources), nil + return waitForResources, nil } utils.MarkCondition(catalog.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReason, err.Error()) return ctrl.Result{}, err diff --git a/go/controller/internal/controller/clusterrestore_controller.go b/go/controller/internal/controller/clusterrestore_controller.go index a0fa219261..c2a986531f 100644 --- a/go/controller/internal/controller/clusterrestore_controller.go +++ b/go/controller/internal/controller/clusterrestore_controller.go @@ -99,7 +99,7 @@ func (r *ClusterRestoreReconciler) Reconcile(ctx context.Context, req ctrl.Reque if err != nil { if errors.IsNotFound(err) { utils.MarkCondition(restore.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, notFoundOrReadyErrorMessage(err)) - return RequeueAfter(requeueWaitForResources), nil + return waitForResources, nil } utils.MarkCondition(restore.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReason, err.Error()) return ctrl.Result{}, err diff --git a/go/controller/internal/controller/common.go b/go/controller/internal/controller/common.go index 8704ec18d6..9d858894e1 100644 --- a/go/controller/internal/controller/common.go +++ b/go/controller/internal/controller/common.go @@ -24,19 +24,15 @@ import ( ) const ( - // requeueAfter is the time between scheduled reconciles if there are no changes to the CRD. - requeueAfter = 30 * time.Second + requeueDefault = 30 * time.Second requeueWaitForResources = 5 * time.Second ) var ( - requeue = ctrl.Result{RequeueAfter: requeueAfter} + requeue = ctrl.Result{RequeueAfter: requeueDefault} + waitForResources = ctrl.Result{RequeueAfter: requeueWaitForResources} ) -func RequeueAfter(after time.Duration) ctrl.Result { - return ctrl.Result{RequeueAfter: after} -} - func ensureBindings(bindings []v1alpha1.Binding, userGroupCache cache.UserGroupCache) ([]v1alpha1.Binding, bool, error) { requeue := false for i := range bindings { diff --git a/go/controller/internal/controller/customstackrun_controller.go b/go/controller/internal/controller/customstackrun_controller.go index 786ace027f..e53a721b9d 100644 --- a/go/controller/internal/controller/customstackrun_controller.go +++ b/go/controller/internal/controller/customstackrun_controller.go @@ -110,7 +110,7 @@ func (r *CustomStackRunReconciler) Reconcile(ctx context.Context, req ctrl.Reque if err != nil { if errors.IsNotFound(err) { utils.MarkCondition(stack.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, notFoundOrReadyErrorMessage(err)) - return RequeueAfter(requeueWaitForResources), nil + return waitForResources, nil } utils.MarkCondition(stack.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, err.Error()) return ctrl.Result{}, err @@ -131,7 +131,7 @@ func (r *CustomStackRunReconciler) Reconcile(ctx context.Context, req ctrl.Reque if err != nil { if errors.IsNotFound(err) { utils.MarkCondition(stack.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, notFoundOrReadyErrorMessage(err)) - return RequeueAfter(requeueWaitForResources), nil + return waitForResources, nil } utils.MarkCondition(stack.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, err.Error()) return ctrl.Result{}, err diff --git a/go/controller/internal/controller/gitrepository_controller.go b/go/controller/internal/controller/gitrepository_controller.go index 3860e6dda0..2554762424 100644 --- a/go/controller/internal/controller/gitrepository_controller.go +++ b/go/controller/internal/controller/gitrepository_controller.go @@ -85,7 +85,7 @@ func (r *GitRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reques return ctrl.Result{}, err } if exists { - logger.Info("repository already exists in the API, running in read-only mode") + logger.V(9).Info("repository already exists, running in read-only mode", "name", repo.Name, "namespace", repo.Namespace) return r.handleExistingRepo(ctx, repo) } @@ -95,7 +95,7 @@ func (r *GitRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reques if err != nil { if errors.IsNotFound(err) { utils.MarkCondition(repo.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, notFoundOrReadyErrorMessage(err)) - return RequeueAfter(requeueWaitForResources), nil + return waitForResources, nil } utils.MarkCondition(repo.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, err.Error()) return requeue, err @@ -117,17 +117,16 @@ func (r *GitRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reques utils.MarkCondition(repo.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, err.Error()) return ctrl.Result{}, err } - logger.Info("repository created") apiRepo = resp.CreateGitRepository + logger.V(9).Info("created repository", "id", apiRepo.ID, "name", repo.Name, "namespace", repo.Namespace) } if repo.Status.HasSHA() && !repo.Status.IsSHAEqual(sha) { - _, err := r.ConsoleClient.UpdateRepository(apiRepo.ID, *attrs) - if err != nil { + if _, err := r.ConsoleClient.UpdateRepository(apiRepo.ID, *attrs); err != nil { utils.MarkCondition(repo.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, err.Error()) return ctrl.Result{}, err } - logger.Info("repository updated") + logger.V(9).Info("updated repository", "id", apiRepo.ID, "name", repo.Name, "namespace", repo.Namespace) } repo.Status.Message = apiRepo.Error @@ -191,13 +190,11 @@ func (r *GitRepositoryReconciler) getRepositoryAttributes(ctx context.Context, r if repo.Spec.CredentialsRef != nil { secret := &corev1.Secret{} name := types.NamespacedName{Name: repo.Spec.CredentialsRef.Name, Namespace: repo.Spec.CredentialsRef.Namespace} - err := r.Get(ctx, name, secret) - if err != nil { + if err := r.Get(ctx, name, secret); err != nil { return nil, err } - err = utils.TryAddOwnerRef(ctx, r.Client, repo, secret, r.Scheme) - if err != nil { + if err := utils.TryAddOwnerRef(ctx, r.Client, repo, secret, r.Scheme); err != nil { return nil, err } diff --git a/go/controller/internal/controller/globalservice_controller.go b/go/controller/internal/controller/globalservice_controller.go index 56700f81cf..0361b567a9 100644 --- a/go/controller/internal/controller/globalservice_controller.go +++ b/go/controller/internal/controller/globalservice_controller.go @@ -111,7 +111,7 @@ func (r *GlobalServiceReconciler) Reconcile(ctx context.Context, req ctrl.Reques if err != nil { if errors.IsNotFound(err) { utils.MarkCondition(globalService.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, notFoundOrReadyErrorMessage(err)) - return RequeueAfter(requeueWaitForResources), nil + return waitForResources, nil } utils.MarkCondition(globalService.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, err.Error()) return ctrl.Result{}, err @@ -121,7 +121,7 @@ func (r *GlobalServiceReconciler) Reconcile(ctx context.Context, req ctrl.Reques if err != nil { if errors.IsNotFound(err) { utils.MarkCondition(globalService.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, notFoundOrReadyErrorMessage(err)) - return RequeueAfter(requeueWaitForResources), nil + return waitForResources, nil } utils.MarkCondition(globalService.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReason, err.Error()) return ctrl.Result{}, err @@ -212,12 +212,12 @@ func (r *GlobalServiceReconciler) getService(ctx context.Context, globalService if err := r.Delete(ctx, globalService); err != nil { return nil, &ctrl.Result{}, err } - return service, lo.ToPtr(RequeueAfter(requeueWaitForResources)), nil + return service, lo.ToPtr(waitForResources), nil } if service.Status.ID == nil { logger.Info("service is not ready") - return service, lo.ToPtr(RequeueAfter(requeueWaitForResources)), nil + return service, lo.ToPtr(waitForResources), nil } return service, nil, nil diff --git a/go/controller/internal/controller/infrastructurestack_controller.go b/go/controller/internal/controller/infrastructurestack_controller.go index 8c0ae12ced..aece9743f8 100644 --- a/go/controller/internal/controller/infrastructurestack_controller.go +++ b/go/controller/internal/controller/infrastructurestack_controller.go @@ -200,7 +200,7 @@ func (r *InfrastructureStackReconciler) Reconcile(ctx context.Context, req ctrl. } utils.MarkCondition(stack.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionTrue, v1alpha1.SynchronizedConditionReason, "") - return RequeueAfter(requeueAfterInfrastructureStack), nil + return ctrl.Result{RequeueAfter: requeueAfterInfrastructureStack}, nil } func (r *InfrastructureStackReconciler) setReadyCondition(ctx context.Context, stack *v1alpha1.InfrastructureStack, exists bool) error { @@ -459,7 +459,7 @@ func (r *InfrastructureStackReconciler) handleClusterRef(ctx context.Context, st if cluster.Status.ID == nil { logger.Info("Cluster is not ready") utils.MarkCondition(stack.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReason, "cluster is not ready") - return "", lo.ToPtr(RequeueAfter(requeueWaitForResources)), nil + return "", lo.ToPtr(waitForResources), nil } return *cluster.Status.ID, nil, nil @@ -469,7 +469,6 @@ func (r *InfrastructureStackReconciler) handleClusterRef(ctx context.Context, st // ready before allowing main reconcile loop to continue. In case project ref is misconfigured, it will // return with error and block the reconcile process from continuing. func (r *InfrastructureStackReconciler) handleRepositoryRef(ctx context.Context, stack *v1alpha1.InfrastructureStack) (string, *ctrl.Result, error) { - logger := log.FromContext(ctx) repository := &v1alpha1.GitRepository{} if err := r.Get(ctx, client.ObjectKey{Name: stack.Spec.RepositoryRef.Name, Namespace: stack.Spec.RepositoryRef.Namespace}, repository); err != nil { @@ -478,15 +477,13 @@ func (r *InfrastructureStackReconciler) handleRepositoryRef(ctx context.Context, } if repository.Status.ID == nil { - logger.Info("Repository is not ready") utils.MarkCondition(stack.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReason, "repository is not ready") - return "", lo.ToPtr(RequeueAfter(requeueWaitForResources)), nil + return "", lo.ToPtr(waitForResources), nil } if repository.Status.Health == v1alpha1.GitHealthFailed { - logger.Info("Repository is not healthy") utils.MarkCondition(stack.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReason, "repository is not healthy") - return "", lo.ToPtr(RequeueAfter(requeueWaitForResources)), nil + return "", lo.ToPtr(waitForResources), nil } return *repository.Status.ID, nil, nil @@ -511,7 +508,7 @@ func (r *InfrastructureStackReconciler) handleProjectRef(ctx context.Context, st if project.Status.ID == nil { logger.Info("Project is not ready") utils.MarkCondition(stack.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReason, "project is not ready") - return nil, lo.ToPtr(RequeueAfter(requeueWaitForResources)), nil + return nil, lo.ToPtr(waitForResources), nil } if err := controllerutil.SetOwnerReference(project, stack, r.Scheme); err != nil { @@ -544,7 +541,7 @@ func (r *InfrastructureStackReconciler) handleStackDefinitionRef(ctx context.Con if stackDefinition.Status.ID == nil { logger.Info("StackDefinition is not ready") utils.MarkCondition(stack.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReason, "stack definition is not ready") - return nil, lo.ToPtr(RequeueAfter(requeueWaitForResources)), nil + return nil, lo.ToPtr(waitForResources), nil } return stackDefinition.Status.ID, nil, nil diff --git a/go/controller/internal/controller/managednamespace_controller.go b/go/controller/internal/controller/managednamespace_controller.go index ace28737db..ff7a3bf266 100644 --- a/go/controller/internal/controller/managednamespace_controller.go +++ b/go/controller/internal/controller/managednamespace_controller.go @@ -111,7 +111,7 @@ func (r *ManagedNamespaceReconciler) Reconcile(ctx context.Context, req ctrl.Req attr, err := r.getNamespaceAttributes(ctx, managedNamespace) if err != nil { if errors.IsNotFound(err) { - return RequeueAfter(requeueWaitForResources), nil + return waitForResources, nil } utils.MarkCondition(managedNamespace.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, err.Error()) return ctrl.Result{}, err @@ -132,7 +132,7 @@ func (r *ManagedNamespaceReconciler) Reconcile(ctx context.Context, req ctrl.Req if err != nil { if errors.IsNotFound(err) { utils.MarkCondition(managedNamespace.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, notFoundOrReadyErrorMessage(err)) - return RequeueAfter(requeueWaitForResources), nil + return waitForResources, nil } utils.MarkCondition(managedNamespace.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, err.Error()) return requeue, err diff --git a/go/controller/internal/controller/notificationrouter_controller.go b/go/controller/internal/controller/notificationrouter_controller.go index 95dfe21c58..b1ffb40914 100644 --- a/go/controller/internal/controller/notificationrouter_controller.go +++ b/go/controller/internal/controller/notificationrouter_controller.go @@ -126,7 +126,7 @@ func (r *NotificationRouterReconciler) Reconcile(ctx context.Context, req ctrl.R if err != nil { if errors.IsNotFound(err) { utils.MarkCondition(notificationRouter.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, notFoundOrReadyErrorMessage(err)) - return RequeueAfter(requeueWaitForResources), nil + return waitForResources, nil } utils.MarkCondition(notificationRouter.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, err.Error()) return ctrl.Result{}, err diff --git a/go/controller/internal/controller/notificationsink_controller.go b/go/controller/internal/controller/notificationsink_controller.go index 04f8cfa30f..24f60d4110 100644 --- a/go/controller/internal/controller/notificationsink_controller.go +++ b/go/controller/internal/controller/notificationsink_controller.go @@ -114,7 +114,7 @@ func (r *NotificationSinkReconciler) Reconcile(ctx context.Context, req ctrl.Req err = r.ensureNotificationSink(notificationSink) if goerrors.Is(err, operrors.ErrRetriable) { utils.MarkCondition(notificationSink.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, err.Error()) - return RequeueAfter(requeueWaitForResources), nil + return waitForResources, nil } if err != nil { utils.MarkCondition(notificationSink.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, err.Error()) diff --git a/go/controller/internal/controller/observer_controller.go b/go/controller/internal/controller/observer_controller.go index b6fd16eebe..5e59475098 100644 --- a/go/controller/internal/controller/observer_controller.go +++ b/go/controller/internal/controller/observer_controller.go @@ -105,7 +105,7 @@ func (r *ObserverReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ logger.Error(err, "unable to create or update observer") if errors.IsNotFound(err) { utils.MarkCondition(observer.SetCondition, v1alpha1.SynchronizedConditionType, metav1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, notFoundOrReadyErrorMessage(err)) - return RequeueAfter(requeueWaitForResources), nil + return waitForResources, nil } utils.MarkCondition(observer.SetCondition, v1alpha1.SynchronizedConditionType, metav1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, err.Error()) return ctrl.Result{}, err diff --git a/go/controller/internal/controller/pipeline_controller.go b/go/controller/internal/controller/pipeline_controller.go index 700e788cd1..053f01185c 100644 --- a/go/controller/internal/controller/pipeline_controller.go +++ b/go/controller/internal/controller/pipeline_controller.go @@ -108,7 +108,7 @@ func (r *PipelineReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ if project.Status.ID == nil { logger.Info("Project is not ready") utils.MarkCondition(pipeline.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReason, "project is not ready") - return RequeueAfter(requeueWaitForResources), nil + return waitForResources, nil } if err := controllerutil.SetOwnerReference(project, pipeline, r.Scheme); err != nil { @@ -125,7 +125,7 @@ func (r *PipelineReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ } if apierrors.IsNotFound(err) { utils.MarkCondition(pipeline.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, notFoundOrReadyErrorMessage(err)) - return RequeueAfter(requeueWaitForResources), nil + return waitForResources, nil } utils.MarkCondition(pipeline.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, err.Error()) return ctrl.Result{}, err diff --git a/go/controller/internal/controller/servicedeployment_controller.go b/go/controller/internal/controller/servicedeployment_controller.go index 49f0f583a5..1edadfb2ba 100644 --- a/go/controller/internal/controller/servicedeployment_controller.go +++ b/go/controller/internal/controller/servicedeployment_controller.go @@ -95,14 +95,14 @@ func (r *ServiceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ if cluster.Status.ID == nil { logger.Info("Cluster is not ready") utils.MarkCondition(service.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReason, "cluster is not ready") - return RequeueAfter(requeueWaitForResources), nil + return waitForResources, nil } repository := &v1alpha1.GitRepository{} if service.Spec.RepositoryRef != nil { if err := r.Get(ctx, client.ObjectKey{Name: service.Spec.RepositoryRef.Name, Namespace: service.Spec.RepositoryRef.Namespace}, repository); err != nil { utils.MarkCondition(service.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, err.Error()) - return RequeueAfter(requeueWaitForResources), err + return waitForResources, err } if !repository.DeletionTimestamp.IsZero() { logger.Info("deleting service after repository deletion") @@ -110,25 +110,25 @@ func (r *ServiceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ utils.MarkCondition(service.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, err.Error()) return ctrl.Result{}, err } - return RequeueAfter(requeueWaitForResources), nil + return waitForResources, nil } if repository.Status.ID == nil { logger.Info("Repository is not ready") utils.MarkCondition(service.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReason, "repository is not ready") - return RequeueAfter(requeueWaitForResources), nil + return waitForResources, nil } if repository.Status.Health == v1alpha1.GitHealthFailed { logger.Info("Repository is not healthy") utils.MarkCondition(service.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReason, "repository is not healthy") - return RequeueAfter(requeueWaitForResources), nil + return waitForResources, nil } } err = r.ensureService(service) if goerrors.Is(err, operrors.ErrRetriable) { utils.MarkCondition(service.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, err.Error()) - return RequeueAfter(requeueWaitForResources), nil + return waitForResources, nil } if err != nil { @@ -208,7 +208,7 @@ func (r *ServiceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ updateStatus(service, existingService, sha) if !isServiceReady(service.Status.Components) { - return RequeueAfter(requeueWaitForResources), nil + return waitForResources, nil } utils.MarkCondition(service.SetCondition, v1alpha1.ReadyConditionType, v1.ConditionTrue, v1alpha1.ReadyConditionReason, "")