Skip to content
This repository has been archived by the owner on Nov 9, 2022. It is now read-only.

Prevent context deadlines exceedances #111

Merged
merged 2 commits into from
Feb 17, 2021
Merged

Prevent context deadlines exceedances #111

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f74b1da
Set QPS/Burst also for uncached/direct client
rfranzke Feb 17, 2021
c01c280
Do not use timeout context for condition status updates
rfranzke Feb 17, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 12 additions & 11 deletions pkg/cmd/target.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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.
Expand Down
10 changes: 5 additions & 5 deletions pkg/controller/health/reconciler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand All @@ -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)

Expand Down Expand Up @@ -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)

Expand All @@ -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())
Expand Down
17 changes: 10 additions & 7 deletions pkg/controller/managedresource/reconciler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -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 {
Expand All @@ -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
Expand Down