refac(Cluster) - Merge & Standardize DirectAccess and ClusterStatus Controller

cmd/greenhouse/controllers.go

@@ -50,11 +50,9 @@ var knownControllers = map[string]func(controllerName string, mgr ctrl.Manager)
 	"pluginChartTest": (&plugincontrollers.HelmChartTestReconciler{}).SetupWithManager,
 
 	// Cluster controllers
-	"bootStrap":           (&clustercontrollers.BootstrapReconciler{}).SetupWithManager,
-	"clusterDirectAccess": startClusterDirectAccessReconciler,
+	"bootStrap":         (&clustercontrollers.BootstrapReconciler{}).SetupWithManager,
+	"clusterReconciler": startClusterReconciler,
 	// "clusterPropagation":     (&clustercontrollers.ClusterPropagationReconciler{}).SetupWithManager,
-	"clusterStatus": (&clustercontrollers.ClusterStatusReconciler{}).SetupWithManager,
-	"kubeconfig":    (&clustercontrollers.KubeconfigReconciler{}).SetupWithManager,
 }
 
 // knownControllers lists the name of known controllers.
@@ -87,13 +85,13 @@ func startOrganizationDexReconciler(name string, mgr ctrl.Manager) error {
 	}).SetupWithManager(name, mgr)
 }
 
