Add ready conditon and cleanup finalizer and status patching (#186)

This change adds a ready condition. It also sets a standard for status
fields and documentation. It makes use of helper functions from FluxCD
to better manage patching of finalizers and status.

Signed-off-by: Philip Laine <philip.laine@gmail.com>
This commit is contained in:
Philip Laine
2026-04-21 15:42:11 +02:00
committed by GitHub
parent 1bcd58a7cd
commit af11e31b28
19 changed files with 250 additions and 181 deletions
+22 -28
View File
@@ -3,20 +3,17 @@ package controller
import (
"context"
"github.com/fluxcd/pkg/runtime/conditions"
"github.com/fluxcd/pkg/runtime/patch"
netbird "github.com/netbirdio/netbird/shared/management/client/rest"
"github.com/netbirdio/netbird/shared/management/http/api"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
nbv1alpha1 "github.com/netbirdio/kubernetes-operator/api/v1alpha1"
nbv1alpha1ac "github.com/netbirdio/kubernetes-operator/pkg/applyconfigurations/api/v1alpha1"
)
const (
GroupFinalizer = "netbird.io/group"
)
// GroupReconciler reconciles a Group object
type GroupReconciler struct {
client.Client
@@ -27,28 +24,29 @@ type GroupReconciler struct {
// +kubebuilder:rbac:groups=netbird.io,resources=groups/status,verbs=get;update;patch
// +kubebuilder:rbac:groups=netbird.io,resources=groups/finalizers,verbs=update
func (r *GroupReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
group := nbv1alpha1.Group{}
err := r.Get(ctx, req.NamespacedName, &group)
group := &nbv1alpha1.Group{}
err := r.Get(ctx, req.NamespacedName, group)
if err != nil {
return ctrl.Result{}, client.IgnoreNotFound(err)
}
sp := patch.NewSerialPatcher(group, r.Client)
if !group.DeletionTimestamp.IsZero() {
return r.reconcileDelete(ctx, group)
return r.reconcileDelete(ctx, sp, group)
}
groupAC := nbv1alpha1ac.Group(req.Name, req.Namespace).WithFinalizers(SetupKeyFinalizer)
err = r.Client.Apply(ctx, groupAC)
controllerutil.AddFinalizer(group, nbv1alpha1.NetbirdFinalizer)
err = sp.Patch(ctx, group)
if err != nil {
return ctrl.Result{}, err
}
groupID, err := func() (string, error) {
if group.Status.GroupID != nil {
groupReq := api.GroupRequest{
Name: group.Spec.Name,
}
resp, err := r.Netbird.Groups.Update(ctx, *group.Status.GroupID, groupReq)
groupReq := api.GroupRequest{
Name: group.Spec.Name,
}
if group.Status.GroupID != "" {
resp, err := r.Netbird.Groups.Update(ctx, group.Status.GroupID, groupReq)
if err != nil && !netbird.IsNotFound(err) {
return "", err
}
@@ -56,10 +54,6 @@ func (r *GroupReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl
return resp.Id, nil
}
}
groupReq := api.GroupRequest{
Name: group.Spec.Name,
}
resp, err := r.Netbird.Groups.Create(ctx, groupReq)
if err != nil {
return "", err
@@ -69,26 +63,26 @@ func (r *GroupReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl
if err != nil {
return ctrl.Result{}, err
}
group.Status.GroupID = groupID
groupAC = nbv1alpha1ac.Group(req.Name, req.Namespace).WithStatus(nbv1alpha1ac.GroupStatus().WithGroupID(groupID))
err = r.Client.Status().Apply(ctx, groupAC)
conditions.MarkTrue(group, nbv1alpha1.ReadyCondition, nbv1alpha1.ReconciledReason, "")
err = sp.Patch(ctx, group, patch.WithStatusObservedGeneration{})
if err != nil {
return ctrl.Result{}, err
}
return ctrl.Result{}, nil
}
func (r *GroupReconciler) reconcileDelete(ctx context.Context, group nbv1alpha1.Group) (ctrl.Result, error) {
if group.Status.GroupID != nil {
err := r.Netbird.Groups.Delete(ctx, *group.Status.GroupID)
func (r *GroupReconciler) reconcileDelete(ctx context.Context, sp *patch.SerialPatcher, group *nbv1alpha1.Group) (ctrl.Result, error) {
if group.Status.GroupID != "" {
err := r.Netbird.Groups.Delete(ctx, group.Status.GroupID)
if err != nil && !netbird.IsNotFound(err) {
return ctrl.Result{}, err
}
}
groupAC := nbv1alpha1ac.Group(group.Name, group.Namespace).WithFinalizers()
err := r.Client.Apply(ctx, groupAC)
controllerutil.RemoveFinalizer(group, nbv1alpha1.NetbirdFinalizer)
err := sp.Patch(ctx, group)
if err != nil {
return ctrl.Result{}, err
}
+4 -4
View File
@@ -60,18 +60,18 @@ var _ = Describe("Group Controller", func() {
Expect(err).NotTo(HaveOccurred())
err = k8sClient.Get(ctx, nn, group)
Expect(err).NotTo(HaveOccurred())
Expect(*group.Status.GroupID).NotTo(BeEmpty())
Expect(group.Status.ObservedGeneration).To(Equal(group.Generation))
Expect(group.Status.GroupID).NotTo(BeEmpty())
By("crerating a new group when deleted from API")
err = controllerReconciler.Netbird.Groups.Delete(ctx, *group.Status.GroupID)
err = controllerReconciler.Netbird.Groups.Delete(ctx, group.Status.GroupID)
Expect(err).NotTo(HaveOccurred())
_, err = controllerReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: nn})
Expect(err).NotTo(HaveOccurred())
newGroup := &nbv1alpha1.Group{}
err = k8sClient.Get(ctx, nn, newGroup)
Expect(err).NotTo(HaveOccurred())
Expect(*newGroup.Status.GroupID).NotTo(BeEmpty())
Expect(*newGroup.Status.GroupID).NotTo(Equal(*group.Status.GroupID))
Expect(newGroup.Status.GroupID).NotTo(Equal(group.Status.GroupID))
})
})
})
+39 -36
View File
@@ -6,6 +6,8 @@ import (
"fmt"
"time"
"github.com/fluxcd/pkg/runtime/conditions"
"github.com/fluxcd/pkg/runtime/patch"
netbird "github.com/netbirdio/netbird/shared/management/client/rest"
"github.com/netbirdio/netbird/shared/management/http/api"
corev1 "k8s.io/api/core/v1"
@@ -15,14 +17,13 @@ import (
"k8s.io/utils/ptr"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
nbv1alpha1 "github.com/netbirdio/kubernetes-operator/api/v1alpha1"
"github.com/netbirdio/kubernetes-operator/internal/ssautil"
nbv1alpha1ac "github.com/netbirdio/kubernetes-operator/pkg/applyconfigurations/api/v1alpha1"
)
const (
SetupKeyFinalizer = "netbird.io/setupkey"
SetupKeySecretKey = "setup-key"
)
@@ -36,14 +37,19 @@ type SetupKeyReconciler struct {
// +kubebuilder:rbac:groups=netbird.io,resources=setupkeys/status,verbs=get;update;patch
// +kubebuilder:rbac:groups=netbird.io,resources=setupkeys/finalizers,verbs=update
func (r *SetupKeyReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
setupKey := nbv1alpha1.SetupKey{}
err := r.Get(ctx, req.NamespacedName, &setupKey)
setupKey := &nbv1alpha1.SetupKey{}
err := r.Get(ctx, req.NamespacedName, setupKey)
if err != nil {
return ctrl.Result{}, client.IgnoreNotFound(err)
}
owner, err := ssautil.OwnerReference(setupKey, r.Client.Scheme())
if err != nil {
return ctrl.Result{}, err
}
sp := patch.NewSerialPatcher(setupKey, r.Client)
if !setupKey.DeletionTimestamp.IsZero() {
return r.reconcileDelete(ctx, setupKey)
return r.reconcileDelete(ctx, sp, setupKey)
}
// Get ids for auto groups.
@@ -67,28 +73,27 @@ func (r *SetupKeyReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
if err != nil {
return ctrl.Result{}, err
}
if group.Status.GroupID == nil {
if group.Status.GroupID == "" {
return ctrl.Result{}, fmt.Errorf("group %s in auto groups list is not ready", group.Name)
}
autoGroupIDs = append(autoGroupIDs, *group.Status.GroupID)
autoGroupIDs = append(autoGroupIDs, group.Status.GroupID)
}
}
// Set finalizer on the setup key.
setupKeyAC := nbv1alpha1ac.SetupKey(req.Name, req.Namespace).WithFinalizers(SetupKeyFinalizer)
err = r.Client.Apply(ctx, setupKeyAC)
controllerutil.AddFinalizer(setupKey, nbv1alpha1.NetbirdFinalizer)
err = sp.Patch(ctx, setupKey)
if err != nil {
return ctrl.Result{}, err
}
// Check if setup key is up to date.
ok, err := func() (bool, error) {
if setupKey.Status.SetupKeyID == nil {
if setupKey.Status.SetupKeyID == "" {
return false, nil
}
// Check setup key in Netbird.
resp, err := r.Netbird.SetupKeys.Get(ctx, *setupKey.Status.SetupKeyID)
resp, err := r.Netbird.SetupKeys.Get(ctx, setupKey.Status.SetupKeyID)
if netbird.IsNotFound(err) {
return false, nil
}
@@ -121,7 +126,7 @@ func (r *SetupKeyReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
setupKeyReq := api.PutApiSetupKeysKeyIdJSONRequestBody{
AutoGroups: autoGroupIDs,
}
_, err = r.Netbird.SetupKeys.Update(ctx, *setupKey.Status.SetupKeyID, setupKeyReq)
_, err = r.Netbird.SetupKeys.Update(ctx, setupKey.Status.SetupKeyID, setupKeyReq)
if err != nil {
return false, err
}
@@ -154,19 +159,13 @@ func (r *SetupKeyReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
if err != nil {
return ctrl.Result{}, err
}
// Update the status with the id.
setupKeyAC = nbv1alpha1ac.SetupKey(req.Name, req.Namespace).WithStatus(nbv1alpha1ac.SetupKeyStatus().WithSetupKeyID(resp.Id))
err = r.Client.Status().Apply(ctx, setupKeyAC)
setupKey.Status.SetupKeyID = resp.Id
err = sp.Patch(ctx, setupKey)
if err != nil {
return ctrl.Result{}, err
}
// Create the secret containing the key.
owner, err := ssautil.OwnerReference(&setupKey, r.Scheme())
if err != nil {
return ctrl.Result{}, err
}
data := map[string]string{
SetupKeySecretKey: resp.Key,
}
@@ -179,30 +178,34 @@ func (r *SetupKeyReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
}
// Delete the old status key if we are recreating.
if oldSetupKeyID != nil {
err = r.Netbird.SetupKeys.Delete(ctx, *oldSetupKeyID)
if oldSetupKeyID != "" {
err = r.Netbird.SetupKeys.Delete(ctx, oldSetupKeyID)
if err != nil && !netbird.IsNotFound(err) {
return ctrl.Result{}, err
}
}
return ctrl.Result{RequeueAfter: 15 * time.Minute}, nil
}
func (r *SetupKeyReconciler) reconcileDelete(ctx context.Context, setupKey nbv1alpha1.SetupKey) (ctrl.Result, error) {
if setupKey.Status.SetupKeyID != nil {
err := r.Netbird.SetupKeys.Delete(ctx, *setupKey.Status.SetupKeyID)
if err != nil && !netbird.IsNotFound(err) {
return ctrl.Result{}, err
}
}
setupKeyAC := nbv1alpha1ac.SetupKey(setupKey.Name, setupKey.Namespace).WithFinalizers()
err := r.Client.Apply(ctx, setupKeyAC)
conditions.MarkTrue(setupKey, nbv1alpha1.ReadyCondition, nbv1alpha1.ReconciledReason, "")
err = sp.Patch(ctx, setupKey, patch.WithStatusObservedGeneration{})
if err != nil {
return ctrl.Result{}, err
}
return ctrl.Result{RequeueAfter: 15 * time.Minute}, nil
}
func (r *SetupKeyReconciler) reconcileDelete(ctx context.Context, sp *patch.SerialPatcher, setupKey *nbv1alpha1.SetupKey) (ctrl.Result, error) {
if setupKey.Status.SetupKeyID != "" {
err := r.Netbird.SetupKeys.Delete(ctx, setupKey.Status.SetupKeyID)
if err != nil && !netbird.IsNotFound(err) {
return ctrl.Result{}, err
}
}
controllerutil.RemoveFinalizer(setupKey, nbv1alpha1.NetbirdFinalizer)
err := sp.Patch(ctx, setupKey)
if err != nil {
return ctrl.Result{}, err
}
return ctrl.Result{}, nil
}
@@ -57,7 +57,8 @@ var _ = Describe("SetupKey Controller", func() {
err = k8sClient.Get(ctx, nn, setupKey)
Expect(err).NotTo(HaveOccurred())
Expect(*setupKey.Status.SetupKeyID).NotTo(BeEmpty())
Expect(setupKey.Status.ObservedGeneration).To(Equal(setupKey.Generation))
Expect(setupKey.Status.SetupKeyID).NotTo(BeEmpty())
secret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
@@ -68,7 +69,7 @@ var _ = Describe("SetupKey Controller", func() {
err = k8sClient.Get(ctx, client.ObjectKeyFromObject(secret), secret)
Expect(err).NotTo(HaveOccurred())
resp, err := controllerReconciler.Netbird.SetupKeys.Get(ctx, *setupKey.Status.SetupKeyID)
resp, err := controllerReconciler.Netbird.SetupKeys.Get(ctx, setupKey.Status.SetupKeyID)
Expect(err).NotTo(HaveOccurred())
Expect(string(secret.Data[SetupKeySecretKey])).To(Equal(resp.Key))
})
@@ -114,7 +115,7 @@ var _ = Describe("SetupKey Controller", func() {
Expect(err).NotTo(HaveOccurred())
Expect(k8sClient.Delete(ctx, &secondSecret)).To(Succeed())
Expect(*firstSetupKey.Status.SetupKeyID).ToNot(Equal(*secondSetupKey.Status.SetupKeyID))
Expect(firstSetupKey.Status.SetupKeyID).ToNot(Equal(secondSetupKey.Status.SetupKeyID))
Expect(firstSecret.Data[SetupKeySecretKey]).ToNot(BeEquivalentTo(secondSecret.Data[SetupKeySecretKey]))
})
})