Add PreReconcileOwnerCheck and implement for Kusto Databases (#4976)

* Move PreReconcileCheckResult to separate file for sharing

* Create PreReconcileOwnerChecker

* Tidy up PrereconcileChecker

* Update reconciler

* Modify Kusto database extension

* Add comment

Author: theunrepentantgeek

v2/api/kusto/customizations/database_extensions.go

@@ -19,23 +19,23 @@ import (
 	"github.com/Azure/azure-service-operator/v2/pkg/genruntime/extensions"
 )
 
-var _ extensions.PreReconciliationChecker = &ClusterExtension{}
+var _ extensions.PreReconciliationOwnerChecker = &DatabaseExtension{}
 
 // Ensure we're dealing with the hub version of Cluster
 // If this no longer compiles (due to a newer version being imported), change the import above to that version.
 var _ conversion.Hub = &kusto.Cluster{}
 
-// PreReconcileCheck is called before the reconciliation of the resource to see if the cluster
+// PreReconcileOwnerCheck is called before the reconciliation of the resource to see if the cluster
 // is in a state that will allow reconciliation to proceed.
 // We can't try to create/update a Database unless the cluster is in a state that allows it.
-func (ext *DatabaseExtension) PreReconcileCheck(
+// We use PreReconcileOwnerCheck instead of PreReconcileCheck because you can't even GET a database if the cluster is powered down.
+func (ext *DatabaseExtension) PreReconcileOwnerCheck(
 	ctx context.Context,
-	obj genruntime.MetaObject,
 	owner genruntime.MetaObject,
 	resourceResolver *resolver.Resolver,
 	armClient_ *genericarmclient.GenericClient,
 	log logr.Logger,
-	next extensions.PreReconcileCheckFunc,
+	next extensions.PreReconcileOwnerCheckFunc,
 ) (extensions.PreReconcileCheckResult, error) {
 	// Check to see if the owning cluster is in a state that will block us from reconciling
 	// Owner nil can happen if the owner of the database is referenced by armID
@@ -58,5 +58,5 @@ func (ext *DatabaseExtension) PreReconcileCheck(
 		}
 	}
 
-	return next(ctx, obj, owner, resourceResolver, armClient_, log)
+	return next(ctx, owner, resourceResolver, armClient_, log)
 }

v2/internal/reconcilers/arm/azure_generic_arm_reconciler_instance.go

@@ -296,29 +296,49 @@ func (r *azureDeploymentReconcilerInstance) BeginCreateOrUpdateResource(
 	return ctrl.Result{Requeue: true}, nil
 }
 
-func (r *azureDeploymentReconcilerInstance) preReconciliationCheck(ctx context.Context) (extensions.PreReconcileCheckResult, error) {
-	// Create a checker for access to the extension point, if required
+func (r *azureDeploymentReconcilerInstance) preReconciliationCheck(
+	ctx context.Context,
+) (extensions.PreReconcileCheckResult, error) {
+	// Check to see which extensions are available
 	checker, extensionFound := extensions.CreatePreReconciliationChecker(r.Extension)
-	if !extensionFound {
-		// No extension found, nothing to do
+	ownerChecker, ownerExtensionFound := extensions.CreatePreReconciliationOwnerChecker(r.Extension)
+
+	if !extensionFound && !ownerExtensionFound {
+		// No extensions found, nothing to do
 		return extensions.ProceedWithReconcile(), nil
 	}
 
-	// Having a checker requires our resource to have an up-to-date status
-	r.Log.V(Verbose).Info("Refreshing Status of resource")
+	// Load owner details so it has an an up-to-date status
+	ownerDetails, ownerErr := r.ResourceResolver.ResolveOwner(ctx, r.Obj)
+	if ownerErr != nil {
+		// We can't obtain the owner, so we can't run either extension
+		return extensions.PreReconcileCheckResult{}, ownerErr
+	}
+
+	// Run the PreReconciliationOwnerChecker if we have one
+	if ownerExtensionFound {
+		check, checkErr := ownerChecker(ctx, ownerDetails.Owner, r.ResourceResolver, r.ARMConnection.Client(), r.Log)
+		if checkErr != nil {
+			// Something went wrong running the check.
+			return extensions.PreReconcileCheckResult{}, checkErr
+		}
+
+		// If the check says we're postponing reconcile, we're done for now as there's nothing to do.
+		if check.PostponeReconciliation() {
+			return extensions.PreReconcileCheckResult{}, nil
+		}
+	}
+
+	// Load resource details so it also has an up-to-date status
+	// We defer this until after we've done the owner check as if that says postpone
+	// we don't need to do this work.
+	// Plus, this avoids errors if the owner is in a state where going a GET on the resource will fail.
 	statusErr := r.updateStatus(ctx)
 	if statusErr != nil && !genericarmclient.IsNotFoundError(statusErr) {
 		// We have an error, and it's not because the resource doesn't exist yet
 		return extensions.PreReconcileCheckResult{}, statusErr
 	}
 
-	// We also need to have our owner, it too with an up-to-date status
-	ownerDetails, ownerErr := r.ResourceResolver.ResolveOwner(ctx, r.Obj)
-	if ownerErr != nil {
-		// We can't obtain the owner, so we can't run the extension
-		return extensions.PreReconcileCheckResult{}, ownerErr
-	}
-
 	// Run our pre-reconciliation checker
 	check, checkErr := checker(ctx, r.Obj, ownerDetails.Owner, r.ResourceResolver, r.ARMConnection.Client(), r.Log)
 	if checkErr != nil {

v2/pkg/genruntime/extensions/prereconcile_check_result.go

@@ -0,0 +1,78 @@
+/*
+ * Copyright (c) Microsoft Corporation.
+ * Licensed under the MIT license.
+ */
+
+package extensions
+
+import (
+	"github.com/rotisserie/eris"
+
+	"github.com/Azure/azure-service-operator/v2/pkg/genruntime/conditions"
+)
+
+type PreReconcileCheckResult struct {
+	action   preReconcileCheckResultType
+	severity conditions.ConditionSeverity
+	reason   conditions.Reason
+	message  string
+}
+
+// ProceedWithReconcile indicates that a resource is ready for reconciliation by returning a PreReconcileCheckResult
+// with action `Proceed`.
+func ProceedWithReconcile() PreReconcileCheckResult {
+	return PreReconcileCheckResult{
+		action: preReconcileCheckResultTypeProceed,
+	}
+}
+
+// BlockReconcile indicates reconciliation of a resource is currently blocked by returning a PreReconcileCheckResult
+// with action `Block`. The reconciliation will automatically be retried after a short delay.
+// reason is an explanatory reason to show to the user via a warning condition on the resource.
+func BlockReconcile(reason string) PreReconcileCheckResult {
+	return PreReconcileCheckResult{
+		action:   preReconcileCheckResultTypeBlock,
+		severity: conditions.ConditionSeverityWarning,
+		reason:   conditions.ReasonReconcileBlocked,
+		message:  reason,
+	}
+}
+
+// PostponeReconcile indicates reconciliation of a resource is not currently required by returning a
+// PreReconcileCheckResult with action `Postpone`. Reconciliation will not be retried until the usual scheduled check.
+func PostponeReconcile() PreReconcileCheckResult {
+	return PreReconcileCheckResult{
+		action:   preReconcileCheckResultTypePostpone,
+		severity: conditions.ConditionSeverityInfo,
+		reason:   conditions.ReasonReconcilePostponed,
+	}
+}
+
+func (r PreReconcileCheckResult) PostponeReconciliation() bool {
+	return r.action == preReconcileCheckResultTypePostpone
+}
+
+func (r PreReconcileCheckResult) BlockReconciliation() bool {
+	return r.action == preReconcileCheckResultTypeBlock
+}
+
+func (r PreReconcileCheckResult) Message() string {
+	return r.message
+}
+
+// CreateConditionError returns an error that can be used to set a condition on the resource.
+func (r PreReconcileCheckResult) CreateConditionError() error {
+	return conditions.NewReadyConditionImpactingError(
+		eris.New(r.message),
+		r.severity,
+		r.reason)
+}
+
+// PreReconcileCheckResultType is the type of result returned by PreReconcileCheck.
+type preReconcileCheckResultType string
+
+const (
+	preReconcileCheckResultTypeBlock    preReconcileCheckResultType = "Block"
+	preReconcileCheckResultTypeProceed  preReconcileCheckResultType = "Proceed"
+	preReconcileCheckResultTypePostpone preReconcileCheckResultType = "Postpone"
+)

v2/pkg/genruntime/extensions/prereconciliation_checker.go

@@ -11,12 +11,10 @@ import (
 	. "github.com/Azure/azure-service-operator/v2/internal/logging"
 
 	"github.com/go-logr/logr"
-	"github.com/rotisserie/eris"
 
 	"github.com/Azure/azure-service-operator/v2/internal/genericarmclient"
 	"github.com/Azure/azure-service-operator/v2/internal/resolver"
 	"github.com/Azure/azure-service-operator/v2/pkg/genruntime"
-	"github.com/Azure/azure-service-operator/v2/pkg/genruntime/conditions"
 )
 
 // PreReconciliationChecker is implemented by resources that want to do extra checks before proceeding with
@@ -28,6 +26,8 @@ type PreReconciliationChecker interface {
 	// Returns BlockReconcile and a human-readable reason if the reconciliation should be skipped.
 	// ctx is the current operation context.
 	// obj is the resource about to be reconciled. The resource's State will be freshly updated.
+	// owner is the owner of the resource about to be reconciled. The owner's State will be freshly updated. May be nil
+	// if the resource has no owner, or if it has been referenced via ARMID directly.
 	// kubeClient allows access to the cluster for any required queries.
 	// armClient allows access to ARM for any required queries.
 	// log is the logger for the current operation.
@@ -52,72 +52,6 @@ type PreReconcileCheckFunc func(
 	log logr.Logger,
 ) (PreReconcileCheckResult, error)
 
-type PreReconcileCheckResult struct {
-	action   preReconcileCheckResultType
-	severity conditions.ConditionSeverity
-	reason   conditions.Reason
-	message  string
-}
-
-// ProceedWithReconcile indicates that a resource is ready for reconciliation by returning a PreReconcileCheckResult
-// with action `Proceed`.
-func ProceedWithReconcile() PreReconcileCheckResult {
-	return PreReconcileCheckResult{
-		action: preReconcileCheckResultTypeProceed,
-	}
-}
-
-// BlockReconcile indicates reconciliation of a resource is currently blocked by returning a PreReconcileCheckResult
-// with action `Block`. The reconciliation will automatically be retried after a short delay.
-// reason is an explanatory reason to show to the user via a warning condition on the resource.
-func BlockReconcile(reason string) PreReconcileCheckResult {
-	return PreReconcileCheckResult{
-		action:   preReconcileCheckResultTypeBlock,
-		severity: conditions.ConditionSeverityWarning,
-		reason:   conditions.ReasonReconcileBlocked,
-		message:  reason,
-	}
-}
-
-// PostponeReconcile indicates reconciliation of a resource is not currently required by returning a
-// PreReconcileCheckResult with action `Postpone`. Reconciliation will not be retried until the usual scheduled check.
-func PostponeReconcile() PreReconcileCheckResult {
-	return PreReconcileCheckResult{
-		action:   preReconcileCheckResultTypePostpone,
-		severity: conditions.ConditionSeverityInfo,
-		reason:   conditions.ReasonReconcilePostponed,
-	}
-}
-
-func (r PreReconcileCheckResult) PostponeReconciliation() bool {
-	return r.action == preReconcileCheckResultTypePostpone
-}
-
-func (r PreReconcileCheckResult) BlockReconciliation() bool {
-	return r.action == preReconcileCheckResultTypeBlock
-}
-
-func (r PreReconcileCheckResult) Message() string {
-	return r.message
-}
-
-// CreateConditionError returns an error that can be used to set a condition on the resource.
-func (r PreReconcileCheckResult) CreateConditionError() error {
-	return conditions.NewReadyConditionImpactingError(
-		eris.New(r.message),
-		r.severity,
-		r.reason)
-}
-
-// PreReconcileCheckResultType is the type of result returned by PreReconcileCheck.
-type preReconcileCheckResultType string
-
-const (
-	preReconcileCheckResultTypeBlock    preReconcileCheckResultType = "Block"
-	preReconcileCheckResultTypeProceed  preReconcileCheckResultType = "Proceed"
-	preReconcileCheckResultTypePostpone preReconcileCheckResultType = "Postpone"
-)
-
 // CreatePreReconciliationChecker creates a checker that can be used to check if a resource is ready for reconciliation.
 // If the resource in question has not implemented the PreReconciliationChecker interface, the provided default checker
 // is returned directly.
@@ -128,7 +62,7 @@ func CreatePreReconciliationChecker(
 ) (PreReconcileCheckFunc, bool) {
 	impl, ok := host.(PreReconciliationChecker)
 	if !ok {
-		return alwaysReconcile, false
+		return preReconciliationCheckerAlways, false
 	}
 
 	return func(
@@ -141,7 +75,7 @@ func CreatePreReconciliationChecker(
 	) (PreReconcileCheckResult, error) {
 		log.V(Status).Info("Extension pre-reconcile check running")
 
-		result, err := impl.PreReconcileCheck(ctx, obj, owner, resourceResolver, armClient, log, alwaysReconcile)
+		result, err := impl.PreReconcileCheck(ctx, obj, owner, resourceResolver, armClient, log, preReconciliationCheckerAlways)
 		if err != nil {
 			log.V(Status).Info(
 				"Extension pre-reconcile check failed",
@@ -160,10 +94,10 @@ func CreatePreReconciliationChecker(
 	}, true
 }
 
-// alwaysReconcile is a PreReconciliationChecker that always indicates a reconciliation is required.
+// preReconciliationCheckerAlways is a PreReconciliationChecker that always indicates a reconciliation is required.
 // We have this here so we can set up a chain, even if it's only one link long.
 // When we start doing proper comparisons between Spec and Status, we'll have an actual chain of checkers.
-func alwaysReconcile(
+func preReconciliationCheckerAlways(
 	_ context.Context,
 	_ genruntime.MetaObject,
 	_ genruntime.MetaObject,