-func startClusterDirectAccessReconciler(name string, mgr ctrl.Manager) error {
+func startClusterReconciler(name string, mgr ctrl.Manager) error {
 	if renewRemoteClusterBearerTokenAfter > remoteClusterBearerTokenValidity {
 		setupLog.Info("WARN: remoteClusterBearerTokenValidity is less than renewRemoteClusterBearerTokenAfter")
 		setupLog.Info("Setting renewRemoteClusterBearerTokenAfter to half of remoteClusterBearerTokenValidity")
 		renewRemoteClusterBearerTokenAfter = remoteClusterBearerTokenValidity / 2
 	}
-	return (&clustercontrollers.DirectAccessReconciler{
+	return (&clustercontrollers.RemoteClusterReconciler{
 		RemoteClusterBearerTokenValidity:   remoteClusterBearerTokenValidity,
 		RenewRemoteClusterBearerTokenAfter: renewRemoteClusterBearerTokenAfter,
 	}).SetupWithManager(name, mgr)

docs/reference/api/openapi.yaml

@@ -1,7 +1,7 @@
 openapi: 3.0.0
 info:
   title: Greenhouse
-  version: aecb95b
+  version: cbd78d3
   description: PlusOne operations platform
 paths:
   /TeamMembership:

pkg/apis/greenhouse/v1alpha1/cluster_types.go

@@ -26,9 +26,6 @@ const (
 
 	// KubeConfigValid reflects the validity of the kubeconfig of a cluster.
 	KubeConfigValid ConditionType = "KubeConfigValid"
-
-	// ClusterDeletionScheduled reflects the condition type if a cluster is scheduled for deletion
-	ClusterDeletionScheduled ConditionType = "ClusterDeletionScheduled"
 )
 
 // ClusterStatus defines the observed state of Cluster
@@ -68,6 +65,14 @@ type Cluster struct {
 	Status ClusterStatus `json:"status,omitempty"`
 }
 
+func (c *Cluster) GetConditions() StatusConditions {
+	return c.Status.StatusConditions
+}
+
+func (c *Cluster) SetCondition(condition Condition) {
+	c.Status.StatusConditions.SetConditions(condition)
+}
+
 // GetSecretName returns the Kubernetes secret containing sensitive data for this cluster.
 // The secret is for internal usage only and its content must not be exposed to the user.
 func (c *Cluster) GetSecretName() string {

pkg/apis/greenhouse/v1alpha1/conditions.go

@@ -17,8 +17,8 @@ const (
 	// ReadyCondition reflects the overall readiness status of a resource.
 	ReadyCondition ConditionType = "Ready"
 
-	// DeleteCondition reflects that the resource has finished it's cleanup process.
-	DeleteCondition ConditionType = "Deleted"
+	// DeleteCondition reflects that the resource has finished its cleanup process.
+	DeleteCondition ConditionType = "Delete"
 
 	// ClusterListEmpty is set when the resources ClusterSelector results in an empty ClusterList.
 	ClusterListEmpty ConditionType = "ClusterListEmpty"
@@ -68,7 +68,7 @@ func TrueCondition(t ConditionType, reason ConditionReason, message string) Cond
 	return NewCondition(t, metav1.ConditionTrue, reason, message)
 }
 
-// NewTrue returns a Condition with ConditionFalse and the given type, reason and message. LastTransitionTime is set to now.
+// FalseCondition returns a Condition with ConditionFalse and the given type, reason and message. LastTransitionTime is set to now.
 func FalseCondition(t ConditionType, reason ConditionReason, message string) Condition {
 	return NewCondition(t, metav1.ConditionFalse, reason, message)
 }
@@ -80,20 +80,24 @@ func UnknownCondition(t ConditionType, reason ConditionReason, message string) C
 
 // Equal returns true if the condition is identical to the supplied condition,
 // ignoring the LastTransitionTime.
-func (c Condition) Equal(other Condition) bool {
+func (c *Condition) Equal(other Condition) bool {
 	return c.Type == other.Type &&
 		c.Status == other.Status &&
 		c.Reason == other.Reason &&
 		c.Message == other.Message
 }
 
 // IsTrue returns true if the condition is true.
-func (c Condition) IsTrue() bool {
+func (c *Condition) IsTrue() bool {
 	return c.Status == metav1.ConditionTrue
 }
 
+func (c *Condition) IsUnknown() bool {
+	return c.Status == metav1.ConditionUnknown
+}
+
 // IsFalse returns true if the condition is false.
-func (c Condition) IsFalse() bool {
+func (c *Condition) IsFalse() bool {
 	return c.Status == metav1.ConditionFalse
 }
 

pkg/controllers/cluster/direct_access_controller.go → pkg/controllers/cluster/cluster_controller.go

@@ -7,6 +7,8 @@ import (
 	"context"
 	"time"
 
+	"github.com/cloudoperators/greenhouse/pkg/lifecycle"
+
 	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/types"
@@ -15,7 +17,6 @@ import (
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/builder"
 	"sigs.k8s.io/controller-runtime/pkg/client"
-	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
 
 	greenhouseapis "github.com/cloudoperators/greenhouse/pkg/apis"
@@ -25,8 +26,8 @@ import (
 
 const serviceAccountName = "greenhouse"
 
-// DirectAccessReconciler reconciles a Cluster object with accessMode=direct set.
-type DirectAccessReconciler struct {
+// RemoteClusterReconciler reconciles a Cluster object with accessMode=direct set.
+type RemoteClusterReconciler struct {
 	client.Client
 	recorder                           record.EventRecorder
 	RemoteClusterBearerTokenValidity   time.Duration
@@ -43,7 +44,7 @@ type DirectAccessReconciler struct {
 //+kubebuilder:rbac:groups="rbac",resources=clusterrolebindings,verbs=get;list;watch;update;patch;create
 
 // SetupWithManager sets up the controller with the Manager.
-func (r *DirectAccessReconciler) SetupWithManager(name string, mgr ctrl.Manager) error {
+func (r *RemoteClusterReconciler) SetupWithManager(name string, mgr ctrl.Manager) error {
 	r.Client = mgr.GetClient()
 	r.recorder = mgr.GetEventRecorderFor(name)
 
@@ -57,106 +58,109 @@ func (r *DirectAccessReconciler) SetupWithManager(name string, mgr ctrl.Manager)
 		Complete(r)
 }
 
-func (r *DirectAccessReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
-	var cluster = new(greenhousev1alpha1.Cluster)
-	if err := r.Get(ctx, req.NamespacedName, cluster); err != nil {
-		return ctrl.Result{}, client.IgnoreNotFound(err)
-	}
+func (r *RemoteClusterReconciler) GetEventRecorder() record.EventRecorder {
+	return r.recorder
+}
+
+func (r *RemoteClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
+	return lifecycle.Reconcile(ctx, r.Client, req.NamespacedName, &greenhousev1alpha1.Cluster{}, r, r.setConditions())
+}
 
+func (r *RemoteClusterReconciler) EnsureCreated(ctx context.Context, resource lifecycle.RuntimeObject) (ctrl.Result, lifecycle.ReconcileResult, error) {
+	cluster := resource.(*greenhousev1alpha1.Cluster) //nolint:errcheck
 	if cluster.Spec.AccessMode != greenhousev1alpha1.ClusterAccessModeDirect {
-		return ctrl.Result{}, nil
+		return ctrl.Result{}, lifecycle.Failed, nil
 	}
-
-	// Update metrics at the end of the reconcile function
-	defer updateMetrics(cluster)
-
+	// Deletion Schedule mechanism
 	isScheduled, schedule, err := clientutil.ExtractDeletionSchedule(cluster.GetAnnotations())
 	if err != nil {
-		return ctrl.Result{}, err
+		return ctrl.Result{}, lifecycle.Failed, err
 	}
 	if isScheduled && cluster.DeletionTimestamp == nil {
 		if ok, err := clientutil.ShouldProceedDeletion(time.Now(), schedule); ok && err == nil {
-			return ctrl.Result{}, r.Client.Delete(ctx, cluster)
+			err = r.Client.Delete(ctx, cluster)
+			if err != nil {
+				return ctrl.Result{}, lifecycle.Failed, err
+			}
+			return ctrl.Result{}, lifecycle.Success, nil
 		}
 	}
-
-	// Cleanup logic
-	if cluster.DeletionTimestamp != nil && controllerutil.ContainsFinalizer(cluster, greenhouseapis.FinalizerCleanupCluster) {
-		// delete all plugins that are bound to this cluster
-		deletionCount, err := deletePlugins(ctx, r.Client, cluster)
-		if err != nil {
-			return ctrl.Result{}, err
-		}
-		if deletionCount > 0 {
-			return ctrl.Result{RequeueAfter: 10 * time.Second}, nil
-		}
-
-		var kubeConfigSecret = new(corev1.Secret)
-		if err := r.Client.Get(ctx, types.NamespacedName{Namespace: cluster.GetNamespace(), Name: cluster.GetSecretName()}, kubeConfigSecret); err != nil {
-			return ctrl.Result{}, err
-		}
-		restClientGetter, err := clientutil.NewRestClientGetterFromSecret(kubeConfigSecret, cluster.Namespace)
-		if err != nil {
-			return ctrl.Result{}, err
-		}
-		remoteClient, err := clientutil.NewK8sClientFromRestClientGetter(restClientGetter)
-		if err != nil {
-			return ctrl.Result{}, err
-		}
-		// Delete namespace in remote cluster before the secret.
-		// All remote resources are bound by owner-reference to the namespace
-		if err := deleteNamespaceInRemoteCluster(ctx, remoteClient, cluster); err != nil {
-			return ctrl.Result{}, err
-		}
-		// A simple Delete won't do. The logic should take into consideration the order, that only a portion of the resources have been deleted, etc.
-		err = clientutil.RemoveFinalizer(ctx, r.Client, cluster, greenhouseapis.FinalizerCleanupCluster)
-		return ctrl.Result{}, err
-	}
-
-	// Add finalizer before starting any work.
-	if err := clientutil.EnsureFinalizer(ctx, r.Client, cluster, greenhouseapis.FinalizerCleanupCluster); err != nil {
-		return ctrl.Result{}, client.IgnoreNotFound(err)
-	}
-
+	defer updateMetrics(cluster)
 	var clusterSecret = new(corev1.Secret)
 	if err := r.Get(ctx, types.NamespacedName{Name: cluster.GetSecretName(), Namespace: cluster.GetNamespace()}, clusterSecret); err != nil {
-		return ctrl.Result{}, err
+		return ctrl.Result{}, lifecycle.Failed, err
 	}
 
 	restClientGetter, err := clientutil.NewRestClientGetterFromSecret(clusterSecret, cluster.Namespace)
 	if err != nil {
-		return ctrl.Result{}, err
+		return ctrl.Result{}, lifecycle.Failed, err
 	}
 
 	k8sClientForRemoteCluster, err := clientutil.NewK8sClientFromRestClientGetter(restClientGetter)
 	if err != nil {
-		return ctrl.Result{}, err
+		return ctrl.Result{}, lifecycle.Failed, err
 	}
 
 	if err := reconcileNamespaceInRemoteCluster(ctx, k8sClientForRemoteCluster, cluster); err != nil {
-		return ctrl.Result{}, err
+		return ctrl.Result{}, lifecycle.Failed, err
 	}
 	if err := reconcileServiceAccountInRemoteCluster(ctx, k8sClientForRemoteCluster, cluster); err != nil {
-		return ctrl.Result{}, err
+		return ctrl.Result{}, lifecycle.Failed, err
 	}
 	if err := reconcileClusterRoleBindingInRemoteCluster(ctx, k8sClientForRemoteCluster, cluster); err != nil {
-		return ctrl.Result{}, err
+		return ctrl.Result{}, lifecycle.Failed, err
 	}
 
-	var tokenRequestor = &tokenHelper{
+	var tokenRequest = &tokenHelper{
 		Client:                             r.Client,
 		RemoteClusterBearerTokenValidity:   r.RemoteClusterBearerTokenValidity,
 		RenewRemoteClusterBearerTokenAfter: r.RenewRemoteClusterBearerTokenAfter,
 	}
-	if err := tokenRequestor.ReconcileServiceAccountToken(ctx, restClientGetter, cluster); err != nil {
-		return ctrl.Result{}, err
+	if err := tokenRequest.ReconcileServiceAccountToken(ctx, restClientGetter, cluster); err != nil {
+		return ctrl.Result{}, lifecycle.Failed, err
 	}
 
 	if err := reconcileRemoteAPIServerVersion(ctx, restClientGetter, r.Client, cluster); err != nil {
-		return ctrl.Result{}, err
+		return ctrl.Result{}, lifecycle.Failed, err
+	}
+	return ctrl.Result{RequeueAfter: defaultRequeueInterval}, lifecycle.Success, nil
+}
+
+// EnsureDeleted - handles the deletion / cleanup of cluster resource
+func (r *RemoteClusterReconciler) EnsureDeleted(ctx context.Context, resource lifecycle.RuntimeObject) (ctrl.Result, lifecycle.ReconcileResult, error) {
+	cluster := resource.(*greenhousev1alpha1.Cluster) //nolint:errcheck
+	// delete all plugins that are bound to this cluster
+	deletionCount, err := deletePlugins(ctx, r.Client, cluster)
+	if err != nil {
+		return ctrl.Result{}, lifecycle.Failed, err
+	}
+	if deletionCount > 0 {
+		return ctrl.Result{RequeueAfter: 10 * time.Second}, lifecycle.Pending, nil
 	}
 
-	return ctrl.Result{RequeueAfter: defaultRequeueInterval}, nil
+	defer updateMetrics(cluster)
+
+	kubeConfigSecret := &corev1.Secret{}
+	if err := r.Client.Get(ctx, types.NamespacedName{Namespace: cluster.GetNamespace(), Name: cluster.GetSecretName()}, kubeConfigSecret); err != nil {
+		return ctrl.Result{}, lifecycle.Failed, err
+	}
+	restClientGetter, err := clientutil.NewRestClientGetterFromSecret(kubeConfigSecret, cluster.Namespace)
+	if err != nil {
+		return ctrl.Result{}, lifecycle.Failed, err
+	}
+	remoteClient, err := clientutil.NewK8sClientFromRestClientGetter(restClientGetter)
+	if err != nil {
+		return ctrl.Result{}, lifecycle.Failed, err
+	}
+
+	if err := deleteClusterRoleBindingInRemoteCluster(ctx, remoteClient); err != nil {
+		return ctrl.Result{}, lifecycle.Failed, err
+	}
+
+	if err := deleteServiceAccountInRemoteCluster(ctx, remoteClient, cluster); err != nil {
+		return ctrl.Result{}, lifecycle.Failed, err
+	}
+	return ctrl.Result{}, lifecycle.Success, nil
 }
 
 // generateNewClientKubeConfig generates a kubeconfig for the client to access the cluster from REST config coming from the secret
@@ -165,7 +169,6 @@ func generateNewClientKubeConfig(_ context.Context, restConfigGetter *clientutil
 	if err != nil {
 		return nil, errors.Wrapf(err, "failed to load kube clientConfig for cluster %s", cluster.GetName())
 	}
-
 	// TODO: replace overwrite with https://github.com/kubernetes/kubernetes/pull/119398 after 1.30 upgrade
 	kubeConfigGenerator := &KubeConfigHelper{
 		Host:        restConfig.Host,

pkg/controllers/cluster/direct_access_controller_test.go → pkg/controllers/cluster/cluster_controller_test.go

@@ -5,6 +5,7 @@ package cluster_test
 
 import (
 	"fmt"
+	"time"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
@@ -80,7 +81,7 @@ var _ = Describe("KubeConfig controller", func() {
 				Eventually(func() error {
 					var namespace = new(corev1.Namespace)
 					return remoteClient.Get(test.Ctx, types.NamespacedName{Namespace: "", Name: setup.Namespace()}, namespace)
-				}).Should(Succeed(), fmt.Sprintf("eventually the namespace %s should exist", setup.Namespace()))
+				}, 3*time.Minute).Should(Succeed(), fmt.Sprintf("eventually the namespace %s should exist", setup.Namespace()))
 
 				By("Checking service account has been created in remote cluster")
 				Eventually(func() error {