Skip to content

Commit

Permalink
improve readability
Browse files Browse the repository at this point in the history 
Signed-off-by: lsviben <sviben.lovro@gmail.com>
  • Loading branch information
@lsviben
lsviben committed Jun 27, 2023
1 parent 8aa5b1a commit a834fe4
Show file tree
Hide file tree
Showing 3 changed files with 366 additions and 20 deletions.
4 changes: 2 additions & 2 deletions pkg/meta/meta.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -423,8 +423,8 @@ func AllowsPropagationTo(from metav1.Object) map[types.NamespacedName]bool {

// IsPaused returns true if the object has the AnnotationKeyReconciliationPaused
// annotation set to `true`.
// Deprecated: Use reconciler.IsPaused instead to check if the Crossplane managed resource is paused as it includes
// the check for management policies.
// Deprecated: Use reconciler.IsPaused instead to check if the Crossplane
// managed resource is paused as it includes the check for management policies.
func IsPaused(o metav1.Object) bool {
return o.GetAnnotations()[AnnotationKeyReconciliationPaused] == "true"
}
90 changes: 72 additions & 18 deletions pkg/reconciler/managed/reconciler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -677,10 +677,12 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco
"external-name", meta.GetExternalName(managed),
)

managementPoliciesEnabled := r.features.Enabled(feature.EnableAlphaManagementPolicies)

// Check the pause annotation and return if it has the value "true"
// or if the management policies are enabled and the ManagementPolicy is empty.
// Log, publish an event and update the SYNC status condition.
if IsPaused(managed, r.features.Enabled(feature.EnableAlphaManagementPolicies)) {
if IsPaused(managed, managementPoliciesEnabled) {
log.Debug("Reconciliation is paused through the management policy or the pause annotation")
record.Event(managed, event.Normal(reasonReconciliationPaused, "Reconciliation is paused"))
managed.SetConditions(xpv1.ReconcilePaused())
Expand All @@ -696,7 +698,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco
// not realize that the controller is still trying to reconcile
// (and modify or delete) the resource since they forgot to enable the
// feature flag.
if !r.features.Enabled(feature.EnableAlphaManagementPolicies) && (managed.GetManagementPolicy() != nil && !ContainsOnlyManagementAction(managed.GetManagementPolicy(), xpv1.ManagementActionAll)) {
if isNonDefaultManagementPolicy(managementPoliciesEnabled, managed.GetManagementPolicy()) {
log.Debug(errManagementPolicy, "policy", managed.GetManagementPolicy())
record.Event(managed, event.Warning(reasonManagementPolicyNotEnabled, errors.New(errManagementPolicy)))
managed.SetConditions(xpv1.ReconcileError(errors.New(errManagementPolicy)))
Expand All @@ -706,7 +708,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco
// If managed resource has a deletion timestamp and a deletion policy of
// Orphan, we do not need to observe the external resource before attempting
// to unpublish connection details and remove finalizer.
if meta.WasDeleted(managed) && shouldOrphan(r.features.Enabled(feature.EnableAlphaManagementPolicies), managed) {
if meta.WasDeleted(managed) && shouldOrphan(managementPoliciesEnabled, managed) {
log = log.WithValues("deletion-timestamp", managed.GetDeletionTimestamp())

// Empty ConnectionDetails are passed to UnpublishConnection because we
Expand Down Expand Up @@ -821,7 +823,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco

// In the observe-only mode, !observation.ResourceExists will be an error
// case, and we will explicitly return this information to the user.
if r.features.Enabled(feature.EnableAlphaManagementPolicies) && !observation.ResourceExists && ContainsOnlyManagementAction(managed.GetManagementPolicy(), xpv1.ManagementActionObserve) {
if !observation.ResourceExists && isManagementPolicyObserveOnly(managementPoliciesEnabled, managed.GetManagementPolicy()) {
record.Event(managed, event.Warning(reasonCannotObserve, errors.New(errExternalResourceNotExist)))
managed.SetConditions(xpv1.ReconcileError(errors.Wrap(errors.New(errExternalResourceNotExist), errReconcileObserve)))
return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)
Expand All @@ -841,7 +843,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco
if meta.WasDeleted(managed) {
log = log.WithValues("deletion-timestamp", managed.GetDeletionTimestamp())

if observation.ResourceExists && !shouldOrphan(r.features.Enabled(feature.EnableAlphaManagementPolicies), managed) {
if observation.ResourceExists && !shouldOrphan(managementPoliciesEnabled, managed) {
if err := external.Delete(externalCtx, managed); err != nil {
// We'll hit this condition if we can't delete our external
// resource, for example if our provider credentials don't have
Expand Down Expand Up @@ -914,7 +916,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco
return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)
}

if !observation.ResourceExists && (!r.features.Enabled(feature.EnableAlphaManagementPolicies) || ContainsManagementAction(managed.GetManagementPolicy(), xpv1.ManagementActionCreate)) {
if !observation.ResourceExists && shouldCreate(managementPoliciesEnabled, managed.GetManagementPolicy()) {
// We write this annotation for two reasons. Firstly, it helps
// us to detect the case in which we fail to persist critical
// information (like the external name) that may be set by the
Expand Down Expand Up @@ -1004,7 +1006,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco
return reconcile.Result{Requeue: true}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)
}

if observation.ResourceLateInitialized && (!r.features.Enabled(feature.EnableAlphaManagementPolicies) || ContainsManagementAction(managed.GetManagementPolicy(), xpv1.ManagementActionLateInitialize)) {
if observation.ResourceLateInitialized && shouldLateInit(managementPoliciesEnabled, managed.GetManagementPolicy()) {
// Note that this update may reset any pending updates to the status of
// the managed resource from when it was observed above. This is because
// the API server replies to the update with its unchanged view of the
Expand Down Expand Up @@ -1037,7 +1039,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco
}

// skip the update if the management policy is set to ignore updates
if r.features.Enabled(feature.EnableAlphaManagementPolicies) && !ContainsManagementAction(managed.GetManagementPolicy(), xpv1.ManagementActionUpdate) {
if !shouldUpdate(managementPoliciesEnabled, managed.GetManagementPolicy()) {
log.Debug("Reconciliation succeeded", "requeue-after", time.Now().Add(r.pollInterval))
managed.SetConditions(xpv1.ReconcileSuccess())
return reconcile.Result{RequeueAfter: r.pollInterval}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)
Expand Down Expand Up @@ -1077,7 +1079,8 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco
return reconcile.Result{RequeueAfter: r.pollInterval}, errors.Wrap(r.client.Status().Update(ctx, managed), errUpdateManagedStatus)
}

// ContainsManagementAction returns true if the management policy contains the management action or ManagementActionAll.
// ContainsManagementAction returns true if the management policy contains
// the management action or ManagementActionAll.
func ContainsManagementAction(managementPolicy xpv1.ManagementPolicy, action xpv1.ManagementAction) bool {
for _, a := range managementPolicy {
if a == action || a == xpv1.ManagementActionAll {
Expand All @@ -1087,27 +1090,76 @@ func ContainsManagementAction(managementPolicy xpv1.ManagementPolicy, action xpv
return false
}

// ContainsOnlyManagementAction returns true if the management policy contains only the management action.
// ContainsOnlyManagementAction returns true if the management policy contains
// only the management action.
func ContainsOnlyManagementAction(managementPolicy xpv1.ManagementPolicy, action xpv1.ManagementAction) bool {
return len(managementPolicy) == 1 && managementPolicy[0] == action
}

// shouldUpdate returns true if the management policy is enabled, and
// contains the Update action.
func shouldUpdate(managementPolicyEnabled bool, policy xpv1.ManagementPolicy) bool {
return shouldDoAction(managementPolicyEnabled, policy, xpv1.ManagementActionUpdate)
}

// shouldLateInit returns true if the management policy is enabled, and
// contains the LateInitialize action.
func shouldLateInit(managementPolicyEnabled bool, policy xpv1.ManagementPolicy) bool {
return shouldDoAction(managementPolicyEnabled, policy, xpv1.ManagementActionLateInitialize)
}

// shouldCreate returns true if the management policy is enabled, and
// contains the Create action.
func shouldCreate(managementPolicyEnabled bool, policy xpv1.ManagementPolicy) bool {
return shouldDoAction(managementPolicyEnabled, policy, xpv1.ManagementActionCreate)
}

// shouldDoAction returns true if the management policy is enabled, and
// contains the supplied action.
func shouldDoAction(managementPolicyEnabled bool, policy xpv1.ManagementPolicy, action xpv1.ManagementAction) bool {
if !managementPolicyEnabled {
return true
}
return ContainsManagementAction(policy, action)
}

// isManagementPolicyObserveOnly returns true if the management policy is
// enabled, and contains only the Observe action.
func isManagementPolicyObserveOnly(managementPolicyEnabled bool, policy xpv1.ManagementPolicy) bool {
if !managementPolicyEnabled {
return false
}
return ContainsOnlyManagementAction(policy, xpv1.ManagementActionObserve)
}

// isNonDefaultManagementPolicy returns true if the management policy is
// not enabled, and set to a non-default value.
func isNonDefaultManagementPolicy(managementPolicyEnabled bool, policy xpv1.ManagementPolicy) bool {
if managementPolicyEnabled {
return false
}
return policy != nil && !ContainsOnlyManagementAction(policy, xpv1.ManagementActionAll)
}

// We need to be careful until we completely remove the deletionPolicy in favor
// of managementPolicies which conflicts with the managementPolicy regarding
// orphaning of the external resource. This function implements the proposal in
// the Observe Only design doc under the "Deprecation of `deletionPolicy`"
// section by triggering external resource deletion only when the deletionPolicy
// is set to "Delete" and the managementPolicy includes ManagementPolicyDeleteAction.
// section by triggering external resource deletion only when the
// deletionPolicy is set to "Delete" and the managementPolicy includes
// ManagementPolicyDeleteAction.
func shouldOrphan(managementPoliciesEnabled bool, managed resource.Managed) bool {
if !managementPoliciesEnabled {
return managed.GetDeletionPolicy() == xpv1.DeletionOrphan
}

// delete external resource if both the deletionPolicy and the managementPolicy are set to delete
// delete external resource if both the deletionPolicy and the
// managementPolicy are set to delete
if managed.GetDeletionPolicy() == xpv1.DeletionDelete && ContainsManagementAction(managed.GetManagementPolicy(), xpv1.ManagementActionDelete) {
return false
}
// if the managementPolicy is not default, and it contains the deletion action, we should delete the external resource
// if the managementPolicy is not default, and it contains the deletion
// action, we should delete the external resource
if !ContainsOnlyManagementAction(managed.GetManagementPolicy(), xpv1.ManagementActionAll) &&
ContainsManagementAction(managed.GetManagementPolicy(), xpv1.ManagementActionDelete) {
return false
Expand All @@ -1118,13 +1170,15 @@ func shouldOrphan(managementPoliciesEnabled bool, managed resource.Managed) bool
// DeletionOrphan && ManagementPolicy without Delete Action
// Conflicting cases:
// DeletionOrphan && Management Policy ["*"] (obeys non-default configuration)
// DeletionDelete && ManagementPolicy that does not include the Delete Action (obeys non-default configuration)
// DeletionDelete && ManagementPolicy that does not include the Delete
// Action (obeys non-default configuration)
return true
}

// IsPaused returns true if the object has the AnnotationKeyReconciliationPaused
// annotation set to `true` or if the managed policies alpha feature is enabled and
// the `spec.managementPolicy` array is empty.
// IsPaused returns true if the object has the
// AnnotationKeyReconciliationPaused annotation set to `true` or if the
// managed policies alpha feature is enabled and the `spec.managementPolicy`
// array is empty.
func IsPaused(o resource.Managed, managementPoliciesEnabled bool) bool {
return o.GetAnnotations()[meta.AnnotationKeyReconciliationPaused] == "true" ||
(managementPoliciesEnabled && len(o.GetManagementPolicy()) == 0)
Expand Down
Loading

0 comments on commit a834fe4

Please sign in to comment.