Set best practice defaults for network router deployment (#214)

This change sets some Kubernetes best practices as defaults. Such as
topology spread and pod disruption budget.

It also exposes log level and image settings in the root struct to make
it easier to override commonly configured settings.

Fixes #77 
Fixes #162

Signed-off-by: Philip Laine <philip.laine@gmail.com>

api/v1alpha1/networkrouter_types.go

@@ -11,6 +11,14 @@ type NetworkRouterSpec struct {
 	// +required
 	DNSZoneRef DNSZoneReference `json:"dnsZoneRef"`
 
+	// Netbird client image.
+	// +optional
+	Image string `json:"image,omitempty"`
+
+	// Log level for Netbird client.
+	// +optional
+	LogLevel string `json:"logLevel,omitempty"`
+
 	// WorkloadOverride contains configuration that will override the default workload.
 	// +optional
 	WorkloadOverride *WorkloadOverride `json:"workloadOverride,omitempty"`
@@ -34,6 +42,8 @@ type WorkloadOverride struct {
 
 	// Replicas sets the amount of client replicas.
 	// +optional
+	// +kubebuilder:default=3
+	// +kubebuilder:validation:Minimum=1
 	Replicas *int32 `json:"replicas"`
 
 	// PodTemplate overrides the pod template.

docs/api-reference.md

@@ -255,6 +255,8 @@ _Appears in:_
 | Field | Description | Default | Validation |
 | --- | --- | --- | --- |
 | `dnsZoneRef` _[DNSZoneReference](#dnszonereference)_ | DNSZoneRef is a reference to the DNS zone used to create records for resources. |  | Required: \{\} <br /> |
+| `image` _string_ | Netbird client image. |  | Optional: \{\} <br /> |
+| `logLevel` _string_ | Log level for Netbird client. |  | Optional: \{\} <br /> |
 | `workloadOverride` _[WorkloadOverride](#workloadoverride)_ | WorkloadOverride contains configuration that will override the default workload. |  | Optional: \{\} <br /> |
 
 
@@ -407,7 +409,7 @@ _Appears in:_
 | --- | --- | --- | --- |
 | `labels` _object (keys:string, values:string)_ | Labels that will be added. |  | Optional: \{\} <br /> |
 | `annotations` _object (keys:string, values:string)_ | Annotations that will be added. |  | Optional: \{\} <br /> |
-| `replicas` _integer_ | Replicas sets the amount of client replicas. |  | Optional: \{\} <br /> |
+| `replicas` _integer_ | Replicas sets the amount of client replicas. | 3 | Minimum: 1 <br />Optional: \{\} <br /> |
 | `podTemplate` _[PodTemplateSpec](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.35/#podtemplatespec-v1-core)_ | PodTemplate overrides the pod template. |  | Schemaless: \{\} <br />Optional: \{\} <br /> |
 
 

helm/kubernetes-operator/crds/netbird.io_networkrouters.yaml

@@ -57,6 +57,12 @@ spec:
                 required:
                 - name
                 type: object
+              image:
+                description: Netbird client image.
+                type: string
+              logLevel:
+                description: Log level for Netbird client.
+                type: string
               workloadOverride:
                 description: WorkloadOverride contains configuration that will override
                   the default workload.
@@ -75,8 +81,10 @@ spec:
                     description: PodTemplate overrides the pod template.
                     x-kubernetes-preserve-unknown-fields: true
                   replicas:
+                    default: 3
                     description: Replicas sets the amount of client replicas.
                     format: int32
+                    minimum: 1
                     type: integer
                 type: object
             required:

internal/controller/networkrouter_controller.go

@@ -16,11 +16,16 @@ import (
 	"github.com/netbirdio/netbird/shared/management/http/api"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
+	policyv1 "k8s.io/api/policy/v1"
+	kerrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/intstr"
 	"k8s.io/apimachinery/pkg/util/strategicpatch"
 	appsv1ac "k8s.io/client-go/applyconfigurations/apps/v1"
 	corev1ac "k8s.io/client-go/applyconfigurations/core/v1"
 	metav1ac "k8s.io/client-go/applyconfigurations/meta/v1"
+	policyv1ac "k8s.io/client-go/applyconfigurations/policy/v1"
 	"k8s.io/utils/ptr"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -190,12 +195,31 @@ func (r *NetworkRouterReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 		"app.kubernetes.io/instance": req.Name,
 	}
 
+	logLevel := "info"
+	if netRouter.Spec.LogLevel != "" {
+		logLevel = netRouter.Spec.LogLevel
+	}
+
+	clientImage := r.ClientImage
+	if netRouter.Spec.Image != "" {
+		clientImage = netRouter.Spec.Image
+	}
+
 	podTemplateSpecAC := corev1ac.PodTemplateSpec().
 		WithLabels(selectorLabels).
 		WithSpec(corev1ac.PodSpec().
+			WithTopologySpreadConstraints(
+				corev1ac.TopologySpreadConstraint().
+					WithMaxSkew(1).
+					WithTopologyKey(corev1.LabelHostname).
+					WithWhenUnsatisfiable(corev1.ScheduleAnyway).
+					WithLabelSelector(metav1ac.LabelSelector().
+						WithMatchLabels(selectorLabels),
+					),
+			).
 			WithContainers(corev1ac.Container().
 				WithName("netbird").
-				WithImage(r.ClientImage).
+				WithImage(clientImage).
 				WithEnv(
 					corev1ac.EnvVar().
 						WithName("NB_SETUP_KEY").
@@ -210,7 +234,13 @@ func (r *NetworkRouterReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 						WithValue(r.ManagementURL),
 					corev1ac.EnvVar().
 						WithName("NB_LOG_LEVEL").
-						WithValue("info"),
+						WithValue(logLevel),
+					corev1ac.EnvVar().
+						WithName("NB_LOG_FILE").
+						WithValue("console"),
+					corev1ac.EnvVar().
+						WithName("NB_ENTRYPOINT_SERVICE_TIMEOUT").
+						WithValue("0"),
 				).
 				WithStartupProbe(corev1ac.Probe().WithExec(corev1ac.ExecAction().WithCommand("netbird", "status", "--check", "startup"))).
 				WithReadinessProbe(corev1ac.Probe().WithExec(corev1ac.ExecAction().WithCommand("netbird", "status", "--check", "ready"))).
@@ -221,19 +251,25 @@ func (r *NetworkRouterReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 						WithAdd("SYS_ADMIN"),
 					).
 					WithPrivileged(true),
+				).
+				WithResources(corev1ac.ResourceRequirements().
+					WithRequests(corev1.ResourceList{
+						corev1.ResourceCPU:    resource.MustParse("100m"),
+						corev1.ResourceMemory: resource.MustParse("128Mi"),
+					}),
 				),
 			),
 		)
 
-	depLabels := map[string]string{}
-	depAnnotations := map[string]string{}
+	workloadLabels := map[string]string{}
+	workloadAnnotations := map[string]string{}
 	replicas := int32(3)
 	if netRouter.Spec.WorkloadOverride != nil {
 		if netRouter.Spec.WorkloadOverride.Labels != nil {
-			depLabels = netRouter.Spec.WorkloadOverride.Labels
+			workloadLabels = netRouter.Spec.WorkloadOverride.Labels
 		}
 		if netRouter.Spec.WorkloadOverride.Annotations != nil {
-			depAnnotations = netRouter.Spec.WorkloadOverride.Annotations
+			workloadAnnotations = netRouter.Spec.WorkloadOverride.Annotations
 		}
 		if netRouter.Spec.WorkloadOverride.Replicas != nil {
 			replicas = *netRouter.Spec.WorkloadOverride.Replicas
@@ -257,17 +293,46 @@ func (r *NetworkRouterReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 			}
 		}
 	}
-	maps.Copy(depLabels, selectorLabels)
+	maps.Copy(workloadLabels, selectorLabels)
 
 	depAC := appsv1ac.Deployment(fmt.Sprintf("networkrouter-%s", req.Name), req.Namespace).
 		WithOwnerReferences(ownerRef).
-		WithLabels(depLabels).
-		WithAnnotations(depAnnotations).
+		WithLabels(workloadLabels).
+		WithAnnotations(workloadAnnotations).
 		WithSpec(appsv1ac.DeploymentSpec().WithReplicas(replicas).WithSelector(metav1ac.LabelSelector().WithMatchLabels(selectorLabels)).WithTemplate(podTemplateSpecAC))
 	err = r.Client.Apply(ctx, depAC)
 	if err != nil {
 		return ctrl.Result{}, err
 	}
+
+	if replicas > 1 {
+		pdbAC := policyv1ac.PodDisruptionBudget(fmt.Sprintf("networkrouter-%s", req.Name), req.Namespace).
+			WithOwnerReferences(ownerRef).
+			WithLabels(workloadLabels).
+			WithAnnotations(workloadAnnotations).
+			WithSpec(policyv1ac.PodDisruptionBudgetSpec().
+				WithMaxUnavailable(intstr.FromInt(1)).
+				WithSelector(metav1ac.LabelSelector().
+					WithMatchLabels(selectorLabels),
+				),
+			)
+		err = r.Client.Apply(ctx, pdbAC)
+		if err != nil {
+			return ctrl.Result{}, err
+		}
+	} else {
+		pdb := policyv1.PodDisruptionBudget{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      fmt.Sprintf("networkrouter-%s", req.Name),
+				Namespace: req.Namespace,
+			},
+		}
+		err = r.Client.Delete(ctx, &pdb)
+		if err != nil && !kerrors.IsNotFound(err) {
+			return ctrl.Result{}, err
+		}
+	}
+
 	dep := &appsv1.Deployment{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      *depAC.Name,

pkg/applyconfigurations/api/v1alpha1/networkrouterspec.go

@@ -9,6 +9,10 @@ package v1alpha1
 type NetworkRouterSpecApplyConfiguration struct {
 	// DNSZoneRef is a reference to the DNS zone used to create records for resources.
 	DNSZoneRef *DNSZoneReferenceApplyConfiguration `json:"dnsZoneRef,omitempty"`
+	// Netbird client image.
+	Image *string `json:"image,omitempty"`
+	// Log level for Netbird client.
+	LogLevel *string `json:"logLevel,omitempty"`
 	// WorkloadOverride contains configuration that will override the default workload.
 	WorkloadOverride *WorkloadOverrideApplyConfiguration `json:"workloadOverride,omitempty"`
 }
@@ -27,6 +31,22 @@ func (b *NetworkRouterSpecApplyConfiguration) WithDNSZoneRef(value *DNSZoneRefer
 	return b
 }
 
+// WithImage sets the Image field in the declarative configuration to the given value
+// and returns the receiver, so that objects can be built by chaining "With" function invocations.
+// If called multiple times, the Image field is set to the value of the last call.
+func (b *NetworkRouterSpecApplyConfiguration) WithImage(value string) *NetworkRouterSpecApplyConfiguration {
+	b.Image = &value
+	return b
+}
+
+// WithLogLevel sets the LogLevel field in the declarative configuration to the given value
+// and returns the receiver, so that objects can be built by chaining "With" function invocations.
+// If called multiple times, the LogLevel field is set to the value of the last call.
+func (b *NetworkRouterSpecApplyConfiguration) WithLogLevel(value string) *NetworkRouterSpecApplyConfiguration {
+	b.LogLevel = &value
+	return b
+}
+
 // WithWorkloadOverride sets the WorkloadOverride field in the declarative configuration to the given value
 // and returns the receiver, so that objects can be built by chaining "With" function invocations.
 // If called multiple times, the WorkloadOverride field is set to the value of the last call.