fix: Retry status write races and skip deleted objects

internal/cloud/controller/bgpadvertisement_controller.go

@@ -70,10 +70,22 @@ type BGPAdvertisementReconciler struct {
 // +kubebuilder:rbac:groups=galactic.datumapis.com,resources=vpcattachments/status,verbs=get;update;patch
 
 func (r *BGPAdvertisementReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
+	err := r.reconcile(ctx, req)
+	if apierrors.IsConflict(err) {
+		// NSO writes the same attachment, interface and claim status. Losing
+		// that race only means the cached copy was stale, so read it again
+		// rather than report a controller error.
+		log.FromContext(ctx).V(1).Info("status changed while reconciling; retrying", "reason", err.Error())
+		return ctrl.Result{Requeue: true}, nil
+	}
+	return ctrl.Result{}, err
+}
+
+func (r *BGPAdvertisementReconciler) reconcile(ctx context.Context, req ctrl.Request) error {
 	var advertisement bgpv1alpha1.BGPAdvertisement
 	if err := r.Get(ctx, req.NamespacedName, &advertisement); err != nil {
 		if !apierrors.IsNotFound(err) {
-			return ctrl.Result{}, err
+			return err
 		}
 		if r.APIReader != nil {
 			var live bgpv1alpha1.BGPAdvertisement
@@ -82,38 +94,38 @@ func (r *BGPAdvertisementReconciler) Reconcile(ctx context.Context, req ctrl.Req
 				// Recreated faster than the cache saw it. The informer's
 				// create event reconciles it again, so clearing now would
 				// only flap the status.
-				return ctrl.Result{}, nil
+				return nil
 			}
 			if !apierrors.IsNotFound(err) {
-				return ctrl.Result{}, fmt.Errorf("confirm withdrawn advertisement %s: %w", req.NamespacedName, err)
+				return fmt.Errorf("confirm withdrawn advertisement %s: %w", req.NamespacedName, err)
 			}
 		}
-		return ctrl.Result{}, r.clearWithdrawnAdvertisement(ctx, req.NamespacedName)
+		return r.clearWithdrawnAdvertisement(ctx, req.NamespacedName)
 	}
 
 	vpc, attachmentID, ok := galactic.SplitAdvertisementName(advertisement.Name)
 	if !ok {
-		return ctrl.Result{}, nil
+		return nil
 	}
 
 	// Ownership is checked before any status write: an advertisement on a
 	// router naming a routerClassName (the fabric underlay) never certifies
 	// a tenant attachment, interface or claim, whatever its name looks like.
 	router, err := r.routerFor(ctx, advertisement.Namespace, advertisement.Spec.RouterRef.Name)
 	if err != nil {
-		return ctrl.Result{}, err
+		return err
 	}
 	if router != nil && !bgpv1alpha1.IsLegacyOwned(router) {
-		return ctrl.Result{}, nil
+		return nil
 	}
 
 	var attachments cloudv1alpha1.VPCAttachmentList
 	if err := r.List(ctx, &attachments,
 		client.MatchingFields{IndexVPCAttachmentIdentity: galactic.AdvertisementName(vpc, attachmentID)}); err != nil {
-		return ctrl.Result{}, fmt.Errorf("list VPC attachments for advertisement %s: %w", advertisement.Name, err)
+		return fmt.Errorf("list VPC attachments for advertisement %s: %w", advertisement.Name, err)
 	}
 	if len(attachments.Items) == 0 {
-		return ctrl.Result{}, nil
+		return nil
 	}
 
 	var node string
@@ -136,15 +148,19 @@ func (r *BGPAdvertisementReconciler) Reconcile(ctx context.Context, req ctrl.Req
 		}
 		meta.SetStatusCondition(&attachment.Status.Conditions, programmed)
 		if err := r.Status().Update(ctx, attachment); err != nil {
-			return ctrl.Result{}, fmt.Errorf("update VPC attachment %s status: %w",
+			// Collected with an interface deleted mid-pass.
+			if apierrors.IsNotFound(err) {
+				continue
+			}
+			return fmt.Errorf("update VPC attachment %s status: %w",
 				client.ObjectKeyFromObject(attachment), err)
 		}
 		if err := r.projectOntoInterface(ctx, attachment, programmed); err != nil {
-			return ctrl.Result{}, err
+			return err
 		}
 	}
 
-	return ctrl.Result{}, nil
+	return nil
 }
 
 // clearWithdrawnAdvertisement removes delivery status only when no tenant
@@ -192,7 +208,7 @@ func (r *BGPAdvertisementReconciler) clearWithdrawnAdvertisement(ctx context.Con
 		if err := r.projectOntoInterface(ctx, attachment, programmed); err != nil {
 			return err
 		}
-		if err := r.Status().Update(ctx, attachment); err != nil {
+		if err := r.Status().Update(ctx, attachment); client.IgnoreNotFound(err) != nil {
 			return fmt.Errorf("clear withdrawn attachment %s: %w", client.ObjectKeyFromObject(attachment), err)
 		}
 	}
@@ -244,6 +260,10 @@ func (r *BGPAdvertisementReconciler) projectOntoInterface(
 	interfaceProgrammed.ObservedGeneration = networkInterface.Generation
 	meta.SetStatusCondition(&networkInterface.Status.Conditions, interfaceProgrammed)
 	if err := r.Status().Update(ctx, &networkInterface); err != nil {
+		// A guest torn down mid-pass leaves nothing to certify.
+		if apierrors.IsNotFound(err) {
+			return nil
+		}
 		return fmt.Errorf("update network interface %s status: %w", key, err)
 	}
 	if networkInterface.Spec.ClaimRef == nil {
@@ -265,6 +285,9 @@ func (r *BGPAdvertisementReconciler) projectOntoInterface(
 		})
 	}
 	if err := r.Status().Update(ctx, &claim); err != nil {
+		if apierrors.IsNotFound(err) {
+			return nil
+		}
 		return fmt.Errorf("update network interface claim %s status: %w", claimKey, err)
 	}
 	return nil

internal/cloud/controller/bgpadvertisement_writerace_test.go

@@ -0,0 +1,90 @@
+/*
+Copyright © 2026 Datum Technology, Inc. All rights reserved.
+
+This program is free software: you can redistribute it and/or modify
+it under the terms of the GNU Affero General Public License as
+published by the Free Software Foundation, either version 3 of the
+License, or (at your option) any later version.
+
+This program is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU Affero General Public License for more details.
+
+You should have received a copy of the GNU Affero General Public License
+along with this program.  If not, see <https://www.gnu.org/licenses/>.
+*/
+
+package controller
+
+import (
+	"context"
+	"testing"
+
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/types"
+	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/interceptor"
+	ctrlreconcile "sigs.k8s.io/controller-runtime/pkg/reconcile"
+
+	networkingv1alpha "go.datum.net/network-services-operator/api/v1alpha"
+)
+
+// failInterfaceStatus makes every interface status write return err, the way
+// a write from a stale cached copy, or to a deleted interface, does.
+func failInterfaceStatus(f *ownershipFixture, err error) {
+	f.reconciler.Client = interceptor.NewClient(f.client.(client.WithWatch), interceptor.Funcs{
+		SubResourceUpdate: func(ctx context.Context, c client.Client, sub string, obj client.Object,
+			opts ...client.SubResourceUpdateOption,
+		) error {
+			if _, ok := obj.(*networkingv1alpha.NetworkInterface); ok {
+				return err
+			}
+			return c.SubResource(sub).Update(ctx, obj, opts...)
+		},
+	})
+}
+
+// TestStatusConflictRetriesWithoutError: losing the interface status write
+// to another writer is normal convergence, so it requeues instead of
+// counting as a reconcile error.
+func TestStatusConflictRetriesWithoutError(t *testing.T) {
+	f := newOwnershipFixture(t, false)
+	failInterfaceStatus(f, apierrors.NewConflict(
+		schema.GroupResource{Group: networkingv1alpha.GroupVersion.Group, Resource: "networkinterfaces"},
+		f.iface.Name, nil))
+	result, err := f.reconciler.Reconcile(context.Background(), ctrlreconcile.Request{
+		NamespacedName: types.NamespacedName{Namespace: f.tenantAdv.Namespace, Name: f.tenantAdv.Name},
+	})
+	if err != nil {
+		t.Fatalf("Reconcile error = %v, want nil", err)
+	}
+	if result != (ctrl.Result{Requeue: true}) {
+		t.Fatalf("result = %+v, want a requeue", result)
+	}
+}
+
+// TestDeletedInterfaceIsNotAnError: an interface deleted after it was read
+// leaves nothing to certify.
+func TestDeletedInterfaceIsNotAnError(t *testing.T) {
+	f := newOwnershipFixture(t, false)
+	failInterfaceStatus(f, apierrors.NewNotFound(
+		schema.GroupResource{Group: networkingv1alpha.GroupVersion.Group, Resource: "networkinterfaces"},
+		f.iface.Name))
+	f.reconcile(t, f.tenantAdv)
+}
+
+// Any other failure still surfaces as an error.
+func TestOtherStatusWriteFailuresStillSurface(t *testing.T) {
+	f := newOwnershipFixture(t, false)
+	failInterfaceStatus(f, apierrors.NewForbidden(
+		schema.GroupResource{Group: networkingv1alpha.GroupVersion.Group, Resource: "networkinterfaces"},
+		f.iface.Name, nil))
+	if _, err := f.reconciler.Reconcile(context.Background(), ctrlreconcile.Request{
+		NamespacedName: types.NamespacedName{Namespace: f.tenantAdv.Namespace, Name: f.tenantAdv.Name},
+	}); err == nil {
+		t.Fatal("Reconcile error = nil, want the forbidden write to surface")
+	}
+}

internal/trafficpolicy/cell/reconcilers.go

@@ -299,6 +299,11 @@ func (r *BindingReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
 	}
 	if err := r.Client.Status().Patch(ctx, &b, client.RawPatch(types.ApplyPatchType, body),
 		client.FieldOwner(FieldOwner), client.ForceOwnership); err != nil {
+		// The binding goes with its interface, often between the read above
+		// and this apply. A deleted binding has no status to report.
+		if apierrors.IsNotFound(err) {
+			return ctrl.Result{}, nil
+		}
 		return ctrl.Result{}, fmt.Errorf("apply TrafficPolicyBinding %s status: %w", req.NamespacedName, err)
 	}
 	if next > 0 {

internal/trafficpolicy/cell/reconcilers_test.go

@@ -0,0 +1,50 @@
+// Copyright 2026 Datum Cloud, Inc.
+//
+// SPDX-License-Identifier: AGPL-3.0-or-later
+
+package cell
+
+import (
+	"context"
+	"testing"
+
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+	"sigs.k8s.io/controller-runtime/pkg/client/interceptor"
+
+	cloudv1alpha1 "go.datum.net/galactic/api/cloud/v1alpha1"
+	galacticv1alpha1 "go.datum.net/galactic/api/v1alpha1"
+)
+
+// TestBindingDeletedBeforeStatusApply: a binding deleted with its interface
+// between the read and the status apply is not a reconcile error.
+func TestBindingDeletedBeforeStatusApply(t *testing.T) {
+	scheme := runtime.NewScheme()
+	for _, add := range []func(*runtime.Scheme) error{cloudv1alpha1.AddToScheme, galacticv1alpha1.AddToScheme} {
+		if err := add(scheme); err != nil {
+			t.Fatal(err)
+		}
+	}
+	binding := &galacticv1alpha1.TrafficPolicyBinding{ObjectMeta: metav1.ObjectMeta{Name: "nic", Namespace: "tenant"}}
+	cl := interceptor.NewClient(fake.NewClientBuilder().WithScheme(scheme).WithObjects(binding).Build(),
+		interceptor.Funcs{
+			SubResourcePatch: func(_ context.Context, _ client.Client, _ string, obj client.Object, _ client.Patch,
+				_ ...client.SubResourcePatchOption,
+			) error {
+				return apierrors.NewNotFound(galacticv1alpha1.GroupVersion.WithResource("trafficpolicybindings").
+					GroupResource(), obj.GetName())
+			},
+		})
+	r := &BindingReconciler{Client: cl}
+	result, err := r.Reconcile(context.Background(), ctrl.Request{NamespacedName: client.ObjectKeyFromObject(binding)})
+	if err != nil {
+		t.Fatalf("Reconcile error = %v, want nil", err)
+	}
+	if result != (ctrl.Result{}) {
+		t.Fatalf("result = %+v, want none", result)
+	}
+}