diff --git a/clm/cmd/apply.go b/clm/cmd/apply.go index 8a550a1..9be2fa3 100644 --- a/clm/cmd/apply.go +++ b/clm/cmd/apply.go @@ -117,7 +117,7 @@ func newApplyCmd() *cobra.Command { for { release.State = component.StateProcessing - ok, err := reconciler.Apply(context.TODO(), &release.Inventory, objects, namespace, ownerId, release.Revision) + ok, err := reconciler.Apply(context.TODO(), &release.Inventory, objects, namespace, ownerId, fmt.Sprintf("%d", release.Revision)) if err != nil { if !isEphmeralError(err) || errCount >= maxErrCount { return err diff --git a/internal/backoff/backoff.go b/internal/backoff/backoff.go index bbf7298..4bbe17e 100644 --- a/internal/backoff/backoff.go +++ b/internal/backoff/backoff.go @@ -22,16 +22,15 @@ type Backoff struct { func NewBackoff(maxDelay time.Duration) *Backoff { return &Backoff{ activities: make(map[any]any), - // resulting per-item backoff is the maximum of a 200-times-50ms-then-maxDelay per-item limiter, - // and an overall 5-per-second-burst-20 bucket limiter; - // as a consequence, we have up to - // - up to 20 almost immediate retries - // - then then a phase of 5 guaranteed retries per seconnd (could be more if burst capacity is refilled - // because of the duration of the reconcile logic execution itself) - // - finally (after 200 iterations) slow retries at the rate given by maxDelay + // resulting per-item backoff is the maximum of a 120-times-100ms-then-maxDelay per-item limiter, + // and an overall 1-per-second-burst-50 bucket limiter; + // as a consequence, we have + // - a phase of 10 retries per second for the first 5 seconds + // - then a phase of 1 retry per second for the next 60 seconds + // - finally slow retries at the rate given by maxDelay limiter: workqueue.NewMaxOfRateLimiter( - workqueue.NewItemFastSlowRateLimiter(50*time.Millisecond, maxDelay, 200), - &workqueue.BucketRateLimiter{Limiter: rate.NewLimiter(rate.Limit(5), 20)}, + workqueue.NewItemFastSlowRateLimiter(100*time.Millisecond, maxDelay, 120), + &workqueue.BucketRateLimiter{Limiter: rate.NewLimiter(rate.Limit(1), 50)}, ), } } diff --git a/pkg/component/reconciler.go b/pkg/component/reconciler.go index 3b4ae16..99beee9 100644 --- a/pkg/component/reconciler.go +++ b/pkg/component/reconciler.go @@ -62,17 +62,19 @@ import ( // TODO: currently, the reconciler always claims/owns dependent objects entirely; but due to server-side-apply it can happen that // only parts of an object are managed: other parts/fiels might be managed by other actors (or even other components); how to handle such cases? // TODO: we maybe should incorporate metadata.uid into the inventory to better detect (foreign) recreations of objects that were already managed by us +// TODO: maybe it would be better to have a dedicated StateTimeout? const ( - readyConditionReasonNew = "FirstSeen" - readyConditionReasonPending = "Pending" - readyConditionReasonProcessing = "Processing" - readyConditionReasonReady = "Ready" - readyConditionReasonError = "Error" - readyConditionReasonTimeout = "Timeout" - readyConditionReasonDeletionPending = "DeletionPending" - readyConditionReasonDeletionBlocked = "DeletionBlocked" - readyConditionReasonDeletionProcessing = "DeletionProcessing" + ReadyConditionReasonNew = "FirstSeen" + ReadyConditionReasonRetrying = "Reytrying" + ReadyConditionReasonRestarting = "Restarting" + ReadyConditionReasonProcessing = "Processing" + ReadyConditionReasonReady = "Ready" + ReadyConditionReasonError = "Error" + ReadyConditionReasonTimeout = "Timeout" + ReadyConditionReasonDeletionRetrying = "DeletionRetrying" + ReadyConditionReasonDeletionBlocked = "DeletionBlocked" + ReadyConditionReasonDeletionProcessing = "DeletionProcessing" triggerBufferSize = 1024 ) @@ -86,7 +88,9 @@ const ( // Post-hooks will only be called if the according operation (read, reconcile, delete) // has been successful. // Note: hooks may change the status of the component, but must not alter the metadata or spec, -// since changes might be persisted by the framework (e.g. when updating finalizers). +// since changes might be persisted by the framework (e.g. when updating finalizers), +// and since that may invalidate the already calculated component digest. +// TODO: we might even add a before-after check around each hook invocation to ensure this type HookFunc[T Component] func(ctx context.Context, clnt client.Client, component T) error // NewClientFunc is the function signature that can be used to modify or replace the default @@ -216,7 +220,7 @@ func (r *Reconciler[T]) Reconcile(ctx context.Context, req ctrl.Request) (result return ctrl.Result{}, errors.Wrap(err, "unexpected get error") } component.GetObjectKind().SetGroupVersionKind(r.groupVersionKind) - // componentDigest is populated after post-read hook phase + // componentDigest is populated after setting up the status handler, right before the post-read hook phase componentDigest := "" // fetch requeue interval, retry interval and timeout @@ -263,17 +267,36 @@ func (r *Reconciler[T]) Reconcile(ctx context.Context, req ctrl.Request) (result // this is correct, because in that case, the RequeueAfter will be determined through the RetriableError r.backoff.Forget(req) } - if status.State != StateProcessing || err != nil { - // clear ProcessingDigest and ProcessingSince in all non-error cases where state is StateProcessing - status.ProcessingDigest = "" - status.ProcessingSince = nil - } - if status.State == StateProcessing && err == nil && now.Sub(status.ProcessingSince.Time) >= timeout { - // TODO: maybe it would be better to have a dedicated StateTimeout? - // note: it is guaranteed that status.ProcessingSince is not nil here because - // - it was not cleared above because of the mutually exclusive clauses on status.State and err - // - it was set during reconcile when state was set to StateProcessing - status.SetState(StateError, readyConditionReasonTimeout, "Reconcilation of dependent resources timed out") + + haveTimeout := status.ProcessingSince != nil && now.Sub(status.ProcessingSince.Time) >= timeout + + if component.GetDeletionTimestamp().IsZero() && err == nil { + switch status.State { + case StateReady: + // if getting here from processing state, then trigger one additional immediate reconcile iteration; + // that helps certain implementing operators to check once more (in non-processing state) if something + // remains to be done + if status.ProcessingSince != nil { + result = ctrl.Result{RequeueAfter: 1 * time.Millisecond} + } + // clear processing state; note that processing will be off until the next component digest change; + // if (for whatever reason) the state would again flip to Processing, or an error would occur, then + // this would not start a new processing timeout cycle + status.ProcessingSince = nil + case StateProcessing: + // preserve processing state but set state to error (with timeout reason) if timeout is over + if haveTimeout { + status.SetState(StateError, ReadyConditionReasonTimeout, "Reconcilation of dependent resources timed out") + } + case StatePending, StateError: + // nothing to be done + case StateDeletionPending, StateDeleting: + // because these states can only occur if deletionTimestamp is not zero + panic("this cannot happen") + default: + // this would be an unknown state + panic("this cannot happen") + } } if err != nil { @@ -286,14 +309,22 @@ func (r *Reconciler[T]) Reconcile(ctx context.Context, req ctrl.Request) (result } // TODO: allow RetriableError to provide custom reason and message if component.GetDeletionTimestamp().IsZero() { - status.SetState(StatePending, readyConditionReasonPending, capitalize(retriableError.Error())) + if haveTimeout { + status.SetState(StatePending, ReadyConditionReasonTimeout, capitalize(retriableError.Error())) + } else { + status.SetState(StatePending, ReadyConditionReasonRetrying, capitalize(retriableError.Error())) + } } else { - status.SetState(StateDeletionPending, readyConditionReasonDeletionPending, capitalize(retriableError.Error())) + status.SetState(StateDeletionPending, ReadyConditionReasonDeletionRetrying, capitalize(retriableError.Error())) } result = ctrl.Result{RequeueAfter: *retryAfter} err = nil } else { - status.SetState(StateError, readyConditionReasonError, err.Error()) + if component.GetDeletionTimestamp().IsZero() && haveTimeout { + status.SetState(StateError, ReadyConditionReasonTimeout, err.Error()) + } else { + status.SetState(StateError, ReadyConditionReasonError, err.Error()) + } } } @@ -313,6 +344,8 @@ func (r *Reconciler[T]) Reconcile(ctx context.Context, req ctrl.Request) (result // TODO: should we move this behind the DeepEqual check below to reduce noise? // also note: it seems that no events will be written if the component's namespace is in deletion + // TODO: do not use GetState(); but accessing the condition directly is not safe (see caveat remark on the + // getCondition() and getOrAddCondition() methods) state, reason, message := status.GetState() var eventAnnotations map[string]string // TODO: formalize this into a real published interface @@ -322,9 +355,9 @@ func (r *Reconciler[T]) Reconcile(ctx context.Context, req ctrl.Request) (result // note: the passed component digest might be empty (that is, if we return before the post-read phase) // note: this interface is not released for usage; it may change without announcement if eventAnnotationProvider, ok := Component(component).(interface { - GetEventAnnotations(previousState State, componentDigest string) map[string]string + GetEventAnnotations(componentDigest string) map[string]string }); ok { - eventAnnotations = eventAnnotationProvider.GetEventAnnotations(savedStatus.State, componentDigest) + eventAnnotations = eventAnnotationProvider.GetEventAnnotations(componentDigest) } // TODO: sending events may block a little while (some seconds), in particular if enhanced recorders are installed through options.NewClient(), // such as the flux notfication recorder; should we therefore send the events asynchronously, or start synchronously and continue asynchronous @@ -338,14 +371,7 @@ func (r *Reconciler[T]) Reconcile(ctx context.Context, req ctrl.Request) (result if skipStatusUpdate { return } - if reflect.DeepEqual(status, savedStatus) { - return - } - // note: it's crucial to set the following timestamps late (otherwise the DeepEqual() check above would always be false) - // on the other hand it's a bit weird, because LastObservedAt will not be updated if no other changes have happened to the status; - // and same for the conditions' LastTransitionTime timestamps; - // maybe we should remove this optimization, and always do the Update() call status.LastObservedAt = &now for i := 0; i < len(status.Conditions); i++ { cond := &status.Conditions[i] @@ -361,8 +387,8 @@ func (r *Reconciler[T]) Reconcile(ctx context.Context, req ctrl.Request) (result // set a first status (and requeue, because the status update itself will not trigger another reconciliation because of the event filter set) if status.ObservedGeneration <= 0 { - status.SetState(StatePending, readyConditionReasonNew, "First seen") - return ctrl.Result{Requeue: true}, nil + status.SetState(StatePending, ReadyConditionReasonNew, "First seen") + return ctrl.Result{RequeueAfter: 1 * time.Millisecond}, nil } // resolve references @@ -371,11 +397,27 @@ func (r *Reconciler[T]) Reconcile(ctx context.Context, req ctrl.Request) (result return ctrl.Result{}, errors.Wrap(err, "error resolving references") } + if component.GetDeletionTimestamp().IsZero() { + if componentDigest != status.ProcessingDigest { + // start a new processing timeout cycle if the component digest changes; note that (other than status.ProcessingSince) + // status.ProcessingDigest is never cleared + status.ProcessingSince = &now + status.ProcessingDigest = componentDigest + r.backoff.Forget(req) + status.SetState(StateProcessing, ReadyConditionReasonRestarting, "Restarting processing due to component changes") + return ctrl.Result{RequeueAfter: 1 * time.Millisecond}, nil + } + } else { + status.ProcessingSince = nil + status.ProcessingDigest = "" + } + // run post-read hooks // note: it's important that this happens after deferring the status handler // TODO: enhance ctx with tailored logger and event recorder - // TODO: enhance ctx with the local client - hookCtx := NewContext(ctx).WithReconcilerName(r.name) + // TODO: should ctx enhanced with componentDigest? + hookCtx := NewContext(ctx). + WithReconcilerName(r.name) for hookOrder, hook := range r.postReadHooks { if err := hook(hookCtx, r.client, component); err != nil { return ctrl.Result{}, errors.Wrapf(err, "error running post-read hook (%d)", hookOrder) @@ -394,7 +436,7 @@ func (r *Reconciler[T]) Reconcile(ctx context.Context, req ctrl.Request) (result targetOptions := r.getOptionsForComponent(component) target := newReconcileTarget[T](r.name, r.id, localClient, targetClient, r.resourceGenerator, targetOptions) // TODO: enhance ctx with tailored logger and event recorder - // TODO: enhance ctx with the local client + // TODO: should ctx enhanced with componentDigest? hookCtx = NewContext(ctx). WithReconcilerName(r.name). WithLocalClient(localClient). @@ -412,7 +454,7 @@ func (r *Reconciler[T]) Reconcile(ctx context.Context, req ctrl.Request) (result // this is necessary because the update call invalidates potential changes done to the component by the post-read // hook above; this means, not to the object itself, but for example to loaded secrets or config maps; // in the following round trip, the finalizer will already be there, and the update will not happen again - return ctrl.Result{Requeue: true}, nil + return ctrl.Result{RequeueAfter: 1 * time.Millisecond}, nil } log.V(2).Info("reconciling dependent resources") @@ -421,7 +463,7 @@ func (r *Reconciler[T]) Reconcile(ctx context.Context, req ctrl.Request) (result return ctrl.Result{}, errors.Wrapf(err, "error running pre-reconcile hook (%d)", hookOrder) } } - ok, processingDigest, err := target.Apply(ctx, component, componentDigest) + ok, err := target.Apply(ctx, component, componentDigest) if err != nil { log.V(1).Info("error while reconciling dependent resources") return ctrl.Result{}, errors.Wrap(err, "error reconciling dependent resources") @@ -435,20 +477,15 @@ func (r *Reconciler[T]) Reconcile(ctx context.Context, req ctrl.Request) (result log.V(1).Info("all dependent resources successfully reconciled") status.AppliedGeneration = component.GetGeneration() status.LastAppliedAt = &now - status.SetState(StateReady, readyConditionReasonReady, "Dependent resources successfully reconciled") + status.SetState(StateReady, ReadyConditionReasonReady, "Dependent resources successfully reconciled") return ctrl.Result{RequeueAfter: requeueInterval}, nil } else { log.V(1).Info("not all dependent resources successfully reconciled") - if processingDigest != status.ProcessingDigest { - status.ProcessingDigest = processingDigest - status.ProcessingSince = &now - r.backoff.Forget(req) - } if !reflect.DeepEqual(status.Inventory, savedStatus.Inventory) { r.backoff.Forget(req) } - status.SetState(StateProcessing, readyConditionReasonProcessing, "Reconcilation of dependent resources triggered; waiting until all dependent resources are ready") - return ctrl.Result{RequeueAfter: r.backoff.Next(req, readyConditionReasonProcessing)}, nil + status.SetState(StateProcessing, ReadyConditionReasonProcessing, "Reconcilation of dependent resources triggered; waiting until all dependent resources are ready") + return ctrl.Result{RequeueAfter: r.backoff.Next(req, ReadyConditionReasonProcessing)}, nil } } else { for hookOrder, hook := range r.preDeleteHooks { @@ -466,15 +503,15 @@ func (r *Reconciler[T]) Reconcile(ctx context.Context, req ctrl.Request) (result log.V(1).Info("deletion not allowed") // TODO: have an additional StateDeletionBlocked? // TODO: eliminate this msg logic - status.SetState(StateDeleting, readyConditionReasonDeletionBlocked, "Deletion blocked: "+msg) - return ctrl.Result{RequeueAfter: 1*time.Second + r.backoff.Next(req, readyConditionReasonDeletionBlocked)}, nil + status.SetState(StateDeleting, ReadyConditionReasonDeletionBlocked, "Deletion blocked: "+msg) + return ctrl.Result{RequeueAfter: 1*time.Second + r.backoff.Next(req, ReadyConditionReasonDeletionBlocked)}, nil } if len(slices.Remove(component.GetFinalizers(), *r.options.Finalizer)) > 0 { // deletion is blocked because of foreign finalizers log.V(1).Info("deleted blocked due to existence of foreign finalizers") // TODO: have an additional StateDeletionBlocked? - status.SetState(StateDeleting, readyConditionReasonDeletionBlocked, "Deletion blocked due to existing foreign finalizers") - return ctrl.Result{RequeueAfter: 1*time.Second + r.backoff.Next(req, readyConditionReasonDeletionBlocked)}, nil + status.SetState(StateDeleting, ReadyConditionReasonDeletionBlocked, "Deletion blocked due to existing foreign finalizers") + return ctrl.Result{RequeueAfter: 1*time.Second + r.backoff.Next(req, ReadyConditionReasonDeletionBlocked)}, nil } // deletion case log.V(2).Info("deleting dependent resources") @@ -507,8 +544,8 @@ func (r *Reconciler[T]) Reconcile(ctx context.Context, req ctrl.Request) (result if !reflect.DeepEqual(status.Inventory, savedStatus.Inventory) { r.backoff.Forget(req) } - status.SetState(StateDeleting, readyConditionReasonDeletionProcessing, "Deletion of dependent resources triggered; waiting until dependent resources are deleted") - return ctrl.Result{RequeueAfter: r.backoff.Next(req, readyConditionReasonDeletionProcessing)}, nil + status.SetState(StateDeleting, ReadyConditionReasonDeletionProcessing, "Deletion of dependent resources triggered; waiting until dependent resources are deleted") + return ctrl.Result{RequeueAfter: r.backoff.Next(req, ReadyConditionReasonDeletionProcessing)}, nil } } } diff --git a/pkg/component/reference.go b/pkg/component/reference.go index 6eb3931..8a6b653 100644 --- a/pkg/component/reference.go +++ b/pkg/component/reference.go @@ -49,6 +49,7 @@ type ConfigMapReference struct { } func (r *ConfigMapReference) load(ctx context.Context, clnt client.Client, namespace string, ignoreNotFound bool) error { + // TODO: shouldn't we panic if already loaded? configMap := &corev1.ConfigMap{} if err := clnt.Get(ctx, apitypes.NamespacedName{Namespace: namespace, Name: r.Name}, configMap); err != nil { if apierrors.IsNotFound(err) { @@ -67,7 +68,7 @@ func (r *ConfigMapReference) load(ctx context.Context, clnt client.Client, names func (r *ConfigMapReference) digest() string { if !r.loaded { - // TODO: shouldn't we panic here? + // note: we can't panic here because this might be called in case of not-found situations return "" } return calculateDigest(r.data) @@ -97,6 +98,7 @@ type ConfigMapKeyReference struct { } func (r *ConfigMapKeyReference) load(ctx context.Context, clnt client.Client, namespace string, ignoreNotFound bool, fallbackKeys ...string) error { + // TODO: shouldn't we panic if already loaded? configMap := &corev1.ConfigMap{} if err := clnt.Get(ctx, apitypes.NamespacedName{Namespace: namespace, Name: r.Name}, configMap); err != nil { if apierrors.IsNotFound(err) { @@ -130,7 +132,7 @@ func (r *ConfigMapKeyReference) load(ctx context.Context, clnt client.Client, na func (r *ConfigMapKeyReference) digest() string { if !r.loaded { - // TODO: shouldn't we panic here? + // note: we can't panic here because this might be called in case of not-found situations return "" } return sha256hex([]byte(r.value)) @@ -157,6 +159,7 @@ type SecretReference struct { } func (r *SecretReference) load(ctx context.Context, clnt client.Client, namespace string, ignoreNotFound bool) error { + // TODO: shouldn't we panic if already loaded? secret := &corev1.Secret{} if err := clnt.Get(ctx, apitypes.NamespacedName{Namespace: namespace, Name: r.Name}, secret); err != nil { if apierrors.IsNotFound(err) { @@ -175,7 +178,7 @@ func (r *SecretReference) load(ctx context.Context, clnt client.Client, namespac func (r *SecretReference) digest() string { if !r.loaded { - // TODO: shouldn't we panic here? + // note: we can't panic here because this might be called in case of not-found situations return "" } return calculateDigest(r.data) @@ -205,6 +208,7 @@ type SecretKeyReference struct { } func (r *SecretKeyReference) load(ctx context.Context, clnt client.Client, namespace string, ignoreNotFound bool, fallbackKeys ...string) error { + // TODO: shouldn't we panic if already loaded? secret := &corev1.Secret{} if err := clnt.Get(ctx, apitypes.NamespacedName{Namespace: namespace, Name: r.Name}, secret); err != nil { if apierrors.IsNotFound(err) { @@ -238,7 +242,7 @@ func (r *SecretKeyReference) load(ctx context.Context, clnt client.Client, names func (r *SecretKeyReference) digest() string { if !r.loaded { - // TODO: shouldn't we panic here? + // note: we can't panic here because this might be called in case of not-found situations return "" } return sha256hex(r.value) @@ -253,16 +257,31 @@ func (r *SecretKeyReference) Value() []byte { return r.value } +// Generic reference. All occurrences in the component's spec of types implementing this interface are automatically resolved +// by the framework during reconcile by calling the Load() method. The digests returned by the Digest() methods are +// incorporated into the component's digest. +type Reference[T Component] interface { + // Load the referenced content. The framework calls this at most once. So it is ok if implementation + // errors out or even panics if invoked more than once. The implementation may skip loading in certain cases, + // for example if deletion is ongoing. + Load(ctx context.Context, clnt client.Client, component T) error + // Return a digest of the referenced content. This digest is incorporated into the component digest which + // is passed to generators and hooks (per context) and which decides when the processing timer is reset, + // and therefore influences the timeout behavior of the compoment. In case the reference is not loaded, + // the implementation should return the empty string. + Digest() string +} + func resolveReferences[T Component](ctx context.Context, clnt client.Client, component T) (string, error) { digestData := make(map[string]any) spec := getSpec(component) digestData["generation"] = component.GetGeneration() digestData["annotations"] = component.GetAnnotations() + // TODO: including spec into the digest is actually not required (since generation is included) digestData["spec"] = spec if err := walk.Walk(spec, func(x any, path []string, tag reflect.StructTag) error { // note: this must() is ok because marshalling []string should always work rawPath := must(json.Marshal(path)) - // TODO: allow arbitrary loadable types (with an interface LoadableReference or similar) switch r := x.(type) { case *ConfigMapReference: if r == nil { @@ -308,6 +327,14 @@ func resolveReferences[T Component](ctx context.Context, clnt client.Client, com return err } digestData["refs:"+string(rawPath)] = r.digest() + case Reference[T]: + if v := reflect.ValueOf(r); r == nil || v.Kind() == reflect.Pointer && v.IsNil() { + return nil + } + if err := r.Load(ctx, clnt, component); err != nil { + return err + } + digestData["refs:"+string(rawPath)] = r.Digest() } return nil }); err != nil { diff --git a/pkg/component/target.go b/pkg/component/target.go index f2d99da..63e77d5 100644 --- a/pkg/component/target.go +++ b/pkg/component/target.go @@ -35,7 +35,7 @@ func newReconcileTarget[T Component](reconcilerName string, reconcilerId string, } } -func (t *reconcileTarget[T]) Apply(ctx context.Context, component T, componentDigest string) (bool, string, error) { +func (t *reconcileTarget[T]) Apply(ctx context.Context, component T, componentDigest string) (bool, error) { //log := log.FromContext(ctx) namespace := "" name := "" @@ -63,12 +63,12 @@ func (t *reconcileTarget[T]) Apply(ctx context.Context, component T, componentDi WithComponentDigest(componentDigest) objects, err := t.resourceGenerator.Generate(generateCtx, namespace, name, component.GetSpec()) if err != nil { - return false, "", errors.Wrap(err, "error rendering manifests") + return false, errors.Wrap(err, "error rendering manifests") } - ok, err := t.reconciler.Apply(ctx, &status.Inventory, objects, namespace, ownerId, component.GetGeneration()) + ok, err := t.reconciler.Apply(ctx, &status.Inventory, objects, namespace, ownerId, componentDigest) - return ok, calculateDigest(componentDigest, objects), err + return ok, err } func (t *reconcileTarget[T]) Delete(ctx context.Context, component T) (bool, error) { diff --git a/pkg/reconciler/reconciler.go b/pkg/reconciler/reconciler.go index cd03a60..35bea63 100644 --- a/pkg/reconciler/reconciler.go +++ b/pkg/reconciler/reconciler.go @@ -222,7 +222,7 @@ func NewReconciler(name string, clnt cluster.Client, options ReconcilerOptions) // Objects which are instances of namespaced types will be placed into the namespace passed to Apply(), if they have no namespace defined in their manifest. // An update of an existing object will be performed if it is considered to be out of sync; that means: // - the object's manifest has changed, and the effective reconcile policy is ReconcilePolicyOnObjectChange or ReconcilePolicyOnObjectOrComponentChange or -// - the specified component revision has changed and the effective reconcile policy is ReconcilePolicyOnObjectOrComponentChange or +// - the specified component has changed and the effective reconcile policy is ReconcilePolicyOnObjectOrComponentChange or // - periodically after forceReapplyPeriod. // // The update itself will be done as follows: @@ -242,10 +242,12 @@ func NewReconciler(name string, clnt cluster.Client, options ReconcilerOptions) // This method will change the passed inventory (add or remove elements, change elements). If Apply() returns true, then all objects are successfully reconciled; // otherwise, if it returns false, the caller should re-call it periodically, until it returns true. In any case, the passed inventory should match the state of the // inventory after the previous invocation of Apply(); usually, the caller saves the inventory after calling Apply(), and loads it before calling Apply(). -// The namespace and ownerId arguments should not be changed across subsequent invocations of Apply(); the componentRevision should be incremented only. +// The namespace and ownerId arguments should not be changed across subsequent invocations of Apply(); the supplied componentDigest is included into the +// digest of dependent objects if the effective reconcile policy is ReconcilePolicyOnObjectOrComponentChange (such that in this case, a change of componentDigest +// triggers an immediate reconciliation of all dependent objects). // // Also note: it is absolutely crucial that this method returns (true, nil) immediately (on the first call) if everything is already in the right state. -func (r *Reconciler) Apply(ctx context.Context, inventory *[]*InventoryItem, objects []client.Object, namespace string, ownerId string, componentRevision int64) (bool, error) { +func (r *Reconciler) Apply(ctx context.Context, inventory *[]*InventoryItem, objects []client.Object, namespace string, ownerId string, componentDigest string) (bool, error) { var err error log := log.FromContext(ctx) @@ -417,7 +419,7 @@ func (r *Reconciler) Apply(ctx context.Context, inventory *[]*InventoryItem, obj // calculate object digest // note: if the effective reconcile policy of an object changes, it will always be reconciled at least one more time; // this is in particular the case if the policy changes from or to ReconcilePolicyOnce. - digest, err := calculateObjectDigest(object, componentRevision, getReconcilePolicy(object)) + digest, err := calculateObjectDigest(object, componentDigest, getReconcilePolicy(object)) if err != nil { return false, errors.Wrapf(err, "error calculating digest for object %s", types.ObjectKeyToString(object)) } diff --git a/pkg/reconciler/types.go b/pkg/reconciler/types.go index d22f47d..bbe05e0 100644 --- a/pkg/reconciler/types.go +++ b/pkg/reconciler/types.go @@ -56,7 +56,7 @@ const ( // Reconcile the dependent object if its manifest, as produced by the generator, changes. ReconcilePolicyOnObjectChange ReconcilePolicy = "OnObjectChange" // Reconcile the dependent object if its manifest, as produced by the generator, changes, or if the owning - // component changes (identified by a change of its metadata.generation). + // component changes (identified by a change of its digest, including references). ReconcilePolicyOnObjectOrComponentChange ReconcilePolicy = "OnObjectOrComponentChange" // Reconcile the dependent object only once; afterwards it will never be touched again by the reconciler. ReconcilePolicyOnce ReconcilePolicy = "Once" diff --git a/pkg/reconciler/util.go b/pkg/reconciler/util.go index db1c9fd..f5c62c3 100644 --- a/pkg/reconciler/util.go +++ b/pkg/reconciler/util.go @@ -58,7 +58,7 @@ func checkRange(x int, min int, max int) error { return nil } -func calculateObjectDigest(obj client.Object, revision int64, reconcilePolicy ReconcilePolicy) (string, error) { +func calculateObjectDigest(obj client.Object, componentDigest string, reconcilePolicy ReconcilePolicy) (string, error) { if reconcilePolicy == ReconcilePolicyOnce { return "__once__", nil } @@ -79,7 +79,8 @@ func calculateObjectDigest(obj client.Object, revision int64, reconcilePolicy Re digest := sha256hex(raw) if reconcilePolicy == ReconcilePolicyOnObjectOrComponentChange { - digest = fmt.Sprintf("%s@%d", digest, revision) + // TODO: this becomes rather long; should we hash it once more? + digest = fmt.Sprintf("%s@%s", digest, componentDigest) } return digest, nil