fix: konnectivity clean-up and status updates (#574)

Signed-off-by: Dario Tranchitella <dario@tranchitella.eu>

internal/resources/addons/coredns.go

@@ -21,6 +21,7 @@ import (
 	"github.com/clastix/kamaji/internal/constants"
 	"github.com/clastix/kamaji/internal/kubeadm"
 	"github.com/clastix/kamaji/internal/resources"
+	addons_utils "github.com/clastix/kamaji/internal/resources/addons/utils"
 	"github.com/clastix/kamaji/internal/resources/utils"
 	"github.com/clastix/kamaji/internal/utilities"
 )
@@ -235,32 +236,32 @@ func (c *CoreDNS) decodeManifests(ctx context.Context, tcp *kamajiv1alpha1.Tenan
 	if err = utilities.DecodeFromYAML(string(parts[1]), c.deployment); err != nil {
 		return errors.Wrap(err, "unable to decode Deployment manifest")
 	}
-	setKamajiManagedLabels(c.deployment)
+	addons_utils.SetKamajiManagedLabels(c.deployment)
 
 	if err = utilities.DecodeFromYAML(string(parts[2]), c.configMap); err != nil {
 		return errors.Wrap(err, "unable to decode ConfigMap manifest")
 	}
-	setKamajiManagedLabels(c.configMap)
+	addons_utils.SetKamajiManagedLabels(c.configMap)
 
 	if err = utilities.DecodeFromYAML(string(parts[3]), c.service); err != nil {
 		return errors.Wrap(err, "unable to decode Service manifest")
 	}
-	setKamajiManagedLabels(c.service)
+	addons_utils.SetKamajiManagedLabels(c.service)
 
 	if err = utilities.DecodeFromYAML(string(parts[4]), c.clusterRole); err != nil {
 		return errors.Wrap(err, "unable to decode ClusterRole manifest")
 	}
-	setKamajiManagedLabels(c.clusterRole)
+	addons_utils.SetKamajiManagedLabels(c.clusterRole)
 
 	if err = utilities.DecodeFromYAML(string(parts[5]), c.clusterRoleBinding); err != nil {
 		return errors.Wrap(err, "unable to decode ClusterRoleBinding manifest")
 	}
-	setKamajiManagedLabels(c.clusterRoleBinding)
+	addons_utils.SetKamajiManagedLabels(c.clusterRoleBinding)
 
 	if err = utilities.DecodeFromYAML(string(parts[6]), c.serviceAccount); err != nil {
 		return errors.Wrap(err, "unable to decode ServiceAccount manifest")
 	}
-	setKamajiManagedLabels(c.serviceAccount)
+	addons_utils.SetKamajiManagedLabels(c.serviceAccount)
 
 	return nil
 }

internal/resources/addons/kube_proxy.go

@@ -23,6 +23,7 @@ import (
 	"github.com/clastix/kamaji/internal/constants"
 	"github.com/clastix/kamaji/internal/kubeadm"
 	"github.com/clastix/kamaji/internal/resources"
+	addon_utils "github.com/clastix/kamaji/internal/resources/addons/utils"
 	"github.com/clastix/kamaji/internal/resources/utils"
 	"github.com/clastix/kamaji/internal/utilities"
 )
@@ -397,32 +398,32 @@ func (k *KubeProxy) decodeManifests(ctx context.Context, tcp *kamajiv1alpha1.Ten
 	if err = utilities.DecodeFromYAML(string(parts[1]), k.serviceAccount); err != nil {
 		return errors.Wrap(err, "unable to decode ServiceAccount manifest")
 	}
-	setKamajiManagedLabels(k.serviceAccount)
+	addon_utils.SetKamajiManagedLabels(k.serviceAccount)
 
 	if err = utilities.DecodeFromYAML(string(parts[2]), k.clusterRoleBinding); err != nil {
 		return errors.Wrap(err, "unable to decode ClusterRoleBinding manifest")
 	}
-	setKamajiManagedLabels(k.clusterRoleBinding)
+	addon_utils.SetKamajiManagedLabels(k.clusterRoleBinding)
 
 	if err = utilities.DecodeFromYAML(string(parts[3]), k.role); err != nil {
 		return errors.Wrap(err, "unable to decode Role manifest")
 	}
-	setKamajiManagedLabels(k.role)
+	addon_utils.SetKamajiManagedLabels(k.role)
 
 	if err = utilities.DecodeFromYAML(string(parts[4]), k.roleBinding); err != nil {
 		return errors.Wrap(err, "unable to decode RoleBinding manifest")
 	}
-	setKamajiManagedLabels(k.roleBinding)
+	addon_utils.SetKamajiManagedLabels(k.roleBinding)
 
 	if err = utilities.DecodeFromYAML(string(parts[5]), k.configMap); err != nil {
 		return errors.Wrap(err, "unable to decode ConfigMap manifest")
 	}
-	setKamajiManagedLabels(k.configMap)
+	addon_utils.SetKamajiManagedLabels(k.configMap)
 
 	if err = utilities.DecodeFromYAML(string(parts[6]), k.daemonSet); err != nil {
 		return errors.Wrap(err, "unable to decode DaemonSet manifest")
 	}
-	setKamajiManagedLabels(k.daemonSet)
+	addon_utils.SetKamajiManagedLabels(k.daemonSet)
 
 	return nil
 }

internal/resources/addons/managed_labels.go → internal/resources/addons/utils/managed_labels.go

@@ -1,7 +1,7 @@
 // Copyright 2022 Clastix Labs
 // SPDX-License-Identifier: Apache-2.0
 
-package addons
+package utils
 
 import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -10,7 +10,7 @@ import (
 	"github.com/clastix/kamaji/internal/utilities"
 )
 
-func setKamajiManagedLabels(obj client.Object) {
+func SetKamajiManagedLabels(obj client.Object) {
 	obj.SetLabels(utilities.MergeMaps(obj.GetLabels(), map[string]string{
 		constants.ProjectNameLabelKey: constants.ProjectNameLabelValue,
 	}))

internal/resources/konnectivity/agent.go

@@ -18,6 +18,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/log"
 
 	kamajiv1alpha1 "github.com/clastix/kamaji/api/v1alpha1"
+	"github.com/clastix/kamaji/internal/constants"
 	"github.com/clastix/kamaji/internal/utilities"
 )
 
@@ -27,17 +28,32 @@ type Agent struct {
 	tenantClient client.Client
 }
 
-func (r *Agent) ShouldStatusBeUpdated(_ context.Context, tenantControlPlane *kamajiv1alpha1.TenantControlPlane) bool {
-	return tenantControlPlane.Spec.Addons.Konnectivity == nil && len(tenantControlPlane.Status.Addons.Konnectivity.Agent.Namespace) == 0
+func (r *Agent) ShouldStatusBeUpdated(_ context.Context, tcp *kamajiv1alpha1.TenantControlPlane) bool {
+	return tcp.Spec.Addons.Konnectivity == nil && (tcp.Status.Addons.Konnectivity.Agent.Namespace != "" || tcp.Status.Addons.Konnectivity.Agent.Name != "") ||
+		tcp.Spec.Addons.Konnectivity != nil && (tcp.Status.Addons.Konnectivity.Agent.Namespace != r.resource.Namespace || tcp.Status.Addons.Konnectivity.Agent.Name != r.resource.Name)
 }
 
 func (r *Agent) ShouldCleanup(tenantControlPlane *kamajiv1alpha1.TenantControlPlane) bool {
-	return tenantControlPlane.Spec.Addons.Konnectivity == nil
+	return tenantControlPlane.Spec.Addons.Konnectivity == nil && tenantControlPlane.Status.Addons.Konnectivity.Enabled
 }
 
 func (r *Agent) CleanUp(ctx context.Context, _ *kamajiv1alpha1.TenantControlPlane) (bool, error) {
 	logger := log.FromContext(ctx, "resource", r.GetName())
 
+	if err := r.tenantClient.Get(ctx, client.ObjectKeyFromObject(r.resource), r.resource); err != nil {
+		if k8serrors.IsNotFound(err) {
+			return false, nil
+		}
+
+		logger.Error(err, "cannot retrieve the requested resource for deletion")
+
+		return false, err
+	}
+
+	if labels := r.resource.GetLabels(); labels == nil || labels[constants.ProjectNameLabelKey] != constants.ProjectNameLabelValue {
+		return false, nil
+	}
+
 	if err := r.tenantClient.Delete(ctx, r.resource); err != nil {
 		if k8serrors.IsNotFound(err) {
 			return false, nil
@@ -83,18 +99,16 @@ func (r *Agent) GetName() string {
 }
 
 func (r *Agent) UpdateTenantControlPlaneStatus(_ context.Context, tenantControlPlane *kamajiv1alpha1.TenantControlPlane) error {
+	tenantControlPlane.Status.Addons.Konnectivity.Agent = kamajiv1alpha1.ExternalKubernetesObjectStatus{}
+
 	if tenantControlPlane.Spec.Addons.Konnectivity != nil {
 		tenantControlPlane.Status.Addons.Konnectivity.Agent = kamajiv1alpha1.ExternalKubernetesObjectStatus{
 			Name:       r.resource.GetName(),
 			Namespace:  r.resource.GetNamespace(),
 			LastUpdate: metav1.Now(),
 		}
-
-		return nil
 	}
 
-	tenantControlPlane.Status.Addons.Konnectivity.Agent = kamajiv1alpha1.ExternalKubernetesObjectStatus{}
-
 	return nil
 }
 
@@ -109,7 +123,7 @@ func (r *Agent) mutate(ctx context.Context, tenantControlPlane *kamajiv1alpha1.T
 			return err
 		}
 
-		r.resource.SetLabels(utilities.KamajiLabels(tenantControlPlane.GetName(), r.GetName()))
+		r.resource.SetLabels(utilities.MergeMaps(r.resource.GetLabels(), utilities.KamajiLabels(tenantControlPlane.GetName(), r.GetName())))
 
 		if r.resource.Spec.Selector == nil {
 			r.resource.Spec.Selector = &metav1.LabelSelector{}

internal/resources/konnectivity/certificate_resource.go

@@ -34,7 +34,7 @@ func (r *CertificateResource) ShouldStatusBeUpdated(_ context.Context, tenantCon
 }
 
 func (r *CertificateResource) ShouldCleanup(tenantControlPlane *kamajiv1alpha1.TenantControlPlane) bool {
-	return tenantControlPlane.Spec.Addons.Konnectivity == nil
+	return tenantControlPlane.Spec.Addons.Konnectivity == nil && tenantControlPlane.Status.Addons.Konnectivity.Enabled
 }
 
 func (r *CertificateResource) CleanUp(ctx context.Context, _ *kamajiv1alpha1.TenantControlPlane) (bool, error) {
@@ -65,6 +65,10 @@ func (r *CertificateResource) Define(_ context.Context, tenantControlPlane *kama
 }
 
 func (r *CertificateResource) CreateOrUpdate(ctx context.Context, tenantControlPlane *kamajiv1alpha1.TenantControlPlane) (controllerutil.OperationResult, error) {
+	if tenantControlPlane.Spec.Addons.Konnectivity == nil {
+		return controllerutil.OperationResultNone, nil
+	}
+
 	return controllerutil.CreateOrUpdate(ctx, r.Client, r.resource, r.mutate(ctx, tenantControlPlane))
 }
 
@@ -73,16 +77,14 @@ func (r *CertificateResource) GetName() string {
 }
 
 func (r *CertificateResource) UpdateTenantControlPlaneStatus(_ context.Context, tenantControlPlane *kamajiv1alpha1.TenantControlPlane) error {
+	tenantControlPlane.Status.Addons.Konnectivity.Certificate = kamajiv1alpha1.CertificatePrivateKeyPairStatus{}
+
 	if tenantControlPlane.Spec.Addons.Konnectivity != nil {
 		tenantControlPlane.Status.Addons.Konnectivity.Certificate.LastUpdate = metav1.Now()
 		tenantControlPlane.Status.Addons.Konnectivity.Certificate.SecretName = r.resource.GetName()
 		tenantControlPlane.Status.Addons.Konnectivity.Certificate.Checksum = utilities.GetObjectChecksum(r.resource)
-
-		return nil
 	}
 
-	tenantControlPlane.Status.Addons.Konnectivity.Certificate = kamajiv1alpha1.CertificatePrivateKeyPairStatus{}
-
 	return nil
 }
 
@@ -91,6 +93,7 @@ func (r *CertificateResource) mutate(ctx context.Context, tenantControlPlane *ka
 		logger := log.FromContext(ctx, "resource", r.GetName())
 
 		r.resource.SetLabels(utilities.MergeMaps(
+			r.resource.GetLabels(),
 			utilities.KamajiLabels(tenantControlPlane.GetName(), r.GetName()),
 			map[string]string{
 				constants.ControllerLabelResource: "x509",

internal/resources/konnectivity/cluster_role_binding_resource.go

@@ -14,6 +14,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/log"
 
 	kamajiv1alpha1 "github.com/clastix/kamaji/api/v1alpha1"
+	"github.com/clastix/kamaji/internal/constants"
 	"github.com/clastix/kamaji/internal/utilities"
 )
 
@@ -24,17 +25,33 @@ type ClusterRoleBindingResource struct {
 	tenantClient client.Client
 }
 
-func (r *ClusterRoleBindingResource) ShouldStatusBeUpdated(_ context.Context, tenantControlPlane *kamajiv1alpha1.TenantControlPlane) bool {
-	return tenantControlPlane.Status.Addons.Konnectivity.ClusterRoleBinding.Name != r.resource.GetName()
+func (r *ClusterRoleBindingResource) ShouldStatusBeUpdated(_ context.Context, tcp *kamajiv1alpha1.TenantControlPlane) bool {
+	return tcp.Spec.Addons.Konnectivity == nil && tcp.Status.Addons.Konnectivity.ClusterRoleBinding.Name != "" ||
+		tcp.Spec.Addons.Konnectivity != nil && (tcp.Status.Addons.Konnectivity.ClusterRoleBinding.Name == "" ||
+			tcp.Status.Addons.Konnectivity.ClusterRoleBinding.Name != r.resource.GetName())
 }
 
 func (r *ClusterRoleBindingResource) ShouldCleanup(tenantControlPlane *kamajiv1alpha1.TenantControlPlane) bool {
-	return tenantControlPlane.Spec.Addons.Konnectivity == nil && len(tenantControlPlane.Status.Addons.Konnectivity.ClusterRoleBinding.Name) > 0
+	return tenantControlPlane.Spec.Addons.Konnectivity == nil && tenantControlPlane.Status.Addons.Konnectivity.Enabled
 }
 
 func (r *ClusterRoleBindingResource) CleanUp(ctx context.Context, _ *kamajiv1alpha1.TenantControlPlane) (bool, error) {
 	logger := log.FromContext(ctx, "resource", r.GetName())
 
+	if err := r.tenantClient.Get(ctx, client.ObjectKeyFromObject(r.resource), r.resource); err != nil {
+		if k8serrors.IsNotFound(err) {
+			return false, nil
+		}
+
+		logger.Error(err, "cannot retrieve the requested resource for deletion")
+
+		return false, err
+	}
+
+	if labels := r.resource.GetLabels(); labels == nil || labels[constants.ProjectNameLabelKey] != constants.ProjectNameLabelValue {
+		return false, nil
+	}
+
 	if err := r.tenantClient.Delete(ctx, r.resource); err != nil {
 		if k8serrors.IsNotFound(err) {
 			return false, nil
@@ -67,35 +84,33 @@ func (r *ClusterRoleBindingResource) Define(ctx context.Context, tenantControlPl
 }
 
 func (r *ClusterRoleBindingResource) CreateOrUpdate(ctx context.Context, tcp *kamajiv1alpha1.TenantControlPlane) (controllerutil.OperationResult, error) {
-	if tcp.Spec.Addons.Konnectivity != nil {
-		return controllerutil.CreateOrUpdate(ctx, r.tenantClient, r.resource, r.mutate(tcp))
+	if tcp.Spec.Addons.Konnectivity == nil {
+		return controllerutil.OperationResultNone, nil
 	}
 
-	return controllerutil.OperationResultNone, nil
+	return controllerutil.CreateOrUpdate(ctx, r.tenantClient, r.resource, r.mutate(tcp))
 }
 
 func (r *ClusterRoleBindingResource) GetName() string {
 	return "konnectivity-clusterrolebinding"
 }
 
 func (r *ClusterRoleBindingResource) UpdateTenantControlPlaneStatus(_ context.Context, tenantControlPlane *kamajiv1alpha1.TenantControlPlane) error {
+	tenantControlPlane.Status.Addons.Konnectivity.ClusterRoleBinding = kamajiv1alpha1.ExternalKubernetesObjectStatus{}
+
 	if tenantControlPlane.Spec.Addons.Konnectivity != nil {
-		tenantControlPlane.Status.Addons.Konnectivity.Enabled = true
 		tenantControlPlane.Status.Addons.Konnectivity.ClusterRoleBinding = kamajiv1alpha1.ExternalKubernetesObjectStatus{
 			Name: r.resource.GetName(),
 		}
-
-		return nil
 	}
 
-	tenantControlPlane.Status.Addons.Konnectivity.ClusterRoleBinding = kamajiv1alpha1.ExternalKubernetesObjectStatus{}
-
 	return nil
 }
 
 func (r *ClusterRoleBindingResource) mutate(tenantControlPlane *kamajiv1alpha1.TenantControlPlane) controllerutil.MutateFn {
 	return func() error {
 		r.resource.SetLabels(utilities.MergeMaps(
+			r.resource.GetLabels(),
 			utilities.KamajiLabels(tenantControlPlane.GetName(), r.GetName()),
 			map[string]string{
 				"kubernetes.io/cluster-service":   "true",

internal/resources/konnectivity/deployment_resource.go

@@ -27,9 +27,8 @@ type KubernetesDeploymentResource struct {
 
 func (r *KubernetesDeploymentResource) ShouldStatusBeUpdated(_ context.Context, tenantControlPlane *kamajiv1alpha1.TenantControlPlane) bool {
 	switch {
-	case tenantControlPlane.Spec.Addons.Konnectivity == nil && tenantControlPlane.Status.Addons.Konnectivity.Enabled:
-		fallthrough
-	case tenantControlPlane.Spec.Addons.Konnectivity != nil && !tenantControlPlane.Status.Addons.Konnectivity.Enabled:
+	case tenantControlPlane.Spec.Addons.Konnectivity == nil && tenantControlPlane.Status.Addons.Konnectivity.Enabled,
+		tenantControlPlane.Spec.Addons.Konnectivity != nil && !tenantControlPlane.Status.Addons.Konnectivity.Enabled:
 		return true
 	default:
 		return false
@@ -94,6 +93,10 @@ func (r *KubernetesDeploymentResource) mutate(_ context.Context, tenantControlPl
 }
 
 func (r *KubernetesDeploymentResource) CreateOrUpdate(ctx context.Context, tenantControlPlane *kamajiv1alpha1.TenantControlPlane) (controllerutil.OperationResult, error) {
+	if tenantControlPlane.Spec.Addons.Konnectivity == nil {
+		return controllerutil.OperationResultNone, nil
+	}
+
 	return utilities.CreateOrUpdateWithConflict(ctx, r.Client, r.resource, r.mutate(ctx, tenantControlPlane))
 }
 

internal/resources/konnectivity/egress_selector_configuration_resource.go

@@ -36,7 +36,7 @@ func (r *EgressSelectorConfigurationResource) Define(_ context.Context, tenantCo
 }
 
 func (r *EgressSelectorConfigurationResource) ShouldCleanup(tenantControlPlane *kamajiv1alpha1.TenantControlPlane) bool {
-	return tenantControlPlane.Spec.Addons.Konnectivity == nil
+	return tenantControlPlane.Spec.Addons.Konnectivity == nil && tenantControlPlane.Status.Addons.Konnectivity.Enabled
 }
 
 func (r *EgressSelectorConfigurationResource) CleanUp(ctx context.Context, _ *kamajiv1alpha1.TenantControlPlane) (bool, error) {
@@ -56,6 +56,10 @@ func (r *EgressSelectorConfigurationResource) CleanUp(ctx context.Context, _ *ka
 }
 
 func (r *EgressSelectorConfigurationResource) CreateOrUpdate(ctx context.Context, tenantControlPlane *kamajiv1alpha1.TenantControlPlane) (controllerutil.OperationResult, error) {
+	if tenantControlPlane.Spec.Addons.Konnectivity == nil {
+		return controllerutil.OperationResultNone, nil
+	}
+
 	return controllerutil.CreateOrUpdate(ctx, r.Client, r.resource, r.mutate(ctx, tenantControlPlane))
 }
 
@@ -68,15 +72,13 @@ func (r *EgressSelectorConfigurationResource) ShouldStatusBeUpdated(_ context.Co
 }
 
 func (r *EgressSelectorConfigurationResource) UpdateTenantControlPlaneStatus(_ context.Context, tenantControlPlane *kamajiv1alpha1.TenantControlPlane) error {
+	tenantControlPlane.Status.Addons.Konnectivity.ConfigMap = kamajiv1alpha1.KonnectivityConfigMap{}
+
 	if tenantControlPlane.Spec.Addons.Konnectivity != nil {
 		tenantControlPlane.Status.Addons.Konnectivity.ConfigMap.Name = r.resource.GetName()
 		tenantControlPlane.Status.Addons.Konnectivity.ConfigMap.Checksum = utilities.GetObjectChecksum(r.resource)
-
-		return nil
 	}
 
-	tenantControlPlane.Status.Addons.Konnectivity.ConfigMap = kamajiv1alpha1.KonnectivityConfigMap{}
-
 	return nil
 }