Merge pull request kubernetes-sigs#11428 from sbueringer/pr-cc-v1beta…

…2-conditions

🌱 Add v1beta2 conditions to ClusterClass

api/v1beta1/clusterclass_types.go

@@ -28,17 +28,40 @@ import (
 // ClusterClassKind represents the Kind of ClusterClass.
 const ClusterClassKind = "ClusterClass"
 
-// Conditions that will be used for the ClusterClass object in v1Beta2 API version.
+// ClusterClass VariablesReady condition and corresponding reasons that will be used in v1Beta2 API version.
 const (
 	// ClusterClassVariablesReadyV1Beta2Condition is true if the ClusterClass variables, including both inline and external
 	// variables, have been successfully reconciled and thus ready to be used to default and validate variables on Clusters using
 	// this ClusterClass.
 	ClusterClassVariablesReadyV1Beta2Condition = "VariablesReady"
 
+	// ClusterClassVariablesReadyV1Beta2Reason surfaces that the variables are ready.
+	ClusterClassVariablesReadyV1Beta2Reason = "VariablesReady"
+
+	// ClusterClassVariablesReadyVariableDiscoveryFailedV1Beta2Reason surfaces that variable discovery failed.
+	ClusterClassVariablesReadyVariableDiscoveryFailedV1Beta2Reason = "VariableDiscoveryFailed"
+)
+
+// ClusterClass RefVersionsUpToDate condition and corresponding reasons that will be used in v1Beta2 API version.
+const (
 	// ClusterClassRefVersionsUpToDateV1Beta2Condition documents if the references in the ClusterClass are
 	// up-to-date (i.e. they are using the latest apiVersion of the current Cluster API contract from
 	// the corresponding CRD).
 	ClusterClassRefVersionsUpToDateV1Beta2Condition = "RefVersionsUpToDate"
+
+	// ClusterClassRefVersionsUpToDateV1Beta2Reason surfaces that the references in the ClusterClass are
+	// up-to-date (i.e. they are using the latest apiVersion of the current Cluster API contract from
+	// the corresponding CRD).
+	ClusterClassRefVersionsUpToDateV1Beta2Reason = "RefVersionsUpToDate"
+
+	// ClusterClassRefVersionsNotUpToDateV1Beta2Reason surfaces that the references in the ClusterClass are not
+	// up-to-date (i.e. they are not using the latest apiVersion of the current Cluster API contract from
+	// the corresponding CRD).
+	ClusterClassRefVersionsNotUpToDateV1Beta2Reason = "RefVersionsNotUpToDate"
+
+	// ClusterClassRefVersionsUpToDateInternalErrorV1Beta2Reason surfaces that an unexpected error occurred when validating
+	// if the references are up-to-date.
+	ClusterClassRefVersionsUpToDateInternalErrorV1Beta2Reason = InternalErrorV1Beta2Reason
 )
 
 // +kubebuilder:object:root=true

api/v1beta1/condition_consts.go

@@ -353,4 +353,8 @@ const (
 	// up-to-date (i.e. they are not using the latest apiVersion of the current Cluster API contract from
 	// the corresponding CRD).
 	ClusterClassOutdatedRefVersionsReason = "OutdatedRefVersions"
+
+	// ClusterClassRefVersionsUpToDateInternalErrorReason (Severity=Warning) surfaces that an unexpected error occurred when validating
+	// if the references are up-to-date.
+	ClusterClassRefVersionsUpToDateInternalErrorReason = "InternalError"
 )

internal/controllers/clusterclass/clusterclass_controller.go

@@ -47,7 +47,7 @@ import (
 	"sigs.k8s.io/cluster-api/feature"
 	runtimeclient "sigs.k8s.io/cluster-api/internal/runtime/client"
 	"sigs.k8s.io/cluster-api/internal/topology/variables"
-	"sigs.k8s.io/cluster-api/util/conditions"
+	"sigs.k8s.io/cluster-api/util"
 	"sigs.k8s.io/cluster-api/util/conversion"
 	"sigs.k8s.io/cluster-api/util/patch"
 	"sigs.k8s.io/cluster-api/util/paused"
@@ -94,7 +94,7 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opt
 	return nil
 }
 
-func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Result, reterr error) {
+func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (retres ctrl.Result, reterr error) {
 	clusterClass := &clusterv1.ClusterClass{}
 	if err := r.Client.Get(ctx, req.NamespacedName, clusterClass); err != nil {
 		if apierrors.IsNotFound(err) {
@@ -112,40 +112,94 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Re
 		return ctrl.Result{}, nil
 	}
 
+	s := &scope{
+		clusterClass: clusterClass,
+	}
+
 	patchHelper, err := patch.NewHelper(clusterClass, r.Client)
 	if err != nil {
 		return ctrl.Result{}, err
 	}
 
 	defer func() {
+		updateStatus(ctx, s)
+
+		patchOpts := []patch.Option{
+			patch.WithOwnedConditions{Conditions: []clusterv1.ConditionType{
+				clusterv1.ClusterClassRefVersionsUpToDateCondition,
+				clusterv1.ClusterClassVariablesReconciledCondition,
+			}},
+			patch.WithOwnedV1Beta2Conditions{Conditions: []string{
+				clusterv1.ClusterClassRefVersionsUpToDateV1Beta2Condition,
+				clusterv1.ClusterClassVariablesReadyV1Beta2Condition,
+			}},
+		}
+
 		// Patch ObservedGeneration only if the reconciliation completed successfully
-		patchOpts := []patch.Option{}
 		if reterr == nil {
 			patchOpts = append(patchOpts, patch.WithStatusObservedGeneration{})
 		}
 		if err := patchHelper.Patch(ctx, clusterClass, patchOpts...); err != nil {
 			reterr = kerrors.NewAggregate([]error{reterr, err})
 			return
 		}
+
+		if reterr != nil {
+			retres = ctrl.Result{}
+		}
 	}()
-	return ctrl.Result{}, r.reconcile(ctx, clusterClass)
+
+	reconcileNormal := []clusterClassReconcileFunc{
+		r.reconcileExternalReferences,
+		r.reconcileVariables,
+	}
+	return doReconcile(ctx, reconcileNormal, s)
 }
 
-func (r *Reconciler) reconcile(ctx context.Context, clusterClass *clusterv1.ClusterClass) error {
-	if err := r.reconcileVariables(ctx, clusterClass); err != nil {
-		return err
+type clusterClassReconcileFunc func(context.Context, *scope) (ctrl.Result, error)
+
+func doReconcile(ctx context.Context, phases []clusterClassReconcileFunc, s *scope) (ctrl.Result, error) {
+	res := ctrl.Result{}
+	errs := []error{}
+	for _, phase := range phases {
+		// Call the inner reconciliation methods.
+		phaseResult, err := phase(ctx, s)
+		if err != nil {
+			errs = append(errs, err)
+		}
+		if len(errs) > 0 {
+			continue
+		}
+		res = util.LowestNonZeroResult(res, phaseResult)
 	}
-	outdatedRefs, err := r.reconcileExternalReferences(ctx, clusterClass)
-	if err != nil {
-		return err
+
+	if len(errs) > 0 {
+		return ctrl.Result{}, kerrors.NewAggregate(errs)
 	}
 
-	reconcileConditions(clusterClass, outdatedRefs)
+	return res, nil
+}
 
-	return nil
+// scope holds the different objects that are read and used during the reconcile.
+type scope struct {
+	// clusterClass is the ClusterClass object being reconciled.
+	// It is set at the beginning of the reconcile function.
+	clusterClass *clusterv1.ClusterClass
+
+	reconcileExternalReferencesError error
+	outdatedExternalReferences       []outdatedRef
+
+	variableDiscoveryError error
+}
+
+type outdatedRef struct {
+	Outdated *corev1.ObjectReference
+	UpToDate *corev1.ObjectReference
 }
 
-func (r *Reconciler) reconcileExternalReferences(ctx context.Context, clusterClass *clusterv1.ClusterClass) (map[*corev1.ObjectReference]*corev1.ObjectReference, error) {
+func (r *Reconciler) reconcileExternalReferences(ctx context.Context, s *scope) (ctrl.Result, error) {
+	clusterClass := s.clusterClass
+
 	// Collect all the reference from the ClusterClass to templates.
 	refs := []*corev1.ObjectReference{}
 
@@ -185,7 +239,7 @@ func (r *Reconciler) reconcileExternalReferences(ctx context.Context, clusterCla
 	// or MachinePool classes.
 	errs := []error{}
 	reconciledRefs := sets.Set[string]{}
-	outdatedRefs := map[*corev1.ObjectReference]*corev1.ObjectReference{}
+	outdatedRefs := []outdatedRef{}
 	for i := range refs {
 		ref := refs[i]
 		uniqueKey := uniqueObjectRefKey(ref)
@@ -212,16 +266,25 @@ func (r *Reconciler) reconcileExternalReferences(ctx context.Context, clusterCla
 			errs = append(errs, err)
 		}
 		if ref.GroupVersionKind().Version != updatedRef.GroupVersionKind().Version {
-			outdatedRefs[ref] = updatedRef
+			outdatedRefs = append(outdatedRefs, outdatedRef{
+				Outdated: ref,
+				UpToDate: updatedRef,
+			})
 		}
 	}
 	if len(errs) > 0 {
-		return nil, kerrors.NewAggregate(errs)
+		err := kerrors.NewAggregate(errs)
+		s.reconcileExternalReferencesError = err
+		return ctrl.Result{}, err
 	}
-	return outdatedRefs, nil
+
+	s.outdatedExternalReferences = outdatedRefs
+	return ctrl.Result{}, nil
 }
 
-func (r *Reconciler) reconcileVariables(ctx context.Context, clusterClass *clusterv1.ClusterClass) error {
+func (r *Reconciler) reconcileVariables(ctx context.Context, s *scope) (ctrl.Result, error) {
+	clusterClass := s.clusterClass
+
 	errs := []error{}
 	allVariableDefinitions := map[string]*clusterv1.ClusterClassStatusVariable{}
 	// Add inline variable definitions to the ClusterClass status.
@@ -270,10 +333,10 @@ func (r *Reconciler) reconcileVariables(ctx context.Context, clusterClass *clust
 		}
 	}
 	if len(errs) > 0 {
+		err := kerrors.NewAggregate(errs)
+		s.variableDiscoveryError = errors.Wrapf(err, "VariableDiscovery failed")
 		// TODO: Decide whether to remove old variables if discovery fails.
-		conditions.MarkFalse(clusterClass, clusterv1.ClusterClassVariablesReconciledCondition, clusterv1.VariableDiscoveryFailedReason, clusterv1.ConditionSeverityError,
-			"VariableDiscovery failed: %s", kerrors.NewAggregate(errs))
-		return errors.Wrapf(kerrors.NewAggregate(errs), "failed to discover variables for ClusterClass %s", clusterClass.Name)
+		return ctrl.Result{}, errors.Wrapf(err, "failed to discover variables for ClusterClass %s", clusterClass.Name)
 	}
 
 	statusVarList := []clusterv1.ClusterClassStatusVariable{}
@@ -297,37 +360,11 @@ func (r *Reconciler) reconcileVariables(ctx context.Context, clusterClass *clust
 
 	if len(variablesWithConflict) > 0 {
 		err := fmt.Errorf("the following variables have conflicting schemas: %s", strings.Join(variablesWithConflict, ","))
-		conditions.MarkFalse(clusterClass, clusterv1.ClusterClassVariablesReconciledCondition, clusterv1.VariableDiscoveryFailedReason, clusterv1.ConditionSeverityError,
-			"VariableDiscovery failed: %s", err)
-		return errors.Wrapf(err, "failed to discover variables for ClusterClass %s", clusterClass.Name)
+		s.variableDiscoveryError = errors.Wrapf(err, "VariableDiscovery failed")
+		return ctrl.Result{}, errors.Wrapf(err, "failed to discover variables for ClusterClass %s", clusterClass.Name)
 	}
 
-	conditions.MarkTrue(clusterClass, clusterv1.ClusterClassVariablesReconciledCondition)
-	return nil
-}
-
-func reconcileConditions(clusterClass *clusterv1.ClusterClass, outdatedRefs map[*corev1.ObjectReference]*corev1.ObjectReference) {
-	if len(outdatedRefs) > 0 {
-		var msg []string
-		for currentRef, updatedRef := range outdatedRefs {
-			msg = append(msg, fmt.Sprintf("Ref %q should be %q", refString(currentRef), refString(updatedRef)))
-		}
-		conditions.Set(
-			clusterClass,
-			conditions.FalseCondition(
-				clusterv1.ClusterClassRefVersionsUpToDateCondition,
-				clusterv1.ClusterClassOutdatedRefVersionsReason,
-				clusterv1.ConditionSeverityWarning,
-				strings.Join(msg, ", "),
-			),
-		)
-		return
-	}
-
-	conditions.Set(
-		clusterClass,
-		conditions.TrueCondition(clusterv1.ClusterClassRefVersionsUpToDateCondition),
-	)
+	return ctrl.Result{}, nil
 }
 
 func addNewStatusVariable(variable clusterv1.ClusterClassVariable, from string) *clusterv1.ClusterClassStatusVariable {
@@ -373,7 +410,7 @@ func (r *Reconciler) reconcileExternal(ctx context.Context, clusterClass *cluste
 	obj, err := external.Get(ctx, r.Client, ref, clusterClass.Namespace)
 	if err != nil {
 		if apierrors.IsNotFound(errors.Cause(err)) {
-			return errors.Wrapf(err, "Could not find external object for the ClusterClass. refGroupVersionKind: %s, refName: %s", ref.GroupVersionKind(), ref.Name)
+			return errors.Wrapf(err, "could not find external object for the ClusterClass. refGroupVersionKind: %s, refName: %s", ref.GroupVersionKind(), ref.Name)
 		}
 		return errors.Wrapf(err, "failed to get the external object for the ClusterClass. refGroupVersionKind: %s, refName: %s", ref.GroupVersionKind(), ref.Name)
 	}
@@ -384,7 +421,7 @@ func (r *Reconciler) reconcileExternal(ctx context.Context, clusterClass *cluste
 		return err
 	}
 
-	// Set external object ControllerReference to the ClusterClass.
+	// Set external object owner reference to the ClusterClass.
 	if err := controllerutil.SetOwnerReference(clusterClass, obj, r.Client.Scheme()); err != nil {
 		return errors.Wrapf(err, "failed to set ClusterClass owner reference for %s %s", obj.GetKind(), klog.KObj(obj))
 	}

internal/controllers/clusterclass/clusterclass_controller_status.go

@@ -0,0 +1,107 @@
+/*
+Copyright 2024 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package clusterclass
+
+import (
+	"context"
+	"fmt"
+	"strings"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
+	"sigs.k8s.io/cluster-api/util/conditions"
+	v1beta2conditions "sigs.k8s.io/cluster-api/util/conditions/v1beta2"
+)
+
+func updateStatus(ctx context.Context, s *scope) {
+	setRefVersionsUpToDateCondition(ctx, s.clusterClass, s.outdatedExternalReferences, s.reconcileExternalReferencesError)
+	setVariablesReconciledCondition(ctx, s.clusterClass, s.variableDiscoveryError)
+}
+
+func setRefVersionsUpToDateCondition(_ context.Context, clusterClass *clusterv1.ClusterClass, outdatedRefs []outdatedRef, reconcileExternalReferencesError error) {
+	if reconcileExternalReferencesError != nil {
+		conditions.MarkUnknown(clusterClass,
+			clusterv1.ClusterClassRefVersionsUpToDateCondition,
+			clusterv1.ClusterClassRefVersionsUpToDateInternalErrorReason,
+			"Please check controller logs for errors",
+		)
+		v1beta2conditions.Set(clusterClass, metav1.Condition{
+			Type:    clusterv1.ClusterClassRefVersionsUpToDateV1Beta2Condition,
+			Status:  metav1.ConditionUnknown,
+			Reason:  clusterv1.ClusterClassRefVersionsUpToDateInternalErrorV1Beta2Reason,
+			Message: "Please check controller logs for errors",
+		})
+		return
+	}
+
+	if len(outdatedRefs) > 0 {
+		var msg []string
+		for _, outdatedRef := range outdatedRefs {
+			msg = append(msg, fmt.Sprintf("* Ref %q should be %q", refString(outdatedRef.Outdated), refString(outdatedRef.UpToDate)))
+		}
+		conditions.Set(clusterClass,
+			conditions.FalseCondition(
+				clusterv1.ClusterClassRefVersionsUpToDateCondition,
+				clusterv1.ClusterClassOutdatedRefVersionsReason,
+				clusterv1.ConditionSeverityWarning,
+				strings.Join(msg, "\n"),
+			),
+		)
+		v1beta2conditions.Set(clusterClass, metav1.Condition{
+			Type:    clusterv1.ClusterClassRefVersionsUpToDateV1Beta2Condition,
+			Status:  metav1.ConditionFalse,
+			Reason:  clusterv1.ClusterClassRefVersionsNotUpToDateV1Beta2Reason,
+			Message: strings.Join(msg, "\n"),
+		})
+		return
+	}
+
+	conditions.Set(clusterClass,
+		conditions.TrueCondition(clusterv1.ClusterClassRefVersionsUpToDateCondition),
+	)
+	v1beta2conditions.Set(clusterClass, metav1.Condition{
+		Type:   clusterv1.ClusterClassRefVersionsUpToDateV1Beta2Condition,
+		Status: metav1.ConditionTrue,
+		Reason: clusterv1.ClusterClassRefVersionsUpToDateV1Beta2Reason,
+	})
+}
+
+func setVariablesReconciledCondition(_ context.Context, clusterClass *clusterv1.ClusterClass, variableDiscoveryError error) {
+	if variableDiscoveryError != nil {
+		conditions.MarkFalse(clusterClass,
+			clusterv1.ClusterClassVariablesReconciledCondition,
+			clusterv1.VariableDiscoveryFailedReason,
+			clusterv1.ConditionSeverityError,
+			variableDiscoveryError.Error(),
+		)
+		v1beta2conditions.Set(clusterClass, metav1.Condition{
+			Type:    clusterv1.ClusterClassVariablesReadyV1Beta2Condition,
+			Status:  metav1.ConditionFalse,
+			Reason:  clusterv1.ClusterClassVariablesReadyVariableDiscoveryFailedV1Beta2Reason,
+			Message: variableDiscoveryError.Error(),
+		})
+		return
+	}
+
+	conditions.MarkTrue(clusterClass, clusterv1.ClusterClassVariablesReconciledCondition)
+	v1beta2conditions.Set(clusterClass, metav1.Condition{
+		Type:   clusterv1.ClusterClassVariablesReadyV1Beta2Condition,
+		Status: metav1.ConditionTrue,
+		Reason: clusterv1.ClusterClassVariablesReadyV1Beta2Reason,
+	})
+}