From 37d48b5ca8d277b51ccbd538bd8f25fa588bef06 Mon Sep 17 00:00:00 2001 From: Philip Laine Date: Tue, 28 Apr 2026 15:18:15 +0200 Subject: [PATCH] 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 --- api/v1alpha1/networkrouter_types.go | 10 +++ docs/api-reference.md | 4 +- .../crds/netbird.io_networkrouters.yaml | 8 ++ .../controller/networkrouter_controller.go | 83 +++++++++++++++++-- .../api/v1alpha1/networkrouterspec.go | 20 +++++ 5 files changed, 115 insertions(+), 10 deletions(-) diff --git a/api/v1alpha1/networkrouter_types.go b/api/v1alpha1/networkrouter_types.go index 10640bc..151d43a 100644 --- a/api/v1alpha1/networkrouter_types.go +++ b/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. diff --git a/docs/api-reference.md b/docs/api-reference.md index c88e342..b7a361c 100644 --- a/docs/api-reference.md +++ b/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: \{\}
| +| `image` _string_ | Netbird client image. | | Optional: \{\}
| +| `logLevel` _string_ | Log level for Netbird client. | | Optional: \{\}
| | `workloadOverride` _[WorkloadOverride](#workloadoverride)_ | WorkloadOverride contains configuration that will override the default workload. | | Optional: \{\}
| @@ -407,7 +409,7 @@ _Appears in:_ | --- | --- | --- | --- | | `labels` _object (keys:string, values:string)_ | Labels that will be added. | | Optional: \{\}
| | `annotations` _object (keys:string, values:string)_ | Annotations that will be added. | | Optional: \{\}
| -| `replicas` _integer_ | Replicas sets the amount of client replicas. | | Optional: \{\}
| +| `replicas` _integer_ | Replicas sets the amount of client replicas. | 3 | Minimum: 1
Optional: \{\}
| | `podTemplate` _[PodTemplateSpec](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.35/#podtemplatespec-v1-core)_ | PodTemplate overrides the pod template. | | Schemaless: \{\}
Optional: \{\}
| diff --git a/helm/kubernetes-operator/crds/netbird.io_networkrouters.yaml b/helm/kubernetes-operator/crds/netbird.io_networkrouters.yaml index 6644007..56d4c67 100644 --- a/helm/kubernetes-operator/crds/netbird.io_networkrouters.yaml +++ b/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: diff --git a/internal/controller/networkrouter_controller.go b/internal/controller/networkrouter_controller.go index b714b92..2928460 100644 --- a/internal/controller/networkrouter_controller.go +++ b/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, diff --git a/pkg/applyconfigurations/api/v1alpha1/networkrouterspec.go b/pkg/applyconfigurations/api/v1alpha1/networkrouterspec.go index 6704347..3b49eaf 100644 --- a/pkg/applyconfigurations/api/v1alpha1/networkrouterspec.go +++ b/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.