From 53b0c5fd7bc331678801c0d6f65df65af20e300d Mon Sep 17 00:00:00 2001 From: Viktor Voytovych Date: Tue, 3 Nov 2026 09:50:41 +0200 Subject: [PATCH 1/2] test: Hear a network's deletion at once where no binding declares it A presence that no hub binding declares hears its network's deletion only at its next resync. With the resync at an hour, this test fails on the restart fix's branch: the network waits for its context. --- .../controller/networkpresence_watch_test.go | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/internal/controller/networkpresence_watch_test.go b/internal/controller/networkpresence_watch_test.go index 367528d9..f7c606ba 100644 --- a/internal/controller/networkpresence_watch_test.go +++ b/internal/controller/networkpresence_watch_test.go @@ -358,3 +358,29 @@ func TestAPresenceNothingDeclaresEndsAtItsGraceAfterARestart(t *testing.T) { return apierrors.IsNotFound(err) }) } + +// mirrorNetwork stands in for the replicator, which mirrors every network onto +// the hub and deletes the mirror as soon as the network is deleted. +func (s *presenceScenario) mirrorNetwork() *networkingv1alpha.Network { + s.t.Helper() + mirror := &networkingv1alpha.Network{} + mirror.Namespace = s.hubNamespace + mirror.Name = s.networkName + mirror.Spec = s.network.Spec + require.NoError(s.t, s.hub.Create(s.ctx, mirror)) + return mirror +} + +// The hub's mirror of a network goes as soon as the network is deleted, and a +// presence nothing declares hears the deletion from it, not at its next resync. +func TestANetworkDeletionReachesAPresenceNothingDeclaresAtOnce(t *testing.T) { + s := newPresenceScenario(t, presenceOptions{unclaimedGrace: time.Hour}) + mirror := s.mirrorNetwork() + s.runPresence(time.Hour) + s.unclaimed() + + s.deleteNetwork() + require.NoError(t, s.hub.Delete(s.ctx, mirror)) + + s.eventually("the network's deletion ends the presence well before its resync", s.networkFinishes) +} From 89643f81fe36ffbd4b2286edc0c5510443f6e6f8 Mon Sep 17 00:00:00 2001 From: Viktor Voytovych Date: Tue, 3 Nov 2026 09:50:41 +0200 Subject: [PATCH 2/2] fix: Hear a network's deletion at once where no binding declares it The replicator mirrors every Network onto the hub and deletes the mirror as soon as the Network is deleted. The presence controller now watches those mirrors and maps a mirror's deletion, or the first update of a deletion a finalizer holds, to the hub copies of the network's contexts in the mirror's namespace, as it maps an interface copy to its presence. The resync stays, for an event that never arrives. Closes #829 --- .../controller/networkpresence_controller.go | 53 +++++++++++++--- .../controller/networkpresence_watch_test.go | 60 +++++++++++++++++++ 2 files changed, 106 insertions(+), 7 deletions(-) diff --git a/internal/controller/networkpresence_controller.go b/internal/controller/networkpresence_controller.go index 0771fbbb..7f605a45 100644 --- a/internal/controller/networkpresence_controller.go +++ b/internal/controller/networkpresence_controller.go @@ -15,12 +15,15 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/cluster" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" mcmanager "sigs.k8s.io/multicluster-runtime/pkg/manager" "sigs.k8s.io/multicluster-runtime/pkg/multicluster" @@ -116,6 +119,7 @@ type NetworkPresenceReconciler struct { // +kubebuilder:rbac:groups=networking.datumapis.com,resources=networkbindings/status,verbs=get;update;patch // +kubebuilder:rbac:groups=networking.datumapis.com,resources=networkcontexts,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=networking.datumapis.com,resources=networkinterfaces,verbs=get;list;watch +// +kubebuilder:rbac:groups=networking.datumapis.com,resources=networks,verbs=get;list;watch // +kubebuilder:rbac:groups=locations.miloapis.com,resources=locations,verbs=get;list;watch // +kubebuilder:rbac:groups="",resources=namespaces,verbs=get;list;watch @@ -285,6 +289,38 @@ func presenceOfInterfaceCopy(_ context.Context, obj client.Object) []reconcile.R }}} } +// presencesOfNetworkMirror maps the hub's mirror of a network onto the +// network's presences: the hub's copies of the network's contexts in the +// mirror's namespace. +func (r *NetworkPresenceReconciler) presencesOfNetworkMirror(ctx context.Context, obj client.Object) []reconcile.Request { + var copies networkingv1alpha.NetworkContextList + if err := r.hub.List(ctx, &copies, client.InNamespace(obj.GetNamespace()), client.MatchingLabels{ + networkingv1alpha.NetworkLabel: obj.GetName(), + }); err != nil { + log.FromContext(ctx).Error(err, "failed listing the presences of a network being deleted; each comes back at its resync", + "namespace", obj.GetNamespace(), "network", obj.GetName()) + return nil + } + + requests := make([]reconcile.Request, 0, len(copies.Items)) + for i := range copies.Items { + requests = append(requests, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(&copies.Items[i])}) + } + return requests +} + +// networkMirrorGoing passes the events that say a network is being deleted. +// The replicator deletes the hub's mirror as soon as the network is deleted; +// a mirror that something holds with a finalizer says so with its first +// update. The rest of a mirror's life says nothing a presence reads here. +func networkMirrorGoing() predicate.Predicate { + return predicate.Funcs{ + CreateFunc: func(event.CreateEvent) bool { return false }, + UpdateFunc: func(e event.UpdateEvent) bool { return !e.ObjectNew.GetDeletionTimestamp().IsZero() }, + GenericFunc: func(event.GenericEvent) bool { return false }, + } +} + // ownContext reads the context a presence left in its project when it is this // controller's to end. A nil context means there is nothing to end. func (r *NetworkPresenceReconciler) ownContext( @@ -369,9 +405,9 @@ func (r *NetworkPresenceReconciler) ends( } // waiting brings a presence that waits to end back after at most the resync. -// The sync signals a change on the hub bindings declaring a presence, and one -// that waits to end may have none, so the resync is how the network's deletion -// reaches it. A zero wait has no end of its own. +// The network's deletion reaches such a presence through the hub's mirror of +// the network, and the resync covers an event that never arrives. A zero wait +// has no end of its own. func (r *NetworkPresenceReconciler) waiting(wait time.Duration) ctrl.Result { if r.ResyncPeriod > 0 && (wait == 0 || wait > r.ResyncPeriod) { wait = r.ResyncPeriod @@ -849,7 +885,7 @@ func (r *NetworkPresenceReconciler) SetupWithManager(mgr manager.Manager) error } r.hub = mgr.GetClient() - builder := ctrl.NewControllerManagedBy(mgr). + return ctrl.NewControllerManagedBy(mgr). // A presence is keyed as the hub's copy of its context is: the same // namespace and name. The copy lasts as long as the presence does, so the // list of copies on start brings back every presence, the ones nothing @@ -870,7 +906,10 @@ func (r *NetworkPresenceReconciler) SetupWithManager(mgr manager.Manager) error // A presence ends only once its location holds no interface of the // network, and the copies each cell publishes here are how the hub sees it. Watches(&networkingv1alpha.NetworkInterface{}, handler.EnqueueRequestsFromMapFunc(presenceOfInterfaceCopy)). - Named("networkpresence") - - return builder.Complete(r) + // A presence nothing declares hears its network's deletion from nothing + // else: the replicator deletes the hub's mirror of the network at once. + Watches(&networkingv1alpha.Network{}, handler.EnqueueRequestsFromMapFunc(r.presencesOfNetworkMirror), + builder.WithPredicates(networkMirrorGoing())). + Named("networkpresence"). + Complete(r) } diff --git a/internal/controller/networkpresence_watch_test.go b/internal/controller/networkpresence_watch_test.go index f7c606ba..9fc358b2 100644 --- a/internal/controller/networkpresence_watch_test.go +++ b/internal/controller/networkpresence_watch_test.go @@ -9,14 +9,17 @@ import ( "time" "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" ctrlconfig "sigs.k8s.io/controller-runtime/pkg/config" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/manager" metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" + "sigs.k8s.io/controller-runtime/pkg/reconcile" locationsv1alpha1 "go.miloapis.com/locations/api/v1alpha1" @@ -384,3 +387,60 @@ func TestANetworkDeletionReachesAPresenceNothingDeclaresAtOnce(t *testing.T) { s.eventually("the network's deletion ends the presence well before its resync", s.networkFinishes) } + +// hubCopy writes the hub copy of a context of network at location into a hub +// namespace. +func (s *presenceScenario) hubCopy(namespace, network, location string) reconcile.Request { + s.t.Helper() + copied := &networkingv1alpha.NetworkContext{} + copied.Namespace = namespace + copied.Name = networkContextName(network, locationsv1alpha1.LocationReference{Name: location}) + copied.Labels = map[string]string{ + networkingv1alpha.NetworkLabel: network, + networkingv1alpha.LocationLabel: location, + } + copied.Spec.Network = networkingv1alpha.LocalNetworkRef{Name: network} + copied.Spec.Location = locationsv1alpha1.LocationReference{Name: location} + copied.Spec.IPFamilies = []networkingv1alpha.IPFamily{networkingv1alpha.IPv6Protocol} + copied.Spec.MTU = 1440 + require.NoError(s.t, s.hub.Create(s.ctx, copied)) + return reconcile.Request{NamespacedName: client.ObjectKeyFromObject(copied)} +} + +// A network's mirror maps to its presences in every location, and to no other +// network's, nor to a network of the same name in another project. +func TestANetworkMirrorMapsToItsPresencesInItsNamespace(t *testing.T) { + s := newPresenceScenario(t, presenceOptions{}) + other := &corev1.Namespace{} + other.Name = "other-" + s.hubNamespace[:40] + require.NoError(t, s.hub.Create(s.ctx, other)) + + want := []reconcile.Request{ + s.hubCopy(s.hubNamespace, s.networkName, "us-east-1"), + s.hubCopy(s.hubNamespace, s.networkName, "us-central-1"), + } + s.hubCopy(s.hubNamespace, "another-network", "us-east-1") + s.hubCopy(other.Name, s.networkName, "us-east-1") + + presence := &NetworkPresenceReconciler{hub: s.hub} + got := presence.presencesOfNetworkMirror(s.ctx, s.mirrorNetwork()) + + require.ElementsMatch(t, want, got) +} + +// Only a mirror's deletion brings presences back. Its creation, in every list a +// start makes, and its edits say nothing a waiting presence reads. +func TestOnlyANetworkMirrorGoingBringsItsPresencesBack(t *testing.T) { + live := &networkingv1alpha.Network{} + going := live.DeepCopy() + going.DeletionTimestamp = ptr.To(metav1.Now()) + going.Finalizers = []string{"test.datumapis.com/hold"} + passes := networkMirrorGoing() + + require.False(t, passes.Create(event.CreateEvent{Object: live}), "a creation") + require.False(t, passes.Update(event.UpdateEvent{ObjectOld: live, ObjectNew: live.DeepCopy()}), "an edit") + require.True(t, passes.Update(event.UpdateEvent{ObjectOld: live, ObjectNew: going}), + "a deletion that a finalizer holds") + require.True(t, passes.Delete(event.DeleteEvent{Object: live}), "a deletion") + require.False(t, passes.Generic(event.GenericEvent{Object: live}), "a generic event") +}