diff --git a/api/net/v1alpha1/types.go b/api/net/v1alpha1/types.go index da4343072..01ed0837f 100644 --- a/api/net/v1alpha1/types.go +++ b/api/net/v1alpha1/types.go @@ -337,6 +337,15 @@ type GatewayNodeSpec struct { // GatewayNodeStatus defines the advertised routes and lease heartbeat for a gateway node. // +k8s:deepcopy-gen=true type GatewayNodeStatus struct { + // ForwardingMTU is the effective local forwarding MTU after a successful + // dataplane reconciliation. Workers use this cap for accepted gateway paths. + // Gateways do not consume it, avoiding feedback between advertised minima. + // Absent values identify agents that have not published a forwarding limit. + // +optional + // +kubebuilder:validation:Minimum=1 + // +kubebuilder:validation:Maximum=65535 + ForwardingMTU *int32 `json:"forwardingMTU,omitempty"` + // LastUpdated is the heartbeat timestamp updated by the node agent. // +optional LastUpdated metav1.Time `json:"lastUpdated,omitempty"` diff --git a/api/net/v1alpha1/zz_generated.deepcopy.go b/api/net/v1alpha1/zz_generated.deepcopy.go index c26f73ad4..ccda55002 100644 --- a/api/net/v1alpha1/zz_generated.deepcopy.go +++ b/api/net/v1alpha1/zz_generated.deepcopy.go @@ -116,6 +116,11 @@ func (in *GatewayNodeSpec) DeepCopy() *GatewayNodeSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *GatewayNodeStatus) DeepCopyInto(out *GatewayNodeStatus) { *out = *in + if in.ForwardingMTU != nil { + in, out := &in.ForwardingMTU, &out.ForwardingMTU + *out = new(int32) + **out = **in + } in.LastUpdated.DeepCopyInto(&out.LastUpdated) if in.Routes != nil { in, out := &in.Routes, &out.Routes diff --git a/cmd/unbounded-net-node/gateway_informer_lifecycle.go b/cmd/unbounded-net-node/gateway_informer_lifecycle.go new file mode 100644 index 000000000..ce9ce4928 --- /dev/null +++ b/cmd/unbounded-net-node/gateway_informer_lifecycle.go @@ -0,0 +1,460 @@ +// Copyright (c) Microsoft Corporation. +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "context" + "fmt" + "reflect" + "strings" + "sync" + "time" + + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/client-go/dynamic" + "k8s.io/client-go/tools/cache" + "k8s.io/klog/v2" + + unboundednetv1alpha1 "github.com/Azure/unbounded/api/net/v1alpha1" +) + +type gatewayInformerSnapshot struct { + generation uint64 + available bool + pools cache.SharedIndexInformer + nodes cache.SharedIndexInformer + poolPeerings cache.SharedIndexInformer +} + +type gatewayInformerGeneration struct { + gatewayInformerSnapshot + cancel context.CancelFunc + cleanups []func() +} + +type gatewayInformerLifecycle struct { + client dynamic.Interface + resync time.Duration + syncTimeout time.Duration + + mu sync.RWMutex + required bool + starting bool + retiring bool + generation uint64 + active *gatewayInformerGeneration + onEvent func() + lastErr error + startCancel context.CancelFunc +} + +func newGatewayInformerLifecycle(client dynamic.Interface, resync time.Duration) *gatewayInformerLifecycle { + return &gatewayInformerLifecycle{ + client: client, + resync: resync, + syncTimeout: 30 * time.Second, + } +} + +func (l *gatewayInformerLifecycle) setEventHandler(onEvent func()) { + l.mu.Lock() + defer l.mu.Unlock() + + l.onEvent = onEvent +} + +func (l *gatewayInformerLifecycle) requestUpdate() { + l.mu.RLock() + onEvent := l.onEvent + l.mu.RUnlock() + + if onEvent != nil { + onEvent() + } +} + +func (l *gatewayInformerLifecycle) setRequired(required bool) { + l.mu.Lock() + defer l.mu.Unlock() + + l.required = required + + l.retiring = !required && l.active != nil + if !required && l.startCancel != nil { + l.startCancel() + } +} + +func (l *gatewayInformerLifecycle) readiness() (bool, string) { + l.mu.RLock() + defer l.mu.RUnlock() + + if l.retiring { + return false, "gateway informer caches are retained until dependent routes and peers are removed" + } + + if !l.required { + return true, "" + } + + if l.starting { + return false, "required gateway informer caches are starting" + } + + if l.active == nil { + if l.lastErr != nil { + return false, fmt.Sprintf("required gateway informer caches unavailable: %v", l.lastErr) + } + + return false, "required gateway informer caches are not running" + } + + if !gatewayInformerSnapshotSynced(l.active.gatewayInformerSnapshot) { + return false, "required gateway informer caches are not synced" + } + + return true, "" +} + +func (l *gatewayInformerLifecycle) snapshot() gatewayInformerSnapshot { + l.mu.RLock() + defer l.mu.RUnlock() + + if l.active == nil { + return gatewayInformerSnapshot{} + } + + return l.active.gatewayInformerSnapshot +} + +func (l *gatewayInformerLifecycle) prepare(ctx context.Context, required bool) (gatewayInformerSnapshot, bool, error) { + l.setRequired(required) + + l.mu.RLock() + active := l.active + l.mu.RUnlock() + + if !required { + if active == nil { + return gatewayInformerSnapshot{}, false, nil + } + + return active.gatewayInformerSnapshot, true, nil + } + + if active != nil { + if !gatewayInformerSnapshotSynced(active.gatewayInformerSnapshot) { + return gatewayInformerSnapshot{}, false, fmt.Errorf("required gateway informer generation %d is not synced", active.generation) + } + + return active.gatewayInformerSnapshot, false, nil + } + + return l.start(ctx) +} + +func (l *gatewayInformerLifecycle) start(ctx context.Context) (gatewayInformerSnapshot, bool, error) { + l.mu.Lock() + if l.active != nil { + snapshot := l.active.gatewayInformerSnapshot + l.mu.Unlock() + + return snapshot, false, nil + } + + if !l.required { + l.mu.Unlock() + + return gatewayInformerSnapshot{}, false, nil + } + + if l.starting { + l.mu.Unlock() + + return gatewayInformerSnapshot{}, false, fmt.Errorf("gateway informer caches are already starting") + } + + runCtx, cancel := context.WithCancel(ctx) + l.startCancel = cancel + l.starting = true + l.lastErr = nil + l.generation++ + generation := l.generation + l.mu.Unlock() + + factory := newNodeDynamicInformerFactory(l.client, l.resync) + pools := factory.ForResource(gatewayPoolGVR).Informer() + nodes := factory.ForResource(gatewayNodeGVR).Informer() + poolPeerings := factory.ForResource(gatewayPoolPeeringGVR).Informer() + + cleanups, err := addGatewayInformerEventHandlers(pools, nodes, poolPeerings, l.requestUpdate) + if err != nil { + cancel() + l.recordStartFailure(err) + + return gatewayInformerSnapshot{}, false, err + } + + factory.Start(runCtx.Done()) + + snapshot := gatewayInformerSnapshot{ + generation: generation, + available: true, + pools: pools, + nodes: nodes, + poolPeerings: poolPeerings, + } + + syncCtx, stopSync := context.WithTimeout(runCtx, l.syncTimeout) + defer stopSync() + + if !cache.WaitForCacheSync(syncCtx.Done(), pools.HasSynced, nodes.HasSynced, poolPeerings.HasSynced) { + cancel() + runGatewayInformerCleanups(cleanups) + + err := fmt.Errorf("failed to sync required gateway informer generation %d", generation) + if ctx.Err() != nil { + err = ctx.Err() + } + + l.recordStartFailure(err) + + return gatewayInformerSnapshot{}, false, err + } + + activeGeneration := &gatewayInformerGeneration{ + gatewayInformerSnapshot: snapshot, + cancel: cancel, + cleanups: cleanups, + } + + l.mu.Lock() + if !l.required || runCtx.Err() != nil { + l.starting = false + l.startCancel = nil + l.mu.Unlock() + cancel() + runGatewayInformerCleanups(cleanups) + + return gatewayInformerSnapshot{}, false, fmt.Errorf("gateway informer generation %d start canceled", generation) + } + + l.starting = false + l.startCancel = nil + l.lastErr = nil + l.active = activeGeneration + l.retiring = !l.required + l.mu.Unlock() + + return snapshot, false, nil +} + +func (l *gatewayInformerLifecycle) recordStartFailure(err error) { + l.mu.Lock() + defer l.mu.Unlock() + + l.starting = false + l.startCancel = nil + l.lastErr = err +} + +func (l *gatewayInformerLifecycle) retire(generation uint64) { + l.mu.Lock() + + active := l.active + if active == nil || active.generation != generation || l.required { + l.mu.Unlock() + + return + } + + l.active = nil + l.retiring = false + l.mu.Unlock() + + runGatewayInformerCleanups(active.cleanups) + active.cancel() +} + +func (l *gatewayInformerLifecycle) close() { + l.mu.Lock() + active := l.active + l.active = nil + l.required = false + + l.retiring = false + if l.startCancel != nil { + l.startCancel() + } + l.mu.Unlock() + + if active != nil { + runGatewayInformerCleanups(active.cleanups) + active.cancel() + } +} + +func gatewayInformerSnapshotSynced(snapshot gatewayInformerSnapshot) bool { + return snapshot.available && + snapshot.pools != nil && snapshot.pools.HasSynced() && + snapshot.nodes != nil && snapshot.nodes.HasSynced() && + snapshot.poolPeerings != nil && snapshot.poolPeerings.HasSynced() +} + +func addGatewayInformerEventHandlers( + pools, nodes, poolPeerings cache.SharedIndexInformer, + onEvent func(), +) ([]func(), error) { + if onEvent == nil { + onEvent = func() {} + } + + handler := cache.ResourceEventHandlerFuncs{ + AddFunc: func(interface{}) { onEvent() }, + UpdateFunc: func(interface{}, interface{}) { onEvent() }, + DeleteFunc: func(interface{}) { onEvent() }, + } + + var cleanups []func() + + add := func(name string, informer cache.SharedIndexInformer, eventHandler cache.ResourceEventHandler) error { + registration, err := informer.AddEventHandler(eventHandler) + if err != nil { + return fmt.Errorf("add %s event handler: %w", name, err) + } + + cleanups = append(cleanups, func() { + if err := informer.RemoveEventHandler(registration); err != nil { + klog.V(4).Infof("Failed to remove %s event handler: %v", name, err) + } + }) + + return nil + } + + if err := add("GatewayPool", pools, handler); err != nil { + runGatewayInformerCleanups(cleanups) + + return nil, err + } + + gatewayNodeHandler := cache.ResourceEventHandlerFuncs{ + AddFunc: func(interface{}) { onEvent() }, + UpdateFunc: func(oldObj, newObj interface{}) { + if shouldReconcileGatewayNodeUpdate(oldObj, newObj) { + onEvent() + } + }, + DeleteFunc: func(interface{}) { onEvent() }, + } + if err := add("GatewayPoolNode", nodes, gatewayNodeHandler); err != nil { + runGatewayInformerCleanups(cleanups) + + return nil, err + } + + if err := add("GatewayPoolPeering", poolPeerings, handler); err != nil { + runGatewayInformerCleanups(cleanups) + + return nil, err + } + + return cleanups, nil +} + +func runGatewayInformerCleanups(cleanups []func()) { + for i := len(cleanups) - 1; i >= 0; i-- { + cleanups[i]() + } +} + +func gatewayAssignmentEventHandler(siteName string, refreshRequired, requestUpdate func()) cache.ResourceEventHandlerFuncs { + changed := func(oldObj, newObj interface{}) { + if reflect.DeepEqual(oldObj, newObj) { + return + } + + if gatewayAssignmentTargetsSite(oldObj, siteName) || gatewayAssignmentTargetsSite(newObj, siteName) { + refreshRequired() + } + + requestUpdate() + } + + return cache.ResourceEventHandlerFuncs{ + AddFunc: func(obj interface{}) { changed(nil, obj) }, + UpdateFunc: changed, + DeleteFunc: func(obj interface{}) { changed(obj, nil) }, + } +} + +func gatewayAssignmentTargetsSite(obj interface{}, siteName string) bool { + if tombstone, ok := obj.(cache.DeletedFinalStateUnknown); ok { + obj = tombstone.Obj + } + + assignment, ok := obj.(*unstructured.Unstructured) + if !ok { + return false + } + + sites, _, err := unstructured.NestedStringSlice(assignment.Object, "spec", "sites") + if err != nil { + klog.Warningf("Cannot read SiteGatewayPoolAssignment %s Sites: %v", assignment.GetName(), err) + return false + } + + for _, site := range sites { + if strings.TrimSpace(site) == strings.TrimSpace(siteName) { + return true + } + } + + return false +} + +func gatewayCachesRequired(adopted nodeAssignment, siteName string, assignmentInformer cache.SharedIndexInformer) bool { + if adopted.gatewayPool != "" { + return true + } + + siteName = strings.TrimSpace(siteName) + if siteName == "" || assignmentInformer == nil { + return false + } + + for _, item := range assignmentInformer.GetStore().List() { + unstr, ok := item.(*unstructured.Unstructured) + if !ok { + continue + } + + assignment, err := parseSiteGatewayPoolAssignment(unstr) + if err != nil || !unboundednetv1alpha1.SpecEnabled(assignment.Spec.Enabled) { + continue + } + + targetsSite := false + + for _, assignmentSite := range assignment.Spec.Sites { + if strings.TrimSpace(assignmentSite) == siteName { + targetsSite = true + + break + } + } + + if !targetsSite { + continue + } + + for _, poolName := range assignment.Spec.GatewayPools { + if strings.TrimSpace(poolName) != "" { + return true + } + } + } + + return false +} diff --git a/cmd/unbounded-net-node/gateway_informer_lifecycle_test.go b/cmd/unbounded-net-node/gateway_informer_lifecycle_test.go new file mode 100644 index 000000000..a0fac61d7 --- /dev/null +++ b/cmd/unbounded-net-node/gateway_informer_lifecycle_test.go @@ -0,0 +1,383 @@ +// Copyright (c) Microsoft Corporation. +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "context" + "errors" + "sync" + "sync/atomic" + "testing" + "time" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + dynamicfake "k8s.io/client-go/dynamic/fake" + k8stesting "k8s.io/client-go/testing" + "k8s.io/client-go/tools/cache" + + unboundednetv1alpha1 "github.com/Azure/unbounded/api/net/v1alpha1" +) + +func TestGatewayInformerFailedListCanBeAbandoned(t *testing.T) { + for _, mode := range []string{"assignment removed", "sync timeout", "close"} { + t.Run(mode, func(t *testing.T) { + client := newGatewayLifecycleClient() + listed := make(chan struct{}, 1) + + client.PrependReactor("list", "*", func(k8stesting.Action) (bool, runtime.Object, error) { + select { + case listed <- struct{}{}: + default: + } + + return true, nil, errors.New("list unavailable") + }) + lifecycle := newGatewayInformerLifecycle(client, 0) + t.Cleanup(lifecycle.close) + + if mode == "sync timeout" { + lifecycle.syncTimeout = 50 * time.Millisecond + } + + done := make(chan error, 1) + + go func() { + _, _, err := lifecycle.prepare(t.Context(), true) + done <- err + }() + + select { + case <-listed: + case <-time.After(2 * time.Second): + t.Fatal("informers did not attempt LIST") + } + + switch mode { + case "assignment removed": + lifecycle.setRequired(false) + case "close": + lifecycle.close() + } + + select { + case err := <-done: + if err == nil { + t.Fatal("failed LIST unexpectedly produced synced caches") + } + case <-time.After(2 * time.Second): + t.Fatal("cache sync blocked reconciliation") + } + + if lifecycle.snapshot().available { + t.Fatal("unsynced generation was published") + } + + if mode != "sync timeout" { + if _, retire, err := lifecycle.prepare(t.Context(), false); err != nil || retire { + t.Fatalf("failed to abandon caches: retire=%v err=%v", retire, err) + } + } else if ready, _ := lifecycle.readiness(); ready { + t.Fatal("timed-out required caches reported ready") + } + }) + } +} + +func TestGatewayInformerLifecycleLateEventHandler(t *testing.T) { + client := newGatewayLifecycleClient() + lifecycle := newGatewayInformerLifecycle(client, 0) + t.Cleanup(lifecycle.close) + + if _, _, err := lifecycle.prepare(t.Context(), true); err != nil { + t.Fatal(err) + } + + // Start the generation before installing the callback, reproducing the + // startup ordering that previously captured a permanent no-op. + events := make(chan struct{}, 1) + + lifecycle.setEventHandler(func() { + _ = lifecycle.snapshot() + + events <- struct{}{} + }) + + waitForEvent := func(resource, operation string) { + t.Helper() + + select { + case <-events: + case <-time.After(2 * time.Second): + t.Fatalf("%s %s did not request an immediate reconcile", resource, operation) + } + } + for _, resource := range []struct { + gvr schema.GroupVersionResource + kind string + }{ + {gatewayPoolGVR, "GatewayPool"}, + {gatewayNodeGVR, "GatewayPoolNode"}, + {gatewayPoolPeeringGVR, "GatewayPoolPeering"}, + } { + api := client.Resource(resource.gvr) + object := &unstructured.Unstructured{Object: map[string]any{ + "apiVersion": "net.unbounded-cloud.io/v1alpha1", + "kind": resource.kind, + "metadata": map[string]any{"name": "event-test", "resourceVersion": "1"}, + "spec": map[string]any{"nodeName": "node-a"}, + }} + + created, err := api.Create(t.Context(), object, metav1.CreateOptions{}) + if err != nil { + t.Fatal(err) + } + + waitForEvent(resource.kind, "add") + created.SetResourceVersion("2") + + if err := unstructured.SetNestedField(created.Object, "node-b", "spec", "nodeName"); err != nil { + t.Fatal(err) + } + + if _, err := api.Update(t.Context(), created, metav1.UpdateOptions{}); err != nil { + t.Fatal(err) + } + + waitForEvent(resource.kind, "update") + + if err := api.Delete(t.Context(), created.GetName(), metav1.DeleteOptions{}); err != nil { + t.Fatal(err) + } + + waitForEvent(resource.kind, "delete") + } + + if got := watchActionCount(client); got != 3 { + t.Fatalf("watch actions=%d, want unchanged generation with three watches", got) + } +} + +func TestGatewayAssignmentEventsLimitSynchronousScans(t *testing.T) { + refreshes, requests := 0, 0 + handler := gatewayAssignmentEventHandler("local", func() { refreshes++ }, func() { requests++ }) + + remote := toUnstructuredSiteWatch(t, &unboundednetv1alpha1.SiteGatewayPoolAssignment{ + ObjectMeta: metav1.ObjectMeta{Name: "remote"}, + Spec: unboundednetv1alpha1.SiteGatewayPoolAssignmentSpec{ + Sites: []string{"remote"}, GatewayPools: []string{"pool"}, + }, + }) + for range 1000 { + handler.OnUpdate(remote, remote.DeepCopy()) + } + + if refreshes != 0 || requests != 0 { + t.Fatalf("resync caused work: scans=%d requests=%d", refreshes, requests) + } + + changed := remote.DeepCopy() + changed.SetResourceVersion("2") + handler.OnAdd(remote, false) + handler.OnUpdate(remote, changed) + handler.OnDelete(cache.DeletedFinalStateUnknown{Obj: changed}) + + if refreshes != 0 || requests != 3 { + t.Fatalf("remote events must be coalesced without synchronous scans: scans=%d requests=%d", refreshes, requests) + } + + local := changed.DeepCopy() + if err := unstructured.SetNestedStringSlice(local.Object, []string{"local"}, "spec", "sites"); err != nil { + t.Fatal(err) + } + + handler.OnAdd(local, false) + handler.OnUpdate(local, changed) + handler.OnUpdate(changed, local) + handler.OnDelete(cache.DeletedFinalStateUnknown{Obj: local}) + + if refreshes != 4 || requests != 7 { + t.Fatalf("local membership changes did not refresh requirements immediately: scans=%d requests=%d", refreshes, requests) + } +} + +func TestGatewayCachesRequired(t *testing.T) { + assignments := newTestInformer() + adopted := nodeAssignment{site: "site-a"} + + if gatewayCachesRequired(adopted, adopted.site, assignments) { + t.Fatal("empty assignment cache unexpectedly requires gateway caches") + } + + disabled := false + for _, assignment := range []*unboundednetv1alpha1.SiteGatewayPoolAssignment{ + { + ObjectMeta: metav1.ObjectMeta{Name: "remote"}, + Spec: unboundednetv1alpha1.SiteGatewayPoolAssignmentSpec{ + Sites: []string{"site-b"}, + GatewayPools: []string{"pool-b"}, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{Name: "disabled"}, + Spec: unboundednetv1alpha1.SiteGatewayPoolAssignmentSpec{ + Enabled: &disabled, + Sites: []string{"site-a"}, + GatewayPools: []string{"pool-disabled"}, + }, + }, + } { + if err := assignments.GetStore().Add(toUnstructuredSiteWatch(t, assignment)); err != nil { + t.Fatalf("add assignment: %v", err) + } + } + + if gatewayCachesRequired(adopted, adopted.site, assignments) { + t.Fatal("remote and disabled assignments unexpectedly require gateway caches") + } + + direct := &unboundednetv1alpha1.SiteGatewayPoolAssignment{ + ObjectMeta: metav1.ObjectMeta{Name: "direct"}, + Spec: unboundednetv1alpha1.SiteGatewayPoolAssignmentSpec{ + Sites: []string{"site-a"}, + GatewayPools: []string{"pool-a"}, + }, + } + if err := assignments.GetStore().Add(toUnstructuredSiteWatch(t, direct)); err != nil { + t.Fatalf("add direct assignment: %v", err) + } + + if !gatewayCachesRequired(adopted, adopted.site, assignments) { + t.Fatal("direct assignment did not require gateway caches") + } + + if !gatewayCachesRequired(nodeAssignment{site: "site-a", gatewayPool: "pool-own"}, "site-a", newTestInformer()) { + t.Fatal("pinned gateway role must always require gateway caches") + } +} + +func TestGatewayInformerLifecycle(t *testing.T) { + client := newGatewayLifecycleClient() + lifecycle := newGatewayInformerLifecycle(client, 0) + t.Cleanup(lifecycle.close) + + if ready, reason := lifecycle.readiness(); !ready || reason != "" { + t.Fatalf("intentionally absent caches readiness = %v, %q", ready, reason) + } + + lifecycle.setRequired(true) + + if ready, reason := lifecycle.readiness(); ready || reason != "required gateway informer caches are not running" { + t.Fatalf("missing required caches readiness = %v, %q", ready, reason) + } + + var events atomic.Int32 + + lifecycle.setEventHandler(func() { + events.Add(1) + }) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + snapshot, retire, err := lifecycle.prepare(ctx, true) + if err != nil { + t.Fatalf("prepare required caches: %v", err) + } + + if retire || !gatewayInformerSnapshotSynced(snapshot) { + t.Fatalf("required snapshot = %#v, retire=%v", snapshot, retire) + } + + if ready, reason := lifecycle.readiness(); !ready || reason != "" { + t.Fatalf("synced required caches readiness = %v, %q", ready, reason) + } + + if got := watchActionCount(client); got != 3 { + t.Fatalf("watch actions = %d, want one for each gateway resource", got) + } + + retiring, retire, err := lifecycle.prepare(ctx, false) + if err != nil { + t.Fatalf("prepare cache retirement: %v", err) + } + + if !retire || retiring.generation != snapshot.generation { + t.Fatalf("retiring snapshot generation=%d retire=%v, want generation=%d", retiring.generation, retire, snapshot.generation) + } + + // Readdition before cleanup keeps the synchronized generation alive. + lifecycle.setRequired(true) + lifecycle.retire(retiring.generation) + + if got := lifecycle.snapshot(); !got.available || got.generation != snapshot.generation { + t.Fatalf("readdition lost synchronized generation: %#v", got) + } + + lifecycle.setRequired(false) + lifecycle.retire(snapshot.generation) + + if got := lifecycle.snapshot(); got.available { + t.Fatalf("retired snapshot still available: %#v", got) + } + + if ready, reason := lifecycle.readiness(); !ready || reason != "" { + t.Fatalf("retired intentionally absent readiness = %v, %q", ready, reason) + } +} + +func TestGatewayInformerLifecycleConcurrentReadiness(t *testing.T) { + lifecycle := newGatewayInformerLifecycle(newGatewayLifecycleClient(), 0) + t.Cleanup(lifecycle.close) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + var wg sync.WaitGroup + for range 4 { + wg.Add(1) + + go func() { + defer wg.Done() + + for range 100 { + _, _ = lifecycle.readiness() + _ = lifecycle.snapshot() + } + }() + } + + if _, _, err := lifecycle.prepare(ctx, true); err != nil { + t.Fatalf("prepare required caches: %v", err) + } + + lifecycle.setRequired(false) + lifecycle.retire(lifecycle.snapshot().generation) + wg.Wait() +} + +func newGatewayLifecycleClient() *dynamicfake.FakeDynamicClient { + return dynamicfake.NewSimpleDynamicClientWithCustomListKinds( + runtime.NewScheme(), + map[schema.GroupVersionResource]string{ + gatewayPoolGVR: "GatewayPoolList", + gatewayNodeGVR: "GatewayPoolNodeList", + gatewayPoolPeeringGVR: "GatewayPoolPeeringList", + }, + ) +} + +func watchActionCount(client *dynamicfake.FakeDynamicClient) int { + count := 0 + + for _, action := range client.Actions() { + if action.GetVerb() == "watch" { + count++ + } + } + + return count +} diff --git a/cmd/unbounded-net-node/gateway_pool_peering_protocol_test.go b/cmd/unbounded-net-node/gateway_pool_peering_protocol_test.go index 511ff2fd7..b06e76d4b 100644 --- a/cmd/unbounded-net-node/gateway_pool_peering_protocol_test.go +++ b/cmd/unbounded-net-node/gateway_pool_peering_protocol_test.go @@ -6,6 +6,7 @@ package main import ( "context" "errors" + "fmt" "strings" "testing" @@ -142,6 +143,8 @@ func TestPinnedGatewayPoolAcknowledgmentWaitsForTaint(t *testing.T) { type poolPeeringProtocolFixture struct { peerings cache.SharedIndexInformer pools cache.SharedIndexInformer + nodes cache.SharedIndexInformer + assignments cache.SharedIndexInformer state *wireGuardState update func() error configureCalls int @@ -222,11 +225,16 @@ func newPoolPeeringProtocolFixture(t *testing.T, gateway bool) *poolPeeringProto TunnelProtocol: new(unboundednetv1alpha1.TunnelProtocolNone), }, })) + f.assignments = assignmentInformer empty := newInformerWithObjects() + gatewayNodes := newInformerWithObjects(toUnstructured(t, &unboundednetv1alpha1.GatewayPoolNode{ + ObjectMeta: metav1.ObjectMeta{Name: "remote"}, + })) + f.nodes = gatewayNodes cfg := &config{NodeName: "self", WireGuardPort: 51820, WireGuardInterfacePrefix: "gpp-test-wg", MTU: 576, PreferredPrivateEncap: "WireGuard"} f.update = func() error { return updateWireGuardFromSlices(context.Background(), nil, siteInformer, empty, f.pools, - empty, empty, assignmentInformer, f.peerings, cfg, siteName, "priv", "pub-self", true, f.state) + gatewayNodes, empty, assignmentInformer, f.peerings, cfg, siteName, "priv", "pub-self", true, f.state) } original := configureWireGuardFunc @@ -274,6 +282,44 @@ func (f *poolPeeringProtocolFixture) checkGateway(t *testing.T, protocol, overri } } +func TestGatewayAssignmentMTUDoesNotLeakFromWorkerAssignments(t *testing.T) { + f := newPoolPeeringProtocolFixture(t, true) + f.state.adoptedAssignment = nodeAssignment{site: "local", gatewayPool: "local-a"} + sites := newInformerWithObjects( + toUnstructured(t, &unboundedv1alpha3.Site{ObjectMeta: metav1.ObjectMeta{Name: "local"}}), + toUnstructured(t, &unboundedv1alpha3.Site{ObjectMeta: metav1.ObjectMeta{Name: "site-b"}}), + ) + assignments := newInformerWithObjects( + toUnstructured(t, &unboundednetv1alpha1.SiteGatewayPoolAssignment{ + ObjectMeta: metav1.ObjectMeta{Name: "worker-only"}, + Spec: unboundednetv1alpha1.SiteGatewayPoolAssignmentSpec{ + Sites: []string{"local", "site-b"}, GatewayPools: []string{"remote-pool"}, TunnelMTU: ptrInt32(1100), + }, + }), + toUnstructured(t, &unboundednetv1alpha1.SiteGatewayPoolAssignment{ + ObjectMeta: metav1.ObjectMeta{Name: "gateway-facing"}, + Spec: unboundednetv1alpha1.SiteGatewayPoolAssignmentSpec{ + Sites: []string{"site-b"}, GatewayPools: []string{"local-a"}, TunnelMTU: ptrInt32(1400), + }, + }), + ) + empty := newInformerWithObjects() + + cfg := &config{NodeName: "self", WireGuardPort: 51820, WireGuardInterfacePrefix: "gpp-test-wg", MTU: 576} + if err := updateWireGuardFromSlices(t.Context(), nil, sites, empty, f.pools, empty, empty, assignments, empty, + cfg, "local", "priv", "pub-self", true, f.state); err != nil { + t.Fatal(err) + } + + if got := f.state.assignmentSiteTunnelMTUs["site-b"]; got != 1400 { + t.Fatalf("gateway-to-Site cap=%d, want 1400, ignoring worker assignment via unrelated pool", got) + } + + if got := f.state.assignmentPoolTunnelMTUs["remote-pool"]; got != 0 { + t.Fatalf("worker-side pool cap leaked into gateway path: %d", got) + } +} + func TestGatewayPoolPeeringProtocolUpdates(t *testing.T) { f := newPoolPeeringProtocolFixture(t, true) for i, step := range []struct { @@ -420,6 +466,84 @@ func TestGatewayPoolPeeringProtocolConflictsAndScope(t *testing.T) { }) } +func TestPublishedForwardingMTUWorkerOnly(t *testing.T) { + for _, gateway := range []bool{false, true} { + t.Run(fmt.Sprintf("gateway=%v", gateway), func(t *testing.T) { + f := newPoolPeeringProtocolFixture(t, gateway) + f.putPeering(t, "connect", "local-a", nil, true) + + node := &unboundednetv1alpha1.GatewayPoolNode{ + ObjectMeta: metav1.ObjectMeta{Name: "remote"}, + Status: unboundednetv1alpha1.GatewayNodeStatus{ForwardingMTU: ptrInt32(500)}, + } + if err := f.nodes.GetStore().Update(toUnstructured(t, node)); err != nil { + t.Fatal(err) + } + + if err := f.update(); err != nil { + t.Fatal(err) + } + + want := 500 + if gateway { + want = 0 + } + + if len(f.gateways) != 1 || f.gateways[0].TransitTunnelMTU != want { + t.Fatalf("gateway=%v consumed published cap unexpectedly: %+v", gateway, f.gateways) + } + }) + } +} + +func TestGatewayPolicyFailureRetriesAfterAssignmentReactivation(t *testing.T) { + f := newPoolPeeringProtocolFixture(t, false) + f.putPeering(t, "connect", "local-a", nil, true) + + if err := f.update(); err != nil { + t.Fatal(err) + } + + assignment, exists, err := f.assignments.GetStore().GetByKey("local-to-remote") + if err != nil || !exists { + t.Fatalf("assignment exists=%v err=%v", exists, err) + } + + if err := f.assignments.GetStore().Delete(assignment); err != nil { + t.Fatal(err) + } + + f.configureErr = errors.New("policy table flush failed") + if err := f.update(); !errors.Is(err, f.configureErr) { + t.Fatalf("cleanup error=%v", err) + } + + if !f.state.dataplaneDirty || len(f.state.gatewayPeers) != 1 { + t.Fatal("failed cleanup was not retained as incomplete programming") + } + + if err := f.assignments.GetStore().Add(assignment); err != nil { + t.Fatal(err) + } + + f.configureErr = nil + if err := f.update(); err != nil { + t.Fatal(err) + } + + if f.configureCalls != 3 || len(f.gateways) != 1 || f.state.dataplaneDirty { + t.Fatalf("reactivated assignment skipped policy repair: calls=%d peers=%v dirty=%v", f.configureCalls, f.gateways, f.state.dataplaneDirty) + } + + if err := f.update(); err != nil { + t.Fatal(err) + } + + if f.configureCalls != 3 { + t.Fatal("successful repair did not restore no-op reconciliation") + } +} + func TestGatewayPoolPeeringProtocolFailureRetries(t *testing.T) { f := newPoolPeeringProtocolFixture(t, true) f.putPeering(t, "peering", "local-z", nil, true) diff --git a/cmd/unbounded-net-node/gateway_routes.go b/cmd/unbounded-net-node/gateway_routes.go index 4497cad30..ad79c6d86 100644 --- a/cmd/unbounded-net-node/gateway_routes.go +++ b/cmd/unbounded-net-node/gateway_routes.go @@ -298,11 +298,15 @@ func buildGatewayNodeRoutesForAssignedSitesStatus( return merged } -func syncGatewayNodeRoutesStatus(ctx context.Context, dynamicClient dynamic.Interface, gatewayNodeInformer cache.SharedIndexInformer, nodeName string, routes map[string]unboundednetv1alpha1.GatewayNodeRoute) error { +func syncGatewayNodeRoutesStatus(ctx context.Context, dynamicClient dynamic.Interface, gatewayNodeInformer cache.SharedIndexInformer, nodeName string, routes map[string]unboundednetv1alpha1.GatewayNodeRoute, forwardingMTU int) error { if dynamicClient == nil || nodeName == "" { return nil } + if forwardingMTU <= 0 || forwardingMTU > 65535 { + return fmt.Errorf("invalid gateway forwarding MTU %d", forwardingMTU) + } + // Build the routes patch. MergePatch preserves map keys that are absent // from the patch, so we must explicitly null out any CIDRs that were // previously advertised but are no longer present. Check the informer @@ -331,8 +335,9 @@ func syncGatewayNodeRoutesStatus(ctx context.Context, dynamicClient dynamic.Inte statusPatch := map[string]interface{}{ "status": map[string]interface{}{ - "lastUpdated": time.Now().UTC().Format(time.RFC3339), - "routes": routesPatch, + "lastUpdated": time.Now().UTC().Format(time.RFC3339), + "routes": routesPatch, + "forwardingMTU": forwardingMTU, }, } @@ -348,8 +353,9 @@ func clearGatewayNodeRoutesStatus(ctx context.Context, dynamicClient dynamic.Int return patchGatewayNodeStatus(ctx, dynamicClient, nodeName, map[string]interface{}{ "status": map[string]interface{}{ - "lastUpdated": time.Now().UTC().Format(time.RFC3339), - "routes": nil, + "lastUpdated": time.Now().UTC().Format(time.RFC3339), + "routes": nil, + "forwardingMTU": nil, }, }) } @@ -532,7 +538,7 @@ func routedCIDRsForGatewayPeer( staleAfter time.Duration, ) ([]string, map[string]int, map[string]unboundednetv1alpha1.GatewayNodeRoute) { if gatewayNode == nil { - return fallback, map[string]int{}, map[string]unboundednetv1alpha1.GatewayNodeRoute{} + return nil, map[string]int{}, map[string]unboundednetv1alpha1.GatewayNodeRoute{} } planRoutes := make(map[string]routeplan.LearnedRoute, len(gatewayNode.Status.Routes)) diff --git a/cmd/unbounded-net-node/gateway_routes_test.go b/cmd/unbounded-net-node/gateway_routes_test.go index 6b3c127ff..83d605954 100644 --- a/cmd/unbounded-net-node/gateway_routes_test.go +++ b/cmd/unbounded-net-node/gateway_routes_test.go @@ -6,20 +6,27 @@ package main import ( "context" "encoding/json" + "errors" "fmt" + "os" "testing" + "time" + corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" dynamicfake "k8s.io/client-go/dynamic/fake" + "k8s.io/client-go/kubernetes/fake" ktesting "k8s.io/client-go/testing" "k8s.io/client-go/tools/cache" unboundedv1alpha3 "github.com/Azure/unbounded/api/machina/v1alpha3" unboundednetv1alpha1 "github.com/Azure/unbounded/api/net/v1alpha1" + "github.com/Azure/unbounded/internal/net/healthcheck" + unboundednetnetlink "github.com/Azure/unbounded/internal/net/netlink" ) // testInformer is a minimal cache.SharedIndexInformer for tests that only need GetStore(). @@ -31,14 +38,234 @@ type testInformer struct { func (f *testInformer) GetStore() cache.Store { return f.store } func (f *testInformer) HasSynced() bool { return true } +func TestGatewayForwardingMTURejectsInvalidValue(t *testing.T) { + for _, mtu := range []int{0, -1, 65536} { + client := fakeGatewayDynamic() + if err := syncGatewayNodeRoutesStatus(t.Context(), client, nil, "self", nil, mtu); err == nil { + t.Fatalf("invalid MTU %d was accepted", mtu) + } + + if len(client.Actions()) != 0 { + t.Fatal("invalid MTU reached the API server") + } + } +} + +func TestGatewayForwardingMTUPublishedAfterConfiguration(t *testing.T) { + self := &unboundednetv1alpha1.GatewayPoolNode{ObjectMeta: metav1.ObjectMeta{Name: "self"}} + client := fakeGatewayDynamic(toUnstructured(t, self)) + gatewayNodes := newInformerWithObjects(toUnstructured(t, self)) + sites := newInformerWithObjects(toUnstructured(t, &unboundedv1alpha3.Site{ObjectMeta: metav1.ObjectMeta{Name: "local"}})) + pools := newInformerWithObjects(toUnstructured(t, &unboundednetv1alpha1.GatewayPool{ + ObjectMeta: metav1.ObjectMeta{Name: "pool"}, + Spec: unboundednetv1alpha1.GatewayPoolSpec{Type: "Internal"}, + Status: unboundednetv1alpha1.GatewayPoolStatus{Nodes: []unboundednetv1alpha1.GatewayNodeInfo{{ + Name: "self", SiteName: "local", WireGuardPublicKey: "pub-self", GatewayWireguardPort: 51821, + }}}, + })) + empty := newInformerWithObjects() + state := &wireGuardState{ + nodeName: "self", clientset: fake.NewClientset(&corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: "self"}}), + cniConfigWritten: true, nodePodCIDRs: []string{"10.244.1.0/24"}, + } + cfg := newCNISafetyTestConfig(t) + cfg.NodeName, cfg.WireGuardPort, cfg.MTU = "self", 51820, 1200 + configureErr := errors.New("programming failed") + original, originalBridge, originalVerify := configureWireGuardFunc, ensureCNIBridgeMTUFunc, verifyActiveTunnelMTUsFunc + + t.Cleanup(func() { + configureWireGuardFunc, ensureCNIBridgeMTUFunc, verifyActiveTunnelMTUsFunc = original, originalBridge, originalVerify + }) + + configureCalls := 0 + + var programmingOrder []string + + configureWireGuardFunc = func(context.Context, *config, string, []meshPeerInfo, []gatewayPeerInfo, string, map[string]bool, map[string]bool, map[string]bool, map[string]string, map[string]string, map[string]string, map[string]string, map[string]string, map[string]int, map[string]int, map[string]int, map[string]int, map[string]int, []unboundednetnetlink.DesiredRoute, map[string]bool, *wireGuardState, map[string]healthcheck.HealthCheckSettings) error { + configureCalls++ + + programmingOrder = append(programmingOrder, "tunnels") + + return configureErr + } + + var bridgeErr, verifyErr error + + bridgeMTU, repairedPods := 0, false + ensureCNIBridgeMTUFunc = func(_ string, mtu int, _ *unboundednetnetlink.NetlinkCache, pods bool) error { + bridgeMTU, repairedPods = mtu, pods + + programmingOrder = append(programmingOrder, "fabric") + + return bridgeErr + } + verifyActiveTunnelMTUsFunc = func(*config, int, []meshPeerInfo, []gatewayPeerInfo) error { + return verifyErr + } + + update := func() error { + return updateWireGuardFromSlices(t.Context(), client, sites, empty, pools, gatewayNodes, empty, empty, empty, + cfg, "local", "private", "pub-self", true, state) + } + if err := update(); !errors.Is(err, configureErr) { + t.Fatalf("expected failed dataplane configuration, got %v", err) + } + + for _, action := range client.Actions() { + if action.GetVerb() == "patch" { + t.Fatal("failed configuration advertised a forwarding MTU or routes") + } + } + + configureErr = nil + + if err := update(); err != nil { + t.Fatal(err) + } + + got, err := client.Resource(gatewayNodeGVR).Get(t.Context(), "self", metav1.GetOptions{}) + if err != nil { + t.Fatal(err) + } + + if mtu, ok, err := unstructured.NestedInt64(got.Object, "status", "forwardingMTU"); err != nil || !ok || mtu != 1200 { + t.Fatalf("successful configuration did not publish effective MTU: %v err=%v", got.Object, err) + } + + if err := update(); err != nil { + t.Fatalf("unchanged reconciliation did not refresh advertisement: %v", err) + } + + beforeCalls, beforeActions := configureCalls, len(client.Actions()) + + verifyErr = errors.New("tunnel MTU drift") + if err := update(); !errors.Is(err, verifyErr) { + t.Fatalf("verification error=%v", err) + } + + if !state.dataplaneDirty || configureCalls != beforeCalls { + t.Fatal("unchanged-state verification failure did not request dataplane repair") + } + + for _, action := range client.Actions()[beforeActions:] { + if action.GetVerb() == "patch" { + t.Fatal("failed verification published gateway status") + } + } + + verifyErr = nil + + if err := update(); err != nil { + t.Fatal(err) + } + + if state.dataplaneDirty || configureCalls != beforeCalls+1 { + t.Fatal("verification drift did not trigger programming on retry") + } + + publicationErr := errors.New("status PATCH failed") + failPublication := false + + client.PrependReactor("patch", "gatewaypoolnodes", func(ktesting.Action) (bool, runtime.Object, error) { + if failPublication { + return true, nil, publicationErr + } + + return false, nil, nil + }) + + for _, failure := range []string{"status publication", "partial fabric write"} { + t.Run(failure, func(t *testing.T) { + cfg.MTU = 1400 + failPublication = failure == "status publication" + wantErr := publicationErr + + if !failPublication { + bridgeErr = errors.New("pod MTU update failed") + wantErr = bridgeErr + } + + if err := update(); !errors.Is(err, wantErr) { + t.Fatalf("expected failure after fabric writes, got %v", err) + } + + if state.cniMTU != 1200 || state.appliedFabricMTU != 1400 || !state.dataplaneDirty { + t.Fatalf("lost in-flight MTU: committed=%d attempted=%d dirty=%v", state.cniMTU, state.appliedFabricMTU, state.dataplaneDirty) + } + + current, err := client.Resource(gatewayNodeGVR).Get(t.Context(), "self", metav1.GetOptions{}) + if err != nil { + t.Fatal(err) + } + + if mtu, _, err := unstructured.NestedInt64(current.Object, "status", "forwardingMTU"); err != nil || mtu != 1200 { + t.Fatalf("failed reconcile advertised uncommitted fabric: %v err=%v", current.Object, err) + } + + cfg.MTU, failPublication, bridgeErr = 1200, false, nil + + if err := update(); err != nil { + t.Fatal(err) + } + + if bridgeMTU != 1200 || !repairedPods || state.dataplaneDirty || state.appliedFabricMTU != 1200 { + t.Fatalf("fabric reversion skipped repair: bridge=%d pods=%v attempted=%d dirty=%v", bridgeMTU, repairedPods, state.appliedFabricMTU, state.dataplaneDirty) + } + + data, err := os.ReadFile(cniConfigPath(cfg)) + if err != nil { + t.Fatal(err) + } + + var cni CNIConfig + if err := json.Unmarshal(data, &cni); err != nil { + t.Fatal(err) + } + + if len(cni.Plugins) == 0 || cni.Plugins[0].MTU != 1200 { + t.Fatalf("CNI config retained raised MTU: %s", data) + } + }) + } + + cfg.MTU = 1400 + + if err := update(); err != nil { + t.Fatal(err) + } + + cfg.MTU, bridgeErr = 1200, errors.New("lowering pod MTU failed") + if err := update(); !errors.Is(err, bridgeErr) { + t.Fatalf("expected partial lowering error, got %v", err) + } + + cfg.MTU, bridgeErr, programmingOrder = 1400, nil, nil + + if err := update(); err != nil { + t.Fatal(err) + } + + if len(programmingOrder) != 2 || programmingOrder[0] != "tunnels" || programmingOrder[1] != "fabric" { + t.Fatalf("reversion raised fabric before repairing tunnels: %v", programmingOrder) + } +} + +func TestMissingGatewayNodeHasNoFallbackRoutes(t *testing.T) { + routes, distances, learned := routedCIDRsForGatewayPeer(nil, []string{"10.0.0.0/16"}, "local", nil, nil, time.Time{}, time.Minute) + if len(routes) != 0 || len(distances) != 0 || len(learned) != 0 { + t.Fatalf("missing gateway retained routes: %v %v %v", routes, distances, learned) + } +} + func TestClearGatewayNodeRoutesStatusWithoutCache(t *testing.T) { node := &unstructured.Unstructured{Object: map[string]interface{}{ "apiVersion": "net.unbounded-cloud.io/v1alpha1", "kind": "GatewayPoolNode", "metadata": map[string]interface{}{"name": "gateway-a"}, "status": map[string]interface{}{ - "routes": map[string]interface{}{"10.0.0.0/16": map[string]interface{}{"type": "NodeCidr"}}, - "publicKey": "keep", + "routes": map[string]interface{}{"10.0.0.0/16": map[string]interface{}{"type": "NodeCidr"}}, + "publicKey": "keep", + "forwardingMTU": int64(1200), }, }} @@ -59,6 +286,10 @@ func TestClearGatewayNodeRoutesStatusWithoutCache(t *testing.T) { if value, _, _ := unstructured.NestedString(got.Object, "status", "publicKey"); value != "keep" { t.Fatalf("unrelated status removed: %v", got.Object) } + + if _, found, err := unstructured.NestedInt64(got.Object, "status", "forwardingMTU"); err != nil || found { + t.Fatalf("forwarding MTU remains after clear: %v, err=%v", got.Object, err) + } } func toUnstructured(t *testing.T, obj interface{}) *unstructured.Unstructured { @@ -237,11 +468,11 @@ func TestResolveGatewayPoolSiteName(t *testing.T) { // TestSyncGatewayNodeRoutesStatusEarlyReturn tests SyncGatewayNodeRoutesStatusEarlyReturn. func TestSyncGatewayNodeRoutesStatusEarlyReturn(t *testing.T) { - if err := syncGatewayNodeRoutesStatus(context.TODO(), nil, nil, "node-a", nil); err != nil { + if err := syncGatewayNodeRoutesStatus(context.TODO(), nil, nil, "node-a", nil, 1400); err != nil { t.Fatalf("expected nil client to no-op, got %v", err) } - if err := syncGatewayNodeRoutesStatus(context.TODO(), nil, nil, "", nil); err != nil { + if err := syncGatewayNodeRoutesStatus(context.TODO(), nil, nil, "", nil, 1400); err != nil { t.Fatalf("expected empty node name to no-op, got %v", err) } } @@ -359,7 +590,7 @@ func TestSyncGatewayNodeRoutesStatus(t *testing.T) { routes := map[string]unboundednetv1alpha1.GatewayNodeRoute{ "10.244.0.0/16": {Type: "RoutedCidr"}, } - if err := syncGatewayNodeRoutesStatus(context.Background(), dyn, nil, "gw-a", routes); err != nil { + if err := syncGatewayNodeRoutesStatus(context.Background(), dyn, nil, "gw-a", routes, 1200); err != nil { t.Fatalf("syncGatewayNodeRoutesStatus() error = %v", err) } @@ -375,6 +606,10 @@ func TestSyncGatewayNodeRoutesStatus(t *testing.T) { if _, ok, _ := unstructured.NestedMap(got.Object, "status", "routes"); !ok { t.Fatalf("expected status.routes to be set") } + + if mtu, ok, err := unstructured.NestedInt64(got.Object, "status", "forwardingMTU"); err != nil || !ok || mtu != 1200 { + t.Fatalf("forwardingMTU=%d present=%v err=%v", mtu, ok, err) + } } // TestSyncGatewayNodeRoutesStatusRemovesStaleRoutes verifies that CIDRs @@ -413,7 +648,7 @@ func TestSyncGatewayNodeRoutesStatusRemovesStaleRoutes(t *testing.T) { newRoutes := map[string]unboundednetv1alpha1.GatewayNodeRoute{ "10.244.0.0/16": {Type: "PodCidr"}, } - if err := syncGatewayNodeRoutesStatus(context.Background(), dyn, fakeInf, "gw-a", newRoutes); err != nil { + if err := syncGatewayNodeRoutesStatus(context.Background(), dyn, fakeInf, "gw-a", newRoutes, 1400); err != nil { t.Fatalf("syncGatewayNodeRoutesStatus() error = %v", err) } diff --git a/cmd/unbounded-net-node/informer_factory.go b/cmd/unbounded-net-node/informer_factory.go new file mode 100644 index 000000000..801a217d9 --- /dev/null +++ b/cmd/unbounded-net-node/informer_factory.go @@ -0,0 +1,81 @@ +// Copyright (c) Microsoft Corporation. +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "context" + "time" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/watch" + "k8s.io/client-go/dynamic" + "k8s.io/client-go/dynamic/dynamicinformer" + clientwatchlist "k8s.io/client-go/util/watchlist" +) + +func newNodeDynamicInformerFactory( + client dynamic.Interface, + resync time.Duration, +) dynamicinformer.DynamicSharedInformerFactory { + return dynamicinformer.NewDynamicSharedInformerFactory( + &watchTimeoutDynamicClient{Interface: client}, + resync, + ) +} + +func doubleWatchTimeout(options *metav1.ListOptions) { + if options == nil || options.TimeoutSeconds == nil || *options.TimeoutSeconds <= 0 { + return + } + + timeoutSeconds := *options.TimeoutSeconds * 2 + options.TimeoutSeconds = &timeoutSeconds +} + +type watchTimeoutDynamicClient struct { + dynamic.Interface +} + +func (c *watchTimeoutDynamicClient) Resource(resource schema.GroupVersionResource) dynamic.NamespaceableResourceInterface { + return &watchTimeoutNamespaceableResource{ + NamespaceableResourceInterface: c.Interface.Resource(resource), + } +} + +func (c *watchTimeoutDynamicClient) IsWatchListSemanticsUnSupported() bool { + return clientwatchlist.DoesClientNotSupportWatchListSemantics(c.Interface) +} + +type watchTimeoutNamespaceableResource struct { + dynamic.NamespaceableResourceInterface +} + +func (r *watchTimeoutNamespaceableResource) Namespace(namespace string) dynamic.ResourceInterface { + return &watchTimeoutResource{ + ResourceInterface: r.NamespaceableResourceInterface.Namespace(namespace), + } +} + +func (r *watchTimeoutNamespaceableResource) Watch(ctx context.Context, options metav1.ListOptions) (watch.Interface, error) { + doubleWatchTimeout(&options) + + return r.NamespaceableResourceInterface.Watch(ctx, options) +} + +type watchTimeoutResource struct { + dynamic.ResourceInterface +} + +func (r *watchTimeoutResource) Watch(ctx context.Context, options metav1.ListOptions) (watch.Interface, error) { + doubleWatchTimeout(&options) + + return r.ResourceInterface.Watch(ctx, options) +} + +var ( + _ dynamic.ResourceInterface = (*watchTimeoutResource)(nil) + _ dynamic.NamespaceableResourceInterface = (*watchTimeoutNamespaceableResource)(nil) + _ dynamic.Interface = (*watchTimeoutDynamicClient)(nil) +) diff --git a/cmd/unbounded-net-node/informer_factory_test.go b/cmd/unbounded-net-node/informer_factory_test.go new file mode 100644 index 000000000..8673caa45 --- /dev/null +++ b/cmd/unbounded-net-node/informer_factory_test.go @@ -0,0 +1,199 @@ +// Copyright (c) Microsoft Corporation. +// SPDX-License-Identifier: Apache-2.0 + +package main + +import ( + "context" + "errors" + "sync" + "testing" + "time" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/watch" + "k8s.io/client-go/dynamic" + dynamicfake "k8s.io/client-go/dynamic/fake" + clientfeatures "k8s.io/client-go/features" + clientfeaturestesting "k8s.io/client-go/features/testing" + "k8s.io/client-go/tools/cache" +) + +type recordedInformerRequest struct { + verb string + options metav1.ListOptions +} + +type recordingDynamicClient struct { + dynamic.Interface + + mu sync.Mutex + requests []recordedInformerRequest +} + +func (c *recordingDynamicClient) Resource(resource schema.GroupVersionResource) dynamic.NamespaceableResourceInterface { + return &recordingNamespaceableResource{ + NamespaceableResourceInterface: c.Interface.Resource(resource), + client: c, + } +} + +func (c *recordingDynamicClient) record(verb string, options metav1.ListOptions) { + c.mu.Lock() + defer c.mu.Unlock() + + c.requests = append(c.requests, recordedInformerRequest{verb: verb, options: cloneListOptions(options)}) +} + +func (c *recordingDynamicClient) snapshot() []recordedInformerRequest { + c.mu.Lock() + defer c.mu.Unlock() + + return append([]recordedInformerRequest(nil), c.requests...) +} + +type recordingNamespaceableResource struct { + dynamic.NamespaceableResourceInterface + + client *recordingDynamicClient +} + +func (r *recordingNamespaceableResource) Namespace(namespace string) dynamic.ResourceInterface { + return &recordingResource{ + ResourceInterface: r.NamespaceableResourceInterface.Namespace(namespace), + client: r.client, + } +} + +type recordingResource struct { + dynamic.ResourceInterface + + client *recordingDynamicClient +} + +func (r *recordingResource) List(ctx context.Context, options metav1.ListOptions) (*unstructured.UnstructuredList, error) { + r.client.record("list", options) + + return r.ResourceInterface.List(ctx, options) +} + +func (r *recordingResource) Watch(ctx context.Context, options metav1.ListOptions) (watch.Interface, error) { + r.client.record("watch", options) + + if options.SendInitialEvents != nil { + return nil, errors.New("watch-list unsupported") + } + + return r.ResourceInterface.Watch(ctx, options) +} + +func cloneListOptions(options metav1.ListOptions) metav1.ListOptions { + cloned := options + if options.TimeoutSeconds != nil { + timeoutSeconds := *options.TimeoutSeconds + cloned.TimeoutSeconds = &timeoutSeconds + } + + if options.SendInitialEvents != nil { + sendInitialEvents := *options.SendInitialEvents + cloned.SendInitialEvents = &sendInitialEvents + } + + return cloned +} + +func TestNodeInformerFactoryWatchTimeoutRequests(t *testing.T) { + clientfeaturestesting.SetFeatureDuringTest(t, clientfeatures.WatchListClient, true) + + gvr := schema.GroupVersionResource{Group: "net.unbounded-cloud.io", Version: "v1alpha1", Resource: "testresources"} + fakeClient := dynamicfake.NewSimpleDynamicClientWithCustomListKinds( + runtime.NewScheme(), + map[schema.GroupVersionResource]string{gvr: "TestResourceList"}, + ) + client := &recordingDynamicClient{Interface: fakeClient} + factory := newNodeDynamicInformerFactory(client, 0) + informer := factory.ForResource(gvr).Informer() + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + factory.Start(ctx.Done()) + + if !cache.WaitForCacheSync(ctx.Done(), informer.HasSynced) { + t.Fatal("informer cache did not sync") + } + + deadline := time.Now().Add(time.Second) + + for { + requests := client.snapshot() + if assertInformerRequestOptions(t, requests) { + break + } + + if time.Now().After(deadline) { + t.Fatalf("did not observe watch-list, LIST, and watch requests: %#v", requests) + } + + time.Sleep(10 * time.Millisecond) + } +} + +func assertInformerRequestOptions(t *testing.T, requests []recordedInformerRequest) bool { + t.Helper() + + var watchList, list, normalWatch *metav1.ListOptions + + for i := range requests { + request := &requests[i] + switch { + case request.verb == "watch" && request.options.SendInitialEvents != nil: + watchList = &request.options + case request.verb == "list": + list = &request.options + case request.verb == "watch": + normalWatch = &request.options + } + } + + if watchList == nil || list == nil || normalWatch == nil { + return false + } + + assertDoubledWatchTimeout(t, "watch-list", *watchList) + + if watchList.SendInitialEvents == nil || !*watchList.SendInitialEvents { + t.Fatal("watch-list request did not preserve sendInitialEvents=true") + } + + if !watchList.AllowWatchBookmarks || watchList.ResourceVersionMatch != metav1.ResourceVersionMatchNotOlderThan { + t.Fatalf("watch-list request fields changed: %#v", *watchList) + } + + if list.Watch || list.TimeoutSeconds != nil { + t.Fatalf("normal LIST timeout changed: %#v", *list) + } + + assertDoubledWatchTimeout(t, "watch", *normalWatch) + + if !normalWatch.AllowWatchBookmarks || normalWatch.SendInitialEvents != nil { + t.Fatalf("watch request fields changed: %#v", *normalWatch) + } + + return true +} + +func assertDoubledWatchTimeout(t *testing.T, requestType string, options metav1.ListOptions) { + t.Helper() + + if options.TimeoutSeconds == nil { + t.Fatalf("%s request has no timeoutSeconds", requestType) + } + + if timeout := *options.TimeoutSeconds; timeout < 600 || timeout >= 1200 { + t.Fatalf("%s timeoutSeconds = %d, want randomized [600,1200)", requestType, timeout) + } +} diff --git a/cmd/unbounded-net-node/main.go b/cmd/unbounded-net-node/main.go index 3533265b8..30a7c578b 100644 --- a/cmd/unbounded-net-node/main.go +++ b/cmd/unbounded-net-node/main.go @@ -21,7 +21,6 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/dynamic" - "k8s.io/client-go/dynamic/dynamicinformer" "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" "k8s.io/client-go/tools/cache" @@ -765,26 +764,24 @@ func run(cfg *config) (resultErr error) { klog.Info("Node annotated with WireGuard public key") - informerFactory := dynamicinformer.NewDynamicSharedInformerFactory(dynamicClient, cfg.InformerResyncPeriod) + informerFactory := newNodeDynamicInformerFactory(dynamicClient, cfg.InformerResyncPeriod) sliceInformer := informerFactory.ForResource(siteNodeSliceGVR).Informer() siteInformer := informerFactory.ForResource(siteGVR).Informer() - gatewayPoolInformer := informerFactory.ForResource(gatewayPoolGVR).Informer() - gatewayNodeInformer := informerFactory.ForResource(gatewayNodeGVR).Informer() sitePeeringInformer := informerFactory.ForResource(sitePeeringGVR).Informer() assignmentInformer := informerFactory.ForResource(siteGatewayPoolAssignmentGVR).Informer() - poolPeeringInformer := informerFactory.ForResource(gatewayPoolPeeringGVR).Informer() + + gatewayInformers := newGatewayInformerLifecycle(dynamicClient, cfg.InformerResyncPeriod) + defer gatewayInformers.close() healthState := &nodeHealthState{ nodeName: cfg.NodeName, informersSynced: []cache.InformerSynced{ sliceInformer.HasSynced, siteInformer.HasSynced, - gatewayPoolInformer.HasSynced, - gatewayNodeInformer.HasSynced, sitePeeringInformer.HasSynced, assignmentInformer.HasSynced, - poolPeeringInformer.HasSynced, }, + gatewayInformersReady: gatewayInformers.readiness, } if cfg.HealthPort > 0 { go startHealthServer(cfg.HealthPort, healthState) @@ -836,12 +833,14 @@ func run(cfg *config) (resultErr error) { // Wait for caches to sync klog.Info("Waiting for informer caches to sync") - if !cache.WaitForCacheSync(ctx.Done(), sliceInformer.HasSynced, siteInformer.HasSynced, gatewayPoolInformer.HasSynced, gatewayNodeInformer.HasSynced, sitePeeringInformer.HasSynced, assignmentInformer.HasSynced, poolPeeringInformer.HasSynced) { + if !cache.WaitForCacheSync(ctx.Done(), sliceInformer.HasSynced, siteInformer.HasSynced, sitePeeringInformer.HasSynced, assignmentInformer.HasSynced) { return fmt.Errorf("failed to sync informer caches") } klog.Info("Informer caches synced") + gatewayInformers.setRequired(gatewayCachesRequired(adoptedAssignment, adoptedAssignment.site, assignmentInformer)) + // Start the netlink cache (read-only network state snapshot). netlinkCache := unboundednetnetlink.NewNetlinkCache(cfg.NetlinkResyncPeriod) if err := netlinkCache.Start(ctx); err != nil { @@ -925,5 +924,5 @@ func run(cfg *config) (resultErr error) { return fmt.Errorf("POD_NAME, POD_NAMESPACE, and POD_UID are required for effective assignment acknowledgment") } - return watchSiteAndConfigureWireGuard(ctx, clientset, dynamicClient, cfg, pubKey, nodePodCIDRs, manageCniPlugin, initialCNIConfigMTU, healthState, netlinkCache, adoptedAssignment, podNamespace, podName, podUID, startupWatch, siteInformer, sliceInformer, gatewayPoolInformer, gatewayNodeInformer, sitePeeringInformer, assignmentInformer, poolPeeringInformer) + return watchSiteAndConfigureWireGuard(ctx, clientset, dynamicClient, cfg, pubKey, nodePodCIDRs, manageCniPlugin, initialCNIConfigMTU, healthState, netlinkCache, adoptedAssignment, podNamespace, podName, podUID, startupWatch, siteInformer, sliceInformer, sitePeeringInformer, assignmentInformer, gatewayInformers) } diff --git a/cmd/unbounded-net-node/main_test.go b/cmd/unbounded-net-node/main_test.go index 6bf496340..ed82ed10e 100644 --- a/cmd/unbounded-net-node/main_test.go +++ b/cmd/unbounded-net-node/main_test.go @@ -402,6 +402,30 @@ func TestShouldReconcileGatewayNodeUpdate_HeartbeatOnlyChange(t *testing.T) { } // TestShouldReconcileGatewayNodeUpdate_RoutesChange tests ShouldReconcileGatewayNodeUpdate_RoutesChange. +func TestShouldReconcileGatewayNodeUpdate_ForwardingMTUChange(t *testing.T) { + oldNode := &unboundednetv1alpha1.GatewayPoolNode{ + ObjectMeta: metav1.ObjectMeta{Name: "gateway"}, + } + newNode := oldNode.DeepCopy() + + newNode.Status.ForwardingMTU = ptrInt32(1200) + if !shouldReconcileGatewayNodeUpdate(gatewayNodeToUnstructured(t, oldNode), gatewayNodeToUnstructured(t, newNode)) { + t.Fatal("new forwarding MTU did not trigger reconciliation") + } + + oldNode = newNode.DeepCopy() + + newNode.Status.ForwardingMTU = ptrInt32(1400) + if !shouldReconcileGatewayNodeUpdate(gatewayNodeToUnstructured(t, oldNode), gatewayNodeToUnstructured(t, newNode)) { + t.Fatal("raised forwarding MTU did not trigger reconciliation") + } + + newNode.Status.ForwardingMTU = nil + if !shouldReconcileGatewayNodeUpdate(gatewayNodeToUnstructured(t, oldNode), gatewayNodeToUnstructured(t, newNode)) { + t.Fatal("removed forwarding MTU did not trigger reconciliation") + } +} + func TestShouldReconcileGatewayNodeUpdate_RoutesChange(t *testing.T) { now := time.Now().UTC() oldNode := &unboundednetv1alpha1.GatewayPoolNode{ diff --git a/cmd/unbounded-net-node/main_update_test.go b/cmd/unbounded-net-node/main_update_test.go index 5bdb9c792..9ef7c93a6 100644 --- a/cmd/unbounded-net-node/main_update_test.go +++ b/cmd/unbounded-net-node/main_update_test.go @@ -589,7 +589,9 @@ func TestUpdateWireGuardFromSlices_ManageCniPluginFalseSkipsPodCIDRRoutesForSame }, }), ) - gatewayNodeInformer := newInformerWithObjects() + gatewayNodeInformer := newInformerWithObjects(toUnstructured(t, &unboundednetv1alpha1.GatewayPoolNode{ + ObjectMeta: metav1.ObjectMeta{Name: "gw-site1"}, + })) sitePeeringInformer := newInformerWithObjects() assignmentInformer := newInformerWithObjects( toUnstructured(t, &unboundednetv1alpha1.SiteGatewayPoolAssignment{ diff --git a/cmd/unbounded-net-node/mtu.go b/cmd/unbounded-net-node/mtu.go index 9df4f9bee..bc268f555 100644 --- a/cmd/unbounded-net-node/mtu.go +++ b/cmd/unbounded-net-node/mtu.go @@ -24,8 +24,9 @@ type cniBridgeLinkManager interface { } var ( - ensureCNIBridgeMTUFunc = ensureCNIBridgeMTU - newCNIBridgeLinkManagerFn = func(bridgeName string) cniBridgeLinkManager { + ensureCNIBridgeMTUFunc = ensureCNIBridgeMTU + verifyActiveTunnelMTUsFunc = verifyActiveTunnelMTUs + newCNIBridgeLinkManagerFn = func(bridgeName string) cniBridgeLinkManager { return unboundednetnetlink.NewLinkManager(bridgeName) } ) @@ -153,11 +154,22 @@ func resolveCNIConfigMTU(configuredMTU, siteMTU int, meshPeers []meshPeerInfo, g return resolveInitialCNIConfigMTU(configuredMTU, siteMTU, underlayMTU) } +func localGatewayPoolTunnelMTU(localPools []string, poolMTUs map[string]int) int { + result := 0 + for _, pool := range localPools { + result = resolveTunnelMTU(result, poolMTUs[pool]) + } + + return result +} + func resolvePeerTunnelMTUs( cfg *config, meshPeers []meshPeerInfo, gatewayPeers []gatewayPeerInfo, mySiteName string, + isGatewayNode bool, + localScopeMTU int, peeredSites, networkPeeredSites map[string]bool, siteTunnelMTUs, peeringSiteTunnelMTUs, assignmentSiteTunnelMTUs, assignmentPoolTunnelMTUs, poolTunnelMTUs map[string]int, @@ -166,9 +178,9 @@ func resolvePeerTunnelMTUs( for i := range meshPeers { underlayIP := selectUnderlayIP(meshPeers[i].InternalIPs, cfg.TunnelIPFamily) autoMTU := detectEncapsulatedRouteMTU(cfg, underlayIP, meshPeers[i].TunnelProtocol, cache) - meshPeers[i].TunnelMTU = resolveTunnelMTU(cfg.MTU, autoMTU, + meshPeers[i].TunnelMTU = resolveTunnelMTU(cfg.MTU, autoMTU, localScopeMTU, resolveMeshPeerTunnelMTU(0, meshPeers[i], mySiteName, - siteTunnelMTUs, peeringSiteTunnelMTUs, assignmentSiteTunnelMTUs)) + siteTunnelMTUs, peeringSiteTunnelMTUs, assignmentSiteTunnelMTUs, isGatewayNode)) } for i := range gatewayPeers { @@ -178,7 +190,13 @@ func resolvePeerTunnelMTUs( } autoMTU := detectEncapsulatedRouteMTU(cfg, underlayIP, gatewayPeers[i].TunnelProtocol, cache) - gatewayPeers[i].TunnelMTU = resolveTunnelMTU(cfg.MTU, autoMTU, + + firstHopPeeringMTU := 0 + if !isGatewayNode && gatewayPeers[i].SiteName != mySiteName { + firstHopPeeringMTU = peeringSiteTunnelMTUs[gatewayPeers[i].SiteName] + } + + gatewayPeers[i].TunnelMTU = resolveTunnelMTU(cfg.MTU, autoMTU, localScopeMTU, firstHopPeeringMTU, resolveGatewayPeerTunnelMTU(0, mySiteName, gatewayPeers[i], siteTunnelMTUs, assignmentPoolTunnelMTUs, poolTunnelMTUs)) } diff --git a/cmd/unbounded-net-node/mtu_test.go b/cmd/unbounded-net-node/mtu_test.go index d9afc1812..0d8389cfc 100644 --- a/cmd/unbounded-net-node/mtu_test.go +++ b/cmd/unbounded-net-node/mtu_test.go @@ -24,6 +24,73 @@ type fakeCNIBridgeLinkManager struct { cache *unboundednetnetlink.NetlinkCache } +func TestGatewayLocalForwardingMTUCaps(t *testing.T) { + cfg := &config{MTU: 1420, TunnelIPFamily: "IPv4"} + pools := map[string]int{"local": 1200, "remote": 1400, "unrelated": 900} + + localMTU := localGatewayPoolTunnelMTU([]string{"local"}, pools) + if localMTU != 1200 { + t.Fatalf("local pool MTU=%d, want 1200", localMTU) + } + + mesh := []meshPeerInfo{{SiteName: "same"}} + gateways := []gatewayPeerInfo{{PoolName: "remote", SiteName: "other"}} + resolvePeerTunnelMTUs(cfg, mesh, gateways, "same", true, localMTU, nil, nil, + map[string]int{"same": 1420}, nil, nil, nil, pools, nil) + + if mesh[0].TunnelMTU != 1200 || gateways[0].TunnelMTU != 1200 { + t.Fatalf("local pool cap missing: mesh=%d gateway=%d", mesh[0].TunnelMTU, gateways[0].TunnelMTU) + } + + if got := resolveCNIConfigMTU(cfg.MTU, resolveTunnelMTU(1420, localMTU), nil, nil, 1500); got != 1200 { + t.Fatalf("idle gateway fabric MTU=%d, want 1200", got) + } + + for _, gateway := range []bool{false, true} { + got := resolveMeshPeerTunnelMTU(1420, meshPeerInfo{SiteName: "same"}, "same", + map[string]int{"same": 1420}, nil, map[string]int{"same": 1100}, gateway) + + want := 1420 + if gateway { + want = 1100 + } + + if got != want { + t.Fatalf("same-Site assignment MTU gateway=%v: got=%d want=%d", gateway, got, want) + } + } +} + +func TestWorkerGatewayFirstHopPeeringMTU(t *testing.T) { + for _, tc := range []struct { + name string + site string + gateway bool + peeringCaps map[string]int + want int + }{ + {name: "worker mesh edge", site: "site-b", peeringCaps: map[string]int{"site-b": 1200}, want: 1200}, + {name: "unrelated peering", site: "site-b", peeringCaps: map[string]int{"site-c": 900}, want: 1250}, + {name: "same Site", site: "site-a", peeringCaps: map[string]int{"site-a": 1200}, want: 1250}, + {name: "pool transit is not mesh", site: "site-b", gateway: true, peeringCaps: map[string]int{"site-b": 1200}, want: 1250}, + } { + t.Run(tc.name, func(t *testing.T) { + cfg := &config{MTU: 1250, TunnelIPFamily: "IPv4"} + peers := []gatewayPeerInfo{{SiteName: tc.site, PoolName: "pool-g"}} + resolvePeerTunnelMTUs(cfg, nil, peers, "site-a", tc.gateway, 0, nil, nil, + nil, tc.peeringCaps, nil, nil, nil, nil) + + if peers[0].TunnelMTU != tc.want { + t.Fatalf("gateway peer MTU=%d, want %d", peers[0].TunnelMTU, tc.want) + } + + if got := resolveCNIConfigMTU(cfg.MTU, 0, nil, peers, 1500); got != tc.want { + t.Fatalf("CNI MTU=%d, want %d", got, tc.want) + } + }) + } +} + func (f *fakeCNIBridgeLinkManager) Exists() bool { return f.exists } diff --git a/cmd/unbounded-net-node/node_types.go b/cmd/unbounded-net-node/node_types.go index 62a60e404..c9d0d7e23 100644 --- a/cmd/unbounded-net-node/node_types.go +++ b/cmd/unbounded-net-node/node_types.go @@ -29,7 +29,9 @@ type wireGuardState struct { siteRouting siteRoutingConfig manageCniPlugin bool // Whether this node's site manages CNI plugin and full podCIDR mesh routes cniMTU int // Last MTU written to the managed CNI configuration + appliedFabricMTU int // Last attempted fabric MTU, including writes before a failed reconciliation. cniConfigWritten bool // Whether this process has written the managed CNI configuration + dataplaneDirty bool // A failed programming attempt must not take the unchanged-state fast path. mu sync.Mutex // Kubernetes client for tainting nodes @@ -194,6 +196,7 @@ type gatewayPeerInfo struct { SkipPodCIDRRoutes bool // Keep tunnel reachability host routes but skip explicit PodCIDR routes TunnelProtocol string // Resolved tunnel protocol for this peer (WireGuard or GENEVE) TunnelMTU int // Effective MTU after route, encapsulation, and CRD limits + TransitTunnelMTU int // MTU imposed by an actual downstream GatewayPoolPeering path } // meshPeerInfo contains information about a peer on the main mesh interface. diff --git a/cmd/unbounded-net-node/reconciliation_helpers.go b/cmd/unbounded-net-node/reconciliation_helpers.go index 091872623..789747161 100644 --- a/cmd/unbounded-net-node/reconciliation_helpers.go +++ b/cmd/unbounded-net-node/reconciliation_helpers.go @@ -5,6 +5,7 @@ package main import ( "encoding/json" + "errors" "fmt" "strconv" "strings" @@ -260,152 +261,342 @@ func mergeLowestTunnelMTU(tunnelMTUs map[string]int, scope string, mtu int) { } } -func mergeGatewayPoolPeeringTunnelMTU(tunnelMTUs map[string]int, peering unboundednetv1alpha1.GatewayPoolPeering) { - mtu := tunnelMTUFromSpec(peering.Spec.TunnelMTU) - for _, poolName := range peering.Spec.GatewayPools { - mergeLowestTunnelMTU(tunnelMTUs, poolName, mtu) +func gatewayPoolPeeringEdgeTunnelMTUs(peerings []unboundednetv1alpha1.GatewayPoolPeering) *peeringMTUIndex { + result := newPeeringMTUIndex() + + for _, peering := range peerings { + if !unboundednetv1alpha1.SpecEnabled(peering.Spec.Enabled) { + continue + } + + result.add(peering.Spec.GatewayPools, tunnelMTUFromSpec(peering.Spec.TunnelMTU)) } + + return result } -func resolveSiteTopologyTunnelMTU( - mySiteName string, - siteTunnelMTUs, poolTunnelMTUs map[string]int, - sitePeerings []unboundednetv1alpha1.SitePeering, - assignments []unboundednetv1alpha1.SiteGatewayPoolAssignment, - poolPeerings []unboundednetv1alpha1.GatewayPoolPeering, - localGatewayPools []string, -) int { - const ( - sitePrefix = "site/" - poolPrefix = "pool/" - ) +func gatewayPoolPeeringTunnelMTU(localPools []string, remotePool string, edgeMTUs *peeringMTUIndex) int { + result := 0 + for _, pool := range localPools { + result = resolveTunnelMTU(result, edgeMTUs.between(pool, remotePool)) + } - adjacency := make(map[string]map[string]bool) - scopeTunnelMTUs := make(map[string]int) + return result +} + +type sitePoolMTUKey struct { + site string + pool string +} - addScope := func(scope string, mtu int) { - if scope == "" { - return +func assignmentEdgeTunnelMTUs(assignments []unboundednetv1alpha1.SiteGatewayPoolAssignment) map[sitePoolMTUKey]int { + result := make(map[sitePoolMTUKey]int) + + for _, assignment := range assignments { + if !unboundednetv1alpha1.SpecEnabled(assignment.Spec.Enabled) { + continue } - if adjacency[scope] == nil { - adjacency[scope] = make(map[string]bool) + mtu := tunnelMTUFromSpec(assignment.Spec.TunnelMTU) + if mtu == 0 { + continue } - mergeLowestTunnelMTU(scopeTunnelMTUs, scope, mtu) + for _, site := range assignment.Spec.Sites { + for _, pool := range assignment.Spec.GatewayPools { + key := sitePoolMTUKey{site: site, pool: pool} + result[key] = resolveTunnelMTU(result[key], mtu) + } + } } - connectScopes := func(scopes []string, mtu int) { - unique := make([]string, 0, len(scopes)) - seen := make(map[string]bool, len(scopes)) + return result +} + +func learnedGatewayTransitTunnelMTU( + learnedRoutes map[string]unboundednetv1alpha1.GatewayNodeRoute, + siteMTUs, poolMTUs map[string]int, + assignmentMTUs map[sitePoolMTUKey]int, + poolPeeringMTUs *peeringMTUIndex, +) int { + result := 0 - for _, scope := range scopes { - scope = strings.TrimSpace(scope) - if scope == "" || seen[scope] { - continue + for _, route := range learnedRoutes { + for _, path := range route.Paths { + for index, hop := range path { + switch hop.Type { + case "Site": + result = resolveTunnelMTU(result, siteMTUs[hop.Name]) + case "GatewayPool": + result = resolveTunnelMTU(result, poolMTUs[hop.Name]) + } + + if index == 0 { + continue + } + + // Paths run from origin to advertiser. Only adjacent hops + // identify a forwarding edge; co-occurrence is not enough. + previous := path[index-1] + if previous.Type == "GatewayPool" && hop.Type == "GatewayPool" { + result = resolveTunnelMTU(result, poolPeeringMTUs.between(previous.Name, hop.Name)) + + continue + } + + site, pool := previous, hop + if site.Type == "GatewayPool" && pool.Type == "Site" { + site, pool = pool, site + } + + if site.Type != "Site" || pool.Type != "GatewayPool" { + continue + } + + result = resolveTunnelMTU(result, assignmentMTUs[sitePoolMTUKey{site: site.Name, pool: pool.Name}]) } - - seen[scope] = true - unique = append(unique, scope) - addScope(scope, mtu) } + } - if len(unique) < 2 { - return + return result +} + +type peeringMTUPair struct { + a string + b string +} + +type peeringMTUScope struct { + members map[string]bool + mtu int +} + +type peeringMTUIndex struct { + byMember map[string][]*peeringMTUScope + pairs map[peeringMTUPair]int +} + +func newPeeringMTUIndex() *peeringMTUIndex { + return &peeringMTUIndex{ + byMember: make(map[string][]*peeringMTUScope), + pairs: make(map[peeringMTUPair]int), + } +} + +func (index *peeringMTUIndex) add(members []string, mtu int) { + if mtu <= 0 { + return + } + + scope := &peeringMTUScope{members: make(map[string]bool, len(members)), mtu: mtu} + for _, member := range members { + if member = strings.TrimSpace(member); member != "" { + scope.members[member] = true } + } - first := unique[0] - for _, scope := range unique[1:] { - adjacency[first][scope] = true - adjacency[scope][first] = true + for member := range scope.members { + index.byMember[member] = append(index.byMember[member], scope) + } +} + +func sitePeeringEdgeTunnelMTUs(peerings []unboundednetv1alpha1.SitePeering) *peeringMTUIndex { + result := newPeeringMTUIndex() + + for _, peering := range peerings { + if !unboundednetv1alpha1.SpecEnabled(peering.Spec.Enabled) { + continue } + + result.add(peering.Spec.Sites, tunnelMTUFromSpec(peering.Spec.TunnelMTU)) } - for siteName, mtu := range siteTunnelMTUs { - addScope(sitePrefix+siteName, mtu) + return result +} + +func (index *peeringMTUIndex) between(a, b string) int { + a, b = strings.TrimSpace(a), strings.TrimSpace(b) + if index == nil || a == b { + return 0 } - for poolName, mtu := range poolTunnelMTUs { - addScope(poolPrefix+poolName, mtu) + if b < a { + a, b = b, a } - for _, peering := range sitePeerings { - scopes := make([]string, 0, len(peering.Spec.Sites)) - for _, siteName := range peering.Spec.Sites { - if siteName = strings.TrimSpace(siteName); siteName != "" { - scopes = append(scopes, sitePrefix+siteName) - } - } + key := peeringMTUPair{a: a, b: b} + if mtu, exists := index.pairs[key]; exists { + return mtu + } + + scopes := index.byMember[a] - connectScopes(scopes, tunnelMTUFromSpec(peering.Spec.TunnelMTU)) + other := b + if len(index.byMember[b]) < len(scopes) { + scopes, other = index.byMember[b], a } - for _, assignment := range assignments { - scopes := make([]string, 0, len(assignment.Spec.Sites)+len(assignment.Spec.GatewayPools)) - for _, siteName := range assignment.Spec.Sites { - if siteName = strings.TrimSpace(siteName); siteName != "" { - scopes = append(scopes, sitePrefix+siteName) - } + result := 0 + + for _, scope := range scopes { + if scope.members[other] { + result = resolveTunnelMTU(result, scope.mtu) } + } - for _, poolName := range assignment.Spec.GatewayPools { - if poolName = strings.TrimSpace(poolName); poolName != "" { - scopes = append(scopes, poolPrefix+poolName) - } + index.pairs[key] = result + + return result +} + +func applyPublishedGatewayMTUs(peers []gatewayPeerInfo, pools map[string]*unboundednetv1alpha1.GatewayPool, nodes map[string]*unboundednetv1alpha1.GatewayPoolNode) { + nodeMTU := func(name string) int { + if node := nodes[name]; node != nil && node.Status.ForwardingMTU != nil { + return int(*node.Status.ForwardingMTU) } - connectScopes(scopes, tunnelMTUFromSpec(assignment.Spec.TunnelMTU)) + return 0 } - for _, peering := range poolPeerings { - scopes := make([]string, 0, len(peering.Spec.GatewayPools)) - for _, poolName := range peering.Spec.GatewayPools { - if poolName = strings.TrimSpace(poolName); poolName != "" { - scopes = append(scopes, poolPrefix+poolName) + poolMTUs := make(map[string]int, len(pools)) + for name, pool := range pools { + for _, node := range pool.Status.Nodes { + poolMTUs[name] = resolveTunnelMTU(poolMTUs[name], nodeMTU(node.Name)) + } + } + + for i := range peers { + mtu := nodeMTU(peers[i].Name) + for _, route := range peers[i].LearnedRoutes { + for _, path := range route.Paths { + for _, hop := range path { + if hop.Type == "GatewayPool" && hop.Name != peers[i].PoolName { + mtu = resolveTunnelMTU(mtu, poolMTUs[hop.Name]) + } + } } } - connectScopes(scopes, tunnelMTUFromSpec(peering.Spec.TunnelMTU)) + peers[i].TransitTunnelMTU = resolveTunnelMTU(peers[i].TransitTunnelMTU, mtu) } +} - mySiteName = strings.TrimSpace(mySiteName) - start := sitePrefix + mySiteName - - queue := make([]string, 0, 1+len(localGatewayPools)) - if mySiteName != "" && adjacency[start] != nil { - queue = append(queue, start) +func directMeshComponentTunnelMTU(siteName string, sites map[string]*unboundedv1alpha3.Site, peerings []unboundednetv1alpha1.SitePeering) int { + if sites[siteName] == nil { + return 0 } - for _, poolName := range localGatewayPools { - poolName = strings.TrimSpace(poolName) + edges := make(map[string][]int) + + for index, peering := range peerings { + if !unboundednetv1alpha1.SpecEnabled(peering.Spec.Enabled) || (peering.Spec.MeshNodes != nil && !*peering.Spec.MeshNodes) { + continue + } - scope := poolPrefix + poolName - if poolName != "" && adjacency[scope] != nil { - queue = append(queue, scope) + members := make(map[string]bool) + + for _, name := range peering.Spec.Sites { + if sites[name] != nil { + members[name] = true + } } - } - if len(queue) == 0 { - return 0 + if len(members) < 2 { + continue + } + + for name := range members { + edges[name] = append(edges[name], index) + } } + visitedSites := map[string]bool{siteName: true} + visitedPeerings := make(map[int]bool) + queue := []string{siteName} result := 0 - visited := make(map[string]bool) + // Shared pod and tunnel interfaces couple MTU constraints across a mesh + // component even when routing itself is not transitive. for len(queue) > 0 { - scope := queue[0] + current := queue[0] queue = queue[1:] + result = resolveTunnelMTU(result, tunnelMTUFromSpec(sites[current].Spec.TunnelMTU)) - if visited[scope] { - continue + for _, index := range edges[current] { + if visitedPeerings[index] { + continue + } + + visitedPeerings[index] = true + peering := peerings[index] + result = resolveTunnelMTU(result, tunnelMTUFromSpec(peering.Spec.TunnelMTU)) + + for _, name := range peering.Spec.Sites { + if sites[name] == nil || visitedSites[name] { + continue + } + + visitedSites[name] = true + queue = append(queue, name) + } } + } + + return result +} - visited[scope] = true - result = resolveTunnelMTU(result, scopeTunnelMTUs[scope]) +func gatewayMeshTransitTunnelMTU( + peerPool, peerSite string, + routes map[string]unboundednetv1alpha1.GatewayNodeRoute, + pools map[string]*unboundednetv1alpha1.GatewayPool, + siteMTUs map[string]int, + peeringMTUs *peeringMTUIndex, +) int { + result := 0 + + for _, route := range routes { + for _, path := range route.Paths { + for _, hop := range path { + if hop.Type != "GatewayPool" { + continue + } + + if hop.Name == peerPool { + result = resolveTunnelMTU(result, siteMTUs[peerSite]) + } else if pool := pools[hop.Name]; pool != nil { + for _, node := range pool.Status.Nodes { + result = resolveTunnelMTU(result, siteMTUs[node.SiteName]) + } + } + } - for adjacent := range adjacency[scope] { - if !visited[adjacent] { - queue = append(queue, adjacent) + for i := 1; i < len(path); i++ { + site, pool := path[i-1], path[i] + if site.Type != "Site" || pool.Type != "GatewayPool" { + continue + } + + gatewaySites := []string{peerSite} + if pool.Name != peerPool { + // Intermediate pool paths identify a pool, not a specific + // gateway. Honor every candidate mesh edge in that pool. + gatewaySites = nil + + if downstream := pools[pool.Name]; downstream != nil { + for _, node := range downstream.Status.Nodes { + gatewaySites = append(gatewaySites, node.SiteName) + } + } + } + + for _, gatewaySite := range gatewaySites { + result = resolveTunnelMTU(result, siteMTUs[gatewaySite]) + if gatewaySite == site.Name || gatewaySite == "" { + continue + } + + result = resolveTunnelMTU(result, peeringMTUs.between(gatewaySite, site.Name)) + } } } } @@ -437,13 +628,15 @@ func resolveTunnelMTU(globalMTU int, mtuValues ...int) int { func resolveMeshPeerTunnelMTU(globalMTU int, peer meshPeerInfo, mySiteName string, siteTunnelMTUs, peeringSiteTunnelMTUs map[string]int, assignmentSiteTunnelMTUs map[string]int, + includeAssignmentMTU bool, ) int { - values := []int{siteTunnelMTUs[mySiteName]} + values := []int{siteTunnelMTUs[mySiteName], siteTunnelMTUs[peer.SiteName]} if peer.SiteName != mySiteName { - values = append(values, - peeringSiteTunnelMTUs[peer.SiteName], - assignmentSiteTunnelMTUs[peer.SiteName], - ) + values = append(values, peeringSiteTunnelMTUs[peer.SiteName]) + } + + if includeAssignmentMTU { + values = append(values, assignmentSiteTunnelMTUs[peer.SiteName]) } return resolveTunnelMTU(globalMTU, values...) @@ -458,8 +651,10 @@ func resolveGatewayPeerTunnelMTU(globalMTU int, mySiteName string, peer gatewayP ) int { return resolveTunnelMTU(globalMTU, siteTunnelMTUs[mySiteName], + siteTunnelMTUs[peer.SiteName], assignmentPoolTunnelMTUs[peer.PoolName], poolTunnelMTUs[peer.PoolName], + peer.TransitTunnelMTU, ) } @@ -754,7 +949,12 @@ func strSliceEqual(a, b []string) bool { return true } -func removeUnmanagedWireGuardInterfaces(cfg *config, state *wireGuardState, desiredGatewayIfaces map[string]bool, meshIface string) { +var ( + listWireGuardLinks = netlink.LinkList + deleteWireGuardLink = netlink.LinkDel +) + +func removeUnmanagedWireGuardInterfaces(cfg *config, state *wireGuardState, desiredGatewayIfaces map[string]bool, meshIface string) error { desired := make(map[string]bool, len(desiredGatewayIfaces)+1) desired[meshIface] = true @@ -762,12 +962,13 @@ func removeUnmanagedWireGuardInterfaces(cfg *config, state *wireGuardState, desi desired[name] = true } - links, err := netlink.LinkList() + links, err := listWireGuardLinks() if err != nil { - klog.Warningf("Failed to list links for WireGuard ownership reconciliation: %v", err) - return + return fmt.Errorf("list links for WireGuard ownership reconciliation: %w", err) } + var cleanupErr error + for _, link := range links { name := link.Attrs().Name @@ -785,12 +986,13 @@ func removeUnmanagedWireGuardInterfaces(cfg *config, state *wireGuardState, desi if cfg.EnablePolicyRouting && state.gatewayPolicyManager != nil { if err := state.gatewayPolicyManager.RemoveInterface(name); err != nil { - klog.V(3).Infof("Policy cleanup skipped for unmanaged interface %s: %v", name, err) + cleanupErr = errors.Join(cleanupErr, fmt.Errorf("remove policy for %s: %w", name, err)) + continue } } - if err := netlink.LinkDel(link); err != nil { - klog.Warningf("Failed to delete unmanaged WireGuard interface %s: %v", name, err) + if err := deleteWireGuardLink(link); err != nil { + cleanupErr = errors.Join(cleanupErr, fmt.Errorf("delete unmanaged WireGuard interface %s: %w", name, err)) continue } @@ -814,6 +1016,8 @@ func removeUnmanagedWireGuardInterfaces(cfg *config, state *wireGuardState, desi delete(state.gatewayNames, name) delete(state.gatewaySiteNames, name) } + + return cleanupErr } // removeStaleWireGuardInterfaces deletes kernel-resident WireGuard diff --git a/cmd/unbounded-net-node/reconciliation_helpers_test.go b/cmd/unbounded-net-node/reconciliation_helpers_test.go index 617a69d84..a26a0e5fc 100644 --- a/cmd/unbounded-net-node/reconciliation_helpers_test.go +++ b/cmd/unbounded-net-node/reconciliation_helpers_test.go @@ -5,6 +5,7 @@ package main import ( "encoding/json" + "fmt" "testing" "time" @@ -250,100 +251,388 @@ func TestMergeLowestTunnelMTU(t *testing.T) { } } -func TestMergeGatewayPoolPeeringTunnelMTU(t *testing.T) { - mtu := int32(1280) - peering := unboundednetv1alpha1.GatewayPoolPeering{ - Spec: unboundednetv1alpha1.GatewayPoolPeeringSpec{ - GatewayPools: []string{"pool-a", "pool-b"}, - TunnelMTU: &mtu, - }, +func TestGatewayMeshTransitCaps(t *testing.T) { + routes := map[string]unboundednetv1alpha1.GatewayNodeRoute{ + "10.0.0.0/24": {Paths: [][]unboundednetv1alpha1.GatewayNodePathHop{{ + {Type: "Site", Name: "site-b"}, {Type: "GatewayPool", Name: "pool-g"}, + }}}, } - tunnelMTUs := map[string]int{"pool-a": 1400, "pool-b": 1200} - mergeGatewayPoolPeeringTunnelMTU(tunnelMTUs, peering) + peerings := []unboundednetv1alpha1.SitePeering{ + {Spec: unboundednetv1alpha1.SitePeeringSpec{Sites: []string{"site-b", "site-c"}, TunnelMTU: ptrInt32(1200)}}, + {Spec: unboundednetv1alpha1.SitePeeringSpec{Sites: []string{"site-x", "site-c"}, TunnelMTU: ptrInt32(900)}}, + } + if got := gatewayMeshTransitTunnelMTU("pool-g", "site-c", routes, nil, nil, sitePeeringEdgeTunnelMTUs(peerings)); got != 1200 { + t.Fatalf("gateway-to-origin mesh edge cap=%d, want 1200", got) + } - if got := tunnelMTUs["pool-a"]; got != 1280 { - t.Fatalf("pool-a MTU = %d, want 1280", got) + if got := gatewayMeshTransitTunnelMTU("pool-g", "site-b", routes, nil, nil, sitePeeringEdgeTunnelMTUs(peerings)); got != 0 { + t.Fatalf("same-Site edge constrained by unrelated peering: %d", got) + } + + pools := map[string]*unboundednetv1alpha1.GatewayPool{ + "pool-g": {Status: unboundednetv1alpha1.GatewayPoolStatus{Nodes: []unboundednetv1alpha1.GatewayNodeInfo{{SiteName: "site-c"}}}}, + } + if got := gatewayMeshTransitTunnelMTU("pool-upstream", "site-a", routes, pools, nil, sitePeeringEdgeTunnelMTUs(peerings)); got != 1200 { + t.Fatalf("intermediate pool mesh edge cap=%d, want 1200", got) + } + + if got := resolveGatewayPeerTunnelMTU(1420, "site-a", gatewayPeerInfo{SiteName: "site-c"}, + map[string]int{"site-a": 1420, "site-c": 1200}, nil, nil); got != 1200 { + t.Fatalf("next-hop gateway Site cap=%d, want 1200", got) + } + + if got := gatewayMeshTransitTunnelMTU("pool-g", "site-c", nil, pools, nil, sitePeeringEdgeTunnelMTUs(peerings)); got != 0 { + t.Fatalf("absent transit routes constrained MTU: %d", got) + } +} + +func TestIntermediateGatewaySiteMTUCaps(t *testing.T) { + routes := map[string]unboundednetv1alpha1.GatewayNodeRoute{ + "10.0.0.0/24": {Paths: [][]unboundednetv1alpha1.GatewayNodePathHop{{ + {Type: "Site", Name: "site-b"}, + {Type: "GatewayPool", Name: "pool-h"}, + {Type: "GatewayPool", Name: "pool-j"}, + {Type: "GatewayPool", Name: "pool-g"}, + }}}, + } + pools := map[string]*unboundednetv1alpha1.GatewayPool{ + "pool-h": {Status: unboundednetv1alpha1.GatewayPoolStatus{Nodes: []unboundednetv1alpha1.GatewayNodeInfo{{SiteName: "site-b"}}}}, + "pool-j": {Status: unboundednetv1alpha1.GatewayPoolStatus{Nodes: []unboundednetv1alpha1.GatewayNodeInfo{{SiteName: "site-d"}, {SiteName: "site-e"}}}}, + "pool-g": {Status: unboundednetv1alpha1.GatewayPoolStatus{Nodes: []unboundednetv1alpha1.GatewayNodeInfo{{SiteName: "unused-advertiser"}}}}, + "unrelated": {Status: unboundednetv1alpha1.GatewayPoolStatus{Nodes: []unboundednetv1alpha1.GatewayNodeInfo{{SiteName: "unrelated"}}}}, + } + + caps := map[string]int{"site-b": 1420, "site-d": 1200, "site-e": 1300, "site-c": 1420, "unrelated": 900, "unused-advertiser": 800} + if got := gatewayMeshTransitTunnelMTU("pool-g", "site-c", routes, pools, caps, nil); got != 1200 { + t.Fatalf("three-pool candidate Site cap=%d, want 1200", got) + } + + if got := gatewayMeshTransitTunnelMTU("pool-g", "site-c", nil, pools, caps, nil); got != 0 { + t.Fatalf("unused pools constrained MTU without routes: %d", got) + } +} + +func TestDirectMeshComponentTunnelMTU(t *testing.T) { + sites := map[string]*unboundedv1alpha3.Site{ + "a": {}, "b": {}, "c": {}, "d": {}, "x": {}, "y": {}, + } + + peerings := []unboundednetv1alpha1.SitePeering{ + {Spec: unboundednetv1alpha1.SitePeeringSpec{Sites: []string{"a", "b"}, TunnelMTU: ptrInt32(1200)}}, + {Spec: unboundednetv1alpha1.SitePeeringSpec{Sites: []string{"a", "c"}, TunnelMTU: ptrInt32(1400)}}, + {Spec: unboundednetv1alpha1.SitePeeringSpec{Sites: []string{"c", "d"}, TunnelMTU: ptrInt32(1500)}}, + {Spec: unboundednetv1alpha1.SitePeeringSpec{Sites: []string{"x", "y"}, TunnelMTU: ptrInt32(900)}}, + {Spec: unboundednetv1alpha1.SitePeeringSpec{Sites: []string{"c", "x"}, TunnelMTU: ptrInt32(800), MeshNodes: new(false)}}, + {Spec: unboundednetv1alpha1.SitePeeringSpec{Sites: []string{"c", "x"}, TunnelMTU: ptrInt32(700), Enabled: new(false)}}, + {Spec: unboundednetv1alpha1.SitePeeringSpec{Sites: []string{"c", "missing"}, TunnelMTU: ptrInt32(600)}}, + {Spec: unboundednetv1alpha1.SitePeeringSpec{Sites: []string{"missing", "x"}}}, + {Spec: unboundednetv1alpha1.SitePeeringSpec{Sites: []string{"c", "c"}, TunnelMTU: ptrInt32(576)}}, + } + for _, name := range []string{"a", "b", "c", "d"} { + if got := directMeshComponentTunnelMTU(name, sites, peerings); got != 1200 { + t.Fatalf("Site %s cap=%d, want shared component minimum 1200", name, got) + } + } + + if got := directMeshComponentTunnelMTU("x", sites, peerings); got != 900 { + t.Fatalf("disconnected component cap=%d, want 900", got) + } + + if got := directMeshComponentTunnelMTU("missing", sites, peerings); got != 0 { + t.Fatalf("missing Site cap=%d, want no cap", got) + } + + sites["b"].Spec.TunnelMTU = ptrInt32(1100) + if got := directMeshComponentTunnelMTU("d", sites, peerings); got != 1100 { + t.Fatalf("remote Site cap was not included: %d", got) + } + + sites["b"].Spec.TunnelMTU = nil + + peerings[0].Spec.Enabled = new(false) + if got := directMeshComponentTunnelMTU("c", sites, peerings); got != 1400 { + t.Fatalf("disabled low edge pinned the component: %d", got) } - if got := tunnelMTUs["pool-b"]; got != 1200 { - t.Fatalf("pool-b MTU = %d, want existing lower value 1200", got) + peerings[0].Spec.Enabled = nil + + peerings[0].Spec.MeshNodes = new(false) + if got := directMeshComponentTunnelMTU("c", sites, peerings); got != 1400 { + t.Fatalf("network-only edge pinned the component: %d", got) + } + + peerings[0].Spec.MeshNodes = nil + + peerings[0].Spec.TunnelMTU = ptrInt32(1450) + if got := directMeshComponentTunnelMTU("c", sites, peerings); got != 1400 { + t.Fatalf("raised low edge pinned the component: %d", got) } } -func TestResolveSiteTopologyTunnelMTU(t *testing.T) { +func TestPublishedGatewayForwardingMTUs(t *testing.T) { + mesh := []meshPeerInfo{ + {SiteName: "b", TunnelProtocol: "WireGuard", TunnelMTU: 1200}, + {SiteName: "d", TunnelProtocol: "WireGuard", TunnelMTU: 1400}, + } + + sharedMTU := protocolInterfaceMTU(1420, 0, 1500, "WireGuard", mesh, nil) + if sharedMTU != 1200 { + t.Fatalf("shared mesh MTU=%d, want 1200", sharedMTU) + } + + nodes := map[string]*unboundednetv1alpha1.GatewayPoolNode{ + "g": {Status: unboundednetv1alpha1.GatewayNodeStatus{ForwardingMTU: ptrInt32(int32(sharedMTU))}}, + "h": {Status: unboundednetv1alpha1.GatewayNodeStatus{ForwardingMTU: ptrInt32(1100)}}, + "unrelated": {Status: unboundednetv1alpha1.GatewayNodeStatus{ForwardingMTU: ptrInt32(900)}}, + } + + pools := map[string]*unboundednetv1alpha1.GatewayPool{ + "pool-g": {Status: unboundednetv1alpha1.GatewayPoolStatus{Nodes: []unboundednetv1alpha1.GatewayNodeInfo{{Name: "g"}, {Name: "unrelated"}}}}, + "pool-h": {Status: unboundednetv1alpha1.GatewayPoolStatus{Nodes: []unboundednetv1alpha1.GatewayNodeInfo{{Name: "h"}}}}, + } + for _, intermediate := range []bool{false, true} { + path := []unboundednetv1alpha1.GatewayNodePathHop{{Type: "Site", Name: "d"}} + if intermediate { + path = append(path, unboundednetv1alpha1.GatewayNodePathHop{Type: "GatewayPool", Name: "pool-h"}) + } + + path = append(path, unboundednetv1alpha1.GatewayNodePathHop{Type: "GatewayPool", Name: "pool-g"}) + peers := []gatewayPeerInfo{{Name: "g", PoolName: "pool-g", LearnedRoutes: map[string]unboundednetv1alpha1.GatewayNodeRoute{ + "10.0.0.0/24": {Paths: [][]unboundednetv1alpha1.GatewayNodePathHop{path}}, + }}} + applyPublishedGatewayMTUs(peers, pools, nodes) + + want := 1200 + if intermediate { + want = 1100 + } + + if peers[0].TransitTunnelMTU != want { + t.Fatalf("intermediate=%v: cap=%d, want %d", intermediate, peers[0].TransitTunnelMTU, want) + } + } + + nodes["g"].Status.ForwardingMTU = ptrInt32(1400) + peers := []gatewayPeerInfo{{Name: "g", PoolName: "pool-g"}} + applyPublishedGatewayMTUs(peers, pools, nodes) + + if peers[0].TransitTunnelMTU != 1400 { + t.Fatalf("raised local cap remained pinned: %d", peers[0].TransitTunnelMTU) + } + + nodes["g"].Status.ForwardingMTU = nil + peers = []gatewayPeerInfo{{Name: "g", PoolName: "pool-g", TransitTunnelMTU: 1300}} + applyPublishedGatewayMTUs(peers, pools, nodes) + + if peers[0].TransitTunnelMTU != 1300 { + t.Fatal("legacy gateway without published MTU discarded topology cap") + } +} + +func TestSitePeeringEdgeTunnelMTUs(t *testing.T) { + peerings := []unboundednetv1alpha1.SitePeering{ + {Spec: unboundednetv1alpha1.SitePeeringSpec{Sites: []string{"c", "b", "a"}, TunnelMTU: ptrInt32(1400)}}, + {Spec: unboundednetv1alpha1.SitePeeringSpec{Sites: []string{"b", "a"}, TunnelMTU: ptrInt32(1200)}}, + {Spec: unboundednetv1alpha1.SitePeeringSpec{Sites: []string{"b", "a"}, TunnelMTU: ptrInt32(900), Enabled: new(false)}}, + {Spec: unboundednetv1alpha1.SitePeeringSpec{Sites: []string{"unused", "a"}}}, + } + + got := sitePeeringEdgeTunnelMTUs(peerings) + if len(got.byMember) != 3 || got.between("a", "b") != 1200 || + got.between("b", "c") != 1400 || got.between("a", "c") != 1400 { + t.Fatalf("unexpected enabled peering minima: %v", got) + } + + if got.between("a", "unused") != 0 || got.between("a", "a") != 0 { + t.Fatal("uncapped or same-Site pair acquired a cap") + } +} + +func TestSitePeeringMTUIndexUsesLinearMemberships(t *testing.T) { + sites := make([]string, 1000) + for i := range sites { + sites[i] = fmt.Sprintf("site-%d", i) + } + + for kind, index := range map[string]*peeringMTUIndex{ + "Site": sitePeeringEdgeTunnelMTUs([]unboundednetv1alpha1.SitePeering{{ + Spec: unboundednetv1alpha1.SitePeeringSpec{Sites: sites, TunnelMTU: ptrInt32(1200)}, + }}), + "GatewayPool": gatewayPoolPeeringEdgeTunnelMTUs([]unboundednetv1alpha1.GatewayPoolPeering{{ + Spec: unboundednetv1alpha1.GatewayPoolPeeringSpec{GatewayPools: sites, TunnelMTU: ptrInt32(1200)}, + }}), + } { + t.Run(kind, func(t *testing.T) { + memberships := 0 + for _, scopes := range index.byMember { + memberships += len(scopes) + } + + if len(index.byMember) != 1000 || memberships != 1000 || len(index.pairs) != 0 { + t.Fatalf("index eagerly expanded pairs: members=%d memberships=%d cachedPairs=%d", len(index.byMember), memberships, len(index.pairs)) + } + + for range 1000 { + if index.between(sites[0], sites[999]) != 1200 || index.between(sites[999], sites[0]) != 1200 { + t.Fatal("requested pair lost its MTU cap") + } + } + + if len(index.pairs) != 1 { + t.Fatalf("repeated pair did not reuse cache: %d entries", len(index.pairs)) + } + }) + } +} + +func TestGatewayPoolPeeringEdgeTunnelMTUs(t *testing.T) { + peerings := []unboundednetv1alpha1.GatewayPoolPeering{ + {Spec: unboundednetv1alpha1.GatewayPoolPeeringSpec{GatewayPools: []string{"c", " b ", "a"}, TunnelMTU: ptrInt32(1400)}}, + {Spec: unboundednetv1alpha1.GatewayPoolPeeringSpec{GatewayPools: []string{"b", "a"}, TunnelMTU: ptrInt32(1200)}}, + {Spec: unboundednetv1alpha1.GatewayPoolPeeringSpec{GatewayPools: []string{"b", "a"}, TunnelMTU: ptrInt32(900), Enabled: new(false)}}, + {Spec: unboundednetv1alpha1.GatewayPoolPeeringSpec{GatewayPools: []string{"unused", "a"}}}, + } + + got := gatewayPoolPeeringEdgeTunnelMTUs(peerings) + if len(got.byMember) != 3 || got.between("b", "a") != 1200 || + got.between("b", "c") != 1400 || got.between("a", "c") != 1400 { + t.Fatalf("unexpected enabled peering minima: %v", got) + } + + if mtu := gatewayPoolPeeringTunnelMTU([]string{" a ", "unrelated"}, "b", got); mtu != 1200 { + t.Fatalf("local-pool indexed minimum=%d, want 1200", mtu) + } +} + +func TestAssignmentEdgeTunnelMTUs(t *testing.T) { + assignments := []unboundednetv1alpha1.SiteGatewayPoolAssignment{ + {Spec: unboundednetv1alpha1.SiteGatewayPoolAssignmentSpec{ + Sites: []string{"a", "b"}, GatewayPools: []string{"g", "h"}, TunnelMTU: ptrInt32(1400), + }}, + {Spec: unboundednetv1alpha1.SiteGatewayPoolAssignmentSpec{ + Sites: []string{"b"}, GatewayPools: []string{"g"}, TunnelMTU: ptrInt32(1200), + }}, + {Spec: unboundednetv1alpha1.SiteGatewayPoolAssignmentSpec{ + Sites: []string{"a"}, GatewayPools: []string{"g"}, TunnelMTU: ptrInt32(900), Enabled: new(false), + }}, + {Spec: unboundednetv1alpha1.SiteGatewayPoolAssignmentSpec{ + Sites: []string{"unused"}, GatewayPools: []string{"g"}, + }}, + } + + got := assignmentEdgeTunnelMTUs(assignments) + if len(got) != 4 || got[sitePoolMTUKey{"a", "g"}] != 1400 || + got[sitePoolMTUKey{"b", "g"}] != 1200 || got[sitePoolMTUKey{"b", "h"}] != 1400 { + t.Fatalf("unexpected enabled edge minima: %v", got) + } +} + +func TestLearnedGatewayTransitPathCaps(t *testing.T) { + path := []unboundednetv1alpha1.GatewayNodePathHop{ + {Type: "Site", Name: "site-b"}, + {Type: "GatewayPool", Name: "pool-h"}, + {Type: "GatewayPool", Name: "pool-j"}, + {Type: "GatewayPool", Name: "pool-g"}, + } + routes := map[string]unboundednetv1alpha1.GatewayNodeRoute{ + "10.0.0.0/24": {Paths: [][]unboundednetv1alpha1.GatewayNodePathHop{path}}, + } + + for _, tc := range []struct { + name string + sites map[string]int + pools map[string]int + assignments []unboundednetv1alpha1.SiteGatewayPoolAssignment + peerings []unboundednetv1alpha1.GatewayPoolPeering + want int + }{ + {name: "origin Site", sites: map[string]int{"site-b": 1200, "unrelated": 900}, want: 1200}, + {name: "downstream pool", pools: map[string]int{"pool-h": 1250, "unrelated": 900}, want: 1250}, + {name: "assignment edge", assignments: []unboundednetv1alpha1.SiteGatewayPoolAssignment{ + {Spec: unboundednetv1alpha1.SiteGatewayPoolAssignmentSpec{Sites: []string{"site-b"}, GatewayPools: []string{"pool-h"}, TunnelMTU: ptrInt32(1100)}}, + }, want: 1100}, + {name: "assignment nonadjacent", assignments: []unboundednetv1alpha1.SiteGatewayPoolAssignment{ + {Spec: unboundednetv1alpha1.SiteGatewayPoolAssignmentSpec{Sites: []string{"site-b"}, GatewayPools: []string{"pool-g"}, TunnelMTU: ptrInt32(1100)}}, + }}, + {name: "adjacent peering", peerings: []unboundednetv1alpha1.GatewayPoolPeering{ + {Spec: unboundednetv1alpha1.GatewayPoolPeeringSpec{GatewayPools: []string{"pool-h", "pool-j"}, TunnelMTU: ptrInt32(1300)}}, + }, want: 1300}, + {name: "nonadjacent peering", peerings: []unboundednetv1alpha1.GatewayPoolPeering{ + {Spec: unboundednetv1alpha1.GatewayPoolPeeringSpec{GatewayPools: []string{"pool-h", "pool-g"}, TunnelMTU: ptrInt32(1000)}}, + }}, + } { + t.Run(tc.name, func(t *testing.T) { + got := learnedGatewayTransitTunnelMTU(routes, tc.sites, tc.pools, assignmentEdgeTunnelMTUs(tc.assignments), gatewayPoolPeeringEdgeTunnelMTUs(tc.peerings)) + if got != tc.want { + t.Fatalf("transit MTU=%d want=%d", got, tc.want) + } + }) + } +} + +func TestForwardingAwareTunnelMTU(t *testing.T) { siteMTUs := map[string]int{ - "local": 1450, - "remote": 1250, - "disconnected": 576, + "site-a": 1450, + "site-b": 1300, } poolMTUs := map[string]int{ - "local-pool": 1400, - "remote-pool": 1350, + "pool-g": 1100, + "pool-h": 1400, } - sitePeerings := []unboundednetv1alpha1.SitePeering{{ - Spec: unboundednetv1alpha1.SitePeeringSpec{ - Sites: []string{"local", "remote"}, - TunnelMTU: ptrInt32(1300), - }, - }} - assignments := []unboundednetv1alpha1.SiteGatewayPoolAssignment{ - { - Spec: unboundednetv1alpha1.SiteGatewayPoolAssignmentSpec{ - Sites: []string{"local"}, - GatewayPools: []string{"local-pool"}, - TunnelMTU: ptrInt32(1380), - }, - }, - { - Spec: unboundednetv1alpha1.SiteGatewayPoolAssignmentSpec{ - Sites: []string{"remote"}, - GatewayPools: []string{"remote-pool"}, - }, - }, + meshPeer := meshPeerInfo{SiteName: "site-b"} + if got := resolveMeshPeerTunnelMTU(0, meshPeer, "site-a", siteMTUs, map[string]int{"site-b": 1350}, nil, false); got != 1300 { + t.Fatalf("A-B mesh MTU = %d, want remote Site minimum 1300", got) + } + + // Site A meshes Site B, and only Site B is assigned to pool G. Mesh peers + // advertise B's PodCIDRs, not transit through B to G, so G must not lower A. + if got := resolveCNIConfigMTU(0, siteMTUs["site-a"], []meshPeerInfo{{TunnelMTU: 1300}}, nil, 1500); got != 1300 { + t.Fatalf("A-B-G unsupported transit MTU = %d, want 1300", got) + } + + gatewayPeer := gatewayPeerInfo{PoolName: "pool-g"} + if got := resolveGatewayPeerTunnelMTU(0, "site-a", gatewayPeer, siteMTUs, map[string]int{"pool-g": 1250}, poolMTUs); got != 1100 { + t.Fatalf("direct gateway MTU = %d, want pool minimum 1100", got) } - poolPeerings := []unboundednetv1alpha1.GatewayPoolPeering{{ + + peering := unboundednetv1alpha1.GatewayPoolPeering{ Spec: unboundednetv1alpha1.GatewayPoolPeeringSpec{ - GatewayPools: []string{"local-pool", "remote-pool"}, - TunnelMTU: ptrInt32(1280), + GatewayPools: []string{"pool-g", "pool-h"}, + TunnelMTU: ptrInt32(1200), }, - }} + } - if got := resolveSiteTopologyTunnelMTU( - "local", - siteMTUs, - poolMTUs, - sitePeerings, - assignments, - poolPeerings, - nil, - ); got != 1250 { - t.Fatalf("local topology MTU = %d, want remote site minimum 1250", got) + poolPeeringMTUs := gatewayPoolPeeringEdgeTunnelMTUs([]unboundednetv1alpha1.GatewayPoolPeering{peering}) + + transitMTU := gatewayPoolPeeringTunnelMTU([]string{"pool-g"}, "pool-h", poolPeeringMTUs) + if got := resolveGatewayPeerTunnelMTU(0, "site-a", gatewayPeerInfo{PoolName: "pool-h", TransitTunnelMTU: transitMTU}, siteMTUs, nil, poolMTUs); got != 1200 { + t.Fatalf("gateway multi-pool routed path MTU = %d, want peering minimum 1200", got) } - if got := resolveSiteTopologyTunnelMTU( - "disconnected", - siteMTUs, - poolMTUs, - sitePeerings, - assignments, - poolPeerings, - nil, - ); got != 576 { - t.Fatalf("disconnected topology MTU = %d, want 576", got) + learnedRoutes := map[string]unboundednetv1alpha1.GatewayNodeRoute{ + "10.0.0.0/24": { + Paths: [][]unboundednetv1alpha1.GatewayNodePathHop{{ + {Type: "GatewayPool", Name: "pool-h"}, + {Type: "GatewayPool", Name: "pool-g"}, + }}, + }, + } + if got := learnedGatewayTransitTunnelMTU(learnedRoutes, nil, nil, nil, poolPeeringMTUs); got != 1200 { + t.Fatalf("worker learned gateway transit MTU = %d, want 1200", got) } - if got := resolveSiteTopologyTunnelMTU( - "local", - map[string]int{"local": 1450}, - map[string]int{"gateway-only": 1200}, - nil, - nil, - nil, - []string{"gateway-only"}, - ); got != 1200 { - t.Fatalf("gateway pool membership MTU = %d, want 1200", got) + if got := learnedGatewayTransitTunnelMTU(nil, nil, nil, nil, poolPeeringMTUs); got != 0 { + t.Fatalf("gateway peering without a learned forwarding path constrained worker MTU: %d", got) + } + + if got := resolveMeshPeerTunnelMTU(1420, meshPeerInfo{SiteName: "missing"}, "site-a", siteMTUs, nil, nil, false); got != 1420 { + t.Fatalf("missing remote resources MTU = %d, want configured default 1420", got) + } + + if got := resolveTunnelMTU(0, -1, 0, 1280); got != 1280 { + t.Fatalf("invalid non-positive MTUs changed result: %d", got) } } @@ -380,21 +669,36 @@ func TestResolveMeshPeerTunnelMTU(t *testing.T) { map[string]int{"site-a": 1300}, map[string]int{"site-b": 1200}, map[string]int{"site-b": 1100}, + false, ) if got != 1300 { t.Fatalf("expected 1300 (site MTU), got %d", got) } - // Cross-site peer: min of site, peering, assignment + // An ordinary worker's direct mesh path does not transit the gateway + // assignment, so the assignment MTU must not constrain that peer. peer = meshPeerInfo{SiteName: "site-b"} got = resolveMeshPeerTunnelMTU(1420, peer, "site-a", - map[string]int{"site-a": 1350}, - map[string]int{"site-b": 1200}, - map[string]int{"site-b": 1250}, + map[string]int{"site-a": 1350, "site-b": 1300}, + map[string]int{"site-b": 1280}, + map[string]int{"site-b": 1100}, + false, ) - if got != 1200 { - t.Fatalf("expected 1200 (peering MTU wins), got %d", got) + if got != 1280 { + t.Fatalf("expected 1280 (peering MTU wins without gateway assignment), got %d", got) + } + + // A gateway's mesh link to an assigned Site is part of the assignment + // forwarding path, so the assignment MTU applies there. + got = resolveMeshPeerTunnelMTU(1420, peer, "site-a", + map[string]int{"site-a": 1350, "site-b": 1300}, + map[string]int{"site-b": 1280}, + map[string]int{"site-b": 1100}, + true, + ) + if got != 1100 { + t.Fatalf("expected 1100 (gateway assignment MTU wins), got %d", got) } // No CRD overrides -- global wins @@ -404,6 +708,7 @@ func TestResolveMeshPeerTunnelMTU(t *testing.T) { map[string]int{}, map[string]int{}, map[string]int{}, + false, ) if got != 1420 { t.Fatalf("expected 1420 (global MTU), got %d", got) diff --git a/cmd/unbounded-net-node/site_routing_reconcile_test.go b/cmd/unbounded-net-node/site_routing_reconcile_test.go index 12a4b17c4..4758a631e 100644 --- a/cmd/unbounded-net-node/site_routing_reconcile_test.go +++ b/cmd/unbounded-net-node/site_routing_reconcile_test.go @@ -6,6 +6,7 @@ package main import ( "context" "errors" + "fmt" "slices" "testing" @@ -19,9 +20,106 @@ import ( unboundednetnetlink "github.com/Azure/unbounded/internal/net/netlink" ) +func TestUpdateWireGuardFromSlices_DirectMeshComponentMTU(t *testing.T) { + sites, slices := newInformerWithObjects(), newInformerWithObjects() + + for i, name := range []string{"a", "b", "c"} { + site := &unboundedv1alpha3.Site{ + ObjectMeta: metav1.ObjectMeta{Name: name}, + Spec: unboundedv1alpha3.SiteSpec{TunnelProtocol: new(unboundednetv1alpha1.TunnelProtocolNone)}, + } + if err := sites.GetStore().Add(toUnstructured(t, site)); err != nil { + t.Fatal(err) + } + + slice := &unboundednetv1alpha1.SiteNodeSlice{ + ObjectMeta: metav1.ObjectMeta{Name: name}, SiteName: name, + Nodes: []unboundednetv1alpha1.NodeInfo{{ + Name: name, WireGuardPublicKey: "key-" + name, PodCIDRs: []string{fmt.Sprintf("10.244.%d.0/24", i)}, + InternalIPs: []string{fmt.Sprintf("192.0.2.%d", i+1)}, + }}, + } + if err := slices.GetStore().Add(toUnstructured(t, slice)); err != nil { + t.Fatal(err) + } + } + + peerings := newInformerWithObjects( + toUnstructured(t, &unboundednetv1alpha1.SitePeering{ + ObjectMeta: metav1.ObjectMeta{Name: "a-b"}, + Spec: unboundednetv1alpha1.SitePeeringSpec{Sites: []string{"a", "b"}, TunnelMTU: ptrInt32(1200)}, + }), + toUnstructured(t, &unboundednetv1alpha1.SitePeering{ + ObjectMeta: metav1.ObjectMeta{Name: "a-c"}, + Spec: unboundednetv1alpha1.SitePeeringSpec{Sites: []string{"a", "c"}, TunnelMTU: ptrInt32(1400)}, + }), + ) + original := configureWireGuardFunc + + t.Cleanup(func() { configureWireGuardFunc = original }) + + configureWireGuardFunc = func(context.Context, *config, string, []meshPeerInfo, []gatewayPeerInfo, string, map[string]bool, map[string]bool, map[string]bool, map[string]string, map[string]string, map[string]string, map[string]string, map[string]string, map[string]int, map[string]int, map[string]int, map[string]int, map[string]int, []unboundednetnetlink.DesiredRoute, map[string]bool, *wireGuardState, map[string]healthcheck.HealthCheckSettings) error { + return nil + } + empty := newInformerWithObjects() + + for _, name := range []string{"a", "b", "c"} { + t.Run(name, func(t *testing.T) { + cfg := &config{NodeName: name, MTU: 1420, WireGuardPort: 51820} + + state := &wireGuardState{nodeName: name, clientset: fake.NewClientset(&corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: name}})} + if err := updateWireGuardFromSlices(t.Context(), nil, sites, slices, empty, empty, peerings, empty, empty, + cfg, name, "private", "key-"+name, true, state); err != nil { + t.Fatal(err) + } + + if state.cniMTU != 1200 { + t.Fatalf("Site %s fabric MTU=%d, want common 1200", name, state.cniMTU) + } + + wantPeers := 1 + if name == "a" { + wantPeers = 2 + } + + if len(state.peers) != wantPeers { + t.Fatalf("Site %s gained transitive peers: %+v", name, state.peers) + } + + for _, peer := range state.peers { + if peer.TunnelMTU != 1200 || (name != "a" && peer.SiteName != "a") { + t.Fatalf("Site %s peer is not a capped direct neighbor: %+v", name, peer) + } + } + }) + } +} + func TestUpdateWireGuardFromSlices_LocalGatewayExclusions(t *testing.T) { site := &unboundedv1alpha3.Site{ObjectMeta: metav1.ObjectMeta{Name: "local"}} + siteInformer := newInformerWithObjects(toUnstructured(t, site)) + if err := siteInformer.GetStore().Add(toUnstructured(t, &unboundedv1alpha3.Site{ + ObjectMeta: metav1.ObjectMeta{Name: "origin"}, + Spec: unboundedv1alpha3.SiteSpec{TunnelMTU: ptrInt32(1200)}, + })); err != nil { + t.Fatal(err) + } + + gatewayNodes := newInformerWithObjects(toUnstructured(t, &unboundednetv1alpha1.GatewayPoolNode{ + ObjectMeta: metav1.ObjectMeta{Name: "gateway"}, + Status: unboundednetv1alpha1.GatewayNodeStatus{ + LastUpdated: metav1.Now(), + Routes: map[string]unboundednetv1alpha1.GatewayNodeRoute{ + "192.168.0.0/16": { + Type: "RoutedCidr", + Paths: [][]unboundednetv1alpha1.GatewayNodePathHop{{ + {Type: "Site", Name: "origin"}, {Type: "GatewayPool", Name: "pool"}, + }}, + }, + }, + }, + })) poolInformer := newInformerWithObjects(toUnstructured(t, &unboundednetv1alpha1.GatewayPool{ ObjectMeta: metav1.ObjectMeta{Name: "pool"}, Spec: unboundednetv1alpha1.GatewayPoolSpec{ @@ -40,7 +138,7 @@ func TestUpdateWireGuardFromSlices_LocalGatewayExclusions(t *testing.T) { }, })) empty := newInformerWithObjects() - cfg := &config{NodeName: "self", WireGuardPort: 51820, PreferredPrivateEncap: "WireGuard"} + cfg := &config{NodeName: "self", WireGuardPort: 51820, MTU: 576, PreferredPrivateEncap: "WireGuard"} state := &wireGuardState{ nodeName: "self", clientset: fake.NewClientset(&corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: "self"}}), } @@ -60,7 +158,7 @@ func TestUpdateWireGuardFromSlices_LocalGatewayExclusions(t *testing.T) { t.Cleanup(func() { configureWireGuardFunc = original }) update := func() error { - return updateWireGuardFromSlices(context.Background(), nil, siteInformer, empty, poolInformer, empty, + return updateWireGuardFromSlices(context.Background(), nil, siteInformer, empty, poolInformer, gatewayNodes, empty, assignments, empty, cfg, "local", "priv", "pub-self", true, state) } @@ -92,6 +190,15 @@ func TestUpdateWireGuardFromSlices_LocalGatewayExclusions(t *testing.T) { t.Fatalf("gateway routes=%+v, want %v", gotGatewayPeers, tt.want) } + wantTransitMTU := 1200 + if len(tt.want) == 0 { + wantTransitMTU = 0 + } + + if gotGatewayPeers[0].TransitTunnelMTU != wantTransitMTU { + t.Fatalf("retained path MTU=%d, want %d", gotGatewayPeers[0].TransitTunnelMTU, wantTransitMTU) + } + if gotGatewayPeers[0].WireGuardPublicKey != "pub-gateway" || !slices.Equal(gotGatewayPeers[0].PodCIDRs, []string{"10.245.0.0/24"}) { t.Fatal("local exclusions changed gateway identity") } @@ -105,6 +212,35 @@ func TestUpdateWireGuardFromSlices_LocalGatewayExclusions(t *testing.T) { } }) } + + gatewayObject, exists, err := gatewayNodes.GetStore().GetByKey("gateway") + if err != nil || !exists { + t.Fatalf("get gateway: exists=%v err=%v", exists, err) + } + + if err := gatewayNodes.GetStore().Delete(gatewayObject); err != nil { + t.Fatal(err) + } + + if err := update(); err != nil { + t.Fatal(err) + } + + if len(gotGatewayPeers) != 0 || len(state.gatewayPeers) != 0 { + t.Fatal("missing GatewayPoolNode retained a route destination") + } + + if err := gatewayNodes.GetStore().Add(gatewayObject); err != nil { + t.Fatal(err) + } + + if err := update(); err != nil { + t.Fatal(err) + } + + if len(gotGatewayPeers) != 1 { + t.Fatal("restored GatewayPoolNode did not restore route destination") + } } func TestUpdateWireGuardFromSlices_GatewayRoutingChanges(t *testing.T) { diff --git a/cmd/unbounded-net-node/site_watch_reconcile.go b/cmd/unbounded-net-node/site_watch_reconcile.go index 13536c63b..21473db95 100644 --- a/cmd/unbounded-net-node/site_watch_reconcile.go +++ b/cmd/unbounded-net-node/site_watch_reconcile.go @@ -36,12 +36,12 @@ import ( // watchSitesAndConfigureWireGuard watches SiteNodeSlice objects and configures WireGuard peers for nodes in the same site // Gateway nodes can operate without being assigned to a site - they will still configure themselves to accept connections -// The informers are passed in from run() and are already started and synced. -func watchSiteAndConfigureWireGuard(ctx context.Context, clientset kubernetes.Interface, dynamicClient dynamic.Interface, cfg *config, myPubKey string, nodePodCIDRs []string, manageCniPlugin bool, initialCNIConfigMTU int, healthState *nodeHealthState, netlinkCache *unboundednetnetlink.NetlinkCache, adoptedAssignment nodeAssignment, podNamespace, podName string, podUID types.UID, startupWatch *startupAssignmentWatch, siteInformer, sliceInformer, gatewayPoolInformer, gatewayNodeInformer, sitePeeringInformer, assignmentInformer, poolPeeringInformer cache.SharedIndexInformer) error { +// The discovery informers are passed in from run() and are already started and synced. +func watchSiteAndConfigureWireGuard(ctx context.Context, clientset kubernetes.Interface, dynamicClient dynamic.Interface, cfg *config, myPubKey string, nodePodCIDRs []string, manageCniPlugin bool, initialCNIConfigMTU int, healthState *nodeHealthState, netlinkCache *unboundednetnetlink.NetlinkCache, adoptedAssignment nodeAssignment, podNamespace, podName string, podUID types.UID, startupWatch *startupAssignmentWatch, siteInformer, sliceInformer, sitePeeringInformer, assignmentInformer cache.SharedIndexInformer, gatewayInformers *gatewayInformerLifecycle) error { ctx, stopWorkers := context.WithCancel(ctx) defer stopWorkers() - klog.Info("Watching Site, SiteNodeSlice, GatewayPool, GatewayPoolNode, SitePeering, SiteGatewayPoolAssignment, and GatewayPoolPeering objects for changes") + klog.Info("Watching Site, SiteNodeSlice, SitePeering, and SiteGatewayPoolAssignment objects for topology discovery") // Read private key for WireGuard configuration privKeyPath := filepath.Join(cfg.WireGuardDir, "server.priv") @@ -221,7 +221,9 @@ func watchSiteAndConfigureWireGuard(ctx context.Context, clientset kubernetes.In } } - defer cleanupForAssignmentTransition(cfg, state, dynamicClient, gatewayNodeInformer) + defer func() { + cleanupForAssignmentTransition(cfg, state, dynamicClient, gatewayInformers.snapshot().nodes) + }() // Networking writes do not observe cancellation, so reconciliation and // heartbeat workers must finish before transition cleanup runs, including @@ -242,8 +244,8 @@ func watchSiteAndConfigureWireGuard(ctx context.Context, clientset kubernetes.In clientset: clientset, siteInformer: siteInformer, sliceInformer: sliceInformer, - gatewayPoolInformer: gatewayPoolInformer, sitePeeringInformer: sitePeeringInformer, + gatewayInformers: gatewayInformers, } healthState.setStatusServer(statusSrv) klog.Info("Status server registered with health server") @@ -257,6 +259,8 @@ func watchSiteAndConfigureWireGuard(ctx context.Context, clientset kubernetes.In fatalCh := make(chan error, 1) acknowledged := false + var requestUpdate func() + reconcileUpdate := func() { // Don't reconcile until all informer caches are synced. Event handlers // fire during the initial cache population, but the data from other @@ -264,11 +268,8 @@ func watchSiteAndConfigureWireGuard(ctx context.Context, clientset kubernetes.In for _, synced := range []cache.InformerSynced{ sliceInformer.HasSynced, siteInformer.HasSynced, - gatewayPoolInformer.HasSynced, - gatewayNodeInformer.HasSynced, sitePeeringInformer.HasSynced, assignmentInformer.HasSynced, - poolPeeringInformer.HasSynced, } { if !synced() { klog.V(4).Info("Skipping reconcile: informer caches not fully synced") @@ -279,6 +280,40 @@ func watchSiteAndConfigureWireGuard(ctx context.Context, clientset kubernetes.In start := time.Now() mySiteName := adoptedAssignment.site + var ( + gatewaySnapshot gatewayInformerSnapshot + retireGatewayInformers bool + gatewayStateStable bool + err error + ) + + for range 3 { + gatewayRequired := gatewayCachesRequired(adoptedAssignment, mySiteName, assignmentInformer) + + gatewaySnapshot, retireGatewayInformers, err = gatewayInformers.prepare(ctx, gatewayRequired) + if err != nil { + if ctx.Err() == nil { + klog.Errorf("Failed to prepare required gateway informer caches: %v", err) + nodeReconciliationTotal.WithLabelValues("error").Inc() + } + + return + } + + if gatewayRequired == gatewayCachesRequired(adoptedAssignment, mySiteName, assignmentInformer) { + gatewayStateStable = true + + break + } + } + + if !gatewayStateStable { + klog.Warning("Gateway dependency changed repeatedly while preparing informer caches; retrying reconciliation") + requestUpdate() + + return + } + // Store the site name for status endpoint state.mu.Lock() state.siteName = mySiteName @@ -288,7 +323,7 @@ func watchSiteAndConfigureWireGuard(ctx context.Context, clientset kubernetes.In // Running this in reconcile ties checks to informer updates and resync events. refreshNodeConfigurationProblems(siteInformer, mySiteName, state, cfg.InformerResyncPeriod) - if err := updateWireGuardFromSlices(ctx, dynamicClient, siteInformer, sliceInformer, gatewayPoolInformer, gatewayNodeInformer, sitePeeringInformer, assignmentInformer, poolPeeringInformer, cfg, mySiteName, privKey, myPubKey, manageCniPlugin, state); err != nil { + if err := updateWireGuardFromSlices(ctx, dynamicClient, siteInformer, sliceInformer, gatewaySnapshot.pools, gatewaySnapshot.nodes, sitePeeringInformer, assignmentInformer, gatewaySnapshot.poolPeerings, cfg, mySiteName, privKey, myPubKey, manageCniPlugin, state); err != nil { var guardErr *cniGuardError if errors.As(err, &guardErr) { klog.V(4).Infof("Tunnel reconciliation waiting for CNI safety: %v", err) @@ -300,6 +335,11 @@ func watchSiteAndConfigureWireGuard(ctx context.Context, clientset kubernetes.In } else { nodeReconciliationTotal.WithLabelValues("success").Inc() + if retireGatewayInformers { + gatewayInformers.retire(gatewaySnapshot.generation) + klog.Info("Stopped gateway-specific informer caches after dependent routes and peers were removed") + } + if !acknowledged { if err := acknowledgeEffectiveAssignment(ctx, clientset, podNamespace, podName, podUID, cfg.NodeName, adoptedAssignment); err != nil { if errors.Is(err, errAssignmentChanged) { @@ -330,7 +370,7 @@ func watchSiteAndConfigureWireGuard(ctx context.Context, clientset kubernetes.In var updateMu sync.Mutex updatePending := false - requestUpdate := func() { + requestUpdate = func() { updateMu.Lock() updatePending = true updateMu.Unlock() @@ -340,6 +380,7 @@ func watchSiteAndConfigureWireGuard(ctx context.Context, clientset kubernetes.In default: } } + gatewayInformers.setEventHandler(requestUpdate) workers.Go(func() { for { @@ -421,42 +462,6 @@ func watchSiteAndConfigureWireGuard(ctx context.Context, clientset kubernetes.In } }) - // Set up event handlers for GatewayPools (to detect gateway node and routedCidrs changes) - poolReg, err := gatewayPoolInformer.AddEventHandler(cache.ResourceEventHandlerFuncs{ - AddFunc: func(obj interface{}) { requestUpdate() }, - UpdateFunc: func(oldObj, newObj interface{}) { requestUpdate() }, - DeleteFunc: func(obj interface{}) { requestUpdate() }, - }) - if err != nil { - return fmt.Errorf("failed to add gateway pool event handler: %w", err) - } - - handlerCleanups = append(handlerCleanups, func() { - if err := gatewayPoolInformer.RemoveEventHandler(poolReg); err != nil { - klog.V(4).Infof("Failed to remove gateway pool event handler: %v", err) - } - }) - - // Set up event handlers for GatewayNodes (future route advertisement source). - gwNodeReg, err := gatewayNodeInformer.AddEventHandler(cache.ResourceEventHandlerFuncs{ - AddFunc: func(obj interface{}) { requestUpdate() }, - UpdateFunc: func(oldObj, newObj interface{}) { - if shouldReconcileGatewayNodeUpdate(oldObj, newObj) { - requestUpdate() - } - }, - DeleteFunc: func(obj interface{}) { requestUpdate() }, - }) - if err != nil { - return fmt.Errorf("failed to add gateway node event handler: %w", err) - } - - handlerCleanups = append(handlerCleanups, func() { - if err := gatewayNodeInformer.RemoveEventHandler(gwNodeReg); err != nil { - klog.V(4).Infof("Failed to remove gateway node event handler: %v", err) - } - }) - // Set up event handlers for SitePeerings (to detect direct site peering changes) peeringReg, err := sitePeeringInformer.AddEventHandler(cache.ResourceEventHandlerFuncs{ AddFunc: func(obj interface{}) { requestUpdate() }, @@ -473,11 +478,13 @@ func watchSiteAndConfigureWireGuard(ctx context.Context, clientset kubernetes.In } }) - assignmentReg, err := assignmentInformer.AddEventHandler(cache.ResourceEventHandlerFuncs{ - AddFunc: func(obj interface{}) { requestUpdate() }, - UpdateFunc: func(oldObj, newObj interface{}) { requestUpdate() }, - DeleteFunc: func(obj interface{}) { requestUpdate() }, - }) + assignmentReg, err := assignmentInformer.AddEventHandler(gatewayAssignmentEventHandler( + adoptedAssignment.site, + func() { + gatewayInformers.setRequired(gatewayCachesRequired(adoptedAssignment, adoptedAssignment.site, assignmentInformer)) + }, + requestUpdate, + )) if err != nil { return fmt.Errorf("failed to add site gateway pool assignment event handler: %w", err) } @@ -488,21 +495,6 @@ func watchSiteAndConfigureWireGuard(ctx context.Context, clientset kubernetes.In } }) - poolPeeringReg, err := poolPeeringInformer.AddEventHandler(cache.ResourceEventHandlerFuncs{ - AddFunc: func(obj interface{}) { requestUpdate() }, - UpdateFunc: func(oldObj, newObj interface{}) { requestUpdate() }, - DeleteFunc: func(obj interface{}) { requestUpdate() }, - }) - if err != nil { - return fmt.Errorf("failed to add gateway pool peering event handler: %w", err) - } - - handlerCleanups = append(handlerCleanups, func() { - if err := poolPeeringInformer.RemoveEventHandler(poolPeeringReg); err != nil { - klog.V(4).Infof("Failed to remove pool peering event handler: %v", err) - } - }) - // Initial configuration requestUpdate() @@ -836,7 +828,8 @@ func shouldReconcileGatewayNodeUpdate(oldObj, newObj interface{}) bool { return true } - if !reflect.DeepEqual(oldGatewayNode.Spec, newGatewayNode.Spec) { + if !reflect.DeepEqual(oldGatewayNode.Spec, newGatewayNode.Spec) || + !reflect.DeepEqual(oldGatewayNode.Status.ForwardingMTU, newGatewayNode.Status.ForwardingMTU) { return true } @@ -1064,6 +1057,15 @@ func getManageCniPluginFromCRDs(siteInformer cache.SharedIndexInformer, siteName var configureWireGuardFunc = configureWireGuard +func emptyDynamicInformer() cache.SharedIndexInformer { + return cache.NewSharedIndexInformer( + &cache.ListWatch{}, + &unstructured.Unstructured{}, + 0, + cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc}, + ) +} + // updateWireGuardFromSlices reads Site and SiteNodeSlices from the informer caches and configures WireGuard // It uses per-node podCIDRs from slices and gateway pools for routing // It also looks up gateway pools referenced by the site and adds gateway peers with all remote sites' CIDRs @@ -1071,7 +1073,27 @@ var configureWireGuardFunc = configureWireGuard // When manageCniPlugin is false on non-gateway nodes, same-site peers are skipped but // directly peered remote-site peers are still included for podCIDR routing. // Sites that are directly peered via SitePeering objects are treated as if they were the same site -func updateWireGuardFromSlices(ctx context.Context, dynamicClient dynamic.Interface, siteInformer, sliceInformer, gatewayPoolInformer, gatewayNodeInformer, sitePeeringInformer, assignmentInformer, poolPeeringInformer cache.SharedIndexInformer, cfg *config, mySiteName, privKey, myPubKey string, manageCniPlugin bool, state *wireGuardState) error { +func updateWireGuardFromSlices(ctx context.Context, dynamicClient dynamic.Interface, siteInformer, sliceInformer, gatewayPoolInformer, gatewayNodeInformer, sitePeeringInformer, assignmentInformer, poolPeeringInformer cache.SharedIndexInformer, cfg *config, mySiteName, privKey, myPubKey string, manageCniPlugin bool, state *wireGuardState) (reconcileErr error) { + defer func() { + if reconcileErr != nil { + state.mu.Lock() + state.dataplaneDirty = true + state.mu.Unlock() + } + }() + + if gatewayPoolInformer == nil { + gatewayPoolInformer = emptyDynamicInformer() + } + + if gatewayNodeInformer == nil { + gatewayNodeInformer = emptyDynamicInformer() + } + + if poolPeeringInformer == nil { + poolPeeringInformer = emptyDynamicInformer() + } + // Parse all sites var allSites []unboundedv1alpha3.Site @@ -1338,17 +1360,9 @@ func updateWireGuardFromSlices(ctx context.Context, dynamicClient dynamic.Interf assignmentSiteHealthCheckSourceAssignment, ) - // Collect assignment-level tunnelMTU overrides per site and per pool. - if v := tunnelMTUFromSpec(assignment.Spec.TunnelMTU); v > 0 { - for _, siteName := range assignment.Spec.Sites { - siteName = strings.TrimSpace(siteName) - if siteName == "" || siteName == mySiteName { - continue - } - - mergeLowestTunnelMTU(assignmentSiteTunnelMTUs, siteName, v) - } - + // Worker-to-pool caps follow this Site's assignments. Gateway-to-Site + // caps are collected separately below using only local pool matches. + if v := tunnelMTUFromSpec(assignment.Spec.TunnelMTU); v > 0 && state.adoptedAssignment.gatewayPool == "" { for _, poolName := range assignment.Spec.GatewayPools { poolName = strings.TrimSpace(poolName) if poolName == "" { @@ -1439,13 +1453,6 @@ func updateWireGuardFromSlices(ctx context.Context, dynamicClient dynamic.Interf } } - // A pool peering is a downstream transit path for every participating - // pool. Propagate its MTU cap to each pool so nodes assigned to that pool - // send packets small enough for the complete path, not only the first hop. - for _, peering := range poolPeerings { - mergeGatewayPoolPeeringTunnelMTU(poolTunnelMTUs, peering) - } - gatewayNodeMap := make(map[string]*unboundednetv1alpha1.GatewayPoolNode) for _, item := range gatewayNodeInformer.GetStore().List() { @@ -1660,7 +1667,7 @@ func updateWireGuardFromSlices(ctx context.Context, dynamicClient dynamic.Interf if v := tunnelMTUFromSpec(assignment.Spec.TunnelMTU); v > 0 { for _, siteName := range assignment.Spec.Sites { siteName = strings.TrimSpace(siteName) - if siteName == "" || siteName == mySiteName { + if siteName == "" { continue } @@ -1722,17 +1729,6 @@ func updateWireGuardFromSlices(ctx context.Context, dynamicClient dynamic.Interf } } - siteTopologyMTU := resolveSiteTopologyTunnelMTU( - mySiteName, - siteTunnelMTUs, - poolTunnelMTUs, - sitePeerings, - assignments, - poolPeerings, - localGatewayPools, - ) - mergeLowestTunnelMTU(siteTunnelMTUs, mySiteName, siteTopologyMTU) - if klog.V(4).Enabled() { profileNames := make([]string, 0, len(healthCheckProfiles)) for profileName := range healthCheckProfiles { @@ -1827,6 +1823,12 @@ func updateWireGuardFromSlices(ctx context.Context, dynamicClient dynamic.Interf // Non-gateway nodes: connect to pools from peerings involving their site // Gateway nodes: connect only to peered pools (other pools in the same Peering) var gatewayPeers []gatewayPeerInfo + + var ( + assignmentEdgeMTUs map[sitePoolMTUKey]int + peeringEdgeMTUs *peeringMTUIndex + poolPeeringEdgeMTUs *peeringMTUIndex + ) { var poolsToIterate map[string]*unboundednetv1alpha1.GatewayPool if isGatewayNode { @@ -1881,16 +1883,42 @@ func updateWireGuardFromSlices(ctx context.Context, dynamicClient dynamic.Interf } peerGatewayNode := gatewayNodeMap[gwNode.Name] + if peerGatewayNode == nil { + klog.V(3).Infof("Skipping gateway peer %s: GatewayPoolNode is not present", gwNode.Name) + continue + } + // Never install NodeCidr routes from network-peered sites via gateways. excludedNodeCIDRSites := networkPeeredSites routedCidrs, routeDistances, learnedRoutes := routedCIDRsForGatewayPeer(peerGatewayNode, fallbackRoutedCIDRs, mySiteName, localGatewayPools, excludedNodeCIDRSites, now, staleAfter) + filteredRoutes := excludeLocalGatewayRoutes(gatewayPeerInfo{ + RoutedCidrs: routedCidrs, RouteDistances: routeDistances, LearnedRoutes: learnedRoutes, + }, siteRouting.localCIDRs) + routedCidrs, routeDistances, learnedRoutes = filteredRoutes.RoutedCidrs, filteredRoutes.RouteDistances, filteredRoutes.LearnedRoutes + + if len(learnedRoutes) > 0 && peeringEdgeMTUs == nil { + assignmentEdgeMTUs = assignmentEdgeTunnelMTUs(assignments) + peeringEdgeMTUs = sitePeeringEdgeTunnelMTUs(sitePeerings) + } + + if poolPeeringEdgeMTUs == nil && (isGatewayNode || len(learnedRoutes) > 0) { + poolPeeringEdgeMTUs = gatewayPoolPeeringEdgeTunnelMTUs(poolPeerings) + } + + transitTunnelMTU := learnedGatewayTransitTunnelMTU(learnedRoutes, siteTunnelMTUs, poolTunnelMTUs, assignmentEdgeMTUs, poolPeeringEdgeMTUs) + + transitTunnelMTU = resolveTunnelMTU(transitTunnelMTU, + gatewayMeshTransitTunnelMTU(pool.Name, gwNode.SiteName, learnedRoutes, gatewayPoolMap, siteTunnelMTUs, peeringEdgeMTUs)) + if isGatewayNode { + transitTunnelMTU = resolveTunnelMTU(transitTunnelMTU, gatewayPoolPeeringTunnelMTU(localGatewayPools, pool.Name, poolPeeringEdgeMTUs)) + } peeringTunnelProtocol := poolPeeringTunnelProtocols[pool.Name] if samePoolGatewayKeys[gwNode.WireGuardPublicKey] { peeringTunnelProtocol = "" } - gatewayPeers = append(gatewayPeers, excludeLocalGatewayRoutes(gatewayPeerInfo{ + gatewayPeers = append(gatewayPeers, gatewayPeerInfo{ Name: gwNode.Name, SiteName: gwNode.SiteName, PoolName: pool.Name, @@ -1907,11 +1935,14 @@ func updateWireGuardFromSlices(ctx context.Context, dynamicClient dynamic.Interf LearnedRoutes: learnedRoutes, PodCIDRs: gwNode.PodCIDRs, SkipPodCIDRRoutes: !manageCniPlugin && !isGatewayNode && gwNode.SiteName == mySiteName, - }, siteRouting.localCIDRs)) + TransitTunnelMTU: transitTunnelMTU, + }) } } } + publishGatewayStatus := func(int, []meshPeerInfo, []gatewayPeerInfo) error { return nil } + if isGatewayNode && dynamicClient != nil && len(localGatewayPools) > 0 { selectedPoolName := localGatewayPools[0] selectedPool := gatewayPoolMap[selectedPoolName] @@ -1929,8 +1960,16 @@ func updateWireGuardFromSlices(ctx context.Context, dynamicClient dynamic.Interf state.lastGatewayAdvertSummary = summary } - if err := syncGatewayNodeRoutesStatus(ctx, dynamicClient, gatewayNodeInformer, cfg.NodeName, routes); err != nil { - klog.Warningf("Failed to publish GatewayNode routes for node %s: %v", cfg.NodeName, err) + publishGatewayStatus = func(forwardingMTU int, mesh []meshPeerInfo, gateways []gatewayPeerInfo) error { + if err := verifyActiveTunnelMTUsFunc(cfg, forwardingMTU, mesh, gateways); err != nil { + return fmt.Errorf("verify gateway forwarding MTU before publication: %w", err) + } + + if err := syncGatewayNodeRoutesStatus(ctx, dynamicClient, gatewayNodeInformer, cfg.NodeName, routes, forwardingMTU); err != nil { + return fmt.Errorf("publish GatewayNode routes and forwarding MTU for %s: %w", cfg.NodeName, err) + } + + return nil } } else if isGatewayNode { summary := gatewayAdvertSummary{ @@ -2066,17 +2105,37 @@ func updateWireGuardFromSlices(ctx context.Context, dynamicClient dynamic.Interf cfg.PreferredPrivateEncap, cfg.PreferredPublicEncap, siteTunnelProtocols, peeringSiteTunnelProtocols, assignmentPoolTunnelProtocols, poolTunnelProtocols) - resolvePeerTunnelMTUs(cfg, peers, gatewayPeers, mySiteName, peeredSites, networkPeeredSites, + localPoolMTU := localGatewayPoolTunnelMTU(localGatewayPools, poolTunnelMTUs) + meshComponentMTU := directMeshComponentTunnelMTU(mySiteName, siteMap, sitePeerings) + localScopeMTU := resolveTunnelMTU(siteTunnelMTUs[mySiteName], localPoolMTU, meshComponentMTU) + + if !isGatewayNode { + applyPublishedGatewayMTUs(gatewayPeers, gatewayPoolMap, gatewayNodeMap) + } + + resolvePeerTunnelMTUs(cfg, peers, gatewayPeers, mySiteName, isGatewayNode, localScopeMTU, peeredSites, networkPeeredSites, siteTunnelMTUs, peeringSiteTunnelMTUs, assignmentSiteTunnelMTUs, assignmentPoolTunnelMTUs, poolTunnelMTUs, state.netlinkCache) - fabricMTU := resolveCNIConfigMTU(cfg.MTU, siteTunnelMTUs[mySiteName], peers, gatewayPeers, + fabricMTU := resolveCNIConfigMTU(cfg.MTU, localScopeMTU, peers, gatewayPeers, unboundednetnetlink.DetectDefaultRouteMTUFromCache(state.netlinkCache)) - previousFabricMTU := state.cniMTU - fabricMTUChanged := fabricMTU > 0 && fabricMTU != previousFabricMTU + state.mu.Lock() + + previousFabricMTU := state.appliedFabricMTU + if previousFabricMTU == 0 { + previousFabricMTU = state.cniMTU + } + + fabricMTUChanged := fabricMTU > 0 && (fabricMTU != previousFabricMTU || state.dataplaneDirty) + state.mu.Unlock() + fabricMTUIncreased := fabricMTUChanged && previousFabricMTU > 0 && fabricMTU > previousFabricMTU applyFabricMTU := func() error { + state.mu.Lock() + state.appliedFabricMTU = fabricMTU + state.mu.Unlock() + if manageCniPlugin && state.cniConfigWritten { if managedCNIWriteRequired(fabricMTUChanged, state.healthState) { cniCfg := *cfg @@ -2132,7 +2191,7 @@ func updateWireGuardFromSlices(ctx context.Context, dynamicClient dynamic.Interf !strSliceEqual(prevLocalGatewayPools, localGatewayPools) // Check if tunnel configuration needs to change - wgChanged := roleChanged || + wgChanged := state.dataplaneDirty || roleChanged || !reflect.DeepEqual(state.siteRouting, siteRouting) || !meshPeersEqual(state.peers, peers) || !strSliceEqual(state.sitePodCIDRs, sitePodCIDRs) || @@ -2161,9 +2220,11 @@ func updateWireGuardFromSlices(ctx context.Context, dynamicClient dynamic.Interf state.mu.Unlock() syncMasqueradeRules(state, nonMasqCIDRs) - return nil + return publishGatewayStatus(fabricMTU, peers, gatewayPeers) } + state.dataplaneDirty = true + // Log added and removed peers logMeshPeerChanges(state.peers, peers) logGatewayPeerChanges(state.gatewayPeers, gatewayPeers) @@ -2256,7 +2317,7 @@ func updateWireGuardFromSlices(ctx context.Context, dynamicClient dynamic.Interf klog.Warningf("Tunnel configuration failed (WireGuard will still be configured): %v", sharedTunnelErr) } - unboundedMTU := resolveCNIConfigMTU(cfg.MTU, siteTunnelMTUs[mySiteName], peers, gatewayPeers, + unboundedMTU := resolveCNIConfigMTU(cfg.MTU, localScopeMTU, peers, gatewayPeers, unboundednetnetlink.DetectDefaultRouteMTUFromCache(state.netlinkCache)) if unboundedMTU > 0 { unboundedLinkManager := unboundednetnetlink.NewLinkManager(unbounded0DeviceName) @@ -2288,17 +2349,21 @@ func updateWireGuardFromSlices(ctx context.Context, dynamicClient dynamic.Interf return err } - if sharedTunnelErr != nil && (fabricMTUIncreased || errors.Is(sharedTunnelErr, errRegisterHealthChecks)) { + if sharedTunnelErr != nil && (isGatewayNode || fabricMTUIncreased || errors.Is(sharedTunnelErr, errRegisterHealthChecks)) { if errors.Is(sharedTunnelErr, errRegisterHealthChecks) { return sharedTunnelErr } + if isGatewayNode { + return fmt.Errorf("cannot publish gateway status while tunnel reconciliation is incomplete: %w", sharedTunnelErr) + } + return fmt.Errorf("cannot raise fabric MTU while tunnel reconciliation is incomplete: %w", sharedTunnelErr) } // Relax the edge only after every tunnel has accepted the larger MTU. if fabricMTUIncreased { - if err := verifyActiveTunnelMTUs(cfg, fabricMTU, peers, gatewayPeers); err != nil { + if err := verifyActiveTunnelMTUsFunc(cfg, fabricMTU, peers, gatewayPeers); err != nil { return fmt.Errorf("cannot raise fabric MTU: %w", err) } @@ -2356,6 +2421,10 @@ func updateWireGuardFromSlices(ctx context.Context, dynamicClient dynamic.Interf } } + if err := publishGatewayStatus(fabricMTU, peers, gatewayPeers); err != nil { + return err + } + // Phase 3: Update state fields that getNodeStatus() reads (brief lock). state.mu.Lock() state.peers = peers @@ -2379,6 +2448,7 @@ func updateWireGuardFromSlices(ctx context.Context, dynamicClient dynamic.Interf state.assignmentPoolTunnelProtocols = copyStringMap(assignmentPoolTunnelProtocols) state.poolTunnelProtocols = copyStringMap(poolTunnelProtocols) state.cniMTU = fabricMTU + state.dataplaneDirty = false state.reconcileCount++ state.mu.Unlock() diff --git a/cmd/unbounded-net-node/status_server.go b/cmd/unbounded-net-node/status_server.go index 2cc9f6c26..ea89ca32c 100644 --- a/cmd/unbounded-net-node/status_server.go +++ b/cmd/unbounded-net-node/status_server.go @@ -43,8 +43,9 @@ var routeSubscribeWithOptions = netlink.RouteSubscribeWithOptions // nodeHealthState holds the shared state for health and status endpoints type nodeHealthState struct { - statusServer *nodeStatusServer // Set once WireGuard state is available - informersSynced []cache.InformerSynced // Informer HasSynced funcs for readiness + statusServer *nodeStatusServer // Set once WireGuard state is available + informersSynced []cache.InformerSynced // Informer HasSynced funcs for readiness + gatewayInformersReady func() (bool, string) nodeName string siteName string @@ -503,6 +504,18 @@ func newHealthMux(healthState *nodeHealthState) *http.ServeMux { } } + if healthState.gatewayInformersReady != nil { + if ready, reason := healthState.gatewayInformersReady(); !ready { + w.WriteHeader(http.StatusServiceUnavailable) + + if _, err := w.Write([]byte(reason)); err != nil { + klog.V(4).Infof("readyz write failed: %v", err) + } + + return + } + } + if ready, reason := healthState.cniReadiness(); !ready { w.WriteHeader(http.StatusServiceUnavailable) @@ -2517,6 +2530,7 @@ type nodeStatusServer struct { sliceInformer cache.SharedIndexInformer gatewayPoolInformer cache.SharedIndexInformer sitePeeringInformer cache.SharedIndexInformer + gatewayInformers *gatewayInformerLifecycle routingTableRefreshMu sync.Mutex routingTableCacheMu sync.RWMutex @@ -3078,7 +3092,12 @@ func (s *nodeStatusServer) getNodeStatus() *NodeStatusResponse { // Annotate routes with Expected/Present/PeerDestinations/Info if len(status.RoutingTable.Routes) > 0 { - annotateNodeRoutes(status, s.cfg, s.siteInformer, s.sliceInformer, s.gatewayPoolInformer, s.sitePeeringInformer) + gatewayPoolInformer := s.gatewayPoolInformer + if s.gatewayInformers != nil { + gatewayPoolInformer = s.gatewayInformers.snapshot().pools + } + + annotateNodeRoutes(status, s.cfg, s.siteInformer, s.sliceInformer, gatewayPoolInformer, s.sitePeeringInformer) } // Add managed route info from the unified route manager diff --git a/cmd/unbounded-net-node/status_server_http_test.go b/cmd/unbounded-net-node/status_server_http_test.go index 3f2937441..5e9b5df3d 100644 --- a/cmd/unbounded-net-node/status_server_http_test.go +++ b/cmd/unbounded-net-node/status_server_http_test.go @@ -9,6 +9,7 @@ import ( "io" "net" "net/http" + "net/http/httptest" "os" "strings" "testing" @@ -17,6 +18,27 @@ import ( "k8s.io/client-go/tools/cache" ) +func TestReadyzReportsRequiredGatewayInformerState(t *testing.T) { + healthState := &nodeHealthState{ + informersSynced: []cache.InformerSynced{func() bool { return true }}, + gatewayInformersReady: func() (bool, string) { + return false, "required gateway informer caches are starting" + }, + } + + request := httptest.NewRequest(http.MethodGet, "/readyz", nil) + response := httptest.NewRecorder() + newHealthMux(healthState).ServeHTTP(response, request) + + if response.Code != http.StatusServiceUnavailable { + t.Fatalf("readyz status = %d, want %d", response.Code, http.StatusServiceUnavailable) + } + + if got := response.Body.String(); got != "required gateway informer caches are starting" { + t.Fatalf("readyz body = %q", got) + } +} + func reserveTestPort(t *testing.T) int { t.Helper() diff --git a/cmd/unbounded-net-node/status_summary_routes.go b/cmd/unbounded-net-node/status_summary_routes.go index dcf22d69b..066a7c3c8 100644 --- a/cmd/unbounded-net-node/status_summary_routes.go +++ b/cmd/unbounded-net-node/status_summary_routes.go @@ -51,7 +51,12 @@ func (s *nodeStatusServer) collectRouteSummary(peers []routeplan.Peer, localSite } }() - actx := buildAnnotationContext(s.siteInformer, s.sliceInformer, s.gatewayPoolInformer, s.sitePeeringInformer) + gatewayPoolInformer := s.gatewayPoolInformer + if s.gatewayInformers != nil { + gatewayPoolInformer = s.gatewayInformers.snapshot().pools + } + + actx := buildAnnotationContext(s.siteInformer, s.sliceInformer, gatewayPoolInformer, s.sitePeeringInformer) for i := range peers { peers[i].SitePeered = sitesAreDirectlyPeered(strings.TrimSpace(localSite), peers[i].SiteName, actx.directSitePeerings) } diff --git a/cmd/unbounded-net-node/status_summary_routes_test.go b/cmd/unbounded-net-node/status_summary_routes_test.go index b0ec7b7bf..6ea0255d5 100644 --- a/cmd/unbounded-net-node/status_summary_routes_test.go +++ b/cmd/unbounded-net-node/status_summary_routes_test.go @@ -9,7 +9,9 @@ import ( "github.com/vishvananda/netlink" "golang.org/x/sys/unix" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + unboundednetv1alpha1 "github.com/Azure/unbounded/api/net/v1alpha1" "github.com/Azure/unbounded/internal/net/routeplan" netstatus "github.com/Azure/unbounded/internal/net/status" ) @@ -38,6 +40,41 @@ func summaryRouteFixture() *nodeStatusServer { } } +func TestRouteSummaryUsesActiveGatewayGeneration(t *testing.T) { + s := summaryRouteFixture() + s.netlinkOps.(*fakeNetlinkOps).mainRoutes = map[int][]netlink.Route{ + netlink.FAMILY_V4: {summaryRoute("10.9.0.1/32", 1, 0, 0)}, + } + + pools := newTestInformer() + if err := pools.GetStore().Add(toUnstructuredSiteWatch(t, &unboundednetv1alpha1.GatewayPool{ + ObjectMeta: metav1.ObjectMeta{Name: "pool-g"}, + Spec: unboundednetv1alpha1.GatewayPoolSpec{Type: "External"}, + Status: unboundednetv1alpha1.GatewayPoolStatus{Nodes: []unboundednetv1alpha1.GatewayNodeInfo{ + {Name: "gateway", ExternalIPs: []string{"203.0.113.1"}}, + }}, + })); err != nil { + t.Fatal(err) + } + + s.gatewayInformers = newGatewayInformerLifecycle(nil, 0) + s.gatewayInformers.active = &gatewayInformerGeneration{gatewayInformerSnapshot: gatewayInformerSnapshot{ + available: true, pools: pools, + }} + + peers := []routeplan.Peer{{ + Name: "gateway", PeerType: "gateway", Interface: "wg51820", AllowedIPs: []string{"10.9.0.1/32", "203.0.113.1/32"}, + }} + if count, mismatches := s.collectRouteSummary(peers, "local"); count != 1 || mismatches != 0 { + t.Fatalf("active generation summary = (%d,%d), want (1,0)", count, mismatches) + } + + s.gatewayInformers.active = nil + if _, mismatches := s.collectRouteSummary(peers, "local"); mismatches == 0 { + t.Fatal("fixture did not distinguish absent gateway identities") + } +} + func TestRouteSummaryParity(t *testing.T) { peer := WireGuardPeerStatus{ Name: "peer", PeerType: "site", SiteName: "local", diff --git a/cmd/unbounded-net-node/wireguard_config.go b/cmd/unbounded-net-node/wireguard_config.go index 4c3e486b5..c3c0bd866 100644 --- a/cmd/unbounded-net-node/wireguard_config.go +++ b/cmd/unbounded-net-node/wireguard_config.go @@ -5,6 +5,7 @@ package main import ( "context" + "errors" "fmt" "net" "sort" @@ -94,15 +95,20 @@ func configureWireGuard(ctx context.Context, cfg *config, privKey string, peers // removeUnmanagedWireGuardInterfaces handles both cases by listing wg* // links and deleting any that aren't in desiredGatewayIfaces. if len(peers) == 0 && len(gatewayPeers) == 0 { + var cleanupErr error + if state.linkManager.Exists() { klog.Infof("No WireGuard peers -- removing mesh interface %s", iface) if err := state.linkManager.DeleteLink(); err != nil { - klog.V(2).Infof("Failed to remove unused WireGuard interface %s: %v", iface, err) + cleanupErr = errors.Join(cleanupErr, fmt.Errorf("remove unused WireGuard interface %s: %w", iface, err)) } } // Tear down any tracked or kernel-resident gateway wg* interfaces. - removeUnmanagedWireGuardInterfaces(cfg, state, nil, iface) + cleanupErr = errors.Join(cleanupErr, removeUnmanagedWireGuardInterfaces(cfg, state, nil, iface)) + if cleanupErr == nil && cfg.EnablePolicyRouting && state.gatewayPolicyManager != nil { + cleanupErr = errors.Join(cleanupErr, state.gatewayPolicyManager.ReconcileExpectedInterfaces(nil)) + } // Sync routes (clear WG routes, keep any GENEVE/IPIP additional routes) if state.routeManager != nil { if state.netlinkCache != nil { @@ -110,7 +116,7 @@ func configureWireGuard(ctx context.Context, cfg *config, privKey string, peers } if err := state.routeManager.SyncRoutes(additionalRoutes); err != nil { - klog.Errorf("Failed to sync routes: %v", err) + cleanupErr = errors.Join(cleanupErr, fmt.Errorf("sync routes during WireGuard teardown: %w", err)) } if state.netlinkCache != nil { @@ -123,7 +129,7 @@ func configureWireGuard(ctx context.Context, cfg *config, privKey string, peers for peerName := range state.healthCheckManager.GetAllPeerStatuses() { if !geneveHCPeers[peerName] { if err := state.healthCheckManager.RemovePeer(peerName); err != nil { - klog.V(2).Infof("Healthcheck: failed to remove stale peer %s: %v", peerName, err) + cleanupErr = errors.Join(cleanupErr, fmt.Errorf("remove stale healthcheck peer %s: %w", peerName, err)) } else { klog.V(4).Infof("Healthcheck: removed stale peer %s", peerName) } @@ -131,7 +137,7 @@ func configureWireGuard(ctx context.Context, cfg *config, privKey string, peers } } - return nil + return cleanupErr } // Ensure WireGuard interface exists using netlink @@ -490,8 +496,7 @@ func configureWireGuard(ctx context.Context, cfg *config, privKey string, peers // This ensures return traffic leaves via the same interface it arrived on if cfg.EnablePolicyRouting && state.gatewayPolicyManager != nil { if err := state.gatewayPolicyManager.ConfigureInterface(gwIfaceName, gwPort); err != nil { - klog.Warningf("Failed to configure policy routing for %s: %v", gwIfaceName, err) - // Continue - policy routing is not critical + return fmt.Errorf("configure policy routing for %s: %w", gwIfaceName, err) } } @@ -545,30 +550,28 @@ func configureWireGuard(ctx context.Context, cfg *config, privKey string, peers } } - for ifaceName := range staleGatewayIfaces { - delete(state.gatewayLinkManagers, ifaceName) - delete(state.gatewayWireguardManagers, ifaceName) - delete(state.gatewayHealthEndpoints, ifaceName) - delete(state.gatewayRoutes, ifaceName) - delete(state.gatewayRouteDistances, ifaceName) - delete(state.gatewaySiteCIDRs, ifaceName) - delete(state.gatewayPodCIDRs, ifaceName) - } - // Remove old gateway interfaces that are no longer needed for ifaceName, linkManager := range staleGatewayIfaces { klog.Infof("Removing unused gateway interface %s", ifaceName) if cfg.EnablePolicyRouting && state.gatewayPolicyManager != nil { if err := state.gatewayPolicyManager.RemoveInterface(ifaceName); err != nil { - klog.Warningf("Failed to remove policy routing for %s: %v", ifaceName, err) + return fmt.Errorf("remove policy routing for %s: %w", ifaceName, err) } } if err := linkManager.DeleteLink(); err != nil { - klog.Warningf("Failed to delete gateway interface %s: %v", ifaceName, err) + return fmt.Errorf("delete gateway interface %s: %w", ifaceName, err) } + delete(state.gatewayLinkManagers, ifaceName) + delete(state.gatewayWireguardManagers, ifaceName) + delete(state.gatewayHealthEndpoints, ifaceName) + delete(state.gatewayRoutes, ifaceName) + delete(state.gatewayRouteDistances, ifaceName) + delete(state.gatewaySiteCIDRs, ifaceName) + delete(state.gatewayPodCIDRs, ifaceName) + if state.forwardManager != nil { state.forwardManager.RemoveInterface(ifaceName) } @@ -580,13 +583,15 @@ func configureWireGuard(ctx context.Context, cfg *config, privKey string, peers if cfg.EnablePolicyRouting && state.gatewayPolicyManager != nil { if err := state.gatewayPolicyManager.ReconcileExpectedInterfaces(desiredGatewayPolicyTables); err != nil { - klog.Warningf("Failed to reconcile gateway policy routing chains: %v", err) + return fmt.Errorf("reconcile gateway policy routing chains: %w", err) } } // Full ownership reconcile: remove any unmanaged wg* interfaces not present // in the current desired map (mesh interface + desired gateway interfaces). - removeUnmanagedWireGuardInterfaces(cfg, state, desiredGatewayIfaces, iface) + if err := removeUnmanagedWireGuardInterfaces(cfg, state, desiredGatewayIfaces, iface); err != nil { + return err + } // === Sync all routes via unified route manager (netlink) === // This handles intra-site routes on the mesh interface, gateway-specific routes on gateway interfaces, diff --git a/cmd/unbounded-net-node/wireguard_config_test.go b/cmd/unbounded-net-node/wireguard_config_test.go index 70205e031..e6c388b1c 100644 --- a/cmd/unbounded-net-node/wireguard_config_test.go +++ b/cmd/unbounded-net-node/wireguard_config_test.go @@ -2,3 +2,88 @@ // SPDX-License-Identifier: Apache-2.0 package main + +import ( + "errors" + "testing" + + "github.com/vishvananda/netlink" + + unboundednetnetlink "github.com/Azure/unbounded/internal/net/netlink" +) + +func TestWireGuardTeardownFailureRetainsGatewayGeneration(t *testing.T) { + originalList, originalDelete := listWireGuardLinks, deleteWireGuardLink + + t.Cleanup(func() { + listWireGuardLinks, deleteWireGuardLink = originalList, originalDelete + }) + + cfg := &config{WireGuardInterfacePrefix: "retiretestwg", WireGuardPort: 51820} + state := &wireGuardState{ + linkManager: unboundednetnetlink.NewLinkManager("no-such-test-wg"), + gatewayNames: map[string]string{"retiretestwg51821": "old-gateway"}, + } + injected := errors.New("injected teardown failure") + + for _, mode := range []string{"list", "delete"} { + t.Run(mode, func(t *testing.T) { + lifecycle := newGatewayInformerLifecycle(newGatewayLifecycleClient(), 0) + t.Cleanup(lifecycle.close) + + snapshot, _, err := lifecycle.prepare(t.Context(), true) + if err != nil { + t.Fatal(err) + } + + lifecycle.setRequired(false) + + cleanupFails := true + listWireGuardLinks = func() ([]netlink.Link, error) { + if cleanupFails && mode == "list" { + return nil, injected + } + + return []netlink.Link{&netlink.GenericLink{LinkAttrs: netlink.LinkAttrs{Name: "retiretestwg51821"}, LinkType: "wireguard"}}, nil + } + deleteWireGuardLink = func(netlink.Link) error { + if cleanupFails { + return injected + } + + return nil + } + + reconcile := func() error { + err := configureWireGuard(t.Context(), cfg, "", nil, nil, "local", nil, nil, nil, + nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, state, nil) + if err == nil { + lifecycle.retire(snapshot.generation) + } + + return err + } + if err := reconcile(); !errors.Is(err, injected) { + t.Fatalf("cleanup swallowed failure: %v", err) + } + + if got := lifecycle.snapshot(); !got.available || got.generation != snapshot.generation { + t.Fatal("failed cleanup retired synchronized caches") + } + + if ready, _ := lifecycle.readiness(); ready { + t.Fatal("failed cleanup marked retirement ready") + } + + cleanupFails = false + + if err := reconcile(); err != nil { + t.Fatal(err) + } + + if lifecycle.snapshot().available { + t.Fatal("successful cleanup retained caches") + } + }) + } +} diff --git a/deploy/net/crd/net.unbounded-cloud.io_gatewaypoolnodes.yaml b/deploy/net/crd/net.unbounded-cloud.io_gatewaypoolnodes.yaml index f00514682..26e55f37e 100644 --- a/deploy/net/crd/net.unbounded-cloud.io_gatewaypoolnodes.yaml +++ b/deploy/net/crd/net.unbounded-cloud.io_gatewaypoolnodes.yaml @@ -76,6 +76,16 @@ spec: description: GatewayNodeStatus defines the advertised routes and lease heartbeat for a gateway node. properties: + forwardingMTU: + description: |- + ForwardingMTU is the effective local forwarding MTU after a successful + dataplane reconciliation. Workers use this cap for accepted gateway paths. + Gateways do not consume it, avoiding feedback between advertised minima. + Absent values identify agents that have not published a forwarding limit. + format: int32 + maximum: 65535 + minimum: 1 + type: integer lastUpdated: description: LastUpdated is the heartbeat timestamp updated by the node agent. diff --git a/docs/content/reference/networking/configuration.md b/docs/content/reference/networking/configuration.md index bddefa3aa..77971cc63 100644 --- a/docs/content/reference/networking/configuration.md +++ b/docs/content/reference/networking/configuration.md @@ -213,11 +213,18 @@ encapsulation overhead. For example, WireGuard over a 1500-byte route uses **Behavior:** - Each peer's effective MTU is the lowest applicable configured cap, CRD `tunnelMTU`, and route MTU after encapsulation overhead. -- Sites and GatewayPools form a topology connected by SitePeerings, - SiteGatewayPoolAssignments, and GatewayPoolPeerings. The lowest `tunnelMTU` - in the local site's connected topology is applied site-wide, including - downstream multi-hop gateway paths. -- The CNI MTU is the lowest effective peer or connected-topology MTU. +- Forwarding-path scopes constrain a node. Direct mesh + peers use both endpoint Sites and their enabled SitePeering. Gateway paths + use assignments targeting the worker's own Site, the selected GatewayPool, + and GatewayPoolPeerings present in an advertised transit path. +- Direct-mesh components also share the lowest configured Site or enabled + mesh-peering MTU. If A meshes with B at 1200 and C at 1400, A's shared pod + interfaces use 1200, so C must also use 1200 when sending to A. This propagates + an interface constraint, not B-C routes. Disabled and `meshNodes: false` + peerings, missing Sites, and disconnected components do not contribute. + Gateway assignments and pool peerings do not join direct-mesh components. +- The CNI MTU is the lowest effective MTU among actual mesh and gateway peers + and the configured direct-mesh component cap. - At startup and whenever the effective CNI MTU changes, the node agent updates the CNI bridge and both endpoints of existing pod veth pairs. - Forwarded TCP SYN packets on every interface are MSS-clamped to the diff --git a/docs/content/reference/networking/custom-resources.md b/docs/content/reference/networking/custom-resources.md index 3c3ca5aef..f02442dc7 100644 --- a/docs/content/reference/networking/custom-resources.md +++ b/docs/content/reference/networking/custom-resources.md @@ -228,8 +228,16 @@ advertisements and heartbeat timestamps. | `spec.gatewayPool` | `string` | Yes | Parent GatewayPool. | | `spec.site` | `string` | No | Site name (from node labels). | | `status.lastUpdated` | `string` | -- | Last heartbeat timestamp. | +| `status.forwardingMTU` | `integer` | No | Effective local forwarding MTU in bytes after successful dataplane reconciliation. | | `status.routes` | `map[string]GatewayNodeRoute` | -- | Route advertisements keyed by CIDR. | +Workers honor this MTU on the next-hop gateway and candidate gateways in +intermediate pools on accepted paths. Gateways do not consume other gateways' +published MTUs, avoiding feedback between advertised minima. Install the updated +CRD before upgrading agents; older gateways without the field retain +topology-derived caps but do not provide this additional shared-interface limit. +Gateways without a GatewayPoolNode object are not used as route destinations. + --- ## SitePeering diff --git a/docs/net/architecture.md b/docs/net/architecture.md index 562de7ff6..0e3a63be3 100644 --- a/docs/net/architecture.md +++ b/docs/net/architecture.md @@ -700,6 +700,13 @@ graph TD end ``` +Every node agent continuously watches Sites, SiteNodeSlices, SitePeerings, and +SiteGatewayPoolAssignments. Gateway nodes also watch GatewayPools, +GatewayPoolNodes, and GatewayPoolPeerings. A worker starts those three +gateway-specific watches only while an enabled assignment directly targets +its pinned Site, waits for all three caches to synchronize before consuming +them, and stops them only after dependent routes and peers have been removed. + ### Gateway Node Isolation ```mermaid diff --git a/docs/net/configuration.md b/docs/net/configuration.md index 8d702a703..5a87f8da4 100644 --- a/docs/net/configuration.md +++ b/docs/net/configuration.md @@ -300,10 +300,19 @@ encapsulation overhead, GENEVE adds 58 bytes, VXLAN adds 50 bytes, and IPIP adds 20 bytes. With the default value of `0`, the node agent looks up the actual route to each peer, subtracts the selected encapsulation overhead, and uses the lowest applicable Site, peering, assignment, or pool `tunnelMTU`. -The node agent treats Sites and GatewayPools as a topology graph connected by -SitePeerings, SiteGatewayPoolAssignments, and GatewayPoolPeerings. The lowest -`tunnelMTU` anywhere in the local site's connected topology is applied -site-wide, including downstream multi-hop gateway paths. +Forwarding-path scopes constrain the node. Direct mesh peers +use both endpoint Sites and their enabled `SitePeering`; worker-to-gateway +paths use assignments that target the worker's own Site and the selected +GatewayPool; gateway-to-gateway paths also use the governing +`GatewayPoolPeering`. Direct-mesh components additionally share the lowest +configured Site or enabled mesh-peering MTU in the component. Shared pod and +tunnel interfaces make this necessary even without transitive routing: if A +meshes with B at 1200 and C at 1400, A's pod interfaces use 1200, so C must also +use 1200 when sending to A. This propagates only an MTU constraint, not routes +between B and C. Disabled and `meshNodes: false` peerings, missing Sites, and +disconnected components do not contribute. Gateway assignments and pool +peerings do not join direct-mesh components; their limits still follow actual +gateway paths. For example, a WireGuard peer reached through a 1500-byte Ethernet route gets an automatic MTU of `1420`. @@ -320,7 +329,8 @@ an automatic MTU of `1420`. - The controller reads `node.mtu` from the shared configmap and compares it against every node's `tunnel-mtu` annotation. Mismatches are reported as warnings on the status page and in the controller logs. -- The CNI MTU is the lowest effective peer or connected-topology MTU. At +- The CNI MTU is the lowest effective MTU among actual mesh and gateway peers + and the configured direct-mesh component cap. At startup and whenever that value changes, the node agent updates the CNI bridge and both endpoints of existing pod veth pairs. - Forwarded TCP SYN packets on every interface are MSS-clamped to the diff --git a/docs/net/custom-resources.md b/docs/net/custom-resources.md index 9dbbe9c69..2b36b1f22 100644 --- a/docs/net/custom-resources.md +++ b/docs/net/custom-resources.md @@ -481,12 +481,25 @@ status: | `spec.gatewayPool` | `string` | Yes | GatewayPool this node belongs to. | | `spec.site` | `string` | No | Site name (derived from node labels). | | `status.lastUpdated` | `string` | - | Read-only. Last heartbeat timestamp. | +| `status.forwardingMTU` | `integer` | No | Effective local forwarding MTU in bytes, published after successful dataplane reconciliation. | | `status.routes` | `map[string]GatewayNodeRoute` | - | Read-only. Route advertisements keyed by CIDR. | | `status.routes[].type` | `string` | - | Route source type: `NodeCidr`, `PodCidr`, or `RoutedCidr`. | | `status.routes[].source` | `GatewayNodePathHop` | - | Originating object for this route (`type` + `name`). | | `status.routes[].intermediateHops` | `[]GatewayNodePathHop` | - | Additional path objects between source and destination. | | `status.routes[].paths` | `[][]GatewayNodePathHop` | - | Full end-to-end paths. Each item is an ordered hop sequence from origin to local advertiser (max 100). | +Workers apply `status.forwardingMTU` from their next-hop gateway and candidate +gateways in intermediate pools on accepted paths. This includes shared-interface +limits even when a worker does not use the route that imposed the limit. +Gateways do not consume other gateways' published MTUs, avoiding feedback that +could prevent a previous minimum from increasing. + +Install the updated GatewayPoolNode CRD before upgrading agents so the API server +preserves this optional field. Older gateways without the field retain +topology-derived MTU handling; the additional shared-interface protection requires +the updated gateway agent and CRD. Gateway peers whose GatewayPoolNode object is +missing are not used as route destinations. + --- ## SitePeering diff --git a/internal/net/netlink/gateway_policy_manager.go b/internal/net/netlink/gateway_policy_manager.go index d2c2897f6..4bad2d3e1 100644 --- a/internal/net/netlink/gateway_policy_manager.go +++ b/internal/net/netlink/gateway_policy_manager.go @@ -4,6 +4,7 @@ package netlink import ( + "errors" "fmt" "math" "net" @@ -50,6 +51,7 @@ type GatewayPolicyManager struct { // Track which interfaces have been configured configuredIfaces map[string]int // interface name -> table number + pendingCleanup map[string]bool // Track if the global OUTPUT rule has been added outputRuleAdded bool @@ -177,8 +179,7 @@ func (m *GatewayPolicyManager) deleteIPRulesByMarkSet(tableNum int, marks map[ui r := rule if err := netlinkRuleDel(&r); err != nil { - klog.V(4).Infof("Failed deleting %s ip rule mark=%d table=%d: %v", familyName, rule.Mark, rule.Table, err) - continue + return fmt.Errorf("delete %s ip rule mark=%d table=%d: %w", familyName, rule.Mark, rule.Table, err) } klog.V(2).Infof("Deleted %s ip rule mark=%d table=%d", familyName, rule.Mark, rule.Table) @@ -289,17 +290,46 @@ func (m *GatewayPolicyManager) ConfigureInterface(ifaceName string, tableNum int // Check if already configured with the same table number if existingTable, exists := m.configuredIfaces[ifaceName]; exists { - if existingTable == tableNum { + if existingTable == tableNum && !m.pendingCleanup[ifaceName] { klog.V(3).Infof("Gateway policy routing already configured for %s (table %d)", ifaceName, tableNum) return nil } // Table number changed, need to reconfigure klog.V(2).Infof("Gateway %s table number changed from %d to %d, reconfiguring", ifaceName, existingTable, tableNum) - m.removeInterfaceRulesLocked(ifaceName, existingTable) + + if err := m.removeInterfaceRulesLocked(ifaceName, existingTable); err != nil { + return err + } + + delete(m.configuredIfaces, ifaceName) } klog.V(2).Infof("Configuring gateway policy routing for %s (table %d)", ifaceName, tableNum) + if m.configuredIfaces == nil { + m.configuredIfaces = make(map[string]int) + } + + if m.pendingCleanup == nil { + m.pendingCleanup = make(map[string]bool) + } + + m.configuredIfaces[ifaceName] = tableNum + m.pendingCleanup[ifaceName] = true + + if err := ensureGatewayInterfacePolicy(m, ifaceName, tableNum); err != nil { + return err + } + + delete(m.pendingCleanup, ifaceName) + klog.Infof("Gateway policy routing configured for %s (table %d)", ifaceName, tableNum) + + return nil +} + +var ensureGatewayInterfacePolicy = (*GatewayPolicyManager).ensureInterfacePolicyLocked + +func (m *GatewayPolicyManager) ensureInterfacePolicyLocked(ifaceName string, tableNum int) error { // 1. Add route table entry if not exists if err := EnsureRtTablesEntry(tableNum, ifaceName); err != nil { return fmt.Errorf("failed to ensure route table for %s: %w", ifaceName, err) @@ -329,9 +359,6 @@ func (m *GatewayPolicyManager) ConfigureInterface(ifaceName string, tableNum int m.outputRuleAdded = true } - m.configuredIfaces[ifaceName] = tableNum - klog.Infof("Gateway policy routing configured for %s (table %d)", ifaceName, tableNum) - return nil } @@ -529,30 +556,42 @@ func (m *GatewayPolicyManager) ensureIPRule(tableNum int) error { } // RemoveInterface removes policy routing configuration for an interface +var discoverGatewayInterfaceTable = (*GatewayPolicyManager).findInterfaceTableNumLocked + func (m *GatewayPolicyManager) RemoveInterface(ifaceName string) error { m.mu.Lock() defer m.mu.Unlock() if err := m.cleanupLegacyOutputSourceRulesLocked(); err != nil { - klog.Warningf("Failed cleaning legacy gateway output source rules: %v", err) + return err } if err := m.cleanupOutboundPinningRulesLocked(); err != nil { - klog.Warningf("Failed cleaning legacy gateway outbound pinning rules: %v", err) + return err } tableNum, exists := m.configuredIfaces[ifaceName] if !exists { var discovered bool - tableNum, discovered = m.findInterfaceTableNumLocked(ifaceName) + tableNum, discovered = discoverGatewayInterfaceTable(m, ifaceName) if !discovered { return nil } + + if m.configuredIfaces == nil { + m.configuredIfaces = make(map[string]int) + } + + m.configuredIfaces[ifaceName] = tableNum + } + + if err := m.removeInterfaceRulesLocked(ifaceName, tableNum); err != nil { + return err } - m.removeInterfaceRulesLocked(ifaceName, tableNum) delete(m.configuredIfaces, ifaceName) + delete(m.pendingCleanup, ifaceName) return nil } @@ -565,6 +604,19 @@ func (m *GatewayPolicyManager) ReconcileExpectedInterfaces(expected map[string]i klog.V(4).Infof("Reconciling gateway policy expected interfaces: %s", formatExpectedGatewayPolicyTables(expected)) + for iface, tableNum := range m.configuredIfaces { + if _, keep := expected[iface]; keep { + continue + } + + if err := m.removeInterfaceRulesLocked(iface, tableNum); err != nil { + return err + } + + delete(m.configuredIfaces, iface) + delete(m.pendingCleanup, iface) + } + if err := m.cleanupLegacyOutputSourceRulesLocked(); err != nil { return err } @@ -591,8 +643,16 @@ func (m *GatewayPolicyManager) ReconcileExpectedInterfaces(expected map[string]i return nil } +var flushGatewayRouteTable = FlushRouteTable + // removeInterfaceRulesLocked removes rules for an interface (must hold lock) -func (m *GatewayPolicyManager) removeInterfaceRulesLocked(ifaceName string, tableNum int) { +func (m *GatewayPolicyManager) removeInterfaceRulesLocked(ifaceName string, tableNum int) error { + if m.pendingCleanup == nil { + m.pendingCleanup = make(map[string]bool) + } + + m.pendingCleanup[ifaceName] = true + ingressMark, err := m.gatewayPolicyIngressMark(tableNum) if err != nil { klog.V(3).Infof("Continuing gateway policy cleanup for %s despite mark derivation failure for table %d: %v", ifaceName, tableNum, err) @@ -605,28 +665,28 @@ func (m *GatewayPolicyManager) removeInterfaceRulesLocked(ifaceName string, tabl legacyMarkRule := []string{"-i", ifaceName, "-m", "comment", "--comment", commentPrefix + ": gateway policy mark ingress", "-j", "MARK", "--set-mark", ingressMarkStr} legacySaveRule := []string{"-i", ifaceName, "-m", "comment", "--comment", commentPrefix + ": gateway policy save connmark", "-j", "CONNMARK", "--save-mark"} - if err := m.ipt4.Delete(mangleTable, gatewayPolicyPreroutingChain, setConnmarkRule...); err != nil { - klog.V(4).Infof("Failed to delete IPv4 ingress connmark set rule for %s: %v", ifaceName, err) - } - - if err := m.ipt4.Delete(mangleTable, gatewayPolicyPreroutingChain, legacyMarkRule...); err != nil { - klog.V(4).Infof("Failed to delete legacy IPv4 mark rule for %s: %v", ifaceName, err) - } + var cleanupErr error - if err := m.ipt4.Delete(mangleTable, gatewayPolicyPreroutingChain, legacySaveRule...); err != nil { - klog.V(4).Infof("Failed to delete legacy IPv4 save connmark rule for %s: %v", ifaceName, err) - } + for _, ipt := range []*iptables.IPTables{m.ipt4, m.ipt6} { + if ipt == nil { + continue + } - if err := m.ipt6.Delete(mangleTable, gatewayPolicyPreroutingChain, setConnmarkRule...); err != nil { - klog.V(4).Infof("Failed to delete IPv6 ingress connmark set rule for %s: %v", ifaceName, err) - } + exists, err := ipt.ChainExists(mangleTable, gatewayPolicyPreroutingChain) + if err != nil { + cleanupErr = errors.Join(cleanupErr, err) + continue + } - if err := m.ipt6.Delete(mangleTable, gatewayPolicyPreroutingChain, legacyMarkRule...); err != nil { - klog.V(4).Infof("Failed to delete legacy IPv6 mark rule for %s: %v", ifaceName, err) - } + if !exists { + continue + } - if err := m.ipt6.Delete(mangleTable, gatewayPolicyPreroutingChain, legacySaveRule...); err != nil { - klog.V(4).Infof("Failed to delete legacy IPv6 save connmark rule for %s: %v", ifaceName, err) + for _, rule := range [][]string{setConnmarkRule, legacyMarkRule, legacySaveRule} { + if err := ipt.DeleteIfExists(mangleTable, gatewayPolicyPreroutingChain, rule...); err != nil { + cleanupErr = errors.Join(cleanupErr, err) + } + } } // Remove ip rules (current and legacy marks) by matching against actual listed rules. @@ -636,22 +696,29 @@ func (m *GatewayPolicyManager) removeInterfaceRulesLocked(ifaceName string, tabl } if err := m.deleteIPRulesByMarkSet(tableNum, marks, netlink.FAMILY_V4, "IPv4"); err != nil { - klog.V(4).Infof("Failed deleting IPv4 ip rules for %s table=%d: %v", ifaceName, tableNum, err) + cleanupErr = errors.Join(cleanupErr, err) } if err := m.deleteIPRulesByMarkSet(tableNum, marks, netlink.FAMILY_V6, "IPv6"); err != nil { - klog.V(4).Infof("Failed deleting IPv6 ip rules for %s table=%d: %v", ifaceName, tableNum, err) + cleanupErr = errors.Join(cleanupErr, err) } - if err := FlushRouteTable(tableNum); err != nil { - klog.V(3).Infof("Failed to remove routes for table %d (%s): %v", tableNum, ifaceName, err) + if err := flushGatewayRouteTable(tableNum); err != nil { + cleanupErr = errors.Join(cleanupErr, err) + } + + if cleanupErr != nil { + return fmt.Errorf("remove policy routing for %s table=%d: %w", ifaceName, tableNum, cleanupErr) } if err := m.removeRouteTableEntry(tableNum); err != nil { - klog.V(3).Infof("Failed to remove rt_tables entry for table %d (%s): %v", tableNum, ifaceName, err) + return fmt.Errorf("remove rt_tables entry for %s table=%d: %w", ifaceName, tableNum, err) } klog.V(2).Infof("Removed policy routing rules for %s (table %d)", ifaceName, tableNum) + delete(m.pendingCleanup, ifaceName) + + return nil } // Cleanup removes all gateway policy iptables rules/chains managed by this manager. @@ -718,7 +785,11 @@ func (m *GatewayPolicyManager) pruneStaleInterfacesLocked() error { } klog.V(2).Infof("Removing stale gateway policy rules for missing interface %s (table %d)", iface, tableNum) - m.removeInterfaceRulesLocked(iface, tableNum) + + if err := m.removeInterfaceRulesLocked(iface, tableNum); err != nil { + return err + } + delete(m.configuredIfaces, iface) } @@ -1111,15 +1182,19 @@ func (m *GatewayPolicyManager) cleanupStaleIPRulesLocked(expected map[string]int continue } + if err := flushGatewayRouteTable(tableNum); err != nil { + return fmt.Errorf("flush stale gateway table %d: %w", tableNum, err) + } + + if err := m.removeRouteTableEntry(tableNum); err != nil { + return err + } + r := rule if err := netlinkRuleDel(&r); err != nil { - klog.V(4).Infof("Failed deleting stale %s ip rule mark=%d table=%d: %v", familyName, rule.Mark, rule.Table, err) - continue + return fmt.Errorf("delete stale %s ip rule mark=%d table=%d: %w", familyName, rule.Mark, rule.Table, err) } - _ = FlushRouteTable(tableNum) //nolint:errcheck - _ = m.removeRouteTableEntry(tableNum) //nolint:errcheck - klog.V(2).Infof("Removed stale %s ip rule mark=%d table=%d", familyName, rule.Mark, rule.Table) } diff --git a/internal/net/netlink/gateway_policy_manager_mock_test.go b/internal/net/netlink/gateway_policy_manager_mock_test.go index 7133cf09c..968bb2e08 100644 --- a/internal/net/netlink/gateway_policy_manager_mock_test.go +++ b/internal/net/netlink/gateway_policy_manager_mock_test.go @@ -4,12 +4,185 @@ package netlink import ( + "errors" "fmt" + "os" + "path/filepath" "testing" vishnetlink "github.com/vishvananda/netlink" ) +func TestGatewayPolicyReactivationAfterFailedRemoval(t *testing.T) { + oldList, oldDelete, oldAdd, oldFlush := netlinkRuleList, netlinkRuleDel, netlinkRuleAdd, flushGatewayRouteTable + oldEnsure, oldTables := ensureGatewayInterfacePolicy, rtTablesPath + + t.Cleanup(func() { + netlinkRuleList, netlinkRuleDel, netlinkRuleAdd, flushGatewayRouteTable = oldList, oldDelete, oldAdd, oldFlush + ensureGatewayInterfacePolicy, rtTablesPath = oldEnsure, oldTables + }) + + rtTablesPath = filepath.Join(t.TempDir(), "rt_tables") + if err := os.WriteFile(rtTablesPath, []byte("1001 wg51821\n"), 0o600); err != nil { + t.Fatal(err) + } + + m := &GatewayPolicyManager{ + wireguardBasePort: 51820, gatewayTableBase: 1000, + configuredIfaces: map[string]int{"wg51821": 1001}, + } + rules := map[int][]vishnetlink.Rule{ + vishnetlink.FAMILY_V4: {{Table: 1001, Mark: 1, Family: vishnetlink.FAMILY_V4}}, + vishnetlink.FAMILY_V6: {{Table: 1001, Mark: 1, Family: vishnetlink.FAMILY_V6}}, + } + netlinkRuleList = func(family int) ([]vishnetlink.Rule, error) { return rules[family], nil } + netlinkRuleDel = func(rule *vishnetlink.Rule) error { + rules[rule.Family] = nil + return nil + } + netlinkRuleAdd = func(rule *vishnetlink.Rule) error { + family := rule.Family + if family == 0 { + family = vishnetlink.FAMILY_V4 + } + + rules[family] = append(rules[family], *rule) + + return nil + } + injected := errors.New("table flush failed") + flushGatewayRouteTable = func(int) error { return injected } + + if err := m.RemoveInterface("wg51821"); !errors.Is(err, injected) { + t.Fatalf("removal error=%v", err) + } + + if len(rules[vishnetlink.FAMILY_V4])+len(rules[vishnetlink.FAMILY_V6]) != 0 { + t.Fatal("fixture did not remove policy rules before failed flush") + } + + flushGatewayRouteTable = func(int) error { return nil } + ensureCalls := 0 + ensureErr := errors.New("policy installation failed") + ensureGatewayInterfacePolicy = func(manager *GatewayPolicyManager, _ string, table int) error { + ensureCalls++ + + if ensureErr != nil { + return ensureErr + } + + return manager.ensureIPRule(table) + } + + if err := m.ConfigureInterface("wg51821", 1001); !errors.Is(err, ensureErr) { + t.Fatalf("installation error=%v", err) + } + + if !m.pendingCleanup["wg51821"] { + t.Fatal("failed reinstallation marked interface healthy") + } + + if err := m.ReconcileExpectedInterfaces(map[string]int{"wg51821": 1001}); err != nil { + t.Fatal(err) + } + + if !m.pendingCleanup["wg51821"] { + t.Fatal("expected-interface reconciliation masked failed installation") + } + + ensureErr = nil + + if err := m.ConfigureInterface("wg51821", 1001); err != nil { + t.Fatal(err) + } + + if ensureCalls != 2 || len(rules[vishnetlink.FAMILY_V4]) != 1 || len(rules[vishnetlink.FAMILY_V6]) != 1 || m.pendingCleanup["wg51821"] { + t.Fatalf("reactivation did not restore policy routing: calls=%d rules=%v pending=%v", ensureCalls, rules, m.pendingCleanup) + } +} + +func TestRemoveGatewayPolicyInterfaceRetriesFailedCleanup(t *testing.T) { + oldList, oldDelete, oldFlush, oldTables := netlinkRuleList, netlinkRuleDel, flushGatewayRouteTable, rtTablesPath + oldDiscovery := discoverGatewayInterfaceTable + + t.Cleanup(func() { + netlinkRuleList, netlinkRuleDel, flushGatewayRouteTable, rtTablesPath = oldList, oldDelete, oldFlush, oldTables + discoverGatewayInterfaceTable = oldDiscovery + }) + rtTablesPath = filepath.Join(t.TempDir(), "rt_tables") + injected := errors.New("cleanup failed") + + for _, mode := range []string{"list", "delete", "flush", "discovered flush"} { + t.Run(mode, func(t *testing.T) { + if err := os.WriteFile(rtTablesPath, []byte("1001 wg51821\n"), 0o600); err != nil { + t.Fatal(err) + } + + m := &GatewayPolicyManager{ + wireguardBasePort: 51820, gatewayTableBase: 1000, + configuredIfaces: map[string]int{"wg51821": 1001}, + } + if mode == "discovered flush" { + m.configuredIfaces = nil + discoveryCalls := 0 + discoverGatewayInterfaceTable = func(*GatewayPolicyManager, string) (int, bool) { + discoveryCalls++ + return 1001, discoveryCalls == 1 + } + } + + fail := true + netlinkRuleList = func(int) ([]vishnetlink.Rule, error) { + if fail && mode == "list" { + return nil, injected + } + + return []vishnetlink.Rule{{Table: 1001, Mark: 1}}, nil + } + netlinkRuleDel = func(*vishnetlink.Rule) error { + if fail && mode == "delete" { + return injected + } + + return nil + } + flushGatewayRouteTable = func(int) error { + if fail && (mode == "flush" || mode == "discovered flush") { + return injected + } + + return nil + } + + if err := m.RemoveInterface("wg51821"); !errors.Is(err, injected) { + t.Fatalf("cleanup hid error: %v", err) + } + + if _, tracked := m.configuredIfaces["wg51821"]; !tracked { + t.Fatal("failed cleanup forgot interface") + } + + if err := m.ReconcileExpectedInterfaces(nil); !errors.Is(err, injected) { + t.Fatalf("reconciliation hid pending cleanup failure: %v", err) + } + + if _, tracked := m.configuredIfaces["wg51821"]; !tracked { + t.Fatal("reconciliation forgot failed cleanup") + } + + fail = false + + if err := m.RemoveInterface("wg51821"); err != nil { + t.Fatal(err) + } + + if len(m.configuredIfaces) != 0 { + t.Fatal("successful cleanup retained interface") + } + }) + } +} + // TestDeleteIPRulesByMarkSetDeletesOnlyMatchingEntries tests delete iprules by mark set deletes only matching entries. func TestDeleteIPRulesByMarkSetDeletesOnlyMatchingEntries(t *testing.T) { origList := netlinkRuleList diff --git a/internal/net/netlink/route_table_setup.go b/internal/net/netlink/route_table_setup.go index f95adb57f..aad244441 100644 --- a/internal/net/netlink/route_table_setup.go +++ b/internal/net/netlink/route_table_setup.go @@ -295,7 +295,7 @@ func FlushRouteTable(tableID int) error { for _, route := range routes { r := route if err := netlink.RouteDel(&r); err != nil { - klog.V(4).Infof("Failed removing route from table %d: %v", tableID, err) + return fmt.Errorf("remove route from table %d: %w", tableID, err) } }