From f74b1da30b27f0667193cb26f900da52c244b3a7 Mon Sep 17 00:00:00 2001 From: Rafael Franzke Date: Wed, 17 Feb 2021 13:14:52 +0100 Subject: [PATCH 1/2] Set QPS/Burst also for uncached/direct client --- pkg/cmd/target.go | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/pkg/cmd/target.go b/pkg/cmd/target.go index bb6bc6de..99d0a938 100644 --- a/pkg/cmd/target.go +++ b/pkg/cmd/target.go @@ -91,6 +91,10 @@ func (o *TargetClientOptions) Complete() error { targetClient client.Client ) + // TODO: make this configurable + restConfig.QPS = 100.0 + restConfig.Burst = 130 + if o.disableCache { // create direct client for target cluster targetClient, err = client.New(restConfig, client.Options{ @@ -176,17 +180,14 @@ func getTargetRESTConfig(kubeconfigPath string) (*rest.Config, error) { } func newCachedClient(cache cache.Cache, config rest.Config, options client.Options) (client.Client, error) { - // TODO: make this configurable - config.QPS = 100.0 - config.Burst = 130 - - nonCachedObjects := []client.Object{ - &corev1.Event{}, - &eventsv1beta1.Event{}, - &eventsv1.Event{}, - } - - return manager.NewClientBuilder().WithUncached(nonCachedObjects...).Build(cache, &config, options) + return manager. + NewClientBuilder(). + WithUncached( + &corev1.Event{}, + &eventsv1beta1.Event{}, + &eventsv1.Event{}, + ). + Build(cache, &config, options) } // Start starts the target cache if the client is cached. From c01c280a4744298750ca668ba0079e01bcaa09aa Mon Sep 17 00:00:00 2001 From: Rafael Franzke Date: Wed, 17 Feb 2021 13:46:52 +0100 Subject: [PATCH 2/2] Do not use timeout context for condition status updates --- pkg/controller/health/reconciler.go | 10 +++++----- pkg/controller/managedresource/reconciler.go | 17 ++++++++++------- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/pkg/controller/health/reconciler.go b/pkg/controller/health/reconciler.go index 050efa6a..f261b33f 100644 --- a/pkg/controller/health/reconciler.go +++ b/pkg/controller/health/reconciler.go @@ -60,9 +60,6 @@ func (r *Reconciler) InjectLogger(l logr.Logger) error { // Reconcile performs health checks. func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { - ctx, cancel := context.WithTimeout(ctx, time.Minute) - defer cancel() - log := r.log.WithValues("object", req) log.Info("Starting ManagedResource health checks") @@ -81,6 +78,9 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu return ctrl.Result{}, nil // Do not requeue } + healthCheckCtx, cancel := context.WithTimeout(ctx, time.Minute) + defer cancel() + // Initialize condition based on the current status. conditionResourcesHealthy := resourcesv1alpha1helper.GetOrInitCondition(mr.Status.Conditions, resourcesv1alpha1.ResourcesHealthy) @@ -130,7 +130,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu } } - if err := r.targetClient.Get(ctx, client.ObjectKey{Namespace: ref.Namespace, Name: ref.Name}, obj); err != nil { + if err := r.targetClient.Get(healthCheckCtx, client.ObjectKey{Namespace: ref.Namespace, Name: ref.Name}, obj); err != nil { if apierrors.IsNotFound(err) { log.Info("Could not get object", "namespace", ref.Namespace, "name", ref.Name) @@ -150,7 +150,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu return ctrl.Result{}, err } - if err := CheckHealth(ctx, r.targetClient, r.targetScheme, obj); err != nil { + if err := CheckHealth(healthCheckCtx, r.targetClient, r.targetScheme, obj); err != nil { var ( reason = ref.Kind + "Unhealthy" message = fmt.Sprintf("Required %s %q in namespace %q is unhealthy: %v", ref.Kind, ref.Name, ref.Namespace, err.Error()) diff --git a/pkg/controller/managedresource/reconciler.go b/pkg/controller/managedresource/reconciler.go index 2884139f..6256ef59 100644 --- a/pkg/controller/managedresource/reconciler.go +++ b/pkg/controller/managedresource/reconciler.go @@ -90,9 +90,6 @@ func (r *Reconciler) InjectLogger(l logr.Logger) error { // Reconcile implements `reconcile.Reconciler`. func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { - ctx, cancel := context.WithTimeout(ctx, time.Minute) - defer cancel() - log := r.log.WithValues("object", req) mr := &resourcesv1alpha1.ManagedResource{} @@ -150,12 +147,15 @@ func (r *Reconciler) reconcile(ctx context.Context, mr *resourcesv1alpha1.Manage forceOverwriteAnnotations = *v } + reconcileCtx, cancel := context.WithTimeout(ctx, time.Minute) + defer cancel() + // Initialize condition based on the current status. conditionResourcesApplied := resourcesv1alpha1helper.GetOrInitCondition(mr.Status.Conditions, resourcesv1alpha1.ResourcesApplied) for _, ref := range mr.Spec.SecretRefs { secret := &corev1.Secret{} - if err := r.client.Get(ctx, client.ObjectKey{Namespace: mr.Namespace, Name: ref.Name}, secret); err != nil { + if err := r.client.Get(reconcileCtx, client.ObjectKey{Namespace: mr.Namespace, Name: ref.Name}, secret); err != nil { conditionResourcesApplied = resourcesv1alpha1helper.UpdatedCondition(conditionResourcesApplied, resourcesv1alpha1.ConditionFalse, "CannotReadSecret", err.Error()) if err := tryUpdateManagedResourceConditions(ctx, r.client, mr, conditionResourcesApplied); err != nil { return ctrl.Result{}, fmt.Errorf("could not update the ManagedResource status: %+v ", err) @@ -269,7 +269,7 @@ func (r *Reconciler) reconcile(ctx context.Context, mr *resourcesv1alpha1.Manage } } - if deletionPending, err := r.cleanOldResources(ctx, existingResourcesIndex, mr); err != nil { + if deletionPending, err := r.cleanOldResources(reconcileCtx, existingResourcesIndex, mr); err != nil { var ( reason string status resourcesv1alpha1.ConditionStatus @@ -296,7 +296,7 @@ func (r *Reconciler) reconcile(ctx context.Context, mr *resourcesv1alpha1.Manage } } - if err := r.applyNewResources(ctx, origin, newResourcesObjects, mr.Spec.InjectLabels, equivalences); err != nil { + if err := r.applyNewResources(reconcileCtx, origin, newResourcesObjects, mr.Spec.InjectLabels, equivalences); err != nil { conditionResourcesApplied = resourcesv1alpha1helper.UpdatedCondition(conditionResourcesApplied, resourcesv1alpha1.ConditionFalse, resourcesv1alpha1.ConditionApplyFailed, err.Error()) if err := tryUpdateManagedResourceConditions(ctx, r.client, mr, conditionResourcesApplied); err != nil { return ctrl.Result{}, fmt.Errorf("could not update the ManagedResource status: %+v", err) @@ -322,6 +322,9 @@ func (r *Reconciler) reconcile(ctx context.Context, mr *resourcesv1alpha1.Manage func (r *Reconciler) delete(ctx context.Context, mr *resourcesv1alpha1.ManagedResource, log logr.Logger) (ctrl.Result, error) { log.Info("Starting to delete ManagedResource") + deleteCtx, cancel := context.WithTimeout(ctx, time.Minute) + defer cancel() + conditionResourcesApplied := resourcesv1alpha1helper.GetOrInitCondition(mr.Status.Conditions, resourcesv1alpha1.ResourcesApplied) if keepObjects := mr.Spec.KeepObjects; keepObjects == nil || !*keepObjects { @@ -338,7 +341,7 @@ func (r *Reconciler) delete(ctx context.Context, mr *resourcesv1alpha1.ManagedRe return ctrl.Result{}, fmt.Errorf("could not update the ManagedResource status: %+v", err) } - if deletionPending, err := r.cleanOldResources(ctx, existingResourcesIndex, mr); err != nil { + if deletionPending, err := r.cleanOldResources(deleteCtx, existingResourcesIndex, mr); err != nil { var ( reason string status resourcesv1alpha1.ConditionStatus