From f941814af655d2c508dcd7248e4737552b4a8b3b Mon Sep 17 00:00:00 2001 From: Marcin Maciaszczyk Date: Mon, 10 Feb 2025 16:05:36 +0100 Subject: [PATCH] enhancement: Console controller error handling improvements (#1817) --- ...deployments.plural.sh_bootstraptokens.yaml | 4 +- .../api/v1alpha1/bootstraptoken_types.go | 2 +- ...deployments.plural.sh_bootstraptokens.yaml | 4 +- go/controller/config/samples/catalog.yaml | 52 ++++ .../samples/deployments_v1alpha1_catalog.yaml | 290 ------------------ ...dnamespace.yaml => managed_namespace.yaml} | 0 go/controller/docs/api.md | 2 +- .../controller/bootstraptoken_controller.go | 73 ++--- .../internal/controller/catalog_controller.go | 31 +- .../internal/controller/cluster_controller.go | 20 +- .../controller/clusterrestore_controller.go | 2 +- go/controller/internal/controller/common.go | 37 ++- .../controller/customstackrun_controller.go | 5 +- .../deploymentsettings_controller.go | 1 - .../controller/generatedsecret_controller.go | 2 - .../controller/gitrepository_controller.go | 90 +++--- .../controller/globalservice_controller.go | 82 +++-- .../controller/helmrepository_controller.go | 1 - .../infrastructurestack_controller.go | 18 +- .../controller/managednamespace_controller.go | 109 +++---- .../notificationrouter_controller.go | 3 +- .../controller/notificationsink_controller.go | 5 +- .../observabilityprovider_controller.go | 6 - .../controller/observer_controller.go | 8 +- .../controller/oidcprovider_controller.go | 6 - .../controller/pipeline_controller.go | 4 +- .../controller/prautomation_controller.go | 5 - .../internal/controller/project_controller.go | 1 - .../controller/provider_controller.go | 5 - .../controller/scmconnection_controller.go | 7 - go/controller/internal/controller/scope.go | 8 +- .../controller/service_account_controller.go | 8 +- .../servicedeployment_controller.go | 27 +- .../controller/stackdefinition_controller.go | 7 - .../internal/test/mocks/ConsoleClient_mock.go | 2 +- ...deployments.plural.sh_bootstraptokens.yaml | 4 +- 36 files changed, 290 insertions(+), 641 deletions(-) create mode 100644 go/controller/config/samples/catalog.yaml delete mode 100644 go/controller/config/samples/deployments_v1alpha1_catalog.yaml rename go/controller/config/samples/{deployments_v1alpha1_managednamespace.yaml => managed_namespace.yaml} (100%) diff --git a/charts/controller/crds/deployments.plural.sh_bootstraptokens.yaml b/charts/controller/crds/deployments.plural.sh_bootstraptokens.yaml index 88c2228a78..22d1a49b0c 100644 --- a/charts/controller/crds/deployments.plural.sh_bootstraptokens.yaml +++ b/charts/controller/crds/deployments.plural.sh_bootstraptokens.yaml @@ -45,8 +45,8 @@ spec: description: BootstrapTokenSpec defines the desired state of BootstrapToken properties: projectRef: - description: ProjectRef is the optional project that all clusters - spawned by generated bootstrap token will belong to + description: ProjectRef is the project that all clusters spawned by + generated bootstrap token will belong to properties: apiVersion: description: API version of the referent. diff --git a/go/controller/api/v1alpha1/bootstraptoken_types.go b/go/controller/api/v1alpha1/bootstraptoken_types.go index b5cf1e4186..25fd208dba 100644 --- a/go/controller/api/v1alpha1/bootstraptoken_types.go +++ b/go/controller/api/v1alpha1/bootstraptoken_types.go @@ -38,7 +38,7 @@ type BootstrapTokenSpec struct { // +kubebuilder:validation:Optional User *string `json:"user,omitempty"` - // ProjectRef is the optional project that all clusters spawned by generated bootstrap token will belong to + // ProjectRef is the project that all clusters spawned by generated bootstrap token will belong to // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Project is immutable" // +kubebuilder:validation:Required ProjectRef v1.ObjectReference `json:"projectRef,omitempty"` diff --git a/go/controller/config/crd/bases/deployments.plural.sh_bootstraptokens.yaml b/go/controller/config/crd/bases/deployments.plural.sh_bootstraptokens.yaml index 88c2228a78..22d1a49b0c 100644 --- a/go/controller/config/crd/bases/deployments.plural.sh_bootstraptokens.yaml +++ b/go/controller/config/crd/bases/deployments.plural.sh_bootstraptokens.yaml @@ -45,8 +45,8 @@ spec: description: BootstrapTokenSpec defines the desired state of BootstrapToken properties: projectRef: - description: ProjectRef is the optional project that all clusters - spawned by generated bootstrap token will belong to + description: ProjectRef is the project that all clusters spawned by + generated bootstrap token will belong to properties: apiVersion: description: API version of the referent. diff --git a/go/controller/config/samples/catalog.yaml b/go/controller/config/samples/catalog.yaml new file mode 100644 index 0000000000..1e29ced24f --- /dev/null +++ b/go/controller/config/samples/catalog.yaml @@ -0,0 +1,52 @@ +apiVersion: deployments.plural.sh/v1alpha1 +kind: Project +metadata: + name: default +spec: + name: default +--- +apiVersion: deployments.plural.sh/v1alpha1 +kind: Catalog +metadata: + labels: + app.kubernetes.io/name: base-catalog + app.kubernetes.io/instance: catalog-sample + app.kubernetes.io/part-of: controller + app.kubernetes.io/managed-by: kustomize + app.kubernetes.io/created-by: controller + name: base-catalog +spec: + author: Plural + category: Data + description: The new open-source standard to sync data from applications, APIs & databases. One click deploys for data scientists and developers. + projectRef: + name: default +--- +apiVersion: deployments.plural.sh/v1alpha1 +kind: Catalog +metadata: + labels: + app.kubernetes.io/name: data-catalog + app.kubernetes.io/instance: catalog-sample + app.kubernetes.io/part-of: controller + app.kubernetes.io/managed-by: kustomize + app.kubernetes.io/created-by: controller + name: data-catalog +spec: + author: Plural + category: Data + description: The new open-source standard to sync data from applications, APIs & databases. One click deploys for data scientists and developers. +--- +apiVersion: deployments.plural.sh/v1alpha1 +kind: Catalog +metadata: + labels: + app.kubernetes.io/name: uncategorized-catalog + app.kubernetes.io/instance: catalog-sample + app.kubernetes.io/part-of: controller + app.kubernetes.io/managed-by: kustomize + app.kubernetes.io/created-by: controller + name: uncategorized-catalog +spec: + author: Plural + description: The new open-source standard to sync data from applications, APIs & databases. One click deploys for data scientists and developers. diff --git a/go/controller/config/samples/deployments_v1alpha1_catalog.yaml b/go/controller/config/samples/deployments_v1alpha1_catalog.yaml deleted file mode 100644 index 4fb98aa037..0000000000 --- a/go/controller/config/samples/deployments_v1alpha1_catalog.yaml +++ /dev/null @@ -1,290 +0,0 @@ -apiVersion: deployments.plural.sh/v1alpha1 -kind: Catalog -metadata: - labels: - app.kubernetes.io/name: base-catalog - app.kubernetes.io/instance: catalog-sample - app.kubernetes.io/part-of: controller - app.kubernetes.io/managed-by: kustomize - app.kubernetes.io/created-by: controller - name: base-catalog -spec: - author: Plural - category: Data - description: The new open-source standard to sync data from applications, APIs & databases. One click deploys for data scientists and developers. ---- -apiVersion: deployments.plural.sh/v1alpha1 -kind: Catalog -metadata: - labels: - app.kubernetes.io/name: data-catalog - app.kubernetes.io/instance: catalog-sample - app.kubernetes.io/part-of: controller - app.kubernetes.io/managed-by: kustomize - app.kubernetes.io/created-by: controller - name: data-catalog -spec: - author: Plural - category: Data - description: The new open-source standard to sync data from applications, APIs & databases. One click deploys for data scientists and developers. ---- -apiVersion: deployments.plural.sh/v1alpha1 -kind: Catalog -metadata: - labels: - app.kubernetes.io/name: uncategorized-catalog - app.kubernetes.io/instance: catalog-sample - app.kubernetes.io/part-of: controller - app.kubernetes.io/managed-by: kustomize - app.kubernetes.io/created-by: controller - name: uncategorized-catalog -spec: - author: Plural - description: The new open-source standard to sync data from applications, APIs & databases. One click deploys for data scientists and developers. ---- -apiVersion: deployments.plural.sh/v1alpha1 -kind: Catalog -metadata: - labels: - app.kubernetes.io/name: catalog-a - app.kubernetes.io/instance: catalog-sample - app.kubernetes.io/part-of: controller - app.kubernetes.io/managed-by: kustomize - app.kubernetes.io/created-by: controller - name: catalog-a -spec: - author: test@plural.sh ---- -apiVersion: deployments.plural.sh/v1alpha1 -kind: Catalog -metadata: - labels: - app.kubernetes.io/name: catalog-b - app.kubernetes.io/instance: catalog-sample - app.kubernetes.io/part-of: controller - app.kubernetes.io/managed-by: kustomize - app.kubernetes.io/created-by: controller - name: catalog-b -spec: - author: test@plural.sh ---- -apiVersion: deployments.plural.sh/v1alpha1 -kind: Catalog -metadata: - labels: - app.kubernetes.io/name: catalog-c - app.kubernetes.io/instance: catalog-sample - app.kubernetes.io/part-of: controller - app.kubernetes.io/managed-by: kustomize - app.kubernetes.io/created-by: controller - name: catalog-c -spec: - author: test@plural.sh ---- -apiVersion: deployments.plural.sh/v1alpha1 -kind: Catalog -metadata: - labels: - app.kubernetes.io/name: catalog-d - app.kubernetes.io/instance: catalog-sample - app.kubernetes.io/part-of: controller - app.kubernetes.io/managed-by: kustomize - app.kubernetes.io/created-by: controller - name: catalog-d -spec: - author: test@plural.sh ---- -apiVersion: deployments.plural.sh/v1alpha1 -kind: Catalog -metadata: - labels: - app.kubernetes.io/name: catalog-e - app.kubernetes.io/instance: catalog-sample - app.kubernetes.io/part-of: controller - app.kubernetes.io/managed-by: kustomize - app.kubernetes.io/created-by: controller - name: catalog-e -spec: - author: test@plural.sh ---- -apiVersion: deployments.plural.sh/v1alpha1 -kind: Catalog -metadata: - labels: - app.kubernetes.io/name: catalog-f - app.kubernetes.io/instance: catalog-sample - app.kubernetes.io/part-of: controller - app.kubernetes.io/managed-by: kustomize - app.kubernetes.io/created-by: controller - name: catalog-f -spec: - author: test@plural.sh ---- -apiVersion: deployments.plural.sh/v1alpha1 -kind: Catalog -metadata: - labels: - app.kubernetes.io/name: catalog-g - app.kubernetes.io/instance: catalog-sample - app.kubernetes.io/part-of: controller - app.kubernetes.io/managed-by: kustomize - app.kubernetes.io/created-by: controller - name: catalog-g -spec: - author: test@plural.sh ---- -apiVersion: deployments.plural.sh/v1alpha1 -kind: Catalog -metadata: - labels: - app.kubernetes.io/name: catalog-h - app.kubernetes.io/instance: catalog-sample - app.kubernetes.io/part-of: controller - app.kubernetes.io/managed-by: kustomize - app.kubernetes.io/created-by: controller - name: catalog-h -spec: - author: test@plural.sh ---- -apiVersion: deployments.plural.sh/v1alpha1 -kind: Catalog -metadata: - labels: - app.kubernetes.io/name: catalog-i - app.kubernetes.io/instance: catalog-sample - app.kubernetes.io/part-of: controller - app.kubernetes.io/managed-by: kustomize - app.kubernetes.io/created-by: controller - name: catalog-i -spec: - author: test@plural.sh ---- -apiVersion: deployments.plural.sh/v1alpha1 -kind: Catalog -metadata: - labels: - app.kubernetes.io/name: catalog-j - app.kubernetes.io/instance: catalog-sample - app.kubernetes.io/part-of: controller - app.kubernetes.io/managed-by: kustomize - app.kubernetes.io/created-by: controller - name: catalog-j -spec: - author: test@plural.sh ---- -apiVersion: deployments.plural.sh/v1alpha1 -kind: Catalog -metadata: - labels: - app.kubernetes.io/name: catalog-k - app.kubernetes.io/instance: catalog-sample - app.kubernetes.io/part-of: controller - app.kubernetes.io/managed-by: kustomize - app.kubernetes.io/created-by: controller - name: catalog-k -spec: - author: test@plural.sh ---- -apiVersion: deployments.plural.sh/v1alpha1 -kind: Catalog -metadata: - labels: - app.kubernetes.io/name: catalog-l - app.kubernetes.io/instance: catalog-sample - app.kubernetes.io/part-of: controller - app.kubernetes.io/managed-by: kustomize - app.kubernetes.io/created-by: controller - name: catalog-l -spec: - author: test@plural.sh ---- -apiVersion: deployments.plural.sh/v1alpha1 -kind: Catalog -metadata: - labels: - app.kubernetes.io/name: catalog-m - app.kubernetes.io/instance: catalog-sample - app.kubernetes.io/part-of: controller - app.kubernetes.io/managed-by: kustomize - app.kubernetes.io/created-by: controller - name: catalog-m -spec: - author: test@plural.sh ---- -apiVersion: deployments.plural.sh/v1alpha1 -kind: Catalog -metadata: - labels: - app.kubernetes.io/name: catalog-n - app.kubernetes.io/instance: catalog-sample - app.kubernetes.io/part-of: controller - app.kubernetes.io/managed-by: kustomize - app.kubernetes.io/created-by: controller - name: catalog-n -spec: - author: test@plural.sh ---- -apiVersion: deployments.plural.sh/v1alpha1 -kind: Catalog -metadata: - labels: - app.kubernetes.io/name: catalog-o - app.kubernetes.io/instance: catalog-sample - app.kubernetes.io/part-of: controller - app.kubernetes.io/managed-by: kustomize - app.kubernetes.io/created-by: controller - name: catalog-o -spec: - author: test@plural.sh ---- -apiVersion: deployments.plural.sh/v1alpha1 -kind: Catalog -metadata: - labels: - app.kubernetes.io/name: catalog-p - app.kubernetes.io/instance: catalog-sample - app.kubernetes.io/part-of: controller - app.kubernetes.io/managed-by: kustomize - app.kubernetes.io/created-by: controller - name: catalog-p -spec: - author: test@plural.sh ---- -apiVersion: deployments.plural.sh/v1alpha1 -kind: Catalog -metadata: - labels: - app.kubernetes.io/name: catalog-q - app.kubernetes.io/instance: catalog-sample - app.kubernetes.io/part-of: controller - app.kubernetes.io/managed-by: kustomize - app.kubernetes.io/created-by: controller - name: catalog-q -spec: - author: test@plural.sh ---- -apiVersion: deployments.plural.sh/v1alpha1 -kind: Catalog -metadata: - labels: - app.kubernetes.io/name: catalog-r - app.kubernetes.io/instance: catalog-sample - app.kubernetes.io/part-of: controller - app.kubernetes.io/managed-by: kustomize - app.kubernetes.io/created-by: controller - name: catalog-r -spec: - author: test@plural.sh ---- -apiVersion: deployments.plural.sh/v1alpha1 -kind: Catalog -metadata: - labels: - app.kubernetes.io/name: catalog-s - app.kubernetes.io/instance: catalog-sample - app.kubernetes.io/part-of: controller - app.kubernetes.io/managed-by: kustomize - app.kubernetes.io/created-by: controller - name: catalog-s -spec: - author: test@plural.sh \ No newline at end of file diff --git a/go/controller/config/samples/deployments_v1alpha1_managednamespace.yaml b/go/controller/config/samples/managed_namespace.yaml similarity index 100% rename from go/controller/config/samples/deployments_v1alpha1_managednamespace.yaml rename to go/controller/config/samples/managed_namespace.yaml diff --git a/go/controller/docs/api.md b/go/controller/docs/api.md index 7bc572e198..67c8b19184 100644 --- a/go/controller/docs/api.md +++ b/go/controller/docs/api.md @@ -205,7 +205,7 @@ _Appears in:_ | Field | Description | Default | Validation | | --- | --- | --- | --- | | `user` _string_ | User is an optional email to the user identity for this bootstrap token in audit logs | | Optional: {}
| -| `projectRef` _[ObjectReference](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.29/#objectreference-v1-core)_ | ProjectRef is the optional project that all clusters spawned by generated bootstrap token will belong to | | Required: {}
| +| `projectRef` _[ObjectReference](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.29/#objectreference-v1-core)_ | ProjectRef is the project that all clusters spawned by generated bootstrap token will belong to | | Required: {}
| | `tokenSecretRef` _[SecretReference](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.29/#secretreference-v1-core)_ | TokenSecretRef points to an output secret where bootstrap token will be stored.
It will be created automatically in the same namespace as BootstrapToken and cannot exist. | | Required: {}
| diff --git a/go/controller/internal/controller/bootstraptoken_controller.go b/go/controller/internal/controller/bootstraptoken_controller.go index 7245424faf..88d743bdc6 100644 --- a/go/controller/internal/controller/bootstraptoken_controller.go +++ b/go/controller/internal/controller/bootstraptoken_controller.go @@ -8,10 +8,8 @@ import ( "github.com/samber/lo" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" - apierrors "k8s.io/apimachinery/pkg/api/errors" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/schema" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -51,8 +49,6 @@ type BootstrapTokenReconciler struct { // For more details, check Reconcile and its Result here: // - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.16.3/pkg/reconcile func (in *BootstrapTokenReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Result, retErr error) { - logger := log.FromContext(ctx) - bootstrapToken := new(v1alpha1.BootstrapToken) if err := in.Get(ctx, req.NamespacedName, bootstrapToken); err != nil { return ctrl.Result{}, client.IgnoreNotFound(err) @@ -63,7 +59,6 @@ func (in *BootstrapTokenReconciler) Reconcile(ctx context.Context, req ctrl.Requ scope, err := NewDefaultScope(ctx, in.Client, bootstrapToken) if err != nil { - logger.Error(err, "failed to create scope") utils.MarkCondition(bootstrapToken.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, err.Error()) return ctrl.Result{}, err } @@ -76,8 +71,7 @@ func (in *BootstrapTokenReconciler) Reconcile(ctx context.Context, req ctrl.Requ }() // Handle proper resource deletion via finalizer - result := in.addOrRemoveFinalizer(ctx, bootstrapToken) - if result != nil { + if result := in.addOrRemoveFinalizer(ctx, bootstrapToken); result != nil { return *result, nil } @@ -88,8 +82,13 @@ func (in *BootstrapTokenReconciler) Reconcile(ctx context.Context, req ctrl.Requ return ctrl.Result{}, nil } + project, result, err := in.getProject(ctx, bootstrapToken) + if result != nil || err != nil { + return handleRequeue(result, err, bootstrapToken.SetCondition) + } + // Create token and generate secret - apiBootstrapToken, err := in.sync(ctx, bootstrapToken) + apiBootstrapToken, err := in.sync(ctx, bootstrapToken, *project) if err != nil { if goerrors.Is(err, operrors.ErrRetriable) { utils.MarkCondition(bootstrapToken.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, err.Error()) @@ -107,6 +106,27 @@ func (in *BootstrapTokenReconciler) Reconcile(ctx context.Context, req ctrl.Requ return ctrl.Result{}, nil } +func (in *BootstrapTokenReconciler) getProject(ctx context.Context, bootstrapToken *v1alpha1.BootstrapToken) (*v1alpha1.Project, *ctrl.Result, error) { + project := &v1alpha1.Project{} + if err := in.Get(ctx, client.ObjectKey{Name: bootstrapToken.Spec.ProjectRef.Name}, project); err != nil { + if errors.IsNotFound(err) { + return nil, &waitForResources, err + } + + return nil, nil, err + } + + if project.Status.ID == nil { + return nil, &waitForResources, fmt.Errorf("project is not ready yet") + } + + if err := controllerutil.SetOwnerReference(project, bootstrapToken, in.Scheme); err != nil { + return nil, nil, fmt.Errorf("could not set owner reference: %+v", err) + } + + return project, nil, nil +} + func (in *BootstrapTokenReconciler) addOrRemoveFinalizer(ctx context.Context, bootstrapToken *v1alpha1.BootstrapToken) *ctrl.Result { // If object is not being deleted and if it does not have our finalizer, // then lets add the finalizer. This is equivalent to registering our finalizer. @@ -140,29 +160,9 @@ func (in *BootstrapTokenReconciler) addOrRemoveFinalizer(ctx context.Context, bo return &ctrl.Result{} } -func (in *BootstrapTokenReconciler) getProject(ctx context.Context, bootstrapToken *v1alpha1.BootstrapToken) (*v1alpha1.Project, error) { - logger := log.FromContext(ctx) - project := &v1alpha1.Project{} - if err := in.Get(ctx, client.ObjectKey{Name: bootstrapToken.Spec.ProjectRef.Name}, project); err != nil { - return project, err - } - - if project.Status.ID == nil { - logger.Info("Project is not ready") - return project, apierrors.NewNotFound(schema.GroupResource{Resource: "Project", Group: "deployments.plural.sh"}, bootstrapToken.Spec.ProjectRef.Name) - } +func (in *BootstrapTokenReconciler) sync(ctx context.Context, bootstrapToken *v1alpha1.BootstrapToken, project v1alpha1.Project) (*consoleapi.BootstrapTokenBase, error) { + attributes := consoleapi.BootstrapTokenAttributes{ProjectID: lo.FromPtr(project.ConsoleID())} - if err := controllerutil.SetOwnerReference(project, bootstrapToken, in.Scheme); err != nil { - return project, fmt.Errorf("could not set bootstrapToken owner reference, got error: %+v", err) - } - - return project, nil -} - -func (in *BootstrapTokenReconciler) sync(ctx context.Context, bootstrapToken *v1alpha1.BootstrapToken) (*consoleapi.BootstrapTokenBase, error) { - attributes := consoleapi.BootstrapTokenAttributes{} - - // Configure optional user binding if !lo.IsEmpty(bootstrapToken.Spec.User) { userID, err := in.UserGroupCache.GetUserID(*bootstrapToken.Spec.User) if errors.IsNotFound(err) { @@ -176,19 +176,6 @@ func (in *BootstrapTokenReconciler) sync(ctx context.Context, bootstrapToken *v1 attributes.UserID = lo.ToPtr(userID) } - // Configure required project binding - project, err := in.getProject(ctx, bootstrapToken) - if errors.IsNotFound(err) { - return nil, operrors.ErrRetriable - } - - if err != nil { - return nil, err - } - - attributes.ProjectID = lo.FromPtr(project.ConsoleID()) - - // Create the token apiBootstrapToken, err := in.ConsoleClient.CreateBootstrapToken(ctx, attributes) if err != nil { return nil, err diff --git a/go/controller/internal/controller/catalog_controller.go b/go/controller/internal/controller/catalog_controller.go index bff54100cf..99f5543a27 100644 --- a/go/controller/internal/controller/catalog_controller.go +++ b/go/controller/internal/controller/catalog_controller.go @@ -7,10 +7,8 @@ import ( "github.com/samber/lo" "k8s.io/apimachinery/pkg/api/errors" - apierrors "k8s.io/apimachinery/pkg/api/errors" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/schema" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -57,7 +55,6 @@ func (r *CatalogReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ scope, err := NewDefaultScope(ctx, r.Client, catalog) if err != nil { - logger.Error(err, "failed to create scope") utils.MarkCondition(catalog.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, err.Error()) return ctrl.Result{}, err } @@ -106,15 +103,11 @@ func (r *CatalogReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ return ctrl.Result{}, err } if changed { - project, err := r.getProject(ctx, catalog) - if err != nil { - if errors.IsNotFound(err) { - utils.MarkCondition(catalog.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, notFoundOrReadyErrorMessage(err)) - return RequeueAfter(requeueWaitForResources), nil - } - utils.MarkCondition(catalog.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReason, err.Error()) - return ctrl.Result{}, err + project, res, err := r.getProject(ctx, catalog) + if res != nil || err != nil { + return handleRequeue(res, err, catalog.SetCondition) } + apiCatalog, err := r.ConsoleClient.UpsertCatalog(ctx, catalog.Attributes(project.Status.ID)) if err != nil { logger.Error(err, "unable to create or update catalog") @@ -130,25 +123,27 @@ func (r *CatalogReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ return ctrl.Result{}, nil } -func (r *CatalogReconciler) getProject(ctx context.Context, catalog *v1alpha1.Catalog) (*v1alpha1.Project, error) { - logger := log.FromContext(ctx) +func (r *CatalogReconciler) getProject(ctx context.Context, catalog *v1alpha1.Catalog) (*v1alpha1.Project, *ctrl.Result, error) { project := &v1alpha1.Project{} if catalog.Spec.ProjectRef != nil { if err := r.Get(ctx, client.ObjectKey{Name: catalog.Spec.ProjectRef.Name}, project); err != nil { - return project, err + if errors.IsNotFound(err) { + return nil, &waitForResources, err + } + + return nil, nil, err } if project.Status.ID == nil { - logger.Info("Project is not ready") - return project, apierrors.NewNotFound(schema.GroupResource{Resource: "Project", Group: "deployments.plural.sh"}, catalog.Spec.ProjectRef.Name) + return nil, &waitForResources, fmt.Errorf("project is not ready yet") } if err := controllerutil.SetOwnerReference(project, catalog, r.Scheme); err != nil { - return project, fmt.Errorf("could not set catalog owner reference, got error: %+v", err) + return nil, nil, fmt.Errorf("could not set owner reference: %+v", err) } } - return project, nil + return project, nil, nil } func (r *CatalogReconciler) handleExistingResource(ctx context.Context, catalog *v1alpha1.Catalog) (ctrl.Result, error) { diff --git a/go/controller/internal/controller/cluster_controller.go b/go/controller/internal/controller/cluster_controller.go index 6841761348..14b4912dfe 100644 --- a/go/controller/internal/controller/cluster_controller.go +++ b/go/controller/internal/controller/cluster_controller.go @@ -116,10 +116,9 @@ func (r *ClusterReconciler) Reconcile(ctx context.Context, req reconcile.Request } // Get Project ID from the reference if it is set and ensure that controller reference is set properly. - projectId, result, err := r.getProjectIdAndSetOwnerRef(ctx, cluster) - if result != nil || err != nil { - utils.MarkCondition(cluster.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, "") - return *result, err + projectId, res, err := r.getProjectIdAndSetOwnerRef(ctx, cluster) + if res != nil || err != nil { + return handleRequeue(res, err, cluster.SetCondition) } // Calculate SHA to detect changes that should be applied in the Console API. @@ -308,24 +307,25 @@ func (r *ClusterReconciler) getProviderIdAndSetControllerRef(ctx context.Context } func (r *ClusterReconciler) getProjectIdAndSetOwnerRef(ctx context.Context, cluster *v1alpha1.Cluster) (*string, *ctrl.Result, error) { - logger := log.FromContext(ctx) - if !cluster.Spec.HasProjectRef() { return nil, nil, nil } project := &v1alpha1.Project{} if err := r.Get(ctx, types.NamespacedName{Name: cluster.Spec.ProjectRef.Name}, project); err != nil { - return nil, &ctrl.Result{}, fmt.Errorf("could not get project: %+v", err) + if errors.IsNotFound(err) { + return nil, &waitForResources, err + } + + return nil, nil, err } if !project.Status.HasID() { - logger.Info("project does not have ID set yet") - return nil, &requeue, nil + return nil, &waitForResources, fmt.Errorf("project is not ready yet") } if err := controllerutil.SetOwnerReference(project, cluster, r.Scheme); err != nil { - return nil, &ctrl.Result{}, fmt.Errorf("could not set cluster owner reference, got error: %+v", err) + return nil, nil, fmt.Errorf("could not set owner reference: %+v", err) } return project.Status.ID, nil, nil 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 31e187a1f6..28b0e38e1f 100644 --- a/go/controller/internal/controller/common.go +++ b/go/controller/internal/controller/common.go @@ -9,6 +9,7 @@ import ( "github.com/pluralsh/polly/algorithms" "github.com/samber/lo" corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" @@ -24,17 +25,35 @@ 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} +// handleRequeue allows avoiding rate limiting when some errors occur, +// i.e., when a resource is not created yet, or when it is waiting for an ID. +// +// If the result is set, then any potential error will be saved in a condition +// and ignored in the return to avoid rate limiting. +// +// It is important that at least one from a result or an error have to be non-nil. +func handleRequeue(result *ctrl.Result, err error, setCondition func(condition metav1.Condition)) (ctrl.Result, error) { + utils.MarkCondition(setCondition, v1alpha1.SynchronizedConditionType, metav1.ConditionFalse, + v1alpha1.SynchronizedConditionReasonError, defaultErrMessage(err, "")) + return lo.FromPtr(result), lo.Ternary(result != nil, nil, err) +} + +// defaultErrMessage extracts error message if error is non-nil, otherwise it returns default message. +func defaultErrMessage(err error, defaultMessage string) string { + if err != nil { + return err.Error() + } + + return defaultMessage } func ensureBindings(bindings []v1alpha1.Binding, userGroupCache cache.UserGroupCache) ([]v1alpha1.Binding, bool, error) { @@ -302,14 +321,6 @@ func mergeHelmValues(ctx context.Context, c runtimeclient.Client, secretRef *cor return lo.ToPtr(string(out)), nil } -func defaultErrMessage(err error, defaultMessage string) string { - if err != nil { - return err.Error() - } - - return defaultMessage -} - func notFoundOrReadyErrorMessage(err error) string { return fmt.Sprintf("Referenced object is either not found or not ready, found error: %s", err.Error()) } diff --git a/go/controller/internal/controller/customstackrun_controller.go b/go/controller/internal/controller/customstackrun_controller.go index 786ace027f..24e54ba91a 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 @@ -186,7 +186,6 @@ func (r *CustomStackRunReconciler) handleDelete(ctx context.Context, stack *v1al } } controllerutil.RemoveFinalizer(stack, CustomStackRunFinalizer) - logger.Info("custom stack run deleted successfully") } return nil } diff --git a/go/controller/internal/controller/deploymentsettings_controller.go b/go/controller/internal/controller/deploymentsettings_controller.go index 8c8a5daf29..adc2ecb61c 100644 --- a/go/controller/internal/controller/deploymentsettings_controller.go +++ b/go/controller/internal/controller/deploymentsettings_controller.go @@ -99,7 +99,6 @@ func (r *DeploymentSettingsReconciler) Reconcile(ctx context.Context, req ctrl.R } if !settings.GetDeletionTimestamp().IsZero() { - logger.Info("deployment settings CRD deleted successfully") return ctrl.Result{}, nil } diff --git a/go/controller/internal/controller/generatedsecret_controller.go b/go/controller/internal/controller/generatedsecret_controller.go index 3cc0c47307..cb509c9422 100644 --- a/go/controller/internal/controller/generatedsecret_controller.go +++ b/go/controller/internal/controller/generatedsecret_controller.go @@ -47,7 +47,6 @@ type GeneratedSecretReconciler struct { // For more details, check Reconcile and its Result here: // - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.16.3/pkg/reconcile func (r *GeneratedSecretReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Result, reterr error) { - logger := log.FromContext(ctx) generatedSecret := &v1alpha1.GeneratedSecret{} if err := r.Get(ctx, req.NamespacedName, generatedSecret); err != nil { @@ -59,7 +58,6 @@ func (r *GeneratedSecretReconciler) Reconcile(ctx context.Context, req ctrl.Requ utils.MarkCondition(generatedSecret.SetCondition, v1alpha1.ReadyConditionType, v1.ConditionFalse, v1alpha1.ReadyConditionReason, "") scope, err := NewDefaultScope(ctx, r.Client, generatedSecret) if err != nil { - logger.Error(err, "failed to create scope") utils.MarkCondition(generatedSecret.SetCondition, v1alpha1.ReadyConditionType, 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 2f3f6c31f8..7387e33367 100644 --- a/go/controller/internal/controller/gitrepository_controller.go +++ b/go/controller/internal/controller/gitrepository_controller.go @@ -2,10 +2,9 @@ package controller import ( "context" + "fmt" - apierrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/runtime/schema" - + "github.com/samber/lo" corev1 "k8s.io/api/core/v1" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -62,7 +61,6 @@ func (r *GitRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reques utils.MarkCondition(repo.SetCondition, v1alpha1.ReadyConditionType, v1.ConditionFalse, v1alpha1.ReadyConditionReason, "") scope, err := NewDefaultScope(ctx, r.Client, repo) if err != nil { - logger.Error(err, "failed to create scope") utils.MarkCondition(repo.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, err.Error()) return ctrl.Result{}, err } @@ -85,21 +83,17 @@ 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") - return r.handleExistingRepo(ctx, repo) + logger.V(9).Info("repository already exists, running in read-only mode", "name", repo.Name, "namespace", repo.Namespace) + return r.handleExistingRepo(repo) } utils.MarkCondition(repo.SetCondition, v1alpha1.ReadonlyConditionType, v1.ConditionFalse, v1alpha1.ReadonlyConditionReason, "") - attrs, err := r.getRepositoryAttributes(ctx, repo) - if err != nil { - if errors.IsNotFound(err) { - utils.MarkCondition(repo.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, notFoundOrReadyErrorMessage(err)) - return RequeueAfter(requeueWaitForResources), nil - } - utils.MarkCondition(repo.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, err.Error()) - return requeue, err + attrs, result, err := r.getRepositoryAttributes(ctx, repo) + if result != nil || err != nil { + return handleRequeue(result, err, repo.SetCondition) } + sha, err := utils.HashObject(attrs) if err != nil { utils.MarkCondition(repo.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, err.Error()) @@ -117,17 +111,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 @@ -149,7 +142,6 @@ func (r *GitRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reques func (r *GitRepositoryReconciler) handleDelete(ctx context.Context, repo *v1alpha1.GitRepository) (ctrl.Result, error) { logger := log.FromContext(ctx) if controllerutil.ContainsFinalizer(repo, RepoFinalizer) { - logger.Info("delete git repository") existingRepo, err := r.getRepository(repo.Spec.Url) if err != nil && !errors.IsNotFound(err) { utils.MarkCondition(repo.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, err.Error()) @@ -171,58 +163,50 @@ func (r *GitRepositoryReconciler) handleDelete(ctx context.Context, repo *v1alph return ctrl.Result{}, nil } -func (r *GitRepositoryReconciler) getRepositoryAttributes(ctx context.Context, repo *v1alpha1.GitRepository) (*console.GitAttributes, error) { +func (r *GitRepositoryReconciler) getRepositoryAttributes(ctx context.Context, repo *v1alpha1.GitRepository) (*console.GitAttributes, *ctrl.Result, error) { attrs := console.GitAttributes{URL: repo.Spec.Url} if repo.Spec.ConnectionRef != nil { connection := &v1alpha1.ScmConnection{} - if err := r.Get(ctx, types.NamespacedName{Name: repo.Spec.ConnectionRef.Name, Namespace: repo.Spec.ConnectionRef.Namespace}, connection); err != nil { - return nil, err + ref := repo.Spec.ConnectionRef + if err := r.Get(ctx, types.NamespacedName{Name: ref.Name, Namespace: ref.Namespace}, connection); err != nil { + if errors.IsNotFound(err) { + return nil, &waitForResources, err + } + + return nil, nil, err } if connection.Status.ID == nil { - return nil, apierrors.NewNotFound(schema.GroupResource{Resource: "ScmConnection", Group: "deployments.plural.sh"}, repo.Spec.ConnectionRef.Name) + return nil, &waitForResources, fmt.Errorf("SCM connection is not ready yet") } if err := utils.TryAddOwnerRef(ctx, r.Client, repo, connection, r.Scheme); err != nil { - return nil, err + return nil, nil, err } attrs.ConnectionID = connection.Status.ID } 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 { - return nil, err - } - - err = utils.TryAddOwnerRef(ctx, r.Client, repo, secret, r.Scheme) - if err != nil { - return nil, err - } - - privateKey := string(secret.Data[privateKey]) - passphrase := string(secret.Data[passphrase]) - username := string(secret.Data[username]) - password := string(secret.Data[password]) - if privateKey != "" { - attrs.PrivateKey = &privateKey - } + ref := repo.Spec.CredentialsRef + if err := r.Get(ctx, types.NamespacedName{Name: ref.Name, Namespace: ref.Namespace}, secret); err != nil { + if errors.IsNotFound(err) { + return nil, &waitForResources, err + } - if passphrase != "" { - attrs.Passphrase = &passphrase + return nil, nil, err } - if username != "" { - attrs.Username = &username + if err := utils.TryAddOwnerRef(ctx, r.Client, repo, secret, r.Scheme); err != nil { + return nil, nil, err } - if password != "" { - attrs.Password = &password - } + attrs.PrivateKey = lo.EmptyableToPtr(string(secret.Data[privateKey])) + attrs.Passphrase = lo.EmptyableToPtr(string(secret.Data[passphrase])) + attrs.Username = lo.EmptyableToPtr(string(secret.Data[username])) + attrs.Password = lo.EmptyableToPtr(string(secret.Data[password])) } - return &attrs, nil + return &attrs, nil, nil } func (r *GitRepositoryReconciler) getRepository(url string) (*console.GitRepositoryFragment, error) { @@ -254,16 +238,14 @@ func (r *GitRepositoryReconciler) isAlreadyExists(repository *v1alpha1.GitReposi return !repository.Status.HasID(), nil } -func (r *GitRepositoryReconciler) handleExistingRepo(ctx context.Context, repo *v1alpha1.GitRepository) (reconcile.Result, error) { - logger := log.FromContext(ctx) +func (r *GitRepositoryReconciler) handleExistingRepo(repo *v1alpha1.GitRepository) (reconcile.Result, error) { existingRepo, err := r.getRepository(repo.Spec.Url) if err != nil && !errors.IsNotFound(err) { utils.MarkCondition(repo.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, err.Error()) return ctrl.Result{}, err } if existingRepo == nil { - msg := "Could not find Git repository in Console API" - logger.Info(msg) + msg := "could not find the repository" repo.Status.Message = &msg repo.Status.Health = v1alpha1.GitHealthFailed utils.MarkCondition(repo.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonNotFound, msg) diff --git a/go/controller/internal/controller/globalservice_controller.go b/go/controller/internal/controller/globalservice_controller.go index 7a372b7ceb..c9a5bc6e59 100644 --- a/go/controller/internal/controller/globalservice_controller.go +++ b/go/controller/internal/controller/globalservice_controller.go @@ -20,9 +20,6 @@ import ( "context" "fmt" - apierrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/runtime/schema" - "github.com/samber/lo" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -74,7 +71,6 @@ func (r *GlobalServiceReconciler) Reconcile(ctx context.Context, req ctrl.Reques utils.MarkCondition(globalService.SetCondition, v1alpha1.ReadyConditionType, v1.ConditionFalse, v1alpha1.ReadyConditionReason, "") scope, err := NewDefaultScope(ctx, r.Client, globalService) if err != nil { - logger.Error(err, "failed to create scope") utils.MarkCondition(globalService.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, err.Error()) return ctrl.Result{}, err } @@ -95,37 +91,26 @@ func (r *GlobalServiceReconciler) Reconcile(ctx context.Context, req ctrl.Reques } if !globalService.GetDeletionTimestamp().IsZero() { - return ctrl.Result{}, r.handleDelete(ctx, globalService) + return ctrl.Result{}, r.handleDelete(globalService) } if globalService.Spec.ServiceRef == nil && globalService.Spec.Template == nil { return ctrl.Result{}, fmt.Errorf("the spec.serviceRef and spec.template can't be null") } - service, res, err := r.getService(ctx, globalService) - if res != nil { - utils.MarkCondition(globalService.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, defaultErrMessage(err, "service is not ready")) - return *res, err + service, result, err := r.getService(ctx, globalService) + if result != nil || err != nil { + return handleRequeue(result, err, globalService.SetCondition) } - provider, err := r.getProvider(ctx, globalService) - if err != nil { - if errors.IsNotFound(err) { - utils.MarkCondition(globalService.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, notFoundOrReadyErrorMessage(err)) - return RequeueAfter(requeueWaitForResources), nil - } - utils.MarkCondition(globalService.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, err.Error()) - return ctrl.Result{}, err + provider, result, err := r.getProvider(ctx, globalService) + if result != nil || err != nil { + return handleRequeue(result, err, globalService.SetCondition) } - project, err := r.getProject(ctx, globalService) - if err != nil { - if errors.IsNotFound(err) { - utils.MarkCondition(globalService.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, notFoundOrReadyErrorMessage(err)) - return RequeueAfter(requeueWaitForResources), nil - } - utils.MarkCondition(globalService.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReason, err.Error()) - return ctrl.Result{}, err + project, result, err := r.getProject(ctx, globalService) + if result != nil || err != nil { + return handleRequeue(result, err, globalService.SetCondition) } attr := globalService.Attributes(provider.Status.ID, project.Status.ID) @@ -204,61 +189,68 @@ func (r *GlobalServiceReconciler) getService(ctx context.Context, globalService service := &v1alpha1.ServiceDeployment{} if err := r.Get(ctx, client.ObjectKey{Name: globalService.Spec.ServiceRef.Name, Namespace: globalService.Spec.ServiceRef.Namespace}, service); err != nil { - return service, &ctrl.Result{}, err + if errors.IsNotFound(err) { + return nil, &waitForResources, err + } + + return nil, nil, err } logger := log.FromContext(ctx) if !service.DeletionTimestamp.IsZero() { logger.Info("deleting global service after service deployment deletion") if err := r.Delete(ctx, globalService); err != nil { - return nil, &ctrl.Result{}, err + return nil, nil, err } - return service, lo.ToPtr(RequeueAfter(requeueWaitForResources)), nil + return nil, &waitForResources, nil } if service.Status.ID == nil { - logger.Info("service is not ready") - return service, lo.ToPtr(RequeueAfter(requeueWaitForResources)), nil + return nil, &waitForResources, fmt.Errorf("service is not ready yet") } return service, nil, nil } -func (r *GlobalServiceReconciler) getProvider(ctx context.Context, globalService *v1alpha1.GlobalService) (*v1alpha1.Provider, error) { - logger := log.FromContext(ctx) +func (r *GlobalServiceReconciler) getProvider(ctx context.Context, globalService *v1alpha1.GlobalService) (*v1alpha1.Provider, *ctrl.Result, error) { provider := &v1alpha1.Provider{} if globalService.Spec.ProviderRef != nil { if err := r.Get(ctx, types.NamespacedName{Name: globalService.Spec.ProviderRef.Name}, provider); err != nil { - return provider, err + if errors.IsNotFound(err) { + return nil, &waitForResources, err + } + + return nil, nil, err } if provider.Status.ID == nil { - logger.Info("Provider is not ready") - return provider, apierrors.NewNotFound(schema.GroupResource{Resource: "Provider", Group: "deployments.plural.sh"}, globalService.Spec.ProviderRef.Name) + return nil, &waitForResources, fmt.Errorf("provider is not ready yet") } } - return provider, nil + return provider, nil, nil } -func (r *GlobalServiceReconciler) getProject(ctx context.Context, globalService *v1alpha1.GlobalService) (*v1alpha1.Project, error) { - logger := log.FromContext(ctx) +func (r *GlobalServiceReconciler) getProject(ctx context.Context, globalService *v1alpha1.GlobalService) (*v1alpha1.Project, *ctrl.Result, error) { project := &v1alpha1.Project{} if globalService.Spec.ProjectRef != nil { if err := r.Get(ctx, client.ObjectKey{Name: globalService.Spec.ProjectRef.Name}, project); err != nil { - return project, err + if errors.IsNotFound(err) { + return nil, &waitForResources, err + } + + return nil, nil, err } if project.Status.ID == nil { - logger.Info("Project is not ready") - return project, apierrors.NewNotFound(schema.GroupResource{Resource: "Project", Group: "deployments.plural.sh"}, globalService.Spec.ProjectRef.Name) + return nil, &waitForResources, fmt.Errorf("project is not ready yet") } if err := controllerutil.SetOwnerReference(project, globalService, r.Scheme); err != nil { - return project, fmt.Errorf("could not set global service owner reference, got error: %+v", err) + return nil, nil, fmt.Errorf("could not set owner reference: %+v", err) } } - return project, nil + return project, nil, nil } func (r *GlobalServiceReconciler) handleCreate(sha string, global *v1alpha1.GlobalService, svc *v1alpha1.ServiceDeployment, attrs console.GlobalServiceAttributes) error { @@ -280,10 +272,8 @@ func (r *GlobalServiceReconciler) handleCreate(sha string, global *v1alpha1.Glob return nil } -func (r *GlobalServiceReconciler) handleDelete(ctx context.Context, service *v1alpha1.GlobalService) error { - logger := log.FromContext(ctx) +func (r *GlobalServiceReconciler) handleDelete(service *v1alpha1.GlobalService) error { if controllerutil.ContainsFinalizer(service, GlobalServiceFinalizer) { - logger.Info("try to delete global service") if service.Status.GetID() != "" { existingGlobalService, err := r.ConsoleClient.GetGlobalService(service.Status.GetID()) if err != nil && !errors.IsNotFound(err) { diff --git a/go/controller/internal/controller/helmrepository_controller.go b/go/controller/internal/controller/helmrepository_controller.go index 40594ca802..17a2b8a8e0 100644 --- a/go/controller/internal/controller/helmrepository_controller.go +++ b/go/controller/internal/controller/helmrepository_controller.go @@ -62,7 +62,6 @@ func (in *HelmRepositoryReconciler) Reconcile(ctx context.Context, req reconcile scope, err := NewDefaultScope(ctx, in.Client, helmRepository) if err != nil { - logger.Error(err, "failed to create scope") utils.MarkCondition(helmRepository.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, err.Error()) return ctrl.Result{}, err } diff --git a/go/controller/internal/controller/infrastructurestack_controller.go b/go/controller/internal/controller/infrastructurestack_controller.go index 8c0ae12ced..9383a8a5d6 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 { @@ -246,7 +246,6 @@ func (r *InfrastructureStackReconciler) isAlreadyExists(ctx context.Context, sta func (r *InfrastructureStackReconciler) handleDelete(ctx context.Context, stack *v1alpha1.InfrastructureStack) (ctrl.Result, error) { logger := log.FromContext(ctx) if controllerutil.ContainsFinalizer(stack, InfrastructureStackFinalizer) { - logger.Info("try to delete stack") if stack.Status.GetID() != "" { existingNotificationSink, err := r.ConsoleClient.GetStack(ctx, stack.Status.GetID()) if err != nil && !errors.IsNotFound(err) { @@ -448,7 +447,6 @@ func (r *InfrastructureStackReconciler) stackConfigurationAttributes(conf *v1alp // ready before allowing main reconcile loop to continue. In case cluster reference is misconfigured, // it will return with error and block the reconcile process from continuing. func (r *InfrastructureStackReconciler) handleClusterRef(ctx context.Context, stack *v1alpha1.InfrastructureStack) (string, *ctrl.Result, error) { - logger := log.FromContext(ctx) cluster := &v1alpha1.Cluster{} if err := r.Get(ctx, client.ObjectKey{Name: stack.Spec.ClusterRef.Name, Namespace: stack.Spec.ClusterRef.Namespace}, cluster); err != nil { @@ -457,9 +455,8 @@ 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 +466,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 +474,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 +505,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 +538,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 ec5fda46c7..6c0432ec3f 100644 --- a/go/controller/internal/controller/managednamespace_controller.go +++ b/go/controller/internal/controller/managednamespace_controller.go @@ -20,8 +20,6 @@ import ( "context" "fmt" - "k8s.io/apimachinery/pkg/runtime/schema" - "github.com/pluralsh/polly/algorithms" "github.com/samber/lo" "k8s.io/apimachinery/pkg/api/errors" @@ -73,7 +71,6 @@ func (r *ManagedNamespaceReconciler) Reconcile(ctx context.Context, req ctrl.Req scope, err := NewDefaultScope(ctx, r.Client, managedNamespace) if err != nil { - logger.Error(err, "failed to create scope") utils.MarkCondition(managedNamespace.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, err.Error()) return ctrl.Result{}, err } @@ -109,13 +106,9 @@ func (r *ManagedNamespaceReconciler) Reconcile(ctx context.Context, req ctrl.Req } if !exists { logger.Info("create managed namespace", "name", managedNamespace.Name) - attr, err := r.getNamespaceAttributes(ctx, managedNamespace) - if err != nil { - if errors.IsNotFound(err) { - return RequeueAfter(requeueWaitForResources), nil - } - utils.MarkCondition(managedNamespace.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, err.Error()) - return ctrl.Result{}, err + attr, res, err := r.getNamespaceAttributes(ctx, managedNamespace) + if res != nil || err != nil { + return handleRequeue(res, err, managedNamespace.SetCondition) } ns, err := r.ConsoleClient.CreateNamespace(ctx, *attr) @@ -129,15 +122,11 @@ func (r *ManagedNamespaceReconciler) Reconcile(ctx context.Context, req ctrl.Req } if exists && !managedNamespace.Status.IsSHAEqual(sha) { logger.Info("update managed namespace", "name", managedNamespace.Name) - attr, err := r.getNamespaceAttributes(ctx, managedNamespace) - if err != nil { - if errors.IsNotFound(err) { - utils.MarkCondition(managedNamespace.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, notFoundOrReadyErrorMessage(err)) - return RequeueAfter(requeueWaitForResources), nil - } - utils.MarkCondition(managedNamespace.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, err.Error()) - return requeue, err + attr, res, err := r.getNamespaceAttributes(ctx, managedNamespace) + if res != nil || err != nil { + return handleRequeue(res, err, managedNamespace.SetCondition) } + if !managedNamespace.Status.HasID() { existing, err := r.ConsoleClient.GetNamespaceByName(ctx, managedNamespace.NamespaceName()) if err != nil { @@ -146,6 +135,7 @@ func (r *ManagedNamespaceReconciler) Reconcile(ctx context.Context, req ctrl.Req } managedNamespace.Status.ID = lo.ToPtr(existing.ID) } + _, err = r.ConsoleClient.UpdateNamespace(ctx, managedNamespace.Status.GetID(), *attr) if err != nil { utils.MarkCondition(managedNamespace.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, err.Error()) @@ -171,9 +161,7 @@ func (r *ManagedNamespaceReconciler) SetupWithManager(mgr ctrl.Manager) error { } func (r *ManagedNamespaceReconciler) handleDelete(ctx context.Context, namespace *deploymentsv1alpha1.ManagedNamespace) error { - logger := log.FromContext(ctx) if controllerutil.ContainsFinalizer(namespace, ManagedNamespaceFinalizer) { - logger.Info("try to delete namespace") if namespace.Status.GetID() != "" { existingNotificationSink, err := r.ConsoleClient.GetNamespace(ctx, namespace.Status.GetID()) if err != nil && !errors.IsNotFound(err) { @@ -188,7 +176,6 @@ func (r *ManagedNamespaceReconciler) handleDelete(ctx context.Context, namespace } } controllerutil.RemoveFinalizer(namespace, ManagedNamespaceFinalizer) - logger.Info("namespace deleted successfully") } return nil } @@ -209,8 +196,7 @@ func (r *ManagedNamespaceReconciler) isAlreadyExists(ctx context.Context, namesp return true, nil } -func (r *ManagedNamespaceReconciler) getNamespaceAttributes(ctx context.Context, ns *v1alpha1.ManagedNamespace) (*console.ManagedNamespaceAttributes, error) { - logger := log.FromContext(ctx) +func (r *ManagedNamespaceReconciler) getNamespaceAttributes(ctx context.Context, ns *v1alpha1.ManagedNamespace) (*console.ManagedNamespaceAttributes, *ctrl.Result, error) { attr := &console.ManagedNamespaceAttributes{ Name: ns.NamespaceName(), Namespace: ns.Spec.Name, @@ -235,7 +221,7 @@ func (r *ManagedNamespaceReconciler) getNamespaceAttributes(ctx context.Context, if ns.Spec.Target.Tags != nil { result, err := json.Marshal(ns.Spec.Target.Tags) if err != nil { - return nil, err + return nil, nil, err } rawTags := string(result) attr.Target.Tags = &rawTags @@ -245,7 +231,7 @@ func (r *ManagedNamespaceReconciler) getNamespaceAttributes(ctx context.Context, if ns.Spec.Annotations != nil { result, err := json.Marshal(ns.Spec.Annotations) if err != nil { - return nil, err + return nil, nil, err } rawAnnotations := string(result) attr.Annotations = &rawAnnotations @@ -253,7 +239,7 @@ func (r *ManagedNamespaceReconciler) getNamespaceAttributes(ctx context.Context, if ns.Spec.Labels != nil { result, err := json.Marshal(ns.Spec.Labels) if err != nil { - return nil, err + return nil, nil, err } rawLabels := string(result) attr.Labels = &rawLabels @@ -265,14 +251,15 @@ func (r *ManagedNamespaceReconciler) getNamespaceAttributes(ctx context.Context, } if ns.Spec.Service != nil { srv := ns.Spec.Service - repository, err := r.getRepository(ctx, ns) - if err != nil { - return nil, err + repository, result, err := r.getRepository(ctx, ns) + if result != nil || err != nil { + return nil, result, err } + namespace := ns.GetNamespace() st, err := genServiceTemplate(ctx, r.Client, namespace, srv, repository.Status.ID) if err != nil { - return nil, err + return nil, nil, err } if st.Name == nil { @@ -286,42 +273,56 @@ func (r *ManagedNamespaceReconciler) getNamespaceAttributes(ctx context.Context, attr.Service = st } - project := &v1alpha1.Project{} - if ns.Spec.ProjectRef != nil { - if err := r.Get(ctx, client.ObjectKey{Name: ns.Spec.ProjectRef.Name}, project); err != nil { - utils.MarkCondition(ns.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, err.Error()) - return nil, err - } - - if project.Status.ID == nil { - logger.Info("Project is not ready") - utils.MarkCondition(ns.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReason, "project is not ready") - return nil, errors.NewNotFound(schema.GroupResource{Resource: "Project", Group: "deployments.plural.sh"}, ns.Spec.ProjectRef.Name) - } - - if err := controllerutil.SetOwnerReference(project, ns, r.Scheme); err != nil { - return nil, fmt.Errorf("could not set global service owner reference, got error: %+v", err) - } + project, result, err := r.getProject(ctx, ns) + if result != nil || err != nil { + return nil, result, err } - attr.ProjectID = project.Status.ID - return attr, nil + return attr, nil, nil } -func (r *ManagedNamespaceReconciler) getRepository(ctx context.Context, ns *v1alpha1.ManagedNamespace) (*v1alpha1.GitRepository, error) { +func (r *ManagedNamespaceReconciler) getRepository(ctx context.Context, ns *v1alpha1.ManagedNamespace) (*v1alpha1.GitRepository, *ctrl.Result, error) { repository := &v1alpha1.GitRepository{} if ns.Spec.Service.RepositoryRef != nil { if err := r.Get(ctx, client.ObjectKey{Name: ns.Spec.Service.RepositoryRef.Name, Namespace: ns.Spec.Service.RepositoryRef.Namespace}, repository); err != nil { - return nil, err + if errors.IsNotFound(err) { + return nil, &waitForResources, err + } + + return nil, nil, err } + if repository.Status.ID == nil { - utils.MarkCondition(ns.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReason, "repository is not ready") - return nil, errors.NewNotFound(schema.GroupResource{Resource: "GitRepository", Group: "deployments.plural.sh"}, ns.Spec.Service.RepositoryRef.Name) + return nil, &waitForResources, fmt.Errorf("repository is not ready yet") } + if repository.Status.Health == v1alpha1.GitHealthFailed { - return nil, fmt.Errorf("repository %s is not healthy", repository.Name) + return nil, nil, fmt.Errorf("repository %s is not healthy", repository.Name) + } + } + return repository, nil, nil +} + +func (r *ManagedNamespaceReconciler) getProject(ctx context.Context, ns *v1alpha1.ManagedNamespace) (*v1alpha1.Project, *ctrl.Result, error) { + project := &v1alpha1.Project{} + if ns.Spec.ProjectRef != nil { + if err := r.Get(ctx, client.ObjectKey{Name: ns.Spec.ProjectRef.Name}, project); err != nil { + if errors.IsNotFound(err) { + return nil, &waitForResources, err + } + + return nil, nil, err + } + + if project.Status.ID == nil { + return nil, &waitForResources, fmt.Errorf("project is not ready yet") + } + + if err := controllerutil.SetOwnerReference(project, ns, r.Scheme); err != nil { + return nil, nil, fmt.Errorf("could not set owner reference: %+v", err) } } - return repository, nil + + return project, nil, nil } diff --git a/go/controller/internal/controller/notificationrouter_controller.go b/go/controller/internal/controller/notificationrouter_controller.go index 5d86f240f7..b1ffb40914 100644 --- a/go/controller/internal/controller/notificationrouter_controller.go +++ b/go/controller/internal/controller/notificationrouter_controller.go @@ -71,7 +71,6 @@ func (r *NotificationRouterReconciler) Reconcile(ctx context.Context, req ctrl.R utils.MarkCondition(notificationRouter.SetCondition, v1alpha1.ReadyConditionType, v1.ConditionFalse, v1alpha1.ReadyConditionReason, "") scope, err := NewDefaultScope(ctx, r.Client, notificationRouter) if err != nil { - logger.Error(err, "failed to create scope") utils.MarkCondition(notificationRouter.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, err.Error()) return ctrl.Result{}, err } @@ -127,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 52063af08a..2ce3d1abc1 100644 --- a/go/controller/internal/controller/notificationsink_controller.go +++ b/go/controller/internal/controller/notificationsink_controller.go @@ -75,7 +75,6 @@ func (r *NotificationSinkReconciler) Reconcile(ctx context.Context, req ctrl.Req scope, err := NewDefaultScope(ctx, r.Client, notificationSink) if err != nil { - logger.Error(err, "failed to create scope") utils.MarkCondition(notificationSink.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, err.Error()) return ctrl.Result{}, err } @@ -115,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()) @@ -239,9 +238,7 @@ func (r *NotificationSinkReconciler) isAlreadyExists(ctx context.Context, notifi } func (r *NotificationSinkReconciler) handleDelete(ctx context.Context, notificationSink *v1alpha1.NotificationSink) error { - logger := log.FromContext(ctx) if controllerutil.ContainsFinalizer(notificationSink, NotificationSinkFinalizer) { - logger.Info("try to delete notification sink") if notificationSink.Status.GetID() != "" { existingNotificationSink, err := r.ConsoleClient.GetNotificationSink(ctx, notificationSink.Status.GetID()) if err != nil && !errors.IsNotFound(err) { diff --git a/go/controller/internal/controller/observabilityprovider_controller.go b/go/controller/internal/controller/observabilityprovider_controller.go index 3d15a025d2..932b4ea4cb 100644 --- a/go/controller/internal/controller/observabilityprovider_controller.go +++ b/go/controller/internal/controller/observabilityprovider_controller.go @@ -130,8 +130,6 @@ func (in *ObservabilityProviderReconciler) SetupWithManager(mgr ctrl.Manager) er } func (in *ObservabilityProviderReconciler) addOrRemoveFinalizer(ctx context.Context, provider *v1alpha1.ObservabilityProvider) (*ctrl.Result, error) { - logger := log.FromContext(ctx) - // If object is not being deleted and if it does not have our finalizer, // then lets add the finalizer. This is equivalent to registering our finalizer. if provider.ObjectMeta.DeletionTimestamp.IsZero() && !controllerutil.ContainsFinalizer(provider, ObservabilityProviderFinalizer) { @@ -148,7 +146,6 @@ func (in *ObservabilityProviderReconciler) addOrRemoveFinalizer(ctx context.Cont } if exists && !provider.Status.IsReadonly() { - logger.Info("deleting ObservabilityProvider") if err := in.ConsoleClient.DeleteObservabilityProvider(ctx, provider.Status.GetID()); err != nil { // if it fails to delete the external dependency here, return with error // so that it can be retried. @@ -191,8 +188,6 @@ func (in *ObservabilityProviderReconciler) sync( provider *v1alpha1.ObservabilityProvider, changed bool, ) (*console.ObservabilityProviderFragment, error) { - logger := log.FromContext(ctx) - exists, err := in.ConsoleClient.IsObservabilityProviderExists(ctx, provider.ConsoleName()) if err != nil { return nil, err @@ -209,7 +204,6 @@ func (in *ObservabilityProviderReconciler) sync( return nil, err } - logger.Info("upsert ObservabilityProvider") return in.ConsoleClient.UpsertObservabilityProvider(ctx, provider.Attributes(c)) } diff --git a/go/controller/internal/controller/observer_controller.go b/go/controller/internal/controller/observer_controller.go index b6fd16eebe..5933458b0a 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 @@ -125,8 +125,6 @@ func (r *ObserverReconciler) sync( observer *v1alpha1.Observer, changed bool, ) (*console.ObserverFragment, error) { - logger := log.FromContext(ctx) - exists, err := r.ConsoleClient.IsObserverExists(ctx, observer.ObserverName()) if err != nil { return nil, err @@ -142,7 +140,6 @@ func (r *ObserverReconciler) sync( return nil, err } - logger.Info("upsert Observer") return r.ConsoleClient.UpsertObserver(ctx, observer.Attributes(target, actions, projectID)) } @@ -282,8 +279,6 @@ func (r *ObserverReconciler) handleExisting(ctx context.Context, observer *v1alp } func (r *ObserverReconciler) addOrRemoveFinalizer(ctx context.Context, observer *v1alpha1.Observer) (*ctrl.Result, error) { - logger := log.FromContext(ctx) - // If object is not being deleted and if it does not have our finalizer, // then lets add the finalizer. This is equivalent to registering our finalizer. if observer.ObjectMeta.DeletionTimestamp.IsZero() && !controllerutil.ContainsFinalizer(observer, ObserverFinalizer) { @@ -300,7 +295,6 @@ func (r *ObserverReconciler) addOrRemoveFinalizer(ctx context.Context, observer } if exists && !observer.Status.IsReadonly() { - logger.Info("deleting Observer") if err := r.ConsoleClient.DeleteObserver(ctx, observer.Status.GetID()); err != nil { // if it fails to delete the external dependency here, return with error // so that it can be retried. diff --git a/go/controller/internal/controller/oidcprovider_controller.go b/go/controller/internal/controller/oidcprovider_controller.go index ef167b47a9..89e74c51ad 100644 --- a/go/controller/internal/controller/oidcprovider_controller.go +++ b/go/controller/internal/controller/oidcprovider_controller.go @@ -122,8 +122,6 @@ func (in *OIDCProviderReconciler) Reconcile(ctx context.Context, req reconcile.R } func (in *OIDCProviderReconciler) addOrRemoveFinalizer(ctx context.Context, oidcProvider *v1alpha1.OIDCProvider) (*ctrl.Result, error) { - logger := log.FromContext(ctx) - // If object is not being deleted and if it does not have our finalizer, // then lets add the finalizer. This is equivalent to registering our finalizer. if oidcProvider.ObjectMeta.DeletionTimestamp.IsZero() && !controllerutil.ContainsFinalizer(oidcProvider, OIDCProviderFinalizer) { @@ -137,7 +135,6 @@ func (in *OIDCProviderReconciler) addOrRemoveFinalizer(ctx context.Context, oidc exists := in.isAlreadyExists(oidcProvider) if exists { - logger.Info("deleting OIDCProvider") if err := in.ConsoleClient.DeleteOIDCProvider(ctx, oidcProvider.Status.GetID(), console.OidcProviderTypePlural); errors.IgnoreNotFound(err) != nil { // if it fails to delete the external dependency here, return with error // so that it can be retried. @@ -155,7 +152,6 @@ func (in *OIDCProviderReconciler) addOrRemoveFinalizer(ctx context.Context, oidc } func (in *OIDCProviderReconciler) sync(ctx context.Context, oidcProvider *v1alpha1.OIDCProvider, changed bool) (*console.OIDCProviderFragment, error) { - logger := log.FromContext(ctx) // Currently only Plural type is supported // TODO: parametrize once more types will be supported providerType := console.OidcProviderTypePlural @@ -163,14 +159,12 @@ func (in *OIDCProviderReconciler) sync(ctx context.Context, oidcProvider *v1alph // Update only if OIDCProvider has changed if changed && exists { - logger.Info("updating OIDCProvider") response, err := in.ConsoleClient.UpdateOIDCProvider(ctx, oidcProvider.Status.GetID(), providerType, oidcProvider.Attributes()) return in.backfillCredentialsRefSecret(ctx, oidcProvider, response, err) } // Create the OIDCProvider in Console API if it doesn't exist if !exists { - logger.Info("creating OIDCProvider") response, err := in.ConsoleClient.CreateOIDCProvider(ctx, providerType, oidcProvider.Attributes()) return in.backfillCredentialsRefSecret(ctx, oidcProvider, response, 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/prautomation_controller.go b/go/controller/internal/controller/prautomation_controller.go index b624f9e8e3..695dc6373f 100644 --- a/go/controller/internal/controller/prautomation_controller.go +++ b/go/controller/internal/controller/prautomation_controller.go @@ -104,8 +104,6 @@ func (in *PrAutomationReconciler) Reconcile(ctx context.Context, req reconcile.R } func (in *PrAutomationReconciler) addOrRemoveFinalizer(ctx context.Context, prAutomation *v1alpha1.PrAutomation) (*ctrl.Result, error) { - logger := log.FromContext(ctx) - // If object is not being deleted and if it does not have our finalizer, // then lets add the finalizer. This is equivalent to registering our finalizer. if prAutomation.ObjectMeta.DeletionTimestamp.IsZero() && !controllerutil.ContainsFinalizer(prAutomation, PrAutomationProtectionFinalizerName) { @@ -121,7 +119,6 @@ func (in *PrAutomationReconciler) addOrRemoveFinalizer(ctx context.Context, prAu // Remove PrAutomation from Console API if it exists if exists { - logger.Info("Deleting PR automation") if err = in.ConsoleClient.DeletePrAutomation(ctx, prAutomation.Status.GetID()); err != nil { // if it fails to delete the external dependency here, return with error // so that it can be retried. @@ -172,7 +169,6 @@ func (in *PrAutomationReconciler) sync(ctx context.Context, prAutomation *v1alph // Update only if PrAutomation has changed if prAutomation.Status.SHA != nil && *prAutomation.Status.SHA != sha && exists { - logger.Info("Updating PR automation") pra, err = in.ConsoleClient.UpdatePrAutomation(ctx, prAutomation.Status.GetID(), *attributes) return pra, sha, err } @@ -183,7 +179,6 @@ func (in *PrAutomationReconciler) sync(ctx context.Context, prAutomation *v1alph return pra, sha, err } - logger.Info("Creating PR automation") pra, err = in.ConsoleClient.CreatePrAutomation(ctx, *attributes) return pra, sha, err } diff --git a/go/controller/internal/controller/project_controller.go b/go/controller/internal/controller/project_controller.go index ef4d1acc8a..63bacc11b0 100644 --- a/go/controller/internal/controller/project_controller.go +++ b/go/controller/internal/controller/project_controller.go @@ -60,7 +60,6 @@ func (in *ProjectReconciler) Reconcile(ctx context.Context, req reconcile.Reques scope, err := NewDefaultScope(ctx, in.Client, project) if err != nil { - logger.Error(err, "failed to create scope") utils.MarkCondition(project.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, err.Error()) return ctrl.Result{}, err } diff --git a/go/controller/internal/controller/provider_controller.go b/go/controller/internal/controller/provider_controller.go index fade1bc770..677c0b22b0 100644 --- a/go/controller/internal/controller/provider_controller.go +++ b/go/controller/internal/controller/provider_controller.go @@ -57,7 +57,6 @@ func (r *ProviderReconciler) Reconcile(ctx context.Context, req reconcile.Reques utils.MarkCondition(provider.SetCondition, v1alpha1.ReadyConditionType, v1.ConditionFalse, v1alpha1.ReadyConditionReason, "") scope, err := NewDefaultScope(ctx, r.Client, provider) if err != nil { - logger.Error(err, "failed to create scope") utils.MarkCondition(provider.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, err.Error()) return ctrl.Result{}, err } @@ -194,7 +193,6 @@ func (r *ProviderReconciler) addOrRemoveFinalizer(ctx context.Context, provider // Remove Provider from Console API if it exists if exists && !provider.Status.IsReadonly() { - logger.Info("Deleting provider") if err = r.ConsoleClient.DeleteProvider(ctx, provider.Status.GetID()); err != nil { // if it fails to delete the external dependency here, return with error // so that it can be retried. @@ -216,7 +214,6 @@ func (r *ProviderReconciler) addOrRemoveFinalizer(ctx context.Context, provider } func (r *ProviderReconciler) sync(ctx context.Context, provider *v1alpha1.Provider, changed bool) (*console.ClusterProviderFragment, error) { - logger := log.FromContext(ctx) exists, err := r.ConsoleClient.IsProviderExists(ctx, provider.Status.GetID()) if err != nil { return nil, err @@ -229,7 +226,6 @@ func (r *ProviderReconciler) sync(ctx context.Context, provider *v1alpha1.Provid return nil, err } - logger.Info("Updating provider") return r.ConsoleClient.UpdateProvider(ctx, provider.Status.GetID(), attributes) } @@ -244,7 +240,6 @@ func (r *ProviderReconciler) sync(ctx context.Context, provider *v1alpha1.Provid return nil, err } - logger.Info("Creating provider") return r.ConsoleClient.CreateProvider(ctx, attributes) } diff --git a/go/controller/internal/controller/scmconnection_controller.go b/go/controller/internal/controller/scmconnection_controller.go index ac3d7438f7..3486c5e59a 100644 --- a/go/controller/internal/controller/scmconnection_controller.go +++ b/go/controller/internal/controller/scmconnection_controller.go @@ -59,7 +59,6 @@ func (r *ScmConnectionReconciler) Reconcile(ctx context.Context, req reconcile.R scope, err := NewDefaultScope(ctx, r.Client, scm) if err != nil { - logger.Error(err, "failed to create scope") utils.MarkCondition(scm.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, err.Error()) return ctrl.Result{}, err } @@ -179,8 +178,6 @@ func (r *ScmConnectionReconciler) isAlreadyExists(ctx context.Context, scm *v1al } func (r *ScmConnectionReconciler) addOrRemoveFinalizer(ctx context.Context, scm *v1alpha1.ScmConnection) (*ctrl.Result, error) { - logger := log.FromContext(ctx) - // If object is not being deleted and if it does not have our finalizer, // then lets add the finalizer. This is equivalent to registering our finalizer. if scm.ObjectMeta.DeletionTimestamp.IsZero() && !controllerutil.ContainsFinalizer(scm, ScmConnectionProtectionFinalizerName) { @@ -196,7 +193,6 @@ func (r *ScmConnectionReconciler) addOrRemoveFinalizer(ctx context.Context, scm } if exists && !scm.Status.IsReadonly() { - logger.Info("Deleting ScmConnection") if err = r.ConsoleClient.DeleteScmConnection(ctx, scm.Status.GetID()); err != nil { // if it fails to delete the external dependency here, return with error // so that it can be retried. @@ -218,7 +214,6 @@ func (r *ScmConnectionReconciler) addOrRemoveFinalizer(ctx context.Context, scm } func (r *ScmConnectionReconciler) sync(ctx context.Context, scm *v1alpha1.ScmConnection, changed bool) (*console.ScmConnectionFragment, error) { - logger := log.FromContext(ctx) exists, err := r.ConsoleClient.IsScmConnectionExists(ctx, scm.ConsoleName()) if err != nil { return nil, err @@ -231,7 +226,6 @@ func (r *ScmConnectionReconciler) sync(ctx context.Context, scm *v1alpha1.ScmCon // Update only if ScmConnection has changed if changed && exists { - logger.Info("Updating ScmConnection") attr, err := scm.Attributes(ctx, r.Client, token) if err != nil { return nil, err @@ -245,7 +239,6 @@ func (r *ScmConnectionReconciler) sync(ctx context.Context, scm *v1alpha1.ScmCon } // Create the ScmConnection in Console API if it doesn't exist - logger.Info("Creating ScmConnection") attr, err := scm.Attributes(ctx, r.Client, token) if err != nil { return nil, err diff --git a/go/controller/internal/controller/scope.go b/go/controller/internal/controller/scope.go index 740a40c970..28051d3d1c 100644 --- a/go/controller/internal/controller/scope.go +++ b/go/controller/internal/controller/scope.go @@ -2,10 +2,8 @@ package controller import ( "context" - "errors" "fmt" - "github.com/samber/lo" "sigs.k8s.io/cluster-api/util/patch" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -26,13 +24,9 @@ func (in *DefaultScope[T]) PatchObject() error { } func NewDefaultScope[T client.Object](ctx context.Context, client client.Client, object T) (Scope[T], error) { - if lo.IsNil(object) { - return nil, errors.New("object cannot be nil") - } - helper, err := patch.NewHelper(object, client) if err != nil { - return nil, fmt.Errorf("failed to create new helper, go error: %s", err) + return nil, fmt.Errorf("failed to create scope: %s", err) } return &DefaultScope[T]{ diff --git a/go/controller/internal/controller/service_account_controller.go b/go/controller/internal/controller/service_account_controller.go index b3853cae3e..b62eb4307b 100644 --- a/go/controller/internal/controller/service_account_controller.go +++ b/go/controller/internal/controller/service_account_controller.go @@ -52,7 +52,6 @@ func (r *ServiceAccountReconciler) Reconcile(ctx context.Context, req reconcile. utils.MarkCondition(sa.SetCondition, v1alpha1.ReadyConditionType, v1.ConditionFalse, v1alpha1.ReadyConditionReason, "") scope, err := NewDefaultScope(ctx, r.Client, sa) if err != nil { - logger.Error(err, "failed to create scope") utils.MarkCondition(sa.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, err.Error()) return ctrl.Result{}, err } @@ -177,7 +176,6 @@ func (r *ServiceAccountReconciler) isAlreadyExists(ctx context.Context, sa *v1al } func (r *ServiceAccountReconciler) sync(ctx context.Context, sa *v1alpha1.ServiceAccount, changed bool) (*console.UserFragment, error) { - logger := log.FromContext(ctx) exists, err := r.ConsoleClient.IsServiceAccountExists(ctx, sa.Spec.Email) if err != nil { return nil, err @@ -185,7 +183,6 @@ func (r *ServiceAccountReconciler) sync(ctx context.Context, sa *v1alpha1.Servic // Update only if ServiceAccount has changed if changed && exists { - logger.Info("Updating ServiceAccount") attr := sa.Attributes() return r.ConsoleClient.UpdateServiceAccount(ctx, sa.Status.GetID(), attr) } @@ -196,10 +193,7 @@ func (r *ServiceAccountReconciler) sync(ctx context.Context, sa *v1alpha1.Servic } // Create the ServiceAccount in Console API if it doesn't exist - logger.Info("Creating ServiceAccount") - attr := sa.Attributes() - - return r.ConsoleClient.CreateServiceAccount(ctx, attr) + return r.ConsoleClient.CreateServiceAccount(ctx, sa.Attributes()) } func (r *ServiceAccountReconciler) syncToken(ctx context.Context, sa *v1alpha1.ServiceAccount) error { diff --git a/go/controller/internal/controller/servicedeployment_controller.go b/go/controller/internal/controller/servicedeployment_controller.go index 8834390aba..6b19144d6c 100644 --- a/go/controller/internal/controller/servicedeployment_controller.go +++ b/go/controller/internal/controller/servicedeployment_controller.go @@ -64,7 +64,6 @@ func (r *ServiceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ utils.MarkCondition(service.SetCondition, v1alpha1.ReadyConditionType, v1.ConditionFalse, v1alpha1.ReadyConditionReason, "") scope, err := NewDefaultScope(ctx, r.Client, service) if err != nil { - logger.Error(err, "failed to create scope") utils.MarkCondition(service.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, err.Error()) return ctrl.Result{}, err } @@ -94,16 +93,15 @@ 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") @@ -111,25 +109,23 @@ 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 { @@ -138,13 +134,8 @@ func (r *ServiceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ } attr, result, err := r.genServiceAttributes(ctx, service, repository.Status.ID) - if result != nil { - utils.MarkCondition(service.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, defaultErrMessage(err, "")) - return *result, nil // If the result is set, then error is ignored to requeue. - } - if err != nil { - utils.MarkCondition(service.SetCondition, v1alpha1.SynchronizedConditionType, v1.ConditionFalse, v1alpha1.SynchronizedConditionReasonError, err.Error()) - return ctrl.Result{}, err + if result != nil || err != nil { + return handleRequeue(result, err, service.SetCondition) } existingService, err := r.ConsoleClient.GetService(*cluster.Status.ID, service.ConsoleName()) @@ -209,7 +200,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, "") diff --git a/go/controller/internal/controller/stackdefinition_controller.go b/go/controller/internal/controller/stackdefinition_controller.go index a96b739ac7..9a9d3ff079 100644 --- a/go/controller/internal/controller/stackdefinition_controller.go +++ b/go/controller/internal/controller/stackdefinition_controller.go @@ -116,8 +116,6 @@ func (in *StackDefinitionReconciler) SetupWithManager(mgr ctrl.Manager) error { } func (in *StackDefinitionReconciler) addOrRemoveFinalizer(ctx context.Context, stack *v1alpha1.StackDefinition) (*ctrl.Result, error) { - logger := log.FromContext(ctx) - // If object is not being deleted and if it does not have our finalizer, // then lets add the finalizer. This is equivalent to registering our finalizer. if stack.ObjectMeta.DeletionTimestamp.IsZero() && !controllerutil.ContainsFinalizer(stack, StackDefinitionFinalizer) { @@ -134,7 +132,6 @@ func (in *StackDefinitionReconciler) addOrRemoveFinalizer(ctx context.Context, s } if exists && !stack.Status.IsReadonly() { - logger.Info("deleting StackDefinition") if err := in.ConsoleClient.DeleteStackDefinition(ctx, stack.Status.GetID()); err != nil { // if it fails to delete the external dependency here, return with error // so that it can be retried. @@ -164,8 +161,6 @@ func (in *StackDefinitionReconciler) isAlreadyExists(ctx context.Context, stack } func (in *StackDefinitionReconciler) sync(ctx context.Context, stack *v1alpha1.StackDefinition, changed bool) (*console.StackDefinitionFragment, error) { - logger := log.FromContext(ctx) - exists, err := in.isAlreadyExists(ctx, stack) if err != nil { return nil, err @@ -173,7 +168,6 @@ func (in *StackDefinitionReconciler) sync(ctx context.Context, stack *v1alpha1.S // Update only if StackDefinition has changed if changed && exists { - logger.Info("updating StackDefinition") return in.ConsoleClient.UpdateStackDefinition(ctx, stack.Status.GetID(), stack.Attributes()) } @@ -183,6 +177,5 @@ func (in *StackDefinitionReconciler) sync(ctx context.Context, stack *v1alpha1.S } // Create the StackDefinition in Console API if it doesn't exist - logger.Info("creating StackDefinition") return in.ConsoleClient.CreateStackDefinition(ctx, stack.Attributes()) } diff --git a/go/controller/internal/test/mocks/ConsoleClient_mock.go b/go/controller/internal/test/mocks/ConsoleClient_mock.go index 7feb9f110a..d83fed6fdd 100644 --- a/go/controller/internal/test/mocks/ConsoleClient_mock.go +++ b/go/controller/internal/test/mocks/ConsoleClient_mock.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.45.1. DO NOT EDIT. +// Code generated by mockery v2.43.2. DO NOT EDIT. package mocks diff --git a/plural/helm/console/crds/deployments.plural.sh_bootstraptokens.yaml b/plural/helm/console/crds/deployments.plural.sh_bootstraptokens.yaml index 88c2228a78..22d1a49b0c 100644 --- a/plural/helm/console/crds/deployments.plural.sh_bootstraptokens.yaml +++ b/plural/helm/console/crds/deployments.plural.sh_bootstraptokens.yaml @@ -45,8 +45,8 @@ spec: description: BootstrapTokenSpec defines the desired state of BootstrapToken properties: projectRef: - description: ProjectRef is the optional project that all clusters - spawned by generated bootstrap token will belong to + description: ProjectRef is the project that all clusters spawned by + generated bootstrap token will belong to properties: apiVersion: description: API version of the referent.