diff --git a/go/controller/internal/controller/common.go b/go/controller/internal/controller/common.go index 9d858894e1..eb75c6f627 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" @@ -33,6 +34,25 @@ var ( waitForResources = ctrl.Result{RequeueAfter: requeueWaitForResources} ) +// 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) { + if result != nil { + utils.MarkCondition(setCondition, v1alpha1.SynchronizedConditionType, metav1.ConditionFalse, + v1alpha1.SynchronizedConditionReasonError, defaultErrMessage(err, "")) + return *result, nil + } + + utils.MarkCondition(setCondition, v1alpha1.SynchronizedConditionType, metav1.ConditionFalse, + v1alpha1.SynchronizedConditionReasonError, err.Error()) + return ctrl.Result{}, err +} + func ensureBindings(bindings []v1alpha1.Binding, userGroupCache cache.UserGroupCache) ([]v1alpha1.Binding, bool, error) { requeue := false for i := range bindings { diff --git a/go/controller/internal/controller/gitrepository_controller.go b/go/controller/internal/controller/gitrepository_controller.go index 2554762424..7c6efe8b26 100644 --- a/go/controller/internal/controller/gitrepository_controller.go +++ b/go/controller/internal/controller/gitrepository_controller.go @@ -2,11 +2,9 @@ package controller import ( "context" + "fmt" "github.com/samber/lo" - apierrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/runtime/schema" - corev1 "k8s.io/api/core/v1" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -91,15 +89,11 @@ func (r *GitRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reques 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 waitForResources, 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()) @@ -170,32 +164,41 @@ 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} - if err := r.Get(ctx, name, secret); err != nil { - return nil, err + 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 + } + + return nil, nil, err } if err := utils.TryAddOwnerRef(ctx, r.Client, repo, secret, r.Scheme); err != nil { - return nil, err + return nil, nil, err } attrs.PrivateKey = lo.EmptyableToPtr(string(secret.Data[privateKey])) @@ -204,7 +207,7 @@ func (r *GitRepositoryReconciler) getRepositoryAttributes(ctx context.Context, r attrs.Password = lo.EmptyableToPtr(string(secret.Data[password])) } - return &attrs, nil + return &attrs, nil, nil } func (r *GitRepositoryReconciler) getRepository(url string) (*console.GitRepositoryFragment, error) { @@ -237,15 +240,13 @@ func (r *GitRepositoryReconciler) isAlreadyExists(repository *v1alpha1.GitReposi } func (r *GitRepositoryReconciler) handleExistingRepo(ctx context.Context, repo *v1alpha1.GitRepository) (reconcile.Result, error) { - logger := log.FromContext(ctx) 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)