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

Time limit for cache sync on startup and controller reconciliations #102

Merged
merged 2 commits into from
Jan 8, 2021
Merged

Time limit for cache sync on startup and controller reconciliations #102

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e52ec7c
Time limit for cache sync on startup and controller reconciliations
rfranzke Jan 8, 2021
eb09e7a
make generate for 2021
rfranzke Jan 8, 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
6 changes: 5 additions & 1 deletion cmd/gardener-resource-manager/app/app.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"context"
"fmt"
"sync"
"time"

"github.com/spf13/cobra"
"github.com/spf13/pflag"
Expand Down Expand Up @@ -113,7 +114,10 @@ func NewResourceManagerCommand() *cobra.Command {
}
}()

if !targetClientOpts.Completed().WaitForCacheSync(ctx.Done()) {
ctxWaitForCache, cancelWaitForCache := context.WithTimeout(ctx, 5*time.Minute)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Side-note, doesn't have to be tackled in this PR: we should further improve health checks, by separating liveness and readiness endpoint and report an unready state as long as the caches haven't synced initially.
Though, this is also a general problem in our components.

defer cancelWaitForCache()

if !targetClientOpts.Completed().WaitForCacheSync(ctxWaitForCache.Done()) {
return fmt.Errorf("timed out waiting for target cache to sync")
}

Expand Down
15 changes: 9 additions & 6 deletions pkg/controller/health/reconciler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,14 @@ func (r *Reconciler) InjectLogger(l logr.Logger) error {

// Reconcile performs health checks.
func (r *Reconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
ctx, cancel := context.WithTimeout(r.ctx, time.Minute)
defer cancel()

log := r.log.WithValues("object", req)
log.Info("Starting ManagedResource health checks")

mr := &resourcesv1alpha1.ManagedResource{}
if err := r.client.Get(r.ctx, req.NamespacedName, mr); err != nil {
if err := r.client.Get(ctx, req.NamespacedName, mr); err != nil {
if apierrors.IsNotFound(err) {
log.Info("Stopping health checks for ManagedResource, as it has been deleted")
return reconcile.Result{}, nil
Expand All @@ -89,7 +92,7 @@ func (r *Reconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {

if !mr.DeletionTimestamp.IsZero() {
conditionResourcesHealthy = resourcesv1alpha1helper.UpdatedCondition(conditionResourcesHealthy, resourcesv1alpha1.ConditionFalse, resourcesv1alpha1.ConditionDeletionPending, "The resources are currently being deleted.")
if err := tryUpdateManagedResourceCondition(r.ctx, r.client, mr, conditionResourcesHealthy); err != nil {
if err := tryUpdateManagedResourceCondition(ctx, r.client, mr, conditionResourcesHealthy); err != nil {
return ctrl.Result{}, fmt.Errorf("could not update the ManagedResource status: %+v ", err)
}

Expand Down Expand Up @@ -126,7 +129,7 @@ func (r *Reconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
obj = unstructuredObj
}

if err := r.targetClient.Get(r.ctx, client.ObjectKey{Namespace: ref.Namespace, Name: ref.Name}, obj); err != nil {
if err := r.targetClient.Get(ctx, 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 @@ -136,7 +139,7 @@ func (r *Reconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
)

conditionResourcesHealthy = resourcesv1alpha1helper.UpdatedCondition(conditionResourcesHealthy, resourcesv1alpha1.ConditionFalse, reason, message)
if err := tryUpdateManagedResourceCondition(r.ctx, r.client, mr, conditionResourcesHealthy); err != nil {
if err := tryUpdateManagedResourceCondition(ctx, r.client, mr, conditionResourcesHealthy); err != nil {
return ctrl.Result{}, fmt.Errorf("could not update the ManagedResource status: %+v ", err)
}

Expand All @@ -153,7 +156,7 @@ func (r *Reconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
)

conditionResourcesHealthy = resourcesv1alpha1helper.UpdatedCondition(conditionResourcesHealthy, resourcesv1alpha1.ConditionFalse, reason, message)
if err := tryUpdateManagedResourceCondition(r.ctx, r.client, mr, conditionResourcesHealthy); err != nil {
if err := tryUpdateManagedResourceCondition(ctx, r.client, mr, conditionResourcesHealthy); err != nil {
return ctrl.Result{}, fmt.Errorf("could not update the ManagedResource status: %+v ", err)
}

Expand All @@ -164,7 +167,7 @@ func (r *Reconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
conditionResourcesHealthy = resourcesv1alpha1helper.UpdatedCondition(conditionResourcesHealthy, resourcesv1alpha1.ConditionTrue, "ResourcesHealthy", "All resources are healthy.")

if !apiequality.Semantic.DeepEqual(oldConditions, []resourcesv1alpha1.ManagedResourceCondition{conditionResourcesHealthy}) {
if err := tryUpdateManagedResourceCondition(r.ctx, r.client, mr, conditionResourcesHealthy); err != nil {
if err := tryUpdateManagedResourceCondition(ctx, r.client, mr, conditionResourcesHealthy); err != nil {
return ctrl.Result{}, fmt.Errorf("could not update the ManagedResource status: %+v ", err)
}
}
Expand Down
55 changes: 29 additions & 26 deletions pkg/controller/managedresource/reconciler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,13 @@ func (r *Reconciler) InjectLogger(l logr.Logger) error {

// Reconcile implements `reconcile.Reconciler`.
func (r *Reconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
ctx, cancel := context.WithTimeout(r.ctx, time.Minute)
defer cancel()

log := r.log.WithValues("object", req)

mr := &resourcesv1alpha1.ManagedResource{}
if err := r.client.Get(r.ctx, req.NamespacedName, mr); err != nil {
if err := r.client.Get(ctx, req.NamespacedName, mr); err != nil {
if apierrors.IsNotFound(err) {
log.Info("Stopping reconciliation of ManagedResource, as it has been deleted")
return reconcile.Result{}, nil
Expand All @@ -110,7 +113,7 @@ func (r *Reconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
// If the object should be deleted or the responsibility changed
// the actual deployments have to be deleted
if mr.DeletionTimestamp != nil || (action && !responsible) {
return r.delete(mr, log)
return r.delete(ctx, mr, log)
}

// If the deletion after a change of responsibility is still
Expand All @@ -119,13 +122,13 @@ func (r *Reconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
if responsible && !action {
return ctrl.Result{Requeue: true}, nil
}
return r.reconcile(mr, log)
return r.reconcile(ctx, mr, log)
}

func (r *Reconciler) reconcile(mr *resourcesv1alpha1.ManagedResource, log logr.Logger) (ctrl.Result, error) {
func (r *Reconciler) reconcile(ctx context.Context, mr *resourcesv1alpha1.ManagedResource, log logr.Logger) (ctrl.Result, error) {
log.Info("Starting to reconcile ManagedResource")

if err := utils.EnsureFinalizer(r.ctx, r.client, r.class.FinalizerName(), mr); err != nil {
if err := utils.EnsureFinalizer(ctx, r.client, r.class.FinalizerName(), mr); err != nil {
return reconcile.Result{}, err
}

Expand Down Expand Up @@ -154,9 +157,9 @@ func (r *Reconciler) reconcile(mr *resourcesv1alpha1.ManagedResource, log logr.L

for _, ref := range mr.Spec.SecretRefs {
secret := &corev1.Secret{}
if err := r.client.Get(r.ctx, client.ObjectKey{Namespace: mr.Namespace, Name: ref.Name}, secret); err != nil {
if err := r.client.Get(ctx, client.ObjectKey{Namespace: mr.Namespace, Name: ref.Name}, secret); err != nil {
conditionResourcesApplied = resourcesv1alpha1helper.UpdatedCondition(conditionResourcesApplied, resourcesv1alpha1.ConditionFalse, "CannotReadSecret", err.Error())
if err := tryUpdateManagedResourceConditions(r.ctx, r.client, mr, conditionResourcesApplied); err != nil {
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 @@ -263,12 +266,12 @@ func (r *Reconciler) reconcile(mr *resourcesv1alpha1.ManagedResource, log logr.L
}
conditionResourcesApplied = resourcesv1alpha1helper.UpdatedCondition(conditionResourcesApplied, resourcesv1alpha1.ConditionProgressing, reason, msg)

if err := tryUpdateManagedResourceConditions(r.ctx, r.client, mr, conditionResourcesHealthy, conditionResourcesApplied); err != nil {
if err := tryUpdateManagedResourceConditions(ctx, r.client, mr, conditionResourcesHealthy, conditionResourcesApplied); err != nil {
return ctrl.Result{}, fmt.Errorf("could not update the ManagedResource status: %+v", err)
}
}

if deletionPending, err := r.cleanOldResources(existingResourcesIndex, mr); err != nil {
if deletionPending, err := r.cleanOldResources(ctx, existingResourcesIndex, mr); err != nil {
var (
reason string
status resourcesv1alpha1.ConditionStatus
Expand All @@ -284,7 +287,7 @@ func (r *Reconciler) reconcile(mr *resourcesv1alpha1.ManagedResource, log logr.L
}

conditionResourcesApplied = resourcesv1alpha1helper.UpdatedCondition(conditionResourcesApplied, status, reason, err.Error())
if err := tryUpdateManagedResourceConditions(r.ctx, r.client, mr, conditionResourcesApplied); err != nil {
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 @@ -295,9 +298,9 @@ func (r *Reconciler) reconcile(mr *resourcesv1alpha1.ManagedResource, log logr.L
}
}

if err := r.applyNewResources(newResourcesObjects, mr.Spec.InjectLabels, equivalences); err != nil {
if err := r.applyNewResources(ctx, newResourcesObjects, mr.Spec.InjectLabels, equivalences); err != nil {
conditionResourcesApplied = resourcesv1alpha1helper.UpdatedCondition(conditionResourcesApplied, resourcesv1alpha1.ConditionFalse, resourcesv1alpha1.ConditionApplyFailed, err.Error())
if err := tryUpdateManagedResourceConditions(r.ctx, r.client, mr, conditionResourcesApplied); err != nil {
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 @@ -310,15 +313,15 @@ func (r *Reconciler) reconcile(mr *resourcesv1alpha1.ManagedResource, log logr.L
conditionResourcesApplied = resourcesv1alpha1helper.UpdatedCondition(conditionResourcesApplied, resourcesv1alpha1.ConditionTrue, resourcesv1alpha1.ConditionApplySucceeded, "All resources are applied.")
}

if err := tryUpdateManagedResourceStatus(r.ctx, r.client, mr, newResourcesObjectReferences, conditionResourcesApplied); err != nil {
if err := tryUpdateManagedResourceStatus(ctx, r.client, mr, newResourcesObjectReferences, conditionResourcesApplied); err != nil {
return ctrl.Result{}, fmt.Errorf("could not update the ManagedResource status: %+v", err)
}

log.Info("Finished to reconcile ManagedResource")
return ctrl.Result{RequeueAfter: r.syncPeriod}, nil
}

func (r *Reconciler) delete(mr *resourcesv1alpha1.ManagedResource, log logr.Logger) (ctrl.Result, error) {
func (r *Reconciler) delete(ctx context.Context, mr *resourcesv1alpha1.ManagedResource, log logr.Logger) (ctrl.Result, error) {
log.Info("Starting to delete ManagedResource")

conditionResourcesApplied := resourcesv1alpha1helper.GetOrInitCondition(mr.Status.Conditions, resourcesv1alpha1.ResourcesApplied)
Expand All @@ -333,11 +336,11 @@ func (r *Reconciler) delete(mr *resourcesv1alpha1.ManagedResource, log logr.Logg
msg = conditionResourcesApplied.Message
}
conditionResourcesApplied = resourcesv1alpha1helper.UpdatedCondition(conditionResourcesApplied, resourcesv1alpha1.ConditionProgressing, resourcesv1alpha1.ConditionDeletionPending, msg)
if err := tryUpdateManagedResourceConditions(r.ctx, r.client, mr, conditionResourcesApplied); err != nil {
if err := tryUpdateManagedResourceConditions(ctx, r.client, mr, conditionResourcesApplied); err != nil {
return ctrl.Result{}, fmt.Errorf("could not update the ManagedResource status: %+v", err)
}

if deletionPending, err := r.cleanOldResources(existingResourcesIndex, mr); err != nil {
if deletionPending, err := r.cleanOldResources(ctx, existingResourcesIndex, mr); err != nil {
var (
reason string
status resourcesv1alpha1.ConditionStatus
Expand All @@ -353,7 +356,7 @@ func (r *Reconciler) delete(mr *resourcesv1alpha1.ManagedResource, log logr.Logg
}

conditionResourcesApplied = resourcesv1alpha1helper.UpdatedCondition(conditionResourcesApplied, status, reason, err.Error())
if err := tryUpdateManagedResourceConditions(r.ctx, r.client, mr, conditionResourcesApplied); err != nil {
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 @@ -369,15 +372,15 @@ func (r *Reconciler) delete(mr *resourcesv1alpha1.ManagedResource, log logr.Logg

log.Info("All resources have been deleted, removing finalizers from ManagedResource")

if err := utils.DeleteFinalizer(r.ctx, r.client, r.class.FinalizerName(), mr); err != nil {
if err := utils.DeleteFinalizer(ctx, r.client, r.class.FinalizerName(), mr); err != nil {
return reconcile.Result{}, fmt.Errorf("error removing finalizer from ManagedResource: %+v", err)
}

log.Info("Finished to delete ManagedResource")
return ctrl.Result{}, nil
}

func (r *Reconciler) applyNewResources(newResourcesObjects []object, labelsToInject map[string]string, equivalences Equivalences) error {
func (r *Reconciler) applyNewResources(ctx context.Context, newResourcesObjects []object, labelsToInject map[string]string, equivalences Equivalences) error {
var (
results = make(chan error)
wg sync.WaitGroup
Expand All @@ -389,7 +392,7 @@ func (r *Reconciler) applyNewResources(newResourcesObjects []object, labelsToInj
// get all HPA and HVPA targetRefs to check if we should prevent overwriting replicas and/or resource requirements.
// VPAs don't have to be checked, as they don't update the spec directly and only mutate Pods via a MutatingWebhook
// and therefore don't interfere with the resource manager.
horizontallyScaledObjects, verticallyScaledObjects, err := computeAllScaledObjectKeys(r.ctx, r.targetClient)
horizontallyScaledObjects, verticallyScaledObjects, err := computeAllScaledObjectKeys(ctx, r.targetClient)
if err != nil {
return fmt.Errorf("failed to compute all HPA and HVPA target ref object keys: %w", err)
}
Expand All @@ -410,7 +413,7 @@ func (r *Reconciler) applyNewResources(newResourcesObjects []object, labelsToInj
r.log.Info("Applying", "resource", resource)

results <- retry.RetryOnConflict(retry.DefaultBackoff, func() error {
if operationResult, err := utils.TypedCreateOrUpdate(r.ctx, r.targetClient, r.targetScheme, current, r.alwaysUpdate, func() error {
if operationResult, err := utils.TypedCreateOrUpdate(ctx, r.targetClient, r.targetScheme, current, r.alwaysUpdate, func() error {
metadata, err := meta.Accessor(obj.obj)
if err != nil {
return fmt.Errorf("error getting metadata of object %q: %s", resource, err)
Expand All @@ -437,7 +440,7 @@ func (r *Reconciler) applyNewResources(newResourcesObjects []object, labelsToInj
}

if apierrors.IsInvalid(err) && operationResult == controllerutil.OperationResultUpdated && deleteOnInvalidUpdate(current) {
if deleteErr := r.targetClient.Delete(r.ctx, current); client.IgnoreNotFound(deleteErr) != nil {
if deleteErr := r.targetClient.Delete(ctx, current); client.IgnoreNotFound(deleteErr) != nil {
return fmt.Errorf("error deleting object %q after 'invalid' update error: %s", resource, deleteErr)
}
// return error directly, so that the create after delete will be retried
Expand Down Expand Up @@ -575,7 +578,7 @@ func annotationExistsAndValueTrue(meta metav1.Object, key string) bool {
return annotationExists && valueTrue
}

func (r *Reconciler) cleanOldResources(index *ObjectIndex, mr *resourcesv1alpha1.ManagedResource) (bool, error) {
func (r *Reconciler) cleanOldResources(ctx context.Context, index *ObjectIndex, mr *resourcesv1alpha1.ManagedResource) (bool, error) {
type output struct {
resource string
deletionPending bool
Expand Down Expand Up @@ -608,7 +611,7 @@ func (r *Reconciler) cleanOldResources(index *ObjectIndex, mr *resourcesv1alpha1
r.log.Info("Deleting", "resource", resource)

// get object before deleting to be able to do cleanup work for it
if err := r.targetClient.Get(r.ctx, client.ObjectKey{Namespace: ref.Namespace, Name: ref.Name}, obj); err != nil {
if err := r.targetClient.Get(ctx, client.ObjectKey{Namespace: ref.Namespace, Name: ref.Name}, obj); err != nil {
if !apierrors.IsNotFound(err) && !meta.IsNoMatchError(err) {
r.log.Error(err, "Error during deletion", "resource", resource)
results <- &output{resource, true, err}
Expand All @@ -626,7 +629,7 @@ func (r *Reconciler) cleanOldResources(index *ObjectIndex, mr *resourcesv1alpha1
return
}

if err := cleanup(r.ctx, r.targetClient, r.targetScheme, obj, deletePVCs); err != nil {
if err := cleanup(ctx, r.targetClient, r.targetScheme, obj, deletePVCs); err != nil {
r.log.Error(err, "Error during cleanup", "resource", resource)
results <- &output{resource: resource, deletionPending: true, err: err}
return
Expand All @@ -644,7 +647,7 @@ func (r *Reconciler) cleanOldResources(index *ObjectIndex, mr *resourcesv1alpha1
deleteOptions.PropagationPolicy = &deletePropagationForeground
}

if err := r.targetClient.Delete(r.ctx, obj, deleteOptions); err != nil {
if err := r.targetClient.Delete(ctx, obj, deleteOptions); err != nil {
if !apierrors.IsNotFound(err) && !meta.IsNoMatchError(err) {
r.log.Error(err, "Error during deletion", "resource", resource)
results <- &output{resource, true, err}
Expand Down
9 changes: 6 additions & 3 deletions pkg/controller/secret/reconciler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,13 @@ func (r *Reconciler) InjectLogger(l logr.Logger) error {

// Reconcile implements reconcile.Reconciler.
func (r *Reconciler) Reconcile(req reconcile.Request) (reconcile.Result, error) {
ctx, cancel := context.WithTimeout(r.ctx, time.Minute)
defer cancel()

log := r.log.WithValues("secret", req)

secret := &corev1.Secret{}
if err := r.client.Get(r.ctx, req.NamespacedName, secret); err != nil {
if err := r.client.Get(ctx, req.NamespacedName, secret); err != nil {
if apierrors.IsNotFound(err) {
log.Info("Stopping reconciliation of Secret, as it has been deleted")
return reconcile.Result{}, nil
Expand All @@ -73,7 +76,7 @@ func (r *Reconciler) Reconcile(req reconcile.Request) (reconcile.Result, error)
}

resourceList := &resourcesv1alpha1.ManagedResourceList{}
if err := r.client.List(r.ctx, resourceList, client.InNamespace(secret.Namespace)); err != nil {
if err := r.client.List(ctx, resourceList, client.InNamespace(secret.Namespace)); err != nil {
return reconcile.Result{}, fmt.Errorf("could not fetch ManagedResources in namespace of Secret: %+v", err)
}

Expand Down Expand Up @@ -103,7 +106,7 @@ func (r *Reconciler) Reconcile(req reconcile.Request) (reconcile.Result, error)
}

if addFinalizer || removeFinalizer {
if err := utils.TryUpdate(r.ctx, retry.DefaultBackoff, r.client, secret, func() error {
if err := utils.TryUpdate(ctx, retry.DefaultBackoff, r.client, secret, func() error {
secretFinalizers := sets.NewString(secret.Finalizers...)
if addFinalizer {
secretFinalizers.Insert(controllerFinalizer)
Expand Down
Loading