Cleanup routing peer deployment creation (#155)

This refactors the deployment creation for the routing peer to use the
create or update method. Keeping this a lot cleaner. It also changes to
set controller reference using the util method.

internal/controller/nbroutingpeer_controller.go

@@ -11,9 +11,10 @@ import (
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
-	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
 
 	"github.com/go-logr/logr"
@@ -104,7 +105,7 @@ func (r *NBRoutingPeerReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 	}
 
 	logger.Info("NBRoutingPeer: Checking deployment")
-	err = r.handleDeployment(ctx, req, nbrp, logger)
+	err = r.handleDeployment(ctx, nbrp)
 	if err != nil {
 		return ctrl.Result{}, err
 	}
@@ -114,167 +115,93 @@ func (r *NBRoutingPeerReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 }
 
 // handleDeployment reconcile routing peer Deployment
-func (r *NBRoutingPeerReconciler) handleDeployment(ctx context.Context, req ctrl.Request, nbrp *netbirdiov1.NBRoutingPeer, logger logr.Logger) error {
+func (r *NBRoutingPeerReconciler) handleDeployment(ctx context.Context, nbrp *netbirdiov1.NBRoutingPeer) error {
-	routingPeerDeployment := appsv1.Deployment{}
-	err := r.Client.Get(ctx, req.NamespacedName, &routingPeerDeployment)
-	if err != nil && !errors.IsNotFound(err) {
-		logger.Error(errKubernetesAPI, "error getting Deployment", "err", err)
-		nbrp.Status.Conditions = netbirdiov1.NBConditionFalse("internalError", fmt.Sprintf("error getting Deployment: %v", err))
-		return err
-	}
-
 	labels := r.DefaultLabels
 	maps.Copy(labels, nbrp.Spec.Labels)
 	podLabels := labels
 	podLabels["app.kubernetes.io/name"] = "netbird-router"
 
-	// Create deployment
+	var replicas int32 = 3
-	if errors.IsNotFound(err) {
+	if nbrp.Spec.Replicas != nil {
-		var replicas int32 = 3
+		replicas = *nbrp.Spec.Replicas
-		if nbrp.Spec.Replicas != nil {
-			replicas = *nbrp.Spec.Replicas
-		}
-		routingPeerDeployment = appsv1.Deployment{
-			ObjectMeta: v1.ObjectMeta{
-				Name:      nbrp.Name,
-				Namespace: nbrp.Namespace,
-				OwnerReferences: []v1.OwnerReference{
-					{
-						APIVersion:         netbirdiov1.GroupVersion.Identifier(),
-						Kind:               "NBRoutingPeer",
-						Name:               nbrp.Name,
-						UID:                nbrp.UID,
-						BlockOwnerDeletion: util.Ptr(true),
-					},
-				},
-				Labels:      labels,
-				Annotations: nbrp.Spec.Annotations,
-			},
-			Spec: appsv1.DeploymentSpec{
-				Replicas: &replicas,
-				Selector: &v1.LabelSelector{
-					MatchLabels: map[string]string{
-						"app.kubernetes.io/name": "netbird-router",
-					},
-				},
-				Template: corev1.PodTemplateSpec{
-					ObjectMeta: v1.ObjectMeta{
-						Labels: podLabels,
-					},
-					Spec: corev1.PodSpec{
-						NodeSelector: nbrp.Spec.NodeSelector,
-						Tolerations:  nbrp.Spec.Tolerations,
-						Containers: []corev1.Container{
-							{
-								Name:  "netbird",
-								Image: r.ClientImage,
-								Env: []corev1.EnvVar{
-									{
-										Name: "NB_SETUP_KEY",
-										ValueFrom: &corev1.EnvVarSource{
-											SecretKeyRef: &corev1.SecretKeySelector{
-												LocalObjectReference: corev1.LocalObjectReference{
-													Name: nbrp.Name,
-												},
-												Key: "setupKey",
-											},
-										},
-									},
-									{
-										Name:  "NB_MANAGEMENT_URL",
-										Value: r.ManagementURL,
-									},
-								},
-								SecurityContext: r.buildSecurityContext(nbrp),
-								Resources:       nbrp.Spec.Resources,
-								VolumeMounts:    nbrp.Spec.VolumeMounts,
-							},
-						},
-						Volumes: nbrp.Spec.Volumes,
-					},
-				},
-			},
-		}
-
-		err = r.Client.Create(ctx, &routingPeerDeployment)
-		if err != nil {
-			logger.Error(errKubernetesAPI, "error creating Deployment", "err", err)
-			nbrp.Status.Conditions = netbirdiov1.NBConditionFalse("internalError", fmt.Sprintf("error creating Deployment: %v", err))
-			return err
-		}
-	} else if err == nil {
-		updatedDeployment := routingPeerDeployment.DeepCopy()
-		updatedDeployment.ObjectMeta.Name = nbrp.Name
-		updatedDeployment.ObjectMeta.Namespace = nbrp.Namespace
-		updatedDeployment.ObjectMeta.OwnerReferences = []v1.OwnerReference{
-			{
-				APIVersion:         netbirdiov1.GroupVersion.Identifier(),
-				Kind:               "NBRoutingPeer",
-				Name:               nbrp.Name,
-				UID:                nbrp.UID,
-				BlockOwnerDeletion: util.Ptr(true),
-			},
-		}
-		updatedDeployment.ObjectMeta.Labels = labels
-		for k, v := range nbrp.Spec.Annotations {
-			updatedDeployment.ObjectMeta.Annotations[k] = nbrp.Spec.Annotations[v]
-		}
-		var replicas int32 = 3
-		if nbrp.Spec.Replicas != nil {
-			replicas = *nbrp.Spec.Replicas
-		}
-		updatedDeployment.Spec.Replicas = &replicas
-		updatedDeployment.Spec.Selector = &v1.LabelSelector{
-			MatchLabels: map[string]string{
-				"app.kubernetes.io/name": "netbird-router",
-			},
-		}
-		updatedDeployment.Spec.Template.Spec.Tolerations = nbrp.Spec.Tolerations
-		updatedDeployment.Spec.Template.Spec.NodeSelector = nbrp.Spec.NodeSelector
-		updatedDeployment.Spec.Template.ObjectMeta.Labels = podLabels
-		updatedDeployment.Spec.Template.Spec.Volumes = nbrp.Spec.Volumes
-		if len(updatedDeployment.Spec.Template.Spec.Containers) != 1 {
-			updatedDeployment.Spec.Template.Spec.Containers = []corev1.Container{{}}
-		}
-		updatedDeployment.Spec.Template.Spec.Containers[0].Name = "netbird"
-		updatedDeployment.Spec.Template.Spec.Containers[0].Image = r.ClientImage
-		updatedDeployment.Spec.Template.Spec.Containers[0].Env = []corev1.EnvVar{
-			{
-				Name: "NB_SETUP_KEY",
-				ValueFrom: &corev1.EnvVarSource{
-					SecretKeyRef: &corev1.SecretKeySelector{
-						LocalObjectReference: corev1.LocalObjectReference{
-							Name: nbrp.Name,
-						},
-						Key: "setupKey",
-					},
-				},
-			},
-			{
-				Name:  "NB_MANAGEMENT_URL",
-				Value: r.ManagementURL,
-			},
-		}
-		updatedDeployment.Spec.Template.Spec.Containers[0].SecurityContext = r.buildSecurityContext(nbrp)
-		updatedDeployment.Spec.Template.Spec.Containers[0].Resources = nbrp.Spec.Resources
-		updatedDeployment.Spec.Template.Spec.Containers[0].VolumeMounts = nbrp.Spec.VolumeMounts
-
-		patch := client.StrategicMergeFrom(&routingPeerDeployment)
-		bs, _ := patch.Data(updatedDeployment)
-		// To ensure no useless patching is done to the deployment being watched
-		// Minimum patch size is 2 for "{}"
-		if len(bs) <= 2 {
-			return nil
-		}
-		err = r.Client.Patch(ctx, updatedDeployment, patch)
-		if err != nil {
-			logger.Error(errKubernetesAPI, "error updating Deployment", "err", err)
-			nbrp.Status.Conditions = netbirdiov1.NBConditionFalse("internalError", fmt.Sprintf("error updating Deployment: %v", err))
-			return err
-		}
 	}
 
-	return err
+	securityContext := &corev1.SecurityContext{
+		Capabilities: &corev1.Capabilities{
+			Add: []corev1.Capability{
+				"NET_ADMIN",
+			},
+		},
+	}
+	if nbrp.Spec.Privileged != nil && *nbrp.Spec.Privileged {
+		securityContext.Privileged = nbrp.Spec.Privileged
+	}
+
+	dep := &appsv1.Deployment{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: nbrp.Namespace,
+			Name:      nbrp.Name,
+		},
+	}
+	_, err := controllerutil.CreateOrUpdate(ctx, r.Client, dep, func() error {
+		dep.ObjectMeta.Labels = labels
+		dep.ObjectMeta.Annotations = nbrp.Spec.Annotations
+		dep.Spec = appsv1.DeploymentSpec{
+			Replicas: &replicas,
+			Selector: &metav1.LabelSelector{
+				MatchLabels: map[string]string{
+					"app.kubernetes.io/name": "netbird-router",
+				},
+			},
+			Template: corev1.PodTemplateSpec{
+				ObjectMeta: metav1.ObjectMeta{
+					Labels: podLabels,
+				},
+				Spec: corev1.PodSpec{
+					NodeSelector: nbrp.Spec.NodeSelector,
+					Tolerations:  nbrp.Spec.Tolerations,
+					Containers: []corev1.Container{
+						{
+							Name:  "netbird",
+							Image: r.ClientImage,
+							Env: []corev1.EnvVar{
+								{
+									Name: "NB_SETUP_KEY",
+									ValueFrom: &corev1.EnvVarSource{
+										SecretKeyRef: &corev1.SecretKeySelector{
+											LocalObjectReference: corev1.LocalObjectReference{
+												Name: nbrp.Name,
+											},
+											Key: "setupKey",
+										},
+									},
+								},
+								{
+									Name:  "NB_MANAGEMENT_URL",
+									Value: r.ManagementURL,
+								},
+							},
+							SecurityContext: securityContext,
+							Resources:       nbrp.Spec.Resources,
+							VolumeMounts:    nbrp.Spec.VolumeMounts,
+						},
+					},
+					Volumes: nbrp.Spec.Volumes,
+				},
+			},
+		}
+
+		err := controllerutil.SetControllerReference(nbrp, dep, r.Scheme())
+		if err != nil {
+			return err
+		}
+
+		return nil
+	})
+	if err != nil {
+		return err
+	}
+	return nil
 }
 
 // handleRouter reconcile network routing peer in NetBird management API
@@ -356,10 +283,10 @@ func (r *NBRoutingPeerReconciler) handleSetupKey(ctx context.Context, req ctrl.R
 		nbrp.Status.SetupKeyID = &setupKey.Id
 
 		skSecret := corev1.Secret{
-			ObjectMeta: v1.ObjectMeta{
+			ObjectMeta: metav1.ObjectMeta{
 				Name:      nbrp.Name,
 				Namespace: nbrp.Namespace,
-				OwnerReferences: []v1.OwnerReference{
+				OwnerReferences: []metav1.OwnerReference{
 					{
 						APIVersion:         netbirdiov1.GroupVersion.Identifier(),
 						Kind:               "NBRoutingPeer",
@@ -466,10 +393,10 @@ func (r *NBRoutingPeerReconciler) handleGroup(ctx context.Context, req ctrl.Requ
 
 	if errors.IsNotFound(err) {
 		nbGroup = netbirdiov1.NBGroup{
-			ObjectMeta: v1.ObjectMeta{
+			ObjectMeta: metav1.ObjectMeta{
 				Name:      nbrp.Name,
 				Namespace: nbrp.Namespace,
-				OwnerReferences: []v1.OwnerReference{
+				OwnerReferences: []metav1.OwnerReference{
 					{
 						APIVersion:         netbirdiov1.GroupVersion.Identifier(),
 						Kind:               "NBRoutingPeer",
@@ -643,30 +570,11 @@ func (r *NBRoutingPeerReconciler) handleDelete(ctx context.Context, req ctrl.Req
 	return ctrl.Result{}, nil
 }
 
-// buildSecurityContext creates the appropriate SecurityContext based on the NBRoutingPeer spec
-func (r *NBRoutingPeerReconciler) buildSecurityContext(nbrp *netbirdiov1.NBRoutingPeer) *corev1.SecurityContext {
-	securityContext := &corev1.SecurityContext{
-		Capabilities: &corev1.Capabilities{
-			Add: []corev1.Capability{
-				"NET_ADMIN",
-			},
-		},
-	}
-
-	// Set privileged mode if specified
-	if nbrp.Spec.Privileged != nil && *nbrp.Spec.Privileged {
-		securityContext.Privileged = nbrp.Spec.Privileged
-	}
-
-	return securityContext
-}
-
 // SetupWithManager sets up the controller with the Manager.
 func (r *NBRoutingPeerReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewControllerManagedBy(mgr).
 		For(&netbirdiov1.NBRoutingPeer{}).
-		Named("nbroutingpeer").
+		Owns(&appsv1.Deployment{}).
-		Watches(&appsv1.Deployment{}, handler.EnqueueRequestForOwner(mgr.GetScheme(), mgr.GetRESTMapper(), &netbirdiov1.NBRoutingPeer{})).
 		Watches(&corev1.Secret{}, handler.EnqueueRequestForOwner(mgr.GetScheme(), mgr.GetRESTMapper(), &netbirdiov1.NBRoutingPeer{})).
 		Watches(&netbirdiov1.NBGroup{}, handler.EnqueueRequestForOwner(mgr.GetScheme(), mgr.GetRESTMapper(), &netbirdiov1.NBRoutingPeer{})).
 		Complete(r)