feat: explicitly check for cluster labels

Signed-off-by: Carlos Salas <carlos.salas@suse.com>

internal/controllers/helpers.go

@@ -42,7 +42,6 @@ import (
 
 	managementv3 "github.com/rancher-sandbox/rancher-turtles/internal/rancher/management/v3"
 	"github.com/rancher-sandbox/rancher-turtles/util"
-	turtlesannotations "github.com/rancher-sandbox/rancher-turtles/util/annotations"
 )
 
 const (
@@ -54,26 +53,6 @@ const (
 	defaultRequeueDuration = 1 * time.Minute
 )
 
-func reconcileDelete(ctx context.Context, capiCluster *clusterv1.Cluster) (ctrl.Result, error) {
-	log := log.FromContext(ctx)
-	log.Info("Reconciling rancher cluster deletion")
-
-	// If the Rancher Cluster was already imported, then annotate the CAPI cluster so that we don't auto-import again.
-	log.Info(fmt.Sprintf("Rancher cluster is being removed, annotating CAPI cluster %s with %s",
-		capiCluster.Name,
-		turtlesannotations.ClusterImportedAnnotation))
-
-	annotations := capiCluster.GetAnnotations()
-	if annotations == nil {
-		annotations = map[string]string{}
-	}
-
-	annotations[turtlesannotations.ClusterImportedAnnotation] = "true"
-	capiCluster.SetAnnotations(annotations)
-
-	return ctrl.Result{}, nil
-}
-
 func getClusterRegistrationManifest(ctx context.Context, clusterName, namespace string, rancherClient client.Client,
 	insecureSkipVerify bool,
 ) (string, error) {

internal/controllers/import_controller.go

@@ -44,6 +44,7 @@ import (
 
 	provisioningv1 "github.com/rancher-sandbox/rancher-turtles/internal/rancher/provisioning/v1"
 	"github.com/rancher-sandbox/rancher-turtles/util"
+	turtlesannotations "github.com/rancher-sandbox/rancher-turtles/util/annotations"
 	turtlesnaming "github.com/rancher-sandbox/rancher-turtles/util/naming"
 	turtlespredicates "github.com/rancher-sandbox/rancher-turtles/util/predicates"
 )
@@ -183,7 +184,7 @@ func (r *CAPIImportReconciler) reconcile(ctx context.Context, capiCluster *clust
 	}
 
 	if !rancherCluster.ObjectMeta.DeletionTimestamp.IsZero() {
-		return reconcileDelete(ctx, capiCluster)
+		return r.reconcileDelete(ctx, capiCluster)
 	}
 
 	return r.reconcileNormal(ctx, capiCluster, rancherCluster)
@@ -295,3 +296,23 @@ func (r *CAPIImportReconciler) rancherClusterToCapiCluster(ctx context.Context,
 		return []ctrl.Request{{NamespacedName: client.ObjectKey{Namespace: capiCluster.Namespace, Name: capiCluster.Name}}}
 	}
 }
+
+func (r *CAPIImportReconciler) reconcileDelete(ctx context.Context, capiCluster *clusterv1.Cluster) (ctrl.Result, error) {
+	log := log.FromContext(ctx)
+	log.Info("Reconciling rancher cluster deletion")
+
+	// If the Rancher Cluster was already imported, then annotate the CAPI cluster so that we don't auto-import again.
+	log.Info(fmt.Sprintf("Rancher cluster is being removed, annotating CAPI cluster %s with %s",
+		capiCluster.Name,
+		turtlesannotations.ClusterImportedAnnotation))
+
+	annotations := capiCluster.GetAnnotations()
+	if annotations == nil {
+		annotations = map[string]string{}
+	}
+
+	annotations[turtlesannotations.ClusterImportedAnnotation] = "true"
+	capiCluster.SetAnnotations(annotations)
+
+	return ctrl.Result{}, nil
+}

internal/controllers/import_controller_v3.go

@@ -27,9 +27,11 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	errorutils "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/client-go/tools/record"
+	"k8s.io/client-go/util/retry"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/log"
@@ -44,6 +46,7 @@ import (
 
 	managementv3 "github.com/rancher-sandbox/rancher-turtles/internal/rancher/management/v3"
 	"github.com/rancher-sandbox/rancher-turtles/util"
+	turtlesannotations "github.com/rancher-sandbox/rancher-turtles/util/annotations"
 	turtlespredicates "github.com/rancher-sandbox/rancher-turtles/util/predicates"
 )
 
@@ -135,7 +138,16 @@ func (r *CAPIImportManagementV3Reconciler) Reconcile(ctx context.Context, req ct
 		return ctrl.Result{Requeue: true}, err
 	}
 
-	patchBase := client.MergeFromWithOptions(capiCluster.DeepCopy(), client.MergeFromWithOptimisticLock{})
+	if capiCluster.ObjectMeta.DeletionTimestamp.IsZero() && !turtlesannotations.HasClusterImportAnnotation(capiCluster) {
+		if !controllerutil.ContainsFinalizer(capiCluster, managementv3.CapiClusterFinalizer) {
+			log.Info("capi cluster is imported, adding finalizer")
+			controllerutil.AddFinalizer(capiCluster, managementv3.CapiClusterFinalizer)
+
+			if err := r.Client.Update(ctx, capiCluster); err != nil {
+				return ctrl.Result{}, fmt.Errorf("error adding finalizer: %w", err)
+			}
+		}
+	}
 
 	log = log.WithValues("cluster", capiCluster.Name)
 
@@ -154,8 +166,17 @@ func (r *CAPIImportManagementV3Reconciler) Reconcile(ctx context.Context, req ct
 		errs = append(errs, fmt.Errorf("error reconciling cluster: %w", err))
 	}
 
-	if err := r.Client.Patch(ctx, capiCluster, patchBase); err != nil {
-		errs = append(errs, fmt.Errorf("failed to patch cluster: %w", err))
+	if err := retry.RetryOnConflict(retry.DefaultBackoff, func() error {
+		capiClusterCopy := capiCluster.DeepCopy()
+
+		patchBase := client.MergeFromWithOptions(capiCluster, client.MergeFromWithOptimisticLock{})
+
+		if err := r.Client.Patch(ctx, capiClusterCopy, patchBase); err != nil {
+			errs = append(errs, fmt.Errorf("failed to patch cluster: %w", err))
+		}
+		return nil
+	}); err != nil {
+		return ctrl.Result{}, err
 	}
 
 	if len(errs) > 0 {
@@ -203,15 +224,14 @@ func (r *CAPIImportManagementV3Reconciler) reconcile(ctx context.Context, capiCl
 		rancherCluster = &rancherClusterList.Items[0]
 	}
 
-	if !capiCluster.ObjectMeta.DeletionTimestamp.IsZero() {
-		err := r.deleteDependentRancherCluster(ctx, capiCluster)
-		if err != nil {
+	if !capiCluster.ObjectMeta.DeletionTimestamp.IsZero() && !turtlesannotations.HasClusterImportAnnotation(capiCluster) {
+		if err := r.deleteDependentRancherCluster(ctx, capiCluster); err != nil {
 			return ctrl.Result{Requeue: true}, fmt.Errorf("error deleting associated managementv3.Cluster resources: %w", err)
 		}
 	}
 
 	if !rancherCluster.ObjectMeta.DeletionTimestamp.IsZero() {
-		return reconcileDelete(ctx, capiCluster)
+		return r.reconcileDelete(ctx, capiCluster)
 	}
 
 	return r.reconcileNormal(ctx, capiCluster, rancherCluster)
@@ -261,7 +281,7 @@ func (r *CAPIImportManagementV3Reconciler) reconcileNormal(ctx context.Context,
 		return ctrl.Result{}, err
 	}
 
-	if managementv3.ClusterConditionAgentDeployed.IsTrue(rancherCluster) {
+	if conditions.IsTrue(rancherCluster, managementv3.ClusterConditionAgentDeployed) {
 		log.Info("agent already deployed, no action needed")
 		return ctrl.Result{}, nil
 	}
@@ -297,7 +317,23 @@ func (r *CAPIImportManagementV3Reconciler) rancherClusterToCapiCluster(ctx conte
 	log := log.FromContext(ctx)
 
 	return func(_ context.Context, o client.Object) []ctrl.Request {
-		labels := o.GetLabels()
+		cluster, ok := o.(*managementv3.Cluster)
+		if !ok {
+			log.Error(nil, fmt.Sprintf("Expected a rancher cluster but got a %T", o))
+			return nil
+		}
+
+		labels := cluster.GetLabels()
+		if _, ok := labels[capiClusterOwner]; !ok {
+			log.Error(fmt.Errorf("missing label %s", capiClusterOwner), "getting rancher cluster labels")
+			return nil
+		}
+
+		if _, ok := labels[capiClusterOwnerNamespace]; !ok {
+			log.Error(fmt.Errorf("missing label %s", capiClusterOwnerNamespace), "getting rancher cluster labels")
+			return nil
+		}
+
 		capiCluster := &clusterv1.Cluster{ObjectMeta: metav1.ObjectMeta{
 			Name:      labels[capiClusterOwner],
 			Namespace: labels[capiClusterOwnerNamespace],
@@ -319,6 +355,34 @@ func (r *CAPIImportManagementV3Reconciler) rancherClusterToCapiCluster(ctx conte
 	}
 }
 
+func (r *CAPIImportManagementV3Reconciler) reconcileDelete(ctx context.Context, capiCluster *clusterv1.Cluster) (ctrl.Result, error) {
+	log := log.FromContext(ctx)
+	log.Info("Reconciling rancher cluster deletion")
+
+	// If the Rancher Cluster was already imported, then annotate the CAPI cluster so that we don't auto-import again.
+	log.Info(fmt.Sprintf("Rancher cluster is being removed, annotating CAPI cluster %s with %s",
+		capiCluster.Name,
+		turtlesannotations.ClusterImportedAnnotation))
+
+	annotations := capiCluster.GetAnnotations()
+	if annotations == nil {
+		annotations = map[string]string{}
+	}
+
+	annotations[turtlesannotations.ClusterImportedAnnotation] = "true"
+	capiCluster.SetAnnotations(annotations)
+
+	if controllerutil.ContainsFinalizer(capiCluster, managementv3.CapiClusterFinalizer) {
+		controllerutil.RemoveFinalizer(capiCluster, managementv3.CapiClusterFinalizer)
+
+		if err := r.Client.Update(ctx, capiCluster); err != nil {
+			return ctrl.Result{}, fmt.Errorf("error removing finalizer: %w", err)
+		}
+	}
+
+	return ctrl.Result{}, nil
+}
+
 func (r *CAPIImportManagementV3Reconciler) deleteDependentRancherCluster(ctx context.Context, capiCluster *clusterv1.Cluster) error {
 	log := log.FromContext(ctx)
 	log.Info("capi cluster is being deleted, deleting dependent rancher cluster")

internal/controllers/import_controller_v3_test.go

@@ -35,6 +35,7 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	"sigs.k8s.io/cluster-api/controllers/remote"
+	"sigs.k8s.io/cluster-api/util/conditions"
 	"sigs.k8s.io/cluster-api/util/secret"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
@@ -275,13 +276,14 @@ var _ = Describe("reconcile CAPI Cluster", func() {
 		cluster := rancherClusters.Items[0]
 		Expect(cluster.Name).To(ContainSubstring("c-"))
 
-		cluster.Status.Conditions = []managementv3.ClusterCondition{
+		cluster.Status.Conditions = clusterv1.Conditions{
 			{
-				Type:   managementv3.ClusterConditionType(managementv3.ClusterConditionAgentDeployed),
-				Status: corev1.ConditionTrue,
+				Type:               managementv3.ClusterConditionAgentDeployed,
+				Status:             corev1.ConditionTrue,
+				LastTransitionTime: metav1.Now(),
 			},
 		}
-		Expect(managementv3.ClusterConditionAgentDeployed.IsTrue(&cluster)).To(BeTrue())
+		Expect(conditions.IsTrue(&cluster, managementv3.ClusterConditionAgentDeployed)).To(BeTrue())
 		Expect(cl.Status().Update(ctx, &cluster)).To(Succeed())
 
 		_, err := r.Reconcile(ctx, reconcile.Request{

internal/rancher/management/v3/cluster.go

@@ -17,25 +17,18 @@ limitations under the License.
 package v3
 
 import (
-	"reflect"
-
-	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	runtime "k8s.io/apimachinery/pkg/runtime"
+
+	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 )
 
 const (
 	// ClusterConditionAgentDeployed is the condition type for the agent deployed condition.
-	ClusterConditionAgentDeployed Cond = "AgentDeployed"
+	ClusterConditionAgentDeployed clusterv1.ConditionType = "AgentDeployed"
 	// ClusterConditionReady is the condition type for the ready condition.
-	ClusterConditionReady Cond = "Ready"
-)
-
-type (
-	// Cond represents a condition of a Rancher Cluster.
-	Cond string
-	// ClusterConditionType represents the type of a condition.
-	ClusterConditionType string
+	ClusterConditionReady clusterv1.ConditionType = "Ready"
+	// CapiClusterFinalizer is the finalizer applied to capi clusters.
+	CapiClusterFinalizer = "capicluster.turtles.cattle.io"
 )
 
 // Cluster is the struct representing a Rancher Cluster.
@@ -58,7 +51,7 @@ type ClusterSpec struct {
 
 // ClusterStatus is the struct representing the status of a Rancher Cluster.
 type ClusterStatus struct {
-	Conditions []ClusterCondition `json:"conditions,omitempty"`
+	Conditions clusterv1.Conditions `json:"conditions,omitempty"`
 }
 
 // ClusterList contains a list of ClusterList.
@@ -69,81 +62,9 @@ type ClusterList struct {
 	Items           []Cluster `json:"items"`
 }
 
-// ClusterCondition is the struct representing a condition of a Rancher Cluster.
-type ClusterCondition struct {
-	// Type of cluster condition.
-	Type ClusterConditionType `json:"type"`
-	// Status of the condition, one of True, False, Unknown.
-	Status corev1.ConditionStatus `json:"status"`
-	// The last time this condition was updated.
-	LastUpdateTime string `json:"lastUpdateTime,omitempty"`
-	// Last time the condition transitioned from one status to another.
-	LastTransitionTime string `json:"lastTransitionTime,omitempty"`
-	// The reason for the condition's last transition.
-	Reason string `json:"reason,omitempty"`
-	// Human-readable message indicating details about last transition
-	Message string `json:"message,omitempty"`
-}
-
-// IsTrue returns true if the condition is true.
-func (c Cond) IsTrue(obj runtime.Object) bool {
-	return getStatus(obj, string(c)) == "True"
-}
-
-func getStatus(obj interface{}, condName string) string {
-	cond := findOrNotCreateCond(obj, condName)
-	if cond == nil {
-		return ""
-	}
-
-	return getFieldValue(*cond, "Status").String()
-}
-
-func findOrNotCreateCond(obj interface{}, condName string) *reflect.Value {
-	condSlice := getValue(obj, "Status", "Conditions")
-	return findCond(condSlice, condName)
-}
-
-func findCond(val reflect.Value, name string) *reflect.Value {
-	for i := 0; i < val.Len(); i++ {
-		cond := val.Index(i)
-		typeVal := getFieldValue(cond, "Type")
-
-		if typeVal.String() == name {
-			return &cond
-		}
-	}
-
-	return nil
-}
-
-func getValue(obj interface{}, name ...string) reflect.Value {
-	if obj == nil {
-		return reflect.Value{}
-	}
-
-	v := reflect.ValueOf(obj)
-	t := v.Type()
-
-	if t.Kind() == reflect.Ptr {
-		v = v.Elem()
-	}
-
-	field := v.FieldByName(name[0])
-	if len(name) == 1 {
-		return field
-	}
-
-	return getFieldValue(field, name[1:]...)
-}
-
-func getFieldValue(v reflect.Value, name ...string) reflect.Value {
-	field := v.FieldByName(name[0])
-	if len(name) == 1 {
-		return field
-	}
-
-	return getFieldValue(field, name[1:]...)
+// GetConditions method to implement capi conditions getter interface.
+func (c *Cluster) GetConditions() clusterv1.Conditions {
+	return c.Status.Conditions
 }
 
 func init() {