NETOBSERV-1418: make namespace immutable (#1229)

* NETOBSERV-1418: make namespace immutable

* remove unused functions

Author: jotak

apis/flowcollector/v1beta1/flowcollector_types.go

@@ -49,6 +49,7 @@ type FlowCollectorSpec struct {
 
 	// Namespace where NetObserv pods are deployed.
 	// +kubebuilder:default:=netobserv
+	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Namespace is immutable. If you need to change it, delete and recreate the resource."
 	Namespace string `json:"namespace,omitempty"`
 
 	// Agent configuration for flows extraction.

apis/flowcollector/v1beta2/flowcollector_types.go

@@ -49,6 +49,7 @@ type FlowCollectorSpec struct {
 
 	// Namespace where NetObserv pods are deployed.
 	// +kubebuilder:default:=netobserv
+	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Namespace is immutable. If you need to change it, delete and recreate the resource."
 	Namespace string `json:"namespace,omitempty"`
 
 	// Agent configuration for flows extraction.

bundle/manifests/flows.netobserv.io_flowcollectors.yaml

@@ -1835,6 +1835,10 @@ spec:
                 default: netobserv
                 description: Namespace where NetObserv pods are deployed.
                 type: string
+                x-kubernetes-validations:
+                - message: Namespace is immutable. If you need to change it, delete
+                    and recreate the resource.
+                  rule: self == oldSelf
               processor:
                 description: |-
                   `processor` defines the settings of the component that receives the flows from the agent,
@@ -7066,6 +7070,10 @@ spec:
                 default: netobserv
                 description: Namespace where NetObserv pods are deployed.
                 type: string
+                x-kubernetes-validations:
+                - message: Namespace is immutable. If you need to change it, delete
+                    and recreate the resource.
+                  rule: self == oldSelf
               networkPolicy:
                 description: '`networkPolicy` defines ingress network policy settings
                   for NetObserv components isolation.'

config/crd/bases/flows.netobserv.io_flowcollectors.yaml

@@ -1657,6 +1657,9 @@ spec:
                   default: netobserv
                   description: Namespace where NetObserv pods are deployed.
                   type: string
+                  x-kubernetes-validations:
+                    - message: Namespace is immutable. If you need to change it, delete and recreate the resource.
+                      rule: self == oldSelf
                 processor:
                   description: |-
                     `processor` defines the settings of the component that receives the flows from the agent,
@@ -6476,6 +6479,9 @@ spec:
                   default: netobserv
                   description: Namespace where NetObserv pods are deployed.
                   type: string
+                  x-kubernetes-validations:
+                    - message: Namespace is immutable. If you need to change it, delete and recreate the resource.
+                      rule: self == oldSelf
                 networkPolicy:
                   description: '`networkPolicy` defines ingress network policy settings for NetObserv components isolation.'
                   properties:

controllers/consoleplugin/consoleplugin_reconciler.go

@@ -52,11 +52,6 @@ func NewReconciler(cmn *reconcilers.Instance) CPReconciler {
 	return rec
 }
 
-// CleanupNamespace cleans up old namespace
-func (r *CPReconciler) CleanupNamespace(ctx context.Context) {
-	r.Managed.CleanupPreviousNamespace(ctx)
-}
-
 // Reconcile is the reconciler entry point to reconcile the current plugin state with the desired configuration
 func (r *CPReconciler) Reconcile(ctx context.Context, desired *flowslatest.FlowCollector) error {
 	l := log.FromContext(ctx).WithName("console-plugin")

controllers/ebpf/agent_controller.go

@@ -145,30 +145,6 @@ func (c *AgentController) Reconcile(ctx context.Context, target *flowslatest.Flo
 		return err
 	}
 
-	if c.PreviousPrivilegedNamespace() != c.PrivilegedNamespace() {
-		c.Managed.TryDeleteAll(ctx)
-
-		if current == nil {
-			rlog.Info("nothing to do, namespace already cleaned up", "currentAgent", target.Spec.Agent)
-			return nil
-		}
-		rlog.Info("namespace cleanup: deleting eBPF agent", "currentAgent", target.Spec.Agent)
-		if helper.IsAgentFeatureEnabled(&target.Spec.Agent.EBPF, flowslatest.EbpfManager) {
-			if err := c.bpfmanDetachNetobserv(ctx); err != nil {
-				rlog.Error(err, "failed to delete bpfapplication object")
-				// continue with eBPF agent deletion
-			}
-		}
-		if err := c.Delete(ctx, current); err != nil {
-			if errors.IsNotFound(err) {
-				return nil
-			}
-			return fmt.Errorf("deleting eBPF agent: %w", err)
-		}
-		// Current now has been deleted. Set it to nil to that it triggers actionCreate if we are changing namespace
-		current = nil
-	}
-
 	if err := c.permissions.Reconcile(ctx, &target.Spec.Agent.EBPF); err != nil {
 		return fmt.Errorf("reconciling permissions: %w", err)
 	}
@@ -212,12 +188,12 @@ func (c *AgentController) current(ctx context.Context) (*v1.DaemonSet, error) {
 	agentDS := v1.DaemonSet{}
 	if err := c.Get(ctx, types.NamespacedName{
 		Name:      constants.EBPFAgentName,
-		Namespace: c.PreviousPrivilegedNamespace(),
+		Namespace: c.PrivilegedNamespace(),
 	}, &agentDS); err != nil {
 		if errors.IsNotFound(err) {
 			return nil, nil
 		}
-		return nil, fmt.Errorf("can't read DaemonSet %s/%s: %w", c.PreviousPrivilegedNamespace(), constants.EBPFAgentName, err)
+		return nil, fmt.Errorf("can't read DaemonSet %s/%s: %w", c.PrivilegedNamespace(), constants.EBPFAgentName, err)
 	}
 	return &agentDS, nil
 }

controllers/ebpf/bpfmanager-controller.go

@@ -13,7 +13,6 @@ import (
 	"github.com/sirupsen/logrus"
 	"k8s.io/apimachinery/pkg/api/errors"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/klog"
 	"k8s.io/utils/ptr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
@@ -22,31 +21,6 @@ const (
 	netobservApp = "netobserv"
 )
 
-// bpfmanDetachNetobserv find BpfmanApplication object with all required ebpf hooks and detaches them using bpfman manager
-func (c *AgentController) bpfmanDetachNetobserv(ctx context.Context) error {
-	bpfApp := bpfmaniov1alpha1.ClusterBpfApplication{
-		ObjectMeta: v1.ObjectMeta{
-			Name: netobservApp,
-		},
-		TypeMeta: v1.TypeMeta{
-			Kind: "BpfApplication",
-		},
-	}
-
-	key := client.ObjectKey{Name: netobservApp}
-
-	err := c.Get(ctx, key, &bpfApp)
-	if err != nil {
-		return fmt.Errorf("failed to get BpfApplication: %w", err)
-	}
-
-	err = c.deleteBpfApplication(ctx, &bpfApp)
-	if err != nil {
-		return fmt.Errorf("failed to delete BpfApplication: %w", err)
-	}
-	return nil
-}
-
 // bpfmanAttachNetobserv Creates BpfmanApplication object with all required ebpf hooks and attaches them using bpfman manager
 func (c *AgentController) bpfmanAttachNetobserv(ctx context.Context, fc *flowslatest.FlowCollector) error {
 	var err error
@@ -254,11 +228,6 @@ func prepareBpfApplication(bpfApp *bpfmaniov1alpha1.ClusterBpfApplication, fc *f
 	}
 }
 
-func (c *AgentController) deleteBpfApplication(ctx context.Context, bpfApp *bpfmaniov1alpha1.ClusterBpfApplication) error {
-	klog.Info("Deleting BpfApplication Object")
-	return c.Delete(ctx, bpfApp)
-}
-
 func (c *AgentController) createBpfApplication(ctx context.Context, bpfApp *bpfmaniov1alpha1.ClusterBpfApplication) error {
 	return c.CreateOwned(ctx, bpfApp)
 }

controllers/ebpf/internal/permissions/permissions.go

@@ -54,11 +54,6 @@ func (c *Reconciler) Reconcile(ctx context.Context, desired *flowslatest.FlowCol
 
 func (c *Reconciler) reconcileNamespace(ctx context.Context) error {
 	ns := c.PrivilegedNamespace()
-	if ns != c.PreviousPrivilegedNamespace() {
-		if err := c.cleanupPreviousNamespace(ctx); err != nil {
-			return err
-		}
-	}
 	rlog := log.FromContext(ctx, "PrivilegedNamespace", ns)
 	actual := &v1.Namespace{}
 	if err := c.Get(ctx, client.ObjectKey{Name: ns}, actual); err != nil {
@@ -201,44 +196,3 @@ func (c *Reconciler) reconcileOpenshiftPermissions(
 	rlog.Info("SecurityContextConstraints already reconciled. Doing nothing")
 	return nil
 }
-
-func (c *Reconciler) cleanupPreviousNamespace(ctx context.Context) error {
-	rlog := log.FromContext(ctx, "PreviousPrivilegedNamespace", c.PreviousPrivilegedNamespace())
-
-	// Delete service account
-	if err := c.Delete(ctx, &v1.ServiceAccount{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      constants.EBPFServiceAccount,
-			Namespace: c.PreviousPrivilegedNamespace(),
-		},
-	}); err != nil {
-		if errors.IsNotFound(err) {
-			return nil
-		}
-		return fmt.Errorf("deleting eBPF agent ServiceAccount: %w", err)
-	}
-	// Do not delete SCC as it's not namespace-scoped (it will be reconciled "as usual")
-
-	previous := &v1.Namespace{}
-	if err := c.Get(ctx, client.ObjectKey{Name: c.PreviousPrivilegedNamespace()}, previous); err != nil {
-		if errors.IsNotFound(err) {
-			// Not found => return without error
-			rlog.Info("Previous privileged namespace not found, skipping cleanup")
-			return nil
-		}
-		return fmt.Errorf("can't retrieve previous namespace: %w", err)
-	}
-	// Make sure we own that namespace
-	if helper.IsOwned(previous) {
-		rlog.Info("Owning previous privileged namespace: deleting it")
-		if err := c.Delete(ctx, previous); err != nil {
-			if errors.IsNotFound(err) {
-				return nil
-			}
-			return fmt.Errorf("deleting privileged namespace: %w", err)
-		}
-	} else {
-		rlog.Info("Not owning previous privileged namespace: delete related content only")
-	}
-	return nil
-}

controllers/flowcollector_controller.go

@@ -121,7 +121,7 @@ func (r *FlowCollectorReconciler) reconcile(ctx context.Context, clh *helper.Cli
 	ns := helper.GetNamespace(&desired.Spec)
 	previousNamespace := r.status.GetDeployedNamespace(desired)
 	loki := helper.NewLokiConfig(&desired.Spec.Loki, ns)
-	reconcilersInfo := r.newCommonInfo(clh, ns, previousNamespace, &loki)
+	reconcilersInfo := r.newCommonInfo(clh, ns, &loki)
 
 	if err := r.checkFinalizer(ctx, desired); err != nil {
 		return err
@@ -137,13 +137,6 @@ func (r *FlowCollectorReconciler) reconcile(ctx context.Context, clh *helper.Cli
 
 	// Check namespace changed
 	if ns != previousNamespace {
-		if previousNamespace != "" {
-			// Namespace updated, clean up previous namespace
-			log.FromContext(ctx).
-				Info("FlowCollector namespace change detected: cleaning up previous namespace", "old", previousNamespace, "new", ns)
-			cpReconciler.CleanupNamespace(ctx)
-		}
-
 		// Update namespace in status
 		if err := r.status.SetDeployedNamespace(ctx, r.Client, ns); err != nil {
 			return r.status.Error("ChangeNamespaceError", err)
@@ -176,14 +169,13 @@ func (r *FlowCollectorReconciler) checkFinalizer(ctx context.Context, desired *f
 	return nil
 }
 
-func (r *FlowCollectorReconciler) newCommonInfo(clh *helper.Client, ns, prevNs string, loki *helper.LokiConfig) reconcilers.Common {
+func (r *FlowCollectorReconciler) newCommonInfo(clh *helper.Client, ns string, loki *helper.LokiConfig) reconcilers.Common {
 	return reconcilers.Common{
-		Client:            *clh,
-		Namespace:         ns,
-		PreviousNamespace: prevNs,
-		ClusterInfo:       r.mgr.ClusterInfo,
-		Watcher:           r.watcher,
-		Loki:              loki,
-		IsDownstream:      r.mgr.Config.DownstreamDeployment,
+		Client:       *clh,
+		Namespace:    ns,
+		ClusterInfo:  r.mgr.ClusterInfo,
+		Watcher:      r.watcher,
+		Loki:         loki,
+		IsDownstream: r.mgr.Config.DownstreamDeployment,
 	}
 }

controllers/flowcollector_controller_console_test.go

@@ -356,52 +356,6 @@ func flowCollectorConsolePluginSpecs() {
 		})
 	})
 
-	Context("Changing namespace", func() {
-		const otherNamespace = "other-namespace"
-		cpKey2 := types.NamespacedName{
-			Name:      "netobserv-plugin",
-			Namespace: otherNamespace,
-		}
-
-		It("Should update namespace successfully", func() {
-			updateCR(crKey, func(fc *flowslatest.FlowCollector) {
-				fc.Spec.Namespace = otherNamespace
-			})
-		})
-
-		It("Should redeploy console plugin in new namespace", func() {
-			By("Expecting deployment in previous namespace to be deleted")
-			Eventually(func() interface{} {
-				return k8sClient.Get(ctx, cpKey, &appsv1.Deployment{})
-			}, timeout, interval).Should(MatchError(`deployments.apps "netobserv-plugin" not found`))
-
-			By("Expecting service in previous namespace to be deleted")
-			Eventually(func() interface{} {
-				return k8sClient.Get(ctx, cpKey, &v1.Service{})
-			}, timeout, interval).Should(MatchError(`services "netobserv-plugin" not found`))
-
-			By("Expecting service account in previous namespace to be deleted")
-			Eventually(func() interface{} {
-				return k8sClient.Get(ctx, cpKey, &v1.ServiceAccount{})
-			}, timeout, interval).Should(MatchError(`serviceaccounts "netobserv-plugin" not found`))
-
-			By("Expecting deployment to be created in new namespace")
-			Eventually(func() interface{} {
-				return k8sClient.Get(ctx, cpKey2, &appsv1.Deployment{})
-			}, timeout, interval).Should(Succeed())
-
-			By("Expecting service to be created in new namespace")
-			Eventually(func() interface{} {
-				return k8sClient.Get(ctx, cpKey2, &v1.Service{})
-			}, timeout, interval).Should(Succeed())
-
-			By("Expecting service account to be created in new namespace")
-			Eventually(func() interface{} {
-				return k8sClient.Get(ctx, cpKey2, &v1.ServiceAccount{})
-			}, timeout, interval).Should(Succeed())
-		})
-	})
-
 	Context("Cleanup", func() {
 		It("Should delete CR", func() {
 			cleanupCR(crKey)