From 4fd19540892f839516ce896a609daa21d227d755 Mon Sep 17 00:00:00 2001 From: Kris Raney Date: Wed, 5 Aug 2026 18:41:16 +0000 Subject: [PATCH 01/18] Handle pod restart in the lower-sort pod case In pod pairs, one expects the other to do the veth setup work. If the lazy half restarts, it continues to expect this and things never get fixed. This updates the code to fix even in this case. --- .../meshnet/daemon/grpcwire/gwire_map.go | 18 +++++++ .../meshnet/daemon/grpcwire/gwire_map_test.go | 54 +++++++++++++++++++ .../meshnet/daemon/meshnet/controller.go | 10 +--- .../meshnet/daemon/meshnet/controller_test.go | 28 ++++++++++ 4 files changed, 101 insertions(+), 9 deletions(-) create mode 100644 third_party/meshnet/daemon/grpcwire/gwire_map_test.go diff --git a/third_party/meshnet/daemon/grpcwire/gwire_map.go b/third_party/meshnet/daemon/grpcwire/gwire_map.go index 37ef9ce2..03f87d92 100644 --- a/third_party/meshnet/daemon/grpcwire/gwire_map.go +++ b/third_party/meshnet/daemon/grpcwire/gwire_map.go @@ -32,6 +32,24 @@ func (w *wireMap) GetHandle(key int64) (*os.File, bool) { func (w *wireMap) AddInMem(wire *GRPCWire, handle *os.File) error { w.mu.Lock() defer w.mu.Unlock() + + for key, oldWire := range w.wires { + if oldWire.TopoNamespace == wire.TopoNamespace && + oldWire.LocalPodName == wire.LocalPodName && + oldWire.UID == wire.UID && + oldWire.LocalPodNetNS != wire.LocalPodNetNS { + if oldWire.IsReady { + close(oldWire.StopC) + oldWire.IsReady = false + } + if oldHandle, ok := w.handles[oldWire.LocalNodeIfaceID]; ok && oldHandle != nil { + _ = oldHandle.Close() + delete(w.handles, oldWire.LocalNodeIfaceID) + } + delete(w.wires, key) + } + } + w.wires[linkKey{ namespace: wire.LocalPodNetNS, linkUID: wire.UID, diff --git a/third_party/meshnet/daemon/grpcwire/gwire_map_test.go b/third_party/meshnet/daemon/grpcwire/gwire_map_test.go new file mode 100644 index 00000000..46efa476 --- /dev/null +++ b/third_party/meshnet/daemon/grpcwire/gwire_map_test.go @@ -0,0 +1,54 @@ +package grpcwire + +import ( + "testing" +) + +func TestAddInMem_StaleWireCleanup(t *testing.T) { + stopC1 := make(chan struct{}) + w1 := &GRPCWire{ + UID: 101, + TopoNamespace: "default", + LocalPodName: "pod1", + LocalPodNetNS: "/proc/111/ns/net", + IsReady: true, + StopC: stopC1, + } + + wires.AddInMem(w1, nil) + + if wire, ok := GetWireByUID("/proc/111/ns/net", 101); !ok || wire != w1 { + t.Fatalf("expected w1 in wires map, got ok=%t", ok) + } + + w2 := &GRPCWire{ + UID: 101, + TopoNamespace: "default", + LocalPodName: "pod1", + LocalPodNetNS: "/proc/222/ns/net", + IsReady: true, + StopC: make(chan struct{}), + } + + wires.AddInMem(w2, nil) + + // Old wire for /proc/111/ns/net should be deleted and its StopC closed + if _, ok := GetWireByUID("/proc/111/ns/net", 101); ok { + t.Fatalf("expected old wire /proc/111/ns/net to be deleted") + } + + select { + case <-stopC1: + // expected: stopC1 was closed + default: + t.Fatalf("expected old wire StopC to be closed") + } + + // New wire should be present + if wire, ok := GetWireByUID("/proc/222/ns/net", 101); !ok || wire != w2 { + t.Fatalf("expected w2 in wires map for /proc/222/ns/net, got ok=%t", ok) + } + + // Clean up + wires.AtomicDelete(w2) +} diff --git a/third_party/meshnet/daemon/meshnet/controller.go b/third_party/meshnet/daemon/meshnet/controller.go index 5968fc88..ff7dad59 100644 --- a/third_party/meshnet/daemon/meshnet/controller.go +++ b/third_party/meshnet/daemon/meshnet/controller.go @@ -159,16 +159,8 @@ func (m *Meshnet) reconcilePodLinksInternal(ctx context.Context, topo *unstructu } } else if peerSrcIP != "" { if m.interNodeLinkType == wireutil.INTER_NODE_LINK_GRPC { - // We only initiate gRPC wires from the higher priority pod node (lexicographically) - higherPrio := topo.GetName() > link.PeerPodName - if !higherPrio { - mnetdLogger.Debugf("ReconcilePodLinks: skipping gRPC wire initialization for link UID %d, expecting higher-priority peer %s to initiate", - link.LinkUID, link.PeerPodName) - continue - } - // Check if wire already exists and is ready - if _, ok := grpcwire.GetWireByUID(netNS, int(link.LinkUID)); ok { + if wire, ok := grpcwire.GetWireByUID(netNS, int(link.LinkUID)); ok && wire != nil && wire.IsReady { mnetdLogger.Debugf("ReconcilePodLinks: gRPC wire already exists for link UID %d, skipping", link.LinkUID) continue } diff --git a/third_party/meshnet/daemon/meshnet/controller_test.go b/third_party/meshnet/daemon/meshnet/controller_test.go index 0c12b1b3..86235b82 100644 --- a/third_party/meshnet/daemon/meshnet/controller_test.go +++ b/third_party/meshnet/daemon/meshnet/controller_test.go @@ -4,6 +4,7 @@ import ( "context" "testing" + fakeTopology "github.com/openconfig/kne/third_party/meshnet/api/clientset/v1beta1/fake" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) @@ -146,3 +147,30 @@ func TestReconcilePodLinks_NoLinks(t *testing.T) { t.Fatalf("ReconcilePodLinks failed for pod without links: %v", err) } } + +func TestReconcilePodLinks_LowerPriorityPodInitiatesGRPC(t *testing.T) { + InitLogger() + m := &Meshnet{ + nodeIP: "10.0.0.1", + interNodeLinkType: "GRPC", + } + + // Lower priority pod "a_pod" connected to peer "z_pod" on remote node "10.0.0.2" + lowerPrioPod := createFakePodTopology("a_pod", "default", "10.0.0.1", "/proc/100/ns/net", []string{"z_pod"}) + peerPod := createFakePodTopology("z_pod", "default", "10.0.0.2", "/proc/200/ns/net", []string{"a_pod"}) + + fakeClient, err := fakeTopology.NewSimpleClientset(lowerPrioPod, peerPod) + if err != nil { + t.Fatalf("failed to create fake topology clientset: %v", err) + } + m.tClient = fakeClient + + // Reconcile lower priority pod "a_pod". It should attempt to reconcile without skipping due to lower priority. + // Since CreateGRPCWireLocal will attempt to open TAP device (which fails without root/TAP), we expect a TAP creation error, + // proving it attempted reconciliation rather than skipping. + err = m.ReconcilePodLinks(context.Background(), lowerPrioPod) + if err == nil { + t.Fatalf("expected error during TAP creation without root, but got nil (means it may have skipped)") + } +} + From 82d370cd2ffc339920fb4ab098db99ea292eae1e Mon Sep 17 00:00:00 2001 From: Kris Raney Date: Wed, 5 Aug 2026 18:42:46 +0000 Subject: [PATCH 02/18] Support hot link removal When a link is dynamically removed from the topology, clean up its associated meshnet resources --- .../meshnet/daemon/meshnet/controller.go | 73 ++++++++++++++++++- .../meshnet/daemon/meshnet/controller_test.go | 22 ++++++ 2 files changed, 94 insertions(+), 1 deletion(-) diff --git a/third_party/meshnet/daemon/meshnet/controller.go b/third_party/meshnet/daemon/meshnet/controller.go index ff7dad59..00f08585 100644 --- a/third_party/meshnet/daemon/meshnet/controller.go +++ b/third_party/meshnet/daemon/meshnet/controller.go @@ -7,6 +7,7 @@ import ( "strings" "time" + "github.com/containernetworking/plugins/pkg/ns" "github.com/openconfig/kne/third_party/meshnet/daemon/grpcwire" mpb "github.com/openconfig/kne/third_party/meshnet/daemon/proto/meshnet/v1beta1" "github.com/openconfig/kne/third_party/meshnet/daemon/vxlan" @@ -108,6 +109,70 @@ func (m *Meshnet) ReconcilePodLinks(ctx context.Context, topo *unstructured.Unst return nil } +// cleanupRemovedPodLinks removes any gRPC wires and netns interfaces that belong to link UIDs +// or interface names no longer present in desiredLinks for the active pod. +func (m *Meshnet) cleanupRemovedPodLinks(ctx context.Context, topo *unstructured.Unstructured, netNS string, desiredLinks []wireutil.PodLinkConfig) { + if topo == nil || netNS == "" { + return + } + + desiredUIDs := make(map[int64]bool) + desiredIntfs := make(map[string]bool) + for _, l := range desiredLinks { + desiredUIDs[l.LinkUID] = true + desiredIntfs[l.LocalIntf] = true + } + + // 1. Clean up removed gRPC wires + existingWires, _ := grpcwire.GetWiresByPod(topo.GetNamespace(), topo.GetName()) + for _, wire := range existingWires { + if wire == nil { + continue + } + if wire.LocalPodNetNS == netNS && !desiredUIDs[int64(wire.UID)] { + mnetdLogger.Infof("cleanupRemovedPodLinks: removing hot-deleted gRPC wire (UID %d, pod %s, intf %s)", + wire.UID, wire.LocalPodName, wire.LocalPodIfaceName) + + if wire.PeerNodeIP != "" && wire.PeerNodeIP != m.nodeIP && wire.PeerNodeIP != "localhost" && wire.PeerNodeIP != "127.0.0.1" { + url := fmt.Sprintf("%s:%d", wire.PeerNodeIP, wireutil.GRPCDefaultPort) + url = strings.TrimSpace(url) + if remoteConn, err := grpc.Dial(url, grpc.WithTransportCredentials(insecure.NewCredentials())); err == nil { + remoteClient := mpb.NewRemoteClient(remoteConn) + _, _ = remoteClient.GRPCWireDownRemote(ctx, &mpb.WireDef{ + TopoNs: wire.TopoNamespace, + LocalPodName: wire.LocalPodName, + LocalPodNetNs: wire.LocalPodNetNS, + LinkUid: int64(wire.UID), + }) + remoteConn.Close() + } + } + + _ = grpcwire.RemoveWireAcrosAll(wire, true) + } + } + + // 2. Clean up removed interfaces inside container netns + if podNs, err := ns.GetNS(netNS); err == nil { + _ = podNs.Do(func(_ ns.NetNS) error { + if list, err := netlink.LinkList(); err == nil { + for _, l := range list { + name := l.Attrs().Name + if name == "lo" || name == "eth0" { + continue + } + if !desiredIntfs[name] { + mnetdLogger.Infof("cleanupRemovedPodLinks: removing hot-deleted interface %s from netns %s (pod %s)", name, netNS, topo.GetName()) + _ = netlink.LinkDel(l) + } + } + } + return nil + }) + podNs.Close() + } +} + // reconcilePodLinksInternal performs the actual network interface plumbing work. func (m *Meshnet) reconcilePodLinksInternal(ctx context.Context, topo *unstructured.Unstructured) error { if topo == nil { @@ -122,10 +187,16 @@ func (m *Meshnet) reconcilePodLinksInternal(ctx context.Context, topo *unstructu } links, err := parsePodLinks(topo) - if err != nil || len(links) == 0 { + if err != nil { return err } + m.cleanupRemovedPodLinks(ctx, topo, netNS, links) + + if len(links) == 0 { + return nil + } + peerCache := make(map[string]*unstructured.Unstructured) type grpcPeerBatch struct { peerIP string diff --git a/third_party/meshnet/daemon/meshnet/controller_test.go b/third_party/meshnet/daemon/meshnet/controller_test.go index 86235b82..b5584f1d 100644 --- a/third_party/meshnet/daemon/meshnet/controller_test.go +++ b/third_party/meshnet/daemon/meshnet/controller_test.go @@ -174,3 +174,25 @@ func TestReconcilePodLinks_LowerPriorityPodInitiatesGRPC(t *testing.T) { } } +func TestCleanupRemovedPodLinks_GRPC(t *testing.T) { + InitLogger() + m := &Meshnet{ + nodeIP: "10.0.0.1", + } + + // Pod "p1" initially had 2 links (UID 1 and UID 2) + podWith2Links := createFakePodTopology("p1", "default", "10.0.0.1", "/proc/1/ns/net", []string{"p2", "p3"}) + desiredLinks, err := parsePodLinks(podWith2Links) + if err != nil || len(desiredLinks) != 2 { + t.Fatalf("failed to parse 2 links: %v", err) + } + + // Now remove link UID 2 from spec.links (only UID 1 remains) + podWith1Link := createFakePodTopology("p1", "default", "10.0.0.1", "/proc/1/ns/net", []string{"p2"}) + desired1Link, _ := parsePodLinks(podWith1Link) + + // Call cleanupRemovedPodLinks with 1 link + m.cleanupRemovedPodLinks(context.Background(), podWith1Link, "/proc/1/ns/net", desired1Link) +} + + From 583577b72e9ab3272306936cb1069e263c3898af Mon Sep 17 00:00:00 2001 From: Kris Raney Date: Fri, 14 Aug 2026 21:27:33 +0000 Subject: [PATCH 03/18] Handle case where a pod gets rescheduled to a different node A pod's IP can change on restart, so make sure IP is updated when the pod resyncs --- .../meshnet/daemon/grpcwire/grpcwire.go | 12 +++-- .../meshnet/daemon/grpcwire/gwire_map_test.go | 32 +++++++++++++ .../meshnet/daemon/grpcwire/gwire_recon.go | 48 +++++++++++++++---- .../daemon/grpcwire/gwire_recon_test.go | 2 +- .../daemon/grpcwire/gwire_rpc_handlers.go | 4 +- .../meshnet/daemon/meshnet/controller.go | 14 +++--- 6 files changed, 88 insertions(+), 24 deletions(-) diff --git a/third_party/meshnet/daemon/grpcwire/grpcwire.go b/third_party/meshnet/daemon/grpcwire/grpcwire.go index 00ff41d6..2e1eac6c 100644 --- a/third_party/meshnet/daemon/grpcwire/grpcwire.go +++ b/third_party/meshnet/daemon/grpcwire/grpcwire.go @@ -132,7 +132,7 @@ func CreateGWire(locIfIndex int, locIfNm string, stopC chan struct{}, wireDef *m } // update the wire with the given input and mark the wire ready -func (wire *GRPCWire) UpdateWire(peerIntfId int64, stopC chan struct{}) { +func (wire *GRPCWire) UpdateWire(peerIntfId int64, peerNodeIP string, stopC chan struct{}) { wire.mu.Lock() defer wire.mu.Unlock() if wire.StopC == nil { @@ -142,10 +142,12 @@ func (wire *GRPCWire) UpdateWire(peerIntfId int64, stopC chan struct{}) { wire.StopC = make(chan struct{}) } } - if !wire.IsReady { - wire.WireIfaceIDOnPeerNode = peerIntfId + wire.WireIfaceIDOnPeerNode = peerIntfId + if peerNodeIP != "" { + wire.PeerNodeIP = peerNodeIP } wire.IsReady = true + go wire.K8sStoreGWire() } // GetWireByUID returns wire matching the provided namespace and linkUID. @@ -155,7 +157,7 @@ func GetWireByUID(namespace string, linkUID int) (*GRPCWire, bool) { // For the given uid if the wire exists, then update the wire properties. // Returns true if a wire exists, also the wire structure that got modified -func UpdateWireByUID(namespace string, linkUID int, peerIntfId int64, stopC chan struct{}) (*GRPCWire, bool) { +func UpdateWireByUID(namespace string, linkUID int, peerIntfId int64, peerNodeIP string, stopC chan struct{}) (*GRPCWire, bool) { wires.mu.Lock() wire, ok := wires.wires[linkKey{ namespace: namespace, @@ -163,7 +165,7 @@ func UpdateWireByUID(namespace string, linkUID int, peerIntfId int64, stopC chan }] wires.mu.Unlock() if ok { - wire.UpdateWire(peerIntfId, stopC) + wire.UpdateWire(peerIntfId, peerNodeIP, stopC) } return wire, ok } diff --git a/third_party/meshnet/daemon/grpcwire/gwire_map_test.go b/third_party/meshnet/daemon/grpcwire/gwire_map_test.go index 46efa476..6c129689 100644 --- a/third_party/meshnet/daemon/grpcwire/gwire_map_test.go +++ b/third_party/meshnet/daemon/grpcwire/gwire_map_test.go @@ -52,3 +52,35 @@ func TestAddInMem_StaleWireCleanup(t *testing.T) { // Clean up wires.AtomicDelete(w2) } + +func TestUpdateWireByUID_PeerIPUpdate(t *testing.T) { + w := &GRPCWire{ + UID: 202, + TopoNamespace: "default", + LocalPodName: "podA", + LocalPodNetNS: "/proc/555/ns/net", + WireIfaceIDOnPeerNode: 100, + PeerNodeIP: "10.0.0.2", + IsReady: true, + } + + wires.AddInMem(w, nil) + + // Update with new peer interface ID and new peer node IP (e.g. peer rescheduled to 10.0.0.3) + updated, ok := UpdateWireByUID("/proc/555/ns/net", 202, 300, "10.0.0.3", make(chan struct{})) + if !ok || updated == nil { + t.Fatalf("expected wire to be found and updated") + } + + if updated.PeerNodeIP != "10.0.0.3" { + t.Fatalf("expected PeerNodeIP to be updated to 10.0.0.3, got %s", updated.PeerNodeIP) + } + + if updated.WireIfaceIDOnPeerNode != 300 { + t.Fatalf("expected WireIfaceIDOnPeerNode to be updated to 300, got %d", updated.WireIfaceIDOnPeerNode) + } + + // Clean up + wires.AtomicDelete(w) +} + diff --git a/third_party/meshnet/daemon/grpcwire/gwire_recon.go b/third_party/meshnet/daemon/grpcwire/gwire_recon.go index b41241bf..df64f3fe 100644 --- a/third_party/meshnet/daemon/grpcwire/gwire_recon.go +++ b/third_party/meshnet/daemon/grpcwire/gwire_recon.go @@ -24,12 +24,19 @@ import ( // GWireClient is dynamic client for grpc wire. it is used to read/write grpc wire info from/to k8s api data-store type GWireClient struct { + mu sync.RWMutex di dynamic.NamespaceableResourceInterface gvr schema.GroupVersionResource } var gWClient GWireClient +func (gc *GWireClient) getDI() dynamic.NamespaceableResourceInterface { + gc.mu.RLock() + defer gc.mu.RUnlock() + return gc.di +} + const ( kStatus = "status" // json name of Status of gwire_type, +++TBD: can we make it dynamic kGrpcWireItems = "grpcWireItems" // json name of GWireKItems of gwire_type, +++TBD: can we make it dynamic @@ -37,6 +44,8 @@ const ( // SetGWireClient initializes the dynamic K8s client for gRPC wire CRD management. func SetGWireClient(gClient *dynamic.DynamicClient) { + gWClient.mu.Lock() + defer gWClient.mu.Unlock() // identifier of grpc wire object in k8s apis gWClient.gvr = schema.GroupVersionResource{ Group: grpcwirev1.GroupName, @@ -48,12 +57,18 @@ func SetGWireClient(gClient *dynamic.DynamicClient) { // SetGWireClientInterface sets the K8s dynamic resource interface (used for unit testing). func SetGWireClientInterface(gClient dynamic.NamespaceableResourceInterface) { + gWClient.mu.Lock() + defer gWClient.mu.Unlock() gWClient.di = gClient } // GetWireObjListUS lists unstructured GWireKObj resources for a specified node. -func (gc GWireClient) GetWireObjListUS(ctx context.Context, ndName string) (*unstructured.UnstructuredList, error) { - return gc.di.Namespace("").List(ctx, metav1.ListOptions{ +func (gc *GWireClient) GetWireObjListUS(ctx context.Context, ndName string) (*unstructured.UnstructuredList, error) { + di := gc.getDI() + if di == nil { + return nil, errors.New("gWClient dynamic interface is nil") + } + return di.Namespace("").List(ctx, metav1.ListOptions{ TypeMeta: metav1.TypeMeta{ Kind: reflect.TypeOf(grpcwirev1.GWireKObj{}).Name(), }, @@ -64,18 +79,30 @@ func (gc GWireClient) GetWireObjListUS(ctx context.Context, ndName string) (*uns } // CreatWireObj creates a new unstructured GWireKObj resource in K8s. -func (gc GWireClient) CreatWireObj(ctx context.Context, nSpace string, uWbj map[string]interface{}) (*unstructured.Unstructured, error) { - return gc.di.Namespace(nSpace).Create(ctx, &unstructured.Unstructured{Object: uWbj}, metav1.CreateOptions{}) +func (gc *GWireClient) CreatWireObj(ctx context.Context, nSpace string, uWbj map[string]interface{}) (*unstructured.Unstructured, error) { + di := gc.getDI() + if di == nil { + return nil, errors.New("gWClient dynamic interface is nil") + } + return di.Namespace(nSpace).Create(ctx, &unstructured.Unstructured{Object: uWbj}, metav1.CreateOptions{}) } // UpdateWireObj updates an existing unstructured GWireKObj resource in K8s. -func (gc GWireClient) UpdateWireObj(ctx context.Context, nSpace string, wObjsOnNd *unstructured.Unstructured) (*unstructured.Unstructured, error) { - return gc.di.Namespace(nSpace).Update(ctx, wObjsOnNd, metav1.UpdateOptions{}) +func (gc *GWireClient) UpdateWireObj(ctx context.Context, nSpace string, wObjsOnNd *unstructured.Unstructured) (*unstructured.Unstructured, error) { + di := gc.getDI() + if di == nil { + return nil, errors.New("gWClient dynamic interface is nil") + } + return di.Namespace(nSpace).Update(ctx, wObjsOnNd, metav1.UpdateOptions{}) } // GetWireObjGrpUS retrieves the GWireKObj for a given node and status. -func (gc GWireClient) GetWireObjGrpUS(ctx context.Context, wStatus *grpcwirev1.GWireStatus) (*unstructured.Unstructured, error) { - return gc.di.Namespace(wStatus.TopoNamespace).Get(ctx, wStatus.LocalNodeName, metav1.GetOptions{}) +func (gc *GWireClient) GetWireObjGrpUS(ctx context.Context, wStatus *grpcwirev1.GWireStatus) (*unstructured.Unstructured, error) { + di := gc.getDI() + if di == nil { + return nil, errors.New("gWClient dynamic interface is nil") + } + return di.Namespace(wStatus.TopoNamespace).Get(ctx, wStatus.LocalNodeName, metav1.GetOptions{}) } // ----------------------------------------------------------------------------------------------------------- @@ -177,7 +204,7 @@ type statusGroupKey struct { } func updateGRPCWireStatusBatch(ctx context.Context, updates []wireStatusUpdate) error { - if len(updates) == 0 { + if len(updates) == 0 || gWClient.getDI() == nil { return nil } @@ -417,6 +444,9 @@ func CreateGWireStatInDS(ctx context.Context, wStatus *grpcwirev1.GWireStatus) e // deleteGRPCWireStatus deletes a grpc wire status from 'grpcWireItems' for a specific namespace // for this node. Topology namespace is derived from given 'wStatus'. func deleteGRPCWireStatus(ctx context.Context, wStatus *grpcwirev1.GWireStatus) error { + if gWClient.getDI() == nil { + return nil + } retryErr := retry.RetryOnConflict(retry.DefaultRetry, func() error { node, err := gWClient.GetWireObjGrpUS(ctx, wStatus) diff --git a/third_party/meshnet/daemon/grpcwire/gwire_recon_test.go b/third_party/meshnet/daemon/grpcwire/gwire_recon_test.go index 2a54ae88..e1137647 100644 --- a/third_party/meshnet/daemon/grpcwire/gwire_recon_test.go +++ b/third_party/meshnet/daemon/grpcwire/gwire_recon_test.go @@ -465,7 +465,7 @@ func TestCreateWireStatus_ConcurrentUpdateRace(t *testing.T) { wg.Add(2) go func(id int) { defer wg.Done() - wire.UpdateWire(int64(id), nil) + wire.UpdateWire(int64(id), "10.5.5.5", make(chan struct{})) }(i) go func() { defer wg.Done() diff --git a/third_party/meshnet/daemon/grpcwire/gwire_rpc_handlers.go b/third_party/meshnet/daemon/grpcwire/gwire_rpc_handlers.go index 74a4a076..77a45cdd 100644 --- a/third_party/meshnet/daemon/grpcwire/gwire_rpc_handlers.go +++ b/third_party/meshnet/daemon/grpcwire/gwire_rpc_handlers.go @@ -50,9 +50,9 @@ func CreateUpdateGRPCWireRemoteTriggered(wireDef *mpb.WireDef, stopC chan struct // This can happen due to a race between the local and remote peer. // This can also happen when a pod in one end of the wire is deleted and created again. // In all cases link creation happen only once but it can get updated multiple times. - grpcWire, ok := UpdateWireByUID(wireDef.LocalPodNetNs, int(wireDef.LinkUid), wireDef.WireIfIdOnPeerNode, stopC) + grpcWire, ok := UpdateWireByUID(wireDef.LocalPodNetNs, int(wireDef.LinkUid), wireDef.WireIfIdOnPeerNode, wireDef.PeerNodeIp, stopC) if ok { - grpcOvrlyLogger.Infof("[CREATE-UPDATE-WIRE] At remote end this grpc-wire is already created by %s. Local interface id : %d peer interface id : %d", grpcWire.Originator, grpcWire.LocalNodeIfaceID, grpcWire.WireIfaceIDOnPeerNode) + grpcOvrlyLogger.Infof("[CREATE-UPDATE-WIRE] At remote end this grpc-wire is already created by %s. Local interface id : %d peer interface id : %d peer node IP : %s", grpcWire.Originator, grpcWire.LocalNodeIfaceID, grpcWire.WireIfaceIDOnPeerNode, grpcWire.PeerNodeIP) return grpcWire, false, nil } diff --git a/third_party/meshnet/daemon/meshnet/controller.go b/third_party/meshnet/daemon/meshnet/controller.go index 00f08585..3db1aa4d 100644 --- a/third_party/meshnet/daemon/meshnet/controller.go +++ b/third_party/meshnet/daemon/meshnet/controller.go @@ -230,14 +230,14 @@ func (m *Meshnet) reconcilePodLinksInternal(ctx context.Context, topo *unstructu } } else if peerSrcIP != "" { if m.interNodeLinkType == wireutil.INTER_NODE_LINK_GRPC { - // Check if wire already exists and is ready - if wire, ok := grpcwire.GetWireByUID(netNS, int(link.LinkUID)); ok && wire != nil && wire.IsReady { - mnetdLogger.Debugf("ReconcilePodLinks: gRPC wire already exists for link UID %d, skipping", link.LinkUID) + // Check if wire already exists, is ready, and points to the current peer node IP + if wire, ok := grpcwire.GetWireByUID(netNS, int(link.LinkUID)); ok && wire != nil && wire.IsReady && wire.PeerNodeIP == peerSrcIP { + mnetdLogger.Debugf("ReconcilePodLinks: gRPC wire already exists for link UID %d to peer %s (%s), skipping", link.LinkUID, link.PeerPodName, peerSrcIP) continue } - mnetdLogger.Infof("ReconcilePodLinks: initiating gRPC wire for pod %s <-> peer %s (UID %d)", - topo.GetName(), link.PeerPodName, link.LinkUID) + mnetdLogger.Infof("ReconcilePodLinks: initiating gRPC wire for pod %s <-> peer %s (%s, UID %d)", + topo.GetName(), link.PeerPodName, peerSrcIP, link.LinkUID) // 1. Register local end in meshnet daemon (creates/attaches TAP interface in container netns) wireDefLocal := &mpb.WireDef{ @@ -341,7 +341,7 @@ func (m *Meshnet) reconcilePodLinksInternal(ctx context.Context, topo *unstructu } l := chunkLinks[j] if res != nil && res.Response { - grpcwire.UpdateWireByUID(netNS, int(l.LinkUID), res.PeerIntfId, make(chan struct{})) + grpcwire.UpdateWireByUID(netNS, int(l.LinkUID), res.PeerIntfId, peerIP, make(chan struct{})) } else { linkErr := fmt.Errorf("remote wire creation failed for link UID %d (%s@%s -> %s@%s)", l.LinkUID, topo.GetName(), l.LocalIntf, l.PeerPodName, l.PeerIntf) mnetdLogger.Errorf("ReconcilePodLinks: %v", linkErr) @@ -514,7 +514,7 @@ func (m *Meshnet) RunControllerLoop(ctx context.Context) { } } -// updatePlumbingErrorStatus writes the provided plumbing error message (or clears it if empty) +// updatePlumbingErrorStatus writes the provided plumbing error message (or clears it if empty) // to the Topology resource's status.plumbing_error field. func (m *Meshnet) updatePlumbingErrorStatus(ctx context.Context, topo *unstructured.Unstructured, errMsg string) error { if m.tClient == nil { From c5295f0aa67799a743bfde65ffe8c5f7895bfad2 Mon Sep 17 00:00:00 2001 From: Kris Raney Date: Wed, 5 Aug 2026 18:47:42 +0000 Subject: [PATCH 04/18] Handle the case where a same-node link becomes remote or the reverse When a pod is rescheduled elsewhere, the type of link it needs may change, so clean up and handle that case properly when needed --- .../meshnet/daemon/meshnet/controller.go | 34 +++++++++++++++++++ .../meshnet/daemon/meshnet/controller_test.go | 25 ++++++++++++++ 2 files changed, 59 insertions(+) diff --git a/third_party/meshnet/daemon/meshnet/controller.go b/third_party/meshnet/daemon/meshnet/controller.go index 3db1aa4d..7ae05d83 100644 --- a/third_party/meshnet/daemon/meshnet/controller.go +++ b/third_party/meshnet/daemon/meshnet/controller.go @@ -226,10 +226,44 @@ func (m *Meshnet) reconcilePodLinksInternal(ctx context.Context, topo *unstructu } if peerSrcIP == srcIP || (m.nodeIP == "" && srcIP == "") { if peerNetNS != "" { + // Transition check: If moving from gRPC to same-node veth, clean up any existing gRPC wire + if wire, ok := grpcwire.GetWireByUID(netNS, int(link.LinkUID)); ok && wire != nil { + mnetdLogger.Infof("ReconcilePodLinks: link UID %d for pod %s moved to same node; tearing down old gRPC wire", link.LinkUID, topo.GetName()) + _ = grpcwire.RemoveWireAcrosAll(wire, true) + } + + // If an interface with link.LocalIntf exists in netNS but is not a veth link (e.g. old TAP or VXLAN), remove it + if podNs, err := ns.GetNS(netNS); err == nil { + _ = podNs.Do(func(_ ns.NetNS) error { + if l, err := netlink.LinkByName(link.LocalIntf); err == nil { + if l.Type() != "veth" { + mnetdLogger.Infof("ReconcilePodLinks: removing non-veth interface %s (%s) from netns %s before same-node veth plumbing", link.LocalIntf, l.Type(), netNS) + _ = netlink.LinkDel(l) + } + } + return nil + }) + podNs.Close() + } + sameNodeLinks = append(sameNodeLinks, link) } } else if peerSrcIP != "" { if m.interNodeLinkType == wireutil.INTER_NODE_LINK_GRPC { + // Transition check: If moving from same-node veth or VXLAN to gRPC, clean up non-TAP interface in netNS + if podNs, err := ns.GetNS(netNS); err == nil { + _ = podNs.Do(func(_ ns.NetNS) error { + if l, err := netlink.LinkByName(link.LocalIntf); err == nil { + if l.Type() != "tuntap" { + mnetdLogger.Infof("ReconcilePodLinks: removing non-TAP interface %s (%s) from netns %s before gRPC wire plumbing", link.LocalIntf, l.Type(), netNS) + _ = netlink.LinkDel(l) + } + } + return nil + }) + podNs.Close() + } + // Check if wire already exists, is ready, and points to the current peer node IP if wire, ok := grpcwire.GetWireByUID(netNS, int(link.LinkUID)); ok && wire != nil && wire.IsReady && wire.PeerNodeIP == peerSrcIP { mnetdLogger.Debugf("ReconcilePodLinks: gRPC wire already exists for link UID %d to peer %s (%s), skipping", link.LinkUID, link.PeerPodName, peerSrcIP) diff --git a/third_party/meshnet/daemon/meshnet/controller_test.go b/third_party/meshnet/daemon/meshnet/controller_test.go index b5584f1d..9885c590 100644 --- a/third_party/meshnet/daemon/meshnet/controller_test.go +++ b/third_party/meshnet/daemon/meshnet/controller_test.go @@ -195,4 +195,29 @@ func TestCleanupRemovedPodLinks_GRPC(t *testing.T) { m.cleanupRemovedPodLinks(context.Background(), podWith1Link, "/proc/1/ns/net", desired1Link) } +func TestReconcilePodLinks_TransitionGRPCToSameNode(t *testing.T) { + InitLogger() + m := &Meshnet{ + nodeIP: "10.0.0.1", + } + + // Pod "p1" and "p2" both on same node "10.0.0.1" + p1 := createFakePodTopology("p1", "default", "10.0.0.1", "/proc/1/ns/net", []string{"p2"}) + p2 := createFakePodTopology("p2", "default", "10.0.0.1", "/proc/2/ns/net", []string{"p1"}) + + fakeClient, err := fakeTopology.NewSimpleClientset(p1, p2) + if err != nil { + t.Fatalf("failed to create fake topology clientset: %v", err) + } + m.tClient = fakeClient + + // Reconcile p1. Since netns /proc/1/ns/net doesn't exist, ConfigurePodLinks will return an error opening netns, + // proving it proceeded to same-node veth plumbing rather than skipping or hanging on gRPC. + err = m.ReconcilePodLinks(context.Background(), p1) + if err == nil { + t.Fatalf("expected error opening non-existent netns during same-node plumbing, got nil") + } +} + + From f39f09cc0b1d7251b9f63701d9c5ccc541ce0491 Mon Sep 17 00:00:00 2001 From: Kris Raney Date: Wed, 5 Aug 2026 23:07:04 +0000 Subject: [PATCH 05/18] Handle container restarts that change the netns --- .../meshnet/daemon/grpcwire/grpcwire.go | 2 +- .../meshnet/daemon/grpcwire/gwire_recon.go | 1 - .../meshnet/daemon/meshnet/controller.go | 43 +++++++++++++++++ .../meshnet/daemon/meshnet/controller_test.go | 48 ++++++++++++++++--- 4 files changed, 85 insertions(+), 9 deletions(-) diff --git a/third_party/meshnet/daemon/grpcwire/grpcwire.go b/third_party/meshnet/daemon/grpcwire/grpcwire.go index 2e1eac6c..bc27bc80 100644 --- a/third_party/meshnet/daemon/grpcwire/grpcwire.go +++ b/third_party/meshnet/daemon/grpcwire/grpcwire.go @@ -16,7 +16,7 @@ import ( mpb "github.com/openconfig/kne/third_party/meshnet/daemon/proto/meshnet/v1beta1" ) -var grpcOvrlyLogger *log.Entry = nil +var grpcOvrlyLogger *log.Entry = log.WithFields(log.Fields{"daemon": "meshnetd", "overlay": "gRPC"}) // InitLogger initializes logrus logging for the gRPC overlay daemon. func InitLogger() { diff --git a/third_party/meshnet/daemon/grpcwire/gwire_recon.go b/third_party/meshnet/daemon/grpcwire/gwire_recon.go index df64f3fe..015eee69 100644 --- a/third_party/meshnet/daemon/grpcwire/gwire_recon.go +++ b/third_party/meshnet/daemon/grpcwire/gwire_recon.go @@ -408,7 +408,6 @@ func ReconGWires() error { return nil } - // ----------------------------------------------------------------------------------------------------------- // CreateGWireStatInDS creates grpc wire unstructured object with gvr info populated in it. func CreateGWireStatInDS(ctx context.Context, wStatus *grpcwirev1.GWireStatus) error { diff --git a/third_party/meshnet/daemon/meshnet/controller.go b/third_party/meshnet/daemon/meshnet/controller.go index 7ae05d83..ab76ba23 100644 --- a/third_party/meshnet/daemon/meshnet/controller.go +++ b/third_party/meshnet/daemon/meshnet/controller.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "os" "strings" "time" @@ -22,6 +23,36 @@ import ( "k8s.io/client-go/util/retry" ) +// isNetNSValid returns true if the netns path exists on the host filesystem. +func isNetNSValid(netNS string) bool { + if netNS == "" { + return false + } + _, err := os.Stat(netNS) + return err == nil +} + +// clearPodAliveStatus removes status.src_ip, status.net_ns, status.container_id, and status.plumbing_error +// from the Topology resource when a local pod container netns becomes invalid. +func (m *Meshnet) clearPodAliveStatus(ctx context.Context, topo *unstructured.Unstructured) error { + if m.tClient == nil { + return nil + } + return retry.RetryOnConflict(retry.DefaultRetry, func() error { + latestTopo, err := m.tClient.Topology(topo.GetNamespace()).Unstructured(ctx, topo.GetName(), metav1.GetOptions{}) + if err != nil { + return err + } + unstructured.RemoveNestedField(latestTopo.Object, "status", "src_ip") + unstructured.RemoveNestedField(latestTopo.Object, "status", "net_ns") + unstructured.RemoveNestedField(latestTopo.Object, "status", "container_id") + unstructured.RemoveNestedField(latestTopo.Object, "status", "plumbing_error") + + _, err = m.tClient.Topology(latestTopo.GetNamespace()).Update(ctx, latestTopo, metav1.UpdateOptions{}) + return err + }) +} + // toUnstructured converts any Kubernetes object into *unstructured.Unstructured. func toUnstructured(obj interface{}) (*unstructured.Unstructured, error) { if u, ok := obj.(*unstructured.Unstructured); ok { @@ -178,6 +209,18 @@ func (m *Meshnet) reconcilePodLinksInternal(ctx context.Context, topo *unstructu if topo == nil { return nil } + srcIP, _, _ := unstructured.NestedString(topo.Object, "status", "src_ip") + currentNetNS, _, _ := unstructured.NestedString(topo.Object, "status", "net_ns") + if srcIP != "" && m.nodeIP != "" && srcIP == m.nodeIP { + if currentNetNS != "" && !isNetNSValid(currentNetNS) { + mnetdLogger.Warnf("ReconcilePodLinks: local pod %s has stale netns path %s; clearing active status", topo.GetName(), currentNetNS) + _ = m.CleanupPodLinks(ctx, topo) + _ = grpcwire.DeletePodWires(topo.GetNamespace(), topo.GetName()) + _ = m.clearPodAliveStatus(ctx, topo) + return nil + } + } + srcIP, netNS, active := isPodActive(topo) if !active { return nil diff --git a/third_party/meshnet/daemon/meshnet/controller_test.go b/third_party/meshnet/daemon/meshnet/controller_test.go index 9885c590..6b339e99 100644 --- a/third_party/meshnet/daemon/meshnet/controller_test.go +++ b/third_party/meshnet/daemon/meshnet/controller_test.go @@ -5,6 +5,7 @@ import ( "testing" fakeTopology "github.com/openconfig/kne/third_party/meshnet/api/clientset/v1beta1/fake" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) @@ -156,8 +157,8 @@ func TestReconcilePodLinks_LowerPriorityPodInitiatesGRPC(t *testing.T) { } // Lower priority pod "a_pod" connected to peer "z_pod" on remote node "10.0.0.2" - lowerPrioPod := createFakePodTopology("a_pod", "default", "10.0.0.1", "/proc/100/ns/net", []string{"z_pod"}) - peerPod := createFakePodTopology("z_pod", "default", "10.0.0.2", "/proc/200/ns/net", []string{"a_pod"}) + lowerPrioPod := createFakePodTopology("a_pod", "default", "10.0.0.1", "/proc/self/ns/net", []string{"z_pod"}) + peerPod := createFakePodTopology("z_pod", "default", "10.0.0.2", "/proc/self/ns/net", []string{"a_pod"}) fakeClient, err := fakeTopology.NewSimpleClientset(lowerPrioPod, peerPod) if err != nil { @@ -181,18 +182,18 @@ func TestCleanupRemovedPodLinks_GRPC(t *testing.T) { } // Pod "p1" initially had 2 links (UID 1 and UID 2) - podWith2Links := createFakePodTopology("p1", "default", "10.0.0.1", "/proc/1/ns/net", []string{"p2", "p3"}) + podWith2Links := createFakePodTopology("p1", "default", "10.0.0.1", "/proc/self/ns/net", []string{"p2", "p3"}) desiredLinks, err := parsePodLinks(podWith2Links) if err != nil || len(desiredLinks) != 2 { t.Fatalf("failed to parse 2 links: %v", err) } // Now remove link UID 2 from spec.links (only UID 1 remains) - podWith1Link := createFakePodTopology("p1", "default", "10.0.0.1", "/proc/1/ns/net", []string{"p2"}) + podWith1Link := createFakePodTopology("p1", "default", "10.0.0.1", "/proc/self/ns/net", []string{"p2"}) desired1Link, _ := parsePodLinks(podWith1Link) // Call cleanupRemovedPodLinks with 1 link - m.cleanupRemovedPodLinks(context.Background(), podWith1Link, "/proc/1/ns/net", desired1Link) + m.cleanupRemovedPodLinks(context.Background(), podWith1Link, "/proc/self/ns/net", desired1Link) } func TestReconcilePodLinks_TransitionGRPCToSameNode(t *testing.T) { @@ -202,8 +203,8 @@ func TestReconcilePodLinks_TransitionGRPCToSameNode(t *testing.T) { } // Pod "p1" and "p2" both on same node "10.0.0.1" - p1 := createFakePodTopology("p1", "default", "10.0.0.1", "/proc/1/ns/net", []string{"p2"}) - p2 := createFakePodTopology("p2", "default", "10.0.0.1", "/proc/2/ns/net", []string{"p1"}) + p1 := createFakePodTopology("p1", "default", "10.0.0.1", "/proc/self/ns/net", []string{"p2"}) + p2 := createFakePodTopology("p2", "default", "10.0.0.1", "/proc/self/ns/net", []string{"p1"}) fakeClient, err := fakeTopology.NewSimpleClientset(p1, p2) if err != nil { @@ -219,5 +220,38 @@ func TestReconcilePodLinks_TransitionGRPCToSameNode(t *testing.T) { } } +func TestReconcilePodLinks_StaleLocalNetNSClearsStatus(t *testing.T) { + InitLogger() + m := &Meshnet{ + nodeIP: "10.0.0.1", + } + + // Local pod "p1" with a stale netns path "/nonexistent/netns/path" + p1 := createFakePodTopology("p1", "default", "10.0.0.1", "/nonexistent/netns/path", []string{"p2"}) + + fakeClient, err := fakeTopology.NewSimpleClientset(p1) + if err != nil { + t.Fatalf("failed to create fake topology clientset: %v", err) + } + m.tClient = fakeClient + + // Reconcile p1. Since /nonexistent/netns/path does not exist, reconcilePodLinksInternal should clear alive status and return nil cleanly. + err = m.ReconcilePodLinks(context.Background(), p1) + if err != nil { + t.Fatalf("expected nil return when clearing stale netns status, got: %v", err) + } + + // Verify status fields were cleared from fake K8s client + updatedP1, err := fakeClient.Topology("default").Unstructured(context.Background(), "p1", metav1.GetOptions{}) + if err != nil { + t.Fatalf("failed to fetch updated topology p1: %v", err) + } + _, _, active := isPodActive(updatedP1) + if active { + t.Fatalf("expected active to be false for p1 after clearing stale netns, got true") + } +} + + From f3712b607424cfedcfcc198f185389a38e5eb226 Mon Sep 17 00:00:00 2001 From: Kris Raney Date: Tue, 11 Aug 2026 17:35:57 +0000 Subject: [PATCH 06/18] Avoid thrashing that happens at startup as many pods start MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #### 1. In-Memory Topology Cache (TopologyCache) • Location: controller.go • Maintains a thread-safe in-memory cache of all Topology CRs, populated on controller startup and kept updated in real- time via Kubernetes watch events and local status updates (handler.go). • Resolves handler.go, controller.go, and controller.go from memory in O(1) time with fallback, eliminating the kube- apiserver List/Get request storm. #### 2. Inverted Peer Dependency Indexing & Targeted Reconciliation • Location: controller.go, controller.go • TopologyCache maintains an inverted dependency index (peerPod -> []dependentLocalPods). • When a pod X starts, restarts, or updates: • If X is on this node, X is queued for reconciliation. • All local pods Y that link to X (GetDependents) are identified in O(1) and queued for reconciliation. • Unrelated pods and nodes in the rest of the 1500-node cluster do zero reconciliation work. #### 3. Protection Against Post-Configure State Decay & Restarts • Peer Pod Restart / NetNS Re-creation: If peer pod B restarts (updating its net_ns, src_ip, or container_id), the watch event for B immediately triggers reconciliation for all dependent local pods (such as A). The veth link or gRPC wire is re-plumbed against B's new namespace. • Node Rescheduling: Node IP changes (same-node ↔ remote) trigger the transition logic to tear down stale links and create the new link type. • Safety Net Periodic Resync: A 60-second periodic resync runs in the background as a safety net against any missed watch events or delayed state convergence. #### 4. Coalescing and Debouncing Work Queue (ReconcileQueue) • Location: controller.go • Debounces incoming pod arrival bursts (50ms window) so that simultaneous pod starts coalesce into a single batched pass. --- .../meshnet/daemon/meshnet/controller.go | 434 ++++++++++++++++-- .../meshnet/daemon/meshnet/controller_test.go | 162 +++++++ third_party/meshnet/daemon/meshnet/handler.go | 14 +- third_party/meshnet/daemon/meshnet/meshnet.go | 5 + 4 files changed, 574 insertions(+), 41 deletions(-) diff --git a/third_party/meshnet/daemon/meshnet/controller.go b/third_party/meshnet/daemon/meshnet/controller.go index ab76ba23..5dd9a142 100644 --- a/third_party/meshnet/daemon/meshnet/controller.go +++ b/third_party/meshnet/daemon/meshnet/controller.go @@ -6,6 +6,7 @@ import ( "fmt" "os" "strings" + "sync" "time" "github.com/containernetworking/plugins/pkg/ns" @@ -49,6 +50,9 @@ func (m *Meshnet) clearPodAliveStatus(ctx context.Context, topo *unstructured.Un unstructured.RemoveNestedField(latestTopo.Object, "status", "plumbing_error") _, err = m.tClient.Topology(latestTopo.GetNamespace()).Update(ctx, latestTopo, metav1.UpdateOptions{}) + if err == nil && m.topoCache != nil { + m.topoCache.Put(latestTopo) + } return err }) } @@ -463,24 +467,262 @@ func (m *Meshnet) CleanupPodLinks(ctx context.Context, topo *unstructured.Unstru return nil } +// TopologyCache provides a thread-safe in-memory cache of Kubernetes Topology CRs +// and maintains an inverted dependency map (peerPod -> []localPods) for targeted reconciliation. +type TopologyCache struct { + mu sync.RWMutex + topos map[string]*unstructured.Unstructured // key: "namespace/name" + peerDeps map[string]map[string]bool // key: "namespace/peerPodName" -> set of "namespace/dependentPodName" +} + +// NewTopologyCache creates a new empty TopologyCache instance. +func NewTopologyCache() *TopologyCache { + return &TopologyCache{ + topos: make(map[string]*unstructured.Unstructured), + peerDeps: make(map[string]map[string]bool), + } +} + +// Put updates or inserts a Topology resource into the cache and indexes its link dependencies. +func (c *TopologyCache) Put(topo *unstructured.Unstructured) { + if c == nil || topo == nil { + return + } + c.mu.Lock() + defer c.mu.Unlock() + + ns := topo.GetNamespace() + name := topo.GetName() + key := fmt.Sprintf("%s/%s", ns, name) + + // Remove old peer dependencies registered by this pod if already in cache + if oldTopo, exists := c.topos[key]; exists { + oldLinks, _ := parsePodLinks(oldTopo) + for _, l := range oldLinks { + oldPeerKey := fmt.Sprintf("%s/%s", l.KubeNs, l.PeerPodName) + if deps, ok := c.peerDeps[oldPeerKey]; ok { + delete(deps, key) + if len(deps) == 0 { + delete(c.peerDeps, oldPeerKey) + } + } + } + } + + c.topos[key] = topo + + // Index new peer dependencies + links, _ := parsePodLinks(topo) + for _, l := range links { + peerKey := fmt.Sprintf("%s/%s", l.KubeNs, l.PeerPodName) + if c.peerDeps[peerKey] == nil { + c.peerDeps[peerKey] = make(map[string]bool) + } + c.peerDeps[peerKey][key] = true + } +} + +// Delete removes a Topology resource from the cache and cleans up its link dependencies. +func (c *TopologyCache) Delete(ns, name string) { + if c == nil { + return + } + c.mu.Lock() + defer c.mu.Unlock() + + key := fmt.Sprintf("%s/%s", ns, name) + if oldTopo, exists := c.topos[key]; exists { + oldLinks, _ := parsePodLinks(oldTopo) + for _, l := range oldLinks { + oldPeerKey := fmt.Sprintf("%s/%s", l.KubeNs, l.PeerPodName) + if deps, ok := c.peerDeps[oldPeerKey]; ok { + delete(deps, key) + if len(deps) == 0 { + delete(c.peerDeps, oldPeerKey) + } + } + } + delete(c.topos, key) + } +} + +// Get retrieves a Topology resource by namespace and name from the cache. +func (c *TopologyCache) Get(ns, name string) *unstructured.Unstructured { + if c == nil { + return nil + } + c.mu.RLock() + defer c.mu.RUnlock() + return c.topos[fmt.Sprintf("%s/%s", ns, name)] +} + +// List returns all cached Topology resources matching the given namespace (or all if namespace is empty or metav1.NamespaceAll). +func (c *TopologyCache) List(ns string) []*unstructured.Unstructured { + if c == nil { + return nil + } + c.mu.RLock() + defer c.mu.RUnlock() + + res := make([]*unstructured.Unstructured, 0, len(c.topos)) + for _, topo := range c.topos { + if ns == "" || ns == metav1.NamespaceAll || topo.GetNamespace() == ns { + res = append(res, topo) + } + } + return res +} + +// GetDependents returns all pod keys ("namespace/name") that declare links pointing to the given pod. +func (c *TopologyCache) GetDependents(ns, name string) []string { + if c == nil { + return nil + } + c.mu.RLock() + defer c.mu.RUnlock() + + key := fmt.Sprintf("%s/%s", ns, name) + deps := c.peerDeps[key] + if len(deps) == 0 { + return nil + } + result := make([]string, 0, len(deps)) + for depKey := range deps { + result = append(result, depKey) + } + return result +} + +// ReconcileQueue coalesces and debounces reconciliation requests for individual pods +// or full cluster sync passes. +type ReconcileQueue struct { + mu sync.Mutex + pending map[string]bool + fullPending bool + notifyChan chan struct{} + debounceDur time.Duration + timer *time.Timer +} + +// NewReconcileQueue creates a new ReconcileQueue with the specified debounce duration. +func NewReconcileQueue(debounceDur time.Duration) *ReconcileQueue { + if debounceDur <= 0 { + debounceDur = 50 * time.Millisecond + } + return &ReconcileQueue{ + pending: make(map[string]bool), + notifyChan: make(chan struct{}, 1), + debounceDur: debounceDur, + } +} + +// Enqueue adds a pod key ("namespace/name") to the pending reconciliation set and arms the debounce timer. +func (rq *ReconcileQueue) Enqueue(key string) { + if rq == nil { + return + } + rq.mu.Lock() + defer rq.mu.Unlock() + + rq.pending[key] = true + if rq.timer == nil { + rq.timer = time.AfterFunc(rq.debounceDur, func() { + select { + case rq.notifyChan <- struct{}{}: + default: + } + }) + } +} + +// EnqueueFull requests a full cluster reconciliation pass and arms the debounce timer. +func (rq *ReconcileQueue) EnqueueFull() { + if rq == nil { + return + } + rq.mu.Lock() + defer rq.mu.Unlock() + + rq.fullPending = true + if rq.timer == nil { + rq.timer = time.AfterFunc(rq.debounceDur, func() { + select { + case rq.notifyChan <- struct{}{}: + default: + } + }) + } +} + +// Drain clears and returns the currently queued reconciliation tasks. +func (rq *ReconcileQueue) Drain() (bool, []string) { + if rq == nil { + return true, nil + } + rq.mu.Lock() + defer rq.mu.Unlock() + + isFull := rq.fullPending + rq.fullPending = false + + keys := make([]string, 0, len(rq.pending)) + for k := range rq.pending { + keys = append(keys, k) + } + rq.pending = make(map[string]bool) + rq.timer = nil + + return isFull, keys +} + +func parseKey(key string) (string, string) { + parts := strings.SplitN(key, "/", 2) + if len(parts) == 2 { + return parts[0], parts[1] + } + return "", parts[0] +} + +func (m *Meshnet) enqueueReconcile(key string) { + if m.reconcileQueue != nil { + m.reconcileQueue.Enqueue(key) + } else { + m.triggerReconcile() + } +} + +func (m *Meshnet) enqueueFullReconcile() { + if m.reconcileQueue != nil { + m.reconcileQueue.EnqueueFull() + } else { + m.triggerReconcile() + } +} + // CleanupOrphanedHostVeths scans the host network namespace for any temporary host veths ("vnm-...") // that do not match any link in any currently existing Topology resource and deletes them. // This cleans up partial veths left behind by topologies that were deleted while meshnetd was offline. func (m *Meshnet) CleanupOrphanedHostVeths(ctx context.Context) error { - if m.tClient == nil { - return nil + var topos []*unstructured.Unstructured + if m.topoCache != nil { + topos = m.topoCache.List(metav1.NamespaceAll) } - list, err := m.tClient.Topology(metav1.NamespaceAll).List(ctx, metav1.ListOptions{}) - if err != nil { - return err + if len(topos) == 0 && m.tClient != nil { + list, err := m.tClient.Topology(metav1.NamespaceAll).List(ctx, metav1.ListOptions{}) + if err != nil { + return err + } + for i := range list.Items { + u, err := toUnstructured(&list.Items[i]) + if err != nil { + continue + } + topos = append(topos, u) + } } validHostNames := make(map[string]bool) - for i := range list.Items { - u, err := toUnstructured(&list.Items[i]) - if err != nil { - continue - } + for _, u := range topos { links, _ := parsePodLinks(u) for _, l := range links { side0, side1 := wireutil.HostVethNames(l.KubeNs, l.PodName, l.PeerPodName, l.LinkUID) @@ -506,26 +748,36 @@ func (m *Meshnet) CleanupOrphanedHostVeths(ctx context.Context) error { // ReconcileAllLocalPods scans all Topology resources and reconciles any active local pod scheduled on this node. func (m *Meshnet) ReconcileAllLocalPods(ctx context.Context) error { - if m.tClient == nil { - return nil + var topos []*unstructured.Unstructured + if m.topoCache != nil { + topos = m.topoCache.List(metav1.NamespaceAll) } - list, err := m.tClient.Topology(metav1.NamespaceAll).List(ctx, metav1.ListOptions{}) - if err != nil { - return err - } - for i := range list.Items { - u, err := toUnstructured(&list.Items[i]) + if len(topos) == 0 && m.tClient != nil { + list, err := m.tClient.Topology(metav1.NamespaceAll).List(ctx, metav1.ListOptions{}) if err != nil { - continue + return err + } + for i := range list.Items { + u, err := toUnstructured(&list.Items[i]) + if err != nil { + continue + } + topos = append(topos, u) } + } + for _, u := range topos { _ = m.ReconcilePodLinks(ctx, u) } return nil } -// triggerReconcile sets the dirty token in dirtyChan, coalescing multiple triggers -// into at most one pending reconciliation run. +// triggerReconcile sets the dirty token or triggers a full reconciliation pass, +// maintaining backward compatibility with callers. func (m *Meshnet) triggerReconcile() { + if m.reconcileQueue != nil { + m.reconcileQueue.EnqueueFull() + return + } if m.dirtyChan == nil { return } @@ -537,13 +789,31 @@ func (m *Meshnet) triggerReconcile() { } // runReconcileWorker runs in the background and coalesces incoming reconcile triggers. -// When a trigger is received, it executes a full local reconciliation pass. Any triggers that -// arrive while reconciliation is in progress coalesce into a single follow-up pass. +// When a trigger is received, it executes a targeted or full local reconciliation pass. func (m *Meshnet) runReconcileWorker(ctx context.Context) { + if m.reconcileQueue == nil { + m.reconcileQueue = NewReconcileQueue(50 * time.Millisecond) + } + for { select { case <-ctx.Done(): return + case <-m.reconcileQueue.notifyChan: + isFull, keys := m.reconcileQueue.Drain() + if isFull { + _ = m.CleanupOrphanedHostVeths(ctx) + _ = m.ReconcileAllLocalPods(ctx) + } else { + for _, key := range keys { + ns, name := parseKey(key) + topo, err := m.getPod(ctx, name, ns) + if err != nil || topo == nil { + continue + } + _ = m.ReconcilePodLinks(ctx, topo) + } + } case <-m.dirtyChan: _ = m.CleanupOrphanedHostVeths(ctx) _ = m.ReconcileAllLocalPods(ctx) @@ -552,12 +822,39 @@ func (m *Meshnet) runReconcileWorker(ctx context.Context) { } // RunControllerLoop runs the continuous level-triggered Topology controller in meshnetd. -// It watches for resource changes across namespaces and coalesces incoming events into -// background reconciliation runs. +// It maintains an in-memory topology cache, tracks pod-link dependencies, and coalesces +// incoming events into targeted background reconciliation runs. func (m *Meshnet) RunControllerLoop(ctx context.Context) { mnetdLogger.Infof("Starting Topology controller loop") + if m.topoCache == nil { + m.topoCache = NewTopologyCache() + } + if m.reconcileQueue == nil { + m.reconcileQueue = NewReconcileQueue(50 * time.Millisecond) + } + + // 1. Initial full population of cache from K8s API + if m.tClient != nil { + list, err := m.tClient.Topology(metav1.NamespaceAll).List(ctx, metav1.ListOptions{}) + if err == nil && list != nil { + for i := range list.Items { + if u, err := toUnstructured(&list.Items[i]); err == nil && u != nil { + m.topoCache.Put(u) + } + } + } else if err != nil { + mnetdLogger.Warnf("RunControllerLoop: initial list failed: %v", err) + } + } + go m.runReconcileWorker(ctx) - m.triggerReconcile() + + // Trigger initial full reconciliation on startup + m.enqueueFullReconcile() + + // Periodic resync ticker (every 60s) as a safety net against missed watch events or state decay + resyncTicker := time.NewTicker(60 * time.Second) + defer resyncTicker.Stop() for { if ctx.Err() != nil { @@ -570,24 +867,78 @@ func (m *Meshnet) RunControllerLoop(ctx context.Context) { continue } - for event := range watcher.ResultChan() { - if ctx.Err() != nil { + watchCh := watcher.ResultChan() + loopDone := false + for !loopDone { + select { + case <-ctx.Done(): watcher.Stop() return - } - topo, err := toUnstructured(event.Object) - if err != nil || topo == nil { - continue - } - switch event.Type { - case watch.Added, watch.Modified: - m.triggerReconcile() - case watch.Deleted: - _ = m.CleanupPodLinks(ctx, topo) - m.triggerReconcile() + case <-resyncTicker.C: + m.enqueueFullReconcile() + case event, ok := <-watchCh: + if !ok { + loopDone = true + break + } + topo, err := toUnstructured(event.Object) + if err != nil || topo == nil { + continue + } + + ns := topo.GetNamespace() + name := topo.GetName() + key := fmt.Sprintf("%s/%s", ns, name) + + switch event.Type { + case watch.Added, watch.Modified: + // Update cache + m.topoCache.Put(topo) + + // Check if this pod is local to this node + srcIP, _, active := isPodActive(topo) + isLocal := active && (m.nodeIP == "" || srcIP == m.nodeIP) + if isLocal { + m.enqueueReconcile(key) + } + + // Find all local dependent pods that have links to this pod + dependents := m.topoCache.GetDependents(ns, name) + for _, depKey := range dependents { + depNS, depName := parseKey(depKey) + depTopo := m.topoCache.Get(depNS, depName) + if depTopo != nil { + depSrcIP, _, depActive := isPodActive(depTopo) + if depActive && (m.nodeIP == "" || depSrcIP == m.nodeIP) { + m.enqueueReconcile(depKey) + } + } + } + + case watch.Deleted: + // Before removing from cache, get all dependent pods + dependents := m.topoCache.GetDependents(ns, name) + m.topoCache.Delete(ns, name) + + // If this was a local pod, clean up its links + _ = m.CleanupPodLinks(ctx, topo) + + // Reconcile dependent pods so they update / clean up their link state + for _, depKey := range dependents { + depNS, depName := parseKey(depKey) + depTopo := m.topoCache.Get(depNS, depName) + if depTopo != nil { + depSrcIP, _, depActive := isPodActive(depTopo) + if depActive && (m.nodeIP == "" || depSrcIP == m.nodeIP) { + m.enqueueReconcile(depKey) + } + } + } + } } } - m.triggerReconcile() + // If watch closed, queue full resync on reconnect + m.enqueueFullReconcile() } } @@ -613,6 +964,9 @@ func (m *Meshnet) updatePlumbingErrorStatus(ctx context.Context, topo *unstructu } _, err = m.tClient.Topology(latestTopo.GetNamespace()).Update(ctx, latestTopo, metav1.UpdateOptions{}) + if err == nil && m.topoCache != nil { + m.topoCache.Put(latestTopo) + } return err }) diff --git a/third_party/meshnet/daemon/meshnet/controller_test.go b/third_party/meshnet/daemon/meshnet/controller_test.go index 6b339e99..5c9a1c43 100644 --- a/third_party/meshnet/daemon/meshnet/controller_test.go +++ b/third_party/meshnet/daemon/meshnet/controller_test.go @@ -3,6 +3,7 @@ package meshnet import ( "context" "testing" + "time" fakeTopology "github.com/openconfig/kne/third_party/meshnet/api/clientset/v1beta1/fake" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -252,6 +253,167 @@ func TestReconcilePodLinks_StaleLocalNetNSClearsStatus(t *testing.T) { } } +func TestTopologyCache_PutGetListDelete(t *testing.T) { + cache := NewTopologyCache() + + p1 := createFakePodTopology("p1", "default", "10.0.0.1", "/proc/1/ns/net", []string{"p2"}) + p2 := createFakePodTopology("p2", "default", "10.0.0.2", "/proc/2/ns/net", []string{"p1"}) + p3 := createFakePodTopology("p3", "other-ns", "10.0.0.3", "/proc/3/ns/net", []string{"p4"}) + + cache.Put(p1) + cache.Put(p2) + cache.Put(p3) + + // Get tests + if got := cache.Get("default", "p1"); got == nil || got.GetName() != "p1" { + t.Fatalf("expected p1 in cache, got %+v", got) + } + if got := cache.Get("default", "nonexistent"); got != nil { + t.Fatalf("expected nil for nonexistent pod, got %+v", got) + } + + // List tests + defaultList := cache.List("default") + if len(defaultList) != 2 { + t.Fatalf("expected 2 topologies in default namespace, got %d", len(defaultList)) + } + allList := cache.List("") + if len(allList) != 3 { + t.Fatalf("expected 3 topologies across all namespaces, got %d", len(allList)) + } + + // Delete test + cache.Delete("default", "p1") + if got := cache.Get("default", "p1"); got != nil { + t.Fatalf("expected nil after delete, got %+v", got) + } + if len(cache.List("default")) != 1 { + t.Fatalf("expected 1 topology remaining in default namespace, got %d", len(cache.List("default"))) + } +} + +func TestTopologyCache_DependencyTracking(t *testing.T) { + cache := NewTopologyCache() + + // p1 links to p2 and p3 + p1 := createFakePodTopology("p1", "default", "10.0.0.1", "/proc/1/ns/net", []string{"p2", "p3"}) + // p4 also links to p2 + p4 := createFakePodTopology("p4", "default", "10.0.0.4", "/proc/4/ns/net", []string{"p2"}) + + cache.Put(p1) + cache.Put(p4) + + // Check dependents for p2: should be p1 and p4 + depsP2 := cache.GetDependents("default", "p2") + if len(depsP2) != 2 { + t.Fatalf("expected 2 dependents for p2, got %d: %+v", len(depsP2), depsP2) + } + + // Check dependents for p3: should be only p1 + depsP3 := cache.GetDependents("default", "p3") + if len(depsP3) != 1 || depsP3[0] != "default/p1" { + t.Fatalf("expected [default/p1] for p3, got %+v", depsP3) + } + + // Now delete p1: dependents for p2 should now only be p4, and p3 should have none + cache.Delete("default", "p1") + depsP2After := cache.GetDependents("default", "p2") + if len(depsP2After) != 1 || depsP2After[0] != "default/p4" { + t.Fatalf("expected [default/p4] for p2 after p1 deletion, got %+v", depsP2After) + } + depsP3After := cache.GetDependents("default", "p3") + if len(depsP3After) != 0 { + t.Fatalf("expected 0 dependents for p3 after p1 deletion, got %+v", depsP3After) + } +} + +func TestReconcileQueue_DebounceAndDrain(t *testing.T) { + rq := NewReconcileQueue(20 * time.Millisecond) + + // Enqueue multiple pod keys in rapid succession + rq.Enqueue("default/p1") + rq.Enqueue("default/p2") + rq.Enqueue("default/p1") // duplicate + rq.Enqueue("default/p3") + + // Wait for debounce notification + select { + case <-rq.notifyChan: + case <-time.After(200 * time.Millisecond): + t.Fatalf("timed out waiting for reconcile queue notifyChan") + } + + isFull, keys := rq.Drain() + if isFull { + t.Fatalf("expected isFull=false for targeted enqueue, got true") + } + if len(keys) != 3 { + t.Fatalf("expected 3 unique keys, got %d: %+v", len(keys), keys) + } + + // Test EnqueueFull + rq.EnqueueFull() + select { + case <-rq.notifyChan: + case <-time.After(200 * time.Millisecond): + t.Fatalf("timed out waiting for reconcile queue notifyChan on full reconcile") + } + + isFull, _ = rq.Drain() + if !isFull { + t.Fatalf("expected isFull=true after EnqueueFull, got false") + } +} + +func TestTargetedReconciliation_PeerRestartQueuesDependents(t *testing.T) { + InitLogger() + m := &Meshnet{ + nodeIP: "10.0.0.1", + topoCache: NewTopologyCache(), + reconcileQueue: NewReconcileQueue(10 * time.Millisecond), + } + + // Local pod "p1" on node 10.0.0.1 links to remote peer "p2" on node 10.0.0.2 + p1 := createFakePodTopology("p1", "default", "10.0.0.1", "/proc/self/ns/net", []string{"p2"}) + // Unrelated local pod "p3" on node 10.0.0.1 links to "p4" on node 10.0.0.3 + p3 := createFakePodTopology("p3", "default", "10.0.0.1", "/proc/self/ns/net", []string{"p4"}) + + m.topoCache.Put(p1) + m.topoCache.Put(p3) + + // Remote peer "p2" restarts and updates its status + p2Updated := createFakePodTopology("p2", "default", "10.0.0.2", "/proc/999/ns/net", []string{"p1"}) + m.topoCache.Put(p2Updated) + + // When p2 event arrives, controller queries dependents + dependents := m.topoCache.GetDependents("default", "p2") + for _, depKey := range dependents { + depNS, depName := parseKey(depKey) + depTopo := m.topoCache.Get(depNS, depName) + if depTopo != nil { + depSrcIP, _, depActive := isPodActive(depTopo) + if depActive && (m.nodeIP == "" || depSrcIP == m.nodeIP) { + m.enqueueReconcile(depKey) + } + } + } + + // Wait for debounce notification + select { + case <-m.reconcileQueue.notifyChan: + case <-time.After(200 * time.Millisecond): + t.Fatalf("timed out waiting for reconcile queue notification") + } + + isFull, keys := m.reconcileQueue.Drain() + if isFull { + t.Fatalf("expected targeted reconcile, got full") + } + if len(keys) != 1 || keys[0] != "default/p1" { + t.Fatalf("expected only dependent local pod default/p1 to be queued, got %+v", keys) + } +} + diff --git a/third_party/meshnet/daemon/meshnet/handler.go b/third_party/meshnet/daemon/meshnet/handler.go index 5df49bf7..c8a29c88 100644 --- a/third_party/meshnet/daemon/meshnet/handler.go +++ b/third_party/meshnet/daemon/meshnet/handler.go @@ -20,11 +20,20 @@ import ( ) func (m *Meshnet) getPod(ctx context.Context, name, ns string) (*unstructured.Unstructured, error) { + if m.topoCache != nil { + if topo := m.topoCache.Get(ns, name); topo != nil { + return topo, nil + } + } if m.tClient == nil { return nil, fmt.Errorf("topology client not initialized") } mnetdLogger.Debugf("Reading pod %s from K8s", name) - return m.tClient.Topology(ns).Unstructured(ctx, name, metav1.GetOptions{}) + topo, err := m.tClient.Topology(ns).Unstructured(ctx, name, metav1.GetOptions{}) + if err == nil && topo != nil && m.topoCache != nil { + m.topoCache.Put(topo) + } + return topo, err } func (m *Meshnet) updateStatus(ctx context.Context, obj *unstructured.Unstructured, ns string) error { @@ -33,6 +42,9 @@ func (m *Meshnet) updateStatus(ctx context.Context, obj *unstructured.Unstructur } mnetdLogger.Infof("Update pod status %s from K8s", obj.GetName()) _, err := m.tClient.Topology(ns).Update(ctx, obj, metav1.UpdateOptions{}) + if err == nil && m.topoCache != nil { + m.topoCache.Put(obj) + } return err } diff --git a/third_party/meshnet/daemon/meshnet/meshnet.go b/third_party/meshnet/daemon/meshnet/meshnet.go index e31db49d..8a1e646d 100644 --- a/third_party/meshnet/daemon/meshnet/meshnet.go +++ b/third_party/meshnet/daemon/meshnet/meshnet.go @@ -8,6 +8,7 @@ import ( "net" "os" "path/filepath" + "time" grpc_middleware "github.com/grpc-ecosystem/go-grpc-middleware" grpc_ctxtags "github.com/grpc-ecosystem/go-grpc-middleware/tags" @@ -50,6 +51,8 @@ type Meshnet struct { nodeIP string dirtyChan chan struct{} interNodeLinkType string + topoCache *TopologyCache + reconcileQueue *ReconcileQueue } var mnetdLogger *log.Entry = nil @@ -136,6 +139,8 @@ func New(cfg Config) (*Meshnet, error) { nodeIP: os.Getenv("HOST_IP"), dirtyChan: make(chan struct{}, 1), interNodeLinkType: lnkTyp, + topoCache: NewTopologyCache(), + reconcileQueue: NewReconcileQueue(50 * time.Millisecond), } mpb.RegisterLocalServer(m.s, m) mpb.RegisterRemoteServer(m.s, m) From 931e65bf7c1cf86b4af51f5b5db554ef1ae6e03d Mon Sep 17 00:00:00 2001 From: Kris Raney Date: Tue, 11 Aug 2026 18:38:06 +0000 Subject: [PATCH 07/18] Fix race conditions --- third_party/meshnet/daemon/grpcwire/grpcwire.go | 9 +++++++-- third_party/meshnet/daemon/grpcwire/gwire_recon.go | 2 +- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/third_party/meshnet/daemon/grpcwire/grpcwire.go b/third_party/meshnet/daemon/grpcwire/grpcwire.go index bc27bc80..7cea8128 100644 --- a/third_party/meshnet/daemon/grpcwire/grpcwire.go +++ b/third_party/meshnet/daemon/grpcwire/grpcwire.go @@ -16,11 +16,16 @@ import ( mpb "github.com/openconfig/kne/third_party/meshnet/daemon/proto/meshnet/v1beta1" ) -var grpcOvrlyLogger *log.Entry = log.WithFields(log.Fields{"daemon": "meshnetd", "overlay": "gRPC"}) +var ( + grpcOvrlyLogger *log.Entry = log.WithFields(log.Fields{"daemon": "meshnetd", "overlay": "gRPC"}) + initLoggerOnce sync.Once +) // InitLogger initializes logrus logging for the gRPC overlay daemon. func InitLogger() { - grpcOvrlyLogger = log.WithFields(log.Fields{"daemon": "meshnetd", "overlay": "gRPC"}) + initLoggerOnce.Do(func() { + grpcOvrlyLogger = log.WithFields(log.Fields{"daemon": "meshnetd", "overlay": "gRPC"}) + }) } var packetPool = sync.Pool{ diff --git a/third_party/meshnet/daemon/grpcwire/gwire_recon.go b/third_party/meshnet/daemon/grpcwire/gwire_recon.go index 015eee69..857eadb2 100644 --- a/third_party/meshnet/daemon/grpcwire/gwire_recon.go +++ b/third_party/meshnet/daemon/grpcwire/gwire_recon.go @@ -66,7 +66,7 @@ func SetGWireClientInterface(gClient dynamic.NamespaceableResourceInterface) { func (gc *GWireClient) GetWireObjListUS(ctx context.Context, ndName string) (*unstructured.UnstructuredList, error) { di := gc.getDI() if di == nil { - return nil, errors.New("gWClient dynamic interface is nil") + return nil, fmt.Errorf("gwire client interface not initialized") } return di.Namespace("").List(ctx, metav1.ListOptions{ TypeMeta: metav1.TypeMeta{ From ff96574f69de57f3685204cded36fe8d609477f5 Mon Sep 17 00:00:00 2001 From: Kris Raney Date: Tue, 11 Aug 2026 19:05:13 +0000 Subject: [PATCH 08/18] Robustness fixes: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### 1. Per-RPC Timeout on Remote Handshakes • Locations: controller.go & controller.go • Changes: Wrapped remoteClient.AddGRPCWiresRemoteBatch and remoteClient.GRPCWireDownRemote with a 5*time.Second context timeout (context. WithTimeout(ctx, 5*time.Second)). • Benefit: If a remote worker node is slow, booting, or dropping packets, the call quickly times out and fails gracefully rather than blocking the single-threaded local reconciler loop indefinitely. ────── ### 2. Prevent Duplicate RecvFrmLocalPodThread Spawn • Locations: gwire_rpc_handlers.go & handler.go • Changes: 1. Made gwire_rpc_handlers.go check if the wire is already in memory before creating a new TAP handle or spawning a reader thread. 2. Updated gwire_rpc_handlers.go to return an isNew boolean flag. 3. In AddGRPCWireRemote and AddGRPCWiresRemoteBatch, only spawn go RecvFrmLocalPodThread if isNew == true. • Benefit: When both peer nodes reconcile simultaneously and exchange remote wire creation RPCs, existing TAP handles will not have multiple competing reader goroutines spawned on /dev/net/tun. ────── ### 3. Mitigate Thundering Herd Polling in CNI cmdAdd • Location: meshnet.go • Changes: 1. Batch NetNS Lookup: Discovers all container interfaces in a single netns switch per polling tick instead of executing a netns switch per link. 2. Peer Node IP Cache: Caches the peer pod node IP to avoid querying meshnetClient.Get on every 100ms tick for each link. 3. Exponential Backoff: Replaced the fixed 100ms ticker with an exponential backoff (starting at 100ms and backing off to 1s). 4. Nil-check: Added a peerPod != nil guard in cmdDel to prevent panics during cascading teardowns. --- .../daemon/grpcwire/gwire_rpc_handlers.go | 10 ++++ .../meshnet/daemon/meshnet/controller.go | 8 ++- third_party/meshnet/plugin/meshnet.go | 53 +++++++++++++------ 3 files changed, 52 insertions(+), 19 deletions(-) diff --git a/third_party/meshnet/daemon/grpcwire/gwire_rpc_handlers.go b/third_party/meshnet/daemon/grpcwire/gwire_rpc_handlers.go index 77a45cdd..648f988f 100644 --- a/third_party/meshnet/daemon/grpcwire/gwire_rpc_handlers.go +++ b/third_party/meshnet/daemon/grpcwire/gwire_rpc_handlers.go @@ -10,6 +10,16 @@ import ( ) func CreateGRPCWireLocal(ctx context.Context, wireDef *mpb.WireDef) (*mpb.BoolResponse, error) { + if wire, ok := GetWireByUID(wireDef.LocalPodNetNs, int(wireDef.LinkUid)); ok && wire != nil { + wire.mu.Lock() + wire.Originator = HOST_CREATED_WIRE + if wireDef.PeerNodeIp != "" { + wire.PeerNodeIP = wireDef.PeerNodeIp + } + wire.mu.Unlock() + return &mpb.BoolResponse{Response: true}, nil + } + tapFile, err := wireutil.CreateOrAttachTAP(wireDef.LocalPodNetNs, wireDef.IntfNameInPod, wireDef.LocalPodIp) if err != nil { log.WithFields(log.Fields{ diff --git a/third_party/meshnet/daemon/meshnet/controller.go b/third_party/meshnet/daemon/meshnet/controller.go index 5dd9a142..291cf420 100644 --- a/third_party/meshnet/daemon/meshnet/controller.go +++ b/third_party/meshnet/daemon/meshnet/controller.go @@ -173,12 +173,14 @@ func (m *Meshnet) cleanupRemovedPodLinks(ctx context.Context, topo *unstructured url = strings.TrimSpace(url) if remoteConn, err := grpc.Dial(url, grpc.WithTransportCredentials(insecure.NewCredentials())); err == nil { remoteClient := mpb.NewRemoteClient(remoteConn) - _, _ = remoteClient.GRPCWireDownRemote(ctx, &mpb.WireDef{ + rpcCtx, rpcCancel := context.WithTimeout(ctx, 5*time.Second) + _, _ = remoteClient.GRPCWireDownRemote(rpcCtx, &mpb.WireDef{ TopoNs: wire.TopoNamespace, LocalPodName: wire.LocalPodName, LocalPodNetNs: wire.LocalPodNetNS, LinkUid: int64(wire.UID), }) + rpcCancel() remoteConn.Close() } } @@ -402,7 +404,9 @@ func (m *Meshnet) reconcilePodLinksInternal(ctx context.Context, topo *unstructu chunkLinks := pBatch.links[i:end] mnetdLogger.Infof("ReconcilePodLinks: calling AddGRPCWiresRemoteBatch on %s for batch [%d:%d] of %d links", url, i, end, total) - batchResp, err := remoteClient.AddGRPCWiresRemoteBatch(ctx, &mpb.WireDefBatch{Items: chunkWireDefs}) + rpcCtx, rpcCancel := context.WithTimeout(ctx, 5*time.Second) + batchResp, err := remoteClient.AddGRPCWiresRemoteBatch(rpcCtx, &mpb.WireDefBatch{Items: chunkWireDefs}) + rpcCancel() if err != nil { remoteConn.Close() mnetdLogger.Errorf("ReconcilePodLinks: AddGRPCWiresRemoteBatch failed to %s for batch [%d:%d]: %v", url, i, end, err) diff --git a/third_party/meshnet/plugin/meshnet.go b/third_party/meshnet/plugin/meshnet.go index 17337852..25a41de1 100644 --- a/third_party/meshnet/plugin/meshnet.go +++ b/third_party/meshnet/plugin/meshnet.go @@ -211,31 +211,46 @@ func cmdAdd(args *skel.CmdArgs) error { waitCtx, cancel := context.WithDeadline(ctx, startTime.Add(30*time.Second)) defer cancel() - ticker := time.NewTicker(100 * time.Millisecond) - defer ticker.Stop() + pollInterval := 100 * time.Millisecond + maxInterval := 1 * time.Second + peerNodeCache := make(map[string]string) for { allAreReady := true - for _, link := range localPod.Links { - // Check if interface exists in container netns - _ = ns.WithNetNSPath(args.Netns, func(_ ns.NetNS) error { - if _, err := netlink.LinkByName(link.LocalIntf); err != nil { - allAreReady = false + + // Check all interfaces in container netns in a single netns switch + presentIntfs := make(map[string]bool) + _ = ns.WithNetNSPath(args.Netns, func(_ ns.NetNS) error { + if list, err := netlink.LinkList(); err == nil { + for _, l := range list { + presentIntfs[l.Attrs().Name] = true } - return nil - }) - if !allAreReady { + } + return nil + }) + + for _, link := range localPod.Links { + if !presentIntfs[link.LocalIntf] { + allAreReady = false break } // For inter-node gRPC links, check if the gRPC wire is fully established on the daemon if interNodeLinkType == wireutil.INTER_NODE_LINK_GRPC { - peerPod, err := meshnetClient.Get(ctx, &mpb.PodQuery{ - Name: link.PeerPod, - KubeNs: string(cniArgs.K8S_POD_NAMESPACE), - }) + peerSrcIP, cached := peerNodeCache[link.PeerPod] + if !cached { + peerPod, err := meshnetClient.Get(ctx, &mpb.PodQuery{ + Name: link.PeerPod, + KubeNs: string(cniArgs.K8S_POD_NAMESPACE), + }) + if err == nil && peerPod != nil && peerPod.SrcIp != "" { + peerSrcIP = peerPod.SrcIp + peerNodeCache[link.PeerPod] = peerSrcIP + } + } + // Only check gRPC wire readiness if peer is on a different node - if err == nil && peerPod != nil && peerPod.SrcIp != "" && peerPod.SrcIp != localPod.SrcIp { + if peerSrcIP != "" && peerSrcIP != localPod.SrcIp { wireDef := &mpb.WireDef{ LocalPodNetNs: args.Netns, LinkUid: link.Uid, @@ -258,7 +273,11 @@ func cmdAdd(args *skel.CmdArgs) error { case <-waitCtx.Done(): log.Warnf("Add[%s]: Readiness wait timed out (%d links); proceeding asynchronously", string(cniArgs.K8S_POD_NAME), len(localPod.Links)) goto WaitDone - case <-ticker.C: + case <-time.After(pollInterval): + pollInterval = time.Duration(float64(pollInterval) * 1.5) + if pollInterval > maxInterval { + pollInterval = maxInterval + } } } } @@ -347,7 +366,7 @@ func cmdDel(args *skel.CmdArgs) error { KubeNs: string(cniArgs.K8S_POD_NAMESPACE), }) - if peerPod.SrcIp != localPodSrcIp { + if peerPod != nil && peerPod.SrcIp != localPodSrcIp { // they are on different hosts if interNodeLinkType == wireutil.INTER_NODE_LINK_GRPC { // for this link bring the grpc wire down From 6494b31dd98508926a5ca09e79a36cf8238d2156 Mon Sep 17 00:00:00 2001 From: Kris Raney Date: Tue, 11 Aug 2026 20:36:47 +0000 Subject: [PATCH 09/18] Robustness changes for post-setup decay 1. Added streamMgr.Send(topoNs, peerIP, pkt) to route packets to the multiplexed stream corresponding to (topoNs, peerIP). 2. In grpcwire.go, when a peer pod is reassigned to a different node (wire.PeerNodeIP != newPeerIP), the old stream reference is cleanly released with streamMgr.ReleaseStream and the new stream is registered with streamMgr.GetOrCreateStream. 3. In grpcwire.go, packets dynamically read the current wire.PeerNodeIP under lock, ensuring outgoing packets instantly route to the new node without blackholing traffic or corrupting stream refcounts. --- .../meshnet/daemon/grpcwire/grpcwire.go | 44 ++++++++++++++++--- .../meshnet/daemon/grpcwire/stream_manager.go | 30 +++++++++++++ 2 files changed, 68 insertions(+), 6 deletions(-) diff --git a/third_party/meshnet/daemon/grpcwire/grpcwire.go b/third_party/meshnet/daemon/grpcwire/grpcwire.go index 7cea8128..fe51db64 100644 --- a/third_party/meshnet/daemon/grpcwire/grpcwire.go +++ b/third_party/meshnet/daemon/grpcwire/grpcwire.go @@ -148,7 +148,13 @@ func (wire *GRPCWire) UpdateWire(peerIntfId int64, peerNodeIP string, stopC chan } } wire.WireIfaceIDOnPeerNode = peerIntfId - if peerNodeIP != "" { + if peerNodeIP != "" && wire.PeerNodeIP != peerNodeIP { + if wire.PeerNodeIP != "" { + streamMgr.ReleaseStream(wire.TopoNamespace, wire.PeerNodeIP) + } + wire.PeerNodeIP = peerNodeIP + _ = streamMgr.GetOrCreateStream(wire.TopoNamespace, peerNodeIP) + } else if peerNodeIP != "" { wire.PeerNodeIP = peerNodeIP } wire.IsReady = true @@ -309,10 +315,25 @@ func RecvFrmLocalPodThread(wire *GRPCWire, locIfNm string) error { return err } - nodeStream := streamMgr.GetOrCreateStream(wire.TopoNamespace, wire.PeerNodeIP) - defer streamMgr.ReleaseStream(wire.TopoNamespace, wire.PeerNodeIP) + wire.mu.Lock() + peerIP := wire.PeerNodeIP + topoNs := wire.TopoNamespace + wire.mu.Unlock() - return forwardPackets(tapFile, nodeStream, wire, locIfNm) + if peerIP != "" { + _ = streamMgr.GetOrCreateStream(topoNs, peerIP) + defer func() { + wire.mu.Lock() + currIP := wire.PeerNodeIP + currNs := wire.TopoNamespace + wire.mu.Unlock() + if currIP != "" { + streamMgr.ReleaseStream(currNs, currIP) + } + }() + } + + return forwardPackets(tapFile, nil, wire, locIfNm) } func forwardPackets(reader io.Reader, sender packetSender, wire *GRPCWire, locIfNm string) error { @@ -343,6 +364,9 @@ func forwardPackets(reader io.Reader, sender packetSender, wire *GRPCWire, locIf case <-wire.StopC: grpcOvrlyLogger.Infof("RecvFrmLocalPodThread: closing connection with remote peer-iface@peer-node-ip: %d@%s/%d from %s@%s", wire.WireIfaceIDOnPeerNode, wire.PeerNodeIP, wire.LocalNodeIfaceID, wire.LocalPodName, wire.LocalPodIfaceName) + if closer, ok := reader.(io.Closer); ok { + _ = closer.Close() + } return io.EOF case res := <-readChan: bufPtr := res.buf @@ -371,9 +395,11 @@ func forwardPackets(reader io.Reader, sender packetSender, wire *GRPCWire, locIf wire.mu.Lock() isReady := wire.IsReady peerIntfID := wire.WireIfaceIDOnPeerNode + peerNodeIP := wire.PeerNodeIP + topoNs := wire.TopoNamespace wire.mu.Unlock() - if !isReady || peerIntfID <= 0 { + if !isReady || peerIntfID <= 0 || peerNodeIP == "" { // Remote peer handshake is still in progress; skip sending to unassigned wire ID 0 packetPool.Put(bufPtr) continue @@ -389,7 +415,13 @@ func forwardPackets(reader io.Reader, sender packetSender, wire *GRPCWire, locIf grpcOvrlyLogger.Infof("RecvFrmLocalPodThread: unusually large packet received from local pod (may be GRO enabled). size: %d, pkt:%s", n, pktType) } - if !sender.Send(payload) { + sent := false + if sender != nil { + sent = sender.Send(payload) + } else { + sent = streamMgr.Send(topoNs, peerNodeIP, payload) + } + if !sent { grpcOvrlyLogger.Debugf("RecvFrmLocalPodThread: Could not queue packet over stream %s@%s (queue full)", wire.LocalPodName, wire.LocalNodeIfaceName) } packetPool.Put(bufPtr) diff --git a/third_party/meshnet/daemon/grpcwire/stream_manager.go b/third_party/meshnet/daemon/grpcwire/stream_manager.go index f6c71018..c00b2804 100644 --- a/third_party/meshnet/daemon/grpcwire/stream_manager.go +++ b/third_party/meshnet/daemon/grpcwire/stream_manager.go @@ -94,6 +94,36 @@ func (m *nodeStreamManager) ReleaseStream(topoNs string, peerIP string) { } } +// Send enqueues a packet payload to be transmitted over the multiplexed gRPC stream for the given topo and peer IP. +func (m *nodeStreamManager) Send(topoNs string, peerIP string, pkt *mpb.Packet) bool { + if peerIP == "" || pkt == nil { + return false + } + if topoNs == "" { + topoNs = "default" + } + key := nodeStreamKey{ + topoNs: topoNs, + peerIP: peerIP, + } + + m.mu.Lock() + st, ok := m.streams[key] + if !ok { + st = &NodeStream{ + key: key, + pktChan: make(chan *mpb.Packet, 10000), + stopChan: make(chan struct{}), + refCount: 1, + } + m.streams[key] = st + go st.run() + } + m.mu.Unlock() + + return st.Send(pkt) +} + // Send enqueues a packet payload to be transmitted over the multiplexed gRPC stream. func (s *NodeStream) Send(pkt *mpb.Packet) bool { if pkt == nil { From b539e5631aa46c534123204238b7dfc04f5bfe19 Mon Sep 17 00:00:00 2001 From: Kris Raney Date: Wed, 12 Aug 2026 16:23:47 +0000 Subject: [PATCH 10/18] Clean up code obsoleted by these changes --- .../meshnet/daemon/meshnet/controller.go | 19 ++----------------- third_party/meshnet/daemon/meshnet/meshnet.go | 2 -- third_party/meshnet/plugin/meshnet.go | 2 +- 3 files changed, 3 insertions(+), 20 deletions(-) diff --git a/third_party/meshnet/daemon/meshnet/controller.go b/third_party/meshnet/daemon/meshnet/controller.go index 291cf420..8b3a64cc 100644 --- a/third_party/meshnet/daemon/meshnet/controller.go +++ b/third_party/meshnet/daemon/meshnet/controller.go @@ -775,21 +775,9 @@ func (m *Meshnet) ReconcileAllLocalPods(ctx context.Context) error { return nil } -// triggerReconcile sets the dirty token or triggers a full reconciliation pass, -// maintaining backward compatibility with callers. +// triggerReconcile triggers a full reconciliation pass via the reconcile queue. func (m *Meshnet) triggerReconcile() { - if m.reconcileQueue != nil { - m.reconcileQueue.EnqueueFull() - return - } - if m.dirtyChan == nil { - return - } - select { - case m.dirtyChan <- struct{}{}: - default: - // Already triggered/dirty; worker will execute a pass covering all updates. - } + m.enqueueFullReconcile() } // runReconcileWorker runs in the background and coalesces incoming reconcile triggers. @@ -818,9 +806,6 @@ func (m *Meshnet) runReconcileWorker(ctx context.Context) { _ = m.ReconcilePodLinks(ctx, topo) } } - case <-m.dirtyChan: - _ = m.CleanupOrphanedHostVeths(ctx) - _ = m.ReconcileAllLocalPods(ctx) } } } diff --git a/third_party/meshnet/daemon/meshnet/meshnet.go b/third_party/meshnet/daemon/meshnet/meshnet.go index 8a1e646d..d013d13c 100644 --- a/third_party/meshnet/daemon/meshnet/meshnet.go +++ b/third_party/meshnet/daemon/meshnet/meshnet.go @@ -49,7 +49,6 @@ type Meshnet struct { s *grpc.Server lis net.Listener nodeIP string - dirtyChan chan struct{} interNodeLinkType string topoCache *TopologyCache reconcileQueue *ReconcileQueue @@ -137,7 +136,6 @@ func New(cfg Config) (*Meshnet, error) { lis: lis, s: svr, nodeIP: os.Getenv("HOST_IP"), - dirtyChan: make(chan struct{}, 1), interNodeLinkType: lnkTyp, topoCache: NewTopologyCache(), reconcileQueue: NewReconcileQueue(50 * time.Millisecond), diff --git a/third_party/meshnet/plugin/meshnet.go b/third_party/meshnet/plugin/meshnet.go index 25a41de1..a5a34e21 100644 --- a/third_party/meshnet/plugin/meshnet.go +++ b/third_party/meshnet/plugin/meshnet.go @@ -239,7 +239,7 @@ func cmdAdd(args *skel.CmdArgs) error { if interNodeLinkType == wireutil.INTER_NODE_LINK_GRPC { peerSrcIP, cached := peerNodeCache[link.PeerPod] if !cached { - peerPod, err := meshnetClient.Get(ctx, &mpb.PodQuery{ + peerPod, err := meshnetClient.Get(waitCtx, &mpb.PodQuery{ Name: link.PeerPod, KubeNs: string(cniArgs.K8S_POD_NAMESPACE), }) From f784b295ee1c786c3ac26c3dd54c38e71e3c233f Mon Sep 17 00:00:00 2001 From: Kris Raney Date: Fri, 14 Aug 2026 22:00:03 +0000 Subject: [PATCH 11/18] AI review for leaks and cross-process races MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A. Concurrent Local vs. Remote Wire Creation Race Root Cause: When two nodes (Node A and Node B) reconcile a link simultaneously, Node A calls CreateGRPCWireLocal while sending an AddGRPCWiresRemoteBatch RPC to Node B, and Node B does the inverse. If CreateGRPCWireLocal and CreateUpdateGRPCWireRemoteTriggered execute concurrently on Node B for the same link UID, both check GetWireByUID / UpdateWireByUID before either inserts into wires.wires, resulting in duplicate TAP creation and clobbered map entries. Fix: Added synchronization (createWireMu sync.Mutex) in gwire_rpc_handlers.go across checking if a wire already exists and creating/inserting a new wire. B. Missing Stale NetNS Wire Cleanup in AddInMemNDataStore Root Cause: Commit 81dbf4a added cleanup for old wires with stale LocalPodNetNS paths (when containers restart) to wires.AddInMem. However, gRPC RPC triggers and local reconciliation use AddInMemNDataStore, which omitted that cleanup loop—leaving old netNS TAP handles and reader goroutines running indefinitely when pods restarted in a new netNS. Fix: Extracted a helper method cleanupOldWireLocked and invoked it in both AddInMem and AddInMemNDataStore. Added unit test TestAddInMemNDataStore_StaleWireCleanup to cover this scenario. C. Stream Reference Count Leak on Late Peer Discovery Root Cause: In RecvFrmLocalPodThread , defer func() { ... ReleaseStream(...) }() was registered inside if peerIP != "". When CreateGRPCWireLocal spawns the thread before the peer pod is scheduled (peerIP == ""), the defer was never scheduled. When PeerNodeIP was updated later via wire.UpdateWire, GetOrCreateStream incremented the refcount, but thread termination never released it. Fix: Moved the defer cleanup block outside if peerIP != "" so that currIP := wire.PeerNodeIP is checked upon thread exit and any acquired stream reference is released. D. Reader Thread Leak on Teardown of Non-Ready Wires Root Cause: In RemoveWireAcrosAll and WireDownByUID, wire.StopC was only closed if wire.IsReady. If a wire was locally initiated (IsReady = false) and the pod was deleted before the remote peer completed the handshake, StopC was never closed. Fix: Updated wire teardown methods to unconditionally close wire.StopC (if non-nil) regardless of wire.IsReady. --- .../meshnet/api/types/v1beta1/topology.go | 10 +- .../meshnet/daemon/grpcwire/grpcwire.go | 57 ++-- .../meshnet/daemon/grpcwire/gwire_map.go | 27 +- .../meshnet/daemon/grpcwire/gwire_map_test.go | 307 ++++++++++++++++++ .../daemon/grpcwire/gwire_rpc_handlers.go | 11 +- .../meshnet/daemon/grpcwire/stream_manager.go | 22 +- .../meshnet/daemon/meshnet/controller.go | 17 +- .../meshnet/daemon/meshnet/controller_test.go | 46 +++ third_party/meshnet/daemon/meshnet/meshnet.go | 2 +- third_party/meshnet/utils/wireutil/tap.go | 1 - third_party/meshnet/utils/wireutil/veth.go | 12 +- 11 files changed, 453 insertions(+), 59 deletions(-) diff --git a/third_party/meshnet/api/types/v1beta1/topology.go b/third_party/meshnet/api/types/v1beta1/topology.go index 6df83f83..e942d2f8 100644 --- a/third_party/meshnet/api/types/v1beta1/topology.go +++ b/third_party/meshnet/api/types/v1beta1/topology.go @@ -30,11 +30,11 @@ type TopologySpec struct { type TopologyStatus struct { metav1.TypeMeta `json:",inline"` // Deprecated: Do not use. Skipped links are managed reactively by the daemon controller. - Skipped []Skipped `json:"skipped"` - SrcIP string `json:"src_ip"` - NetNS string `json:"net_ns"` - ContainerID string `json:"container_id"` - PlumbingError string `json:"plumbing_error,omitempty"` + Skipped []Skipped `json:"skipped"` + SrcIP string `json:"src_ip"` + NetNS string `json:"net_ns"` + ContainerID string `json:"container_id"` + PlumbingError string `json:"plumbing_error,omitempty"` } // Skipped represents a skipped interface connection. diff --git a/third_party/meshnet/daemon/grpcwire/grpcwire.go b/third_party/meshnet/daemon/grpcwire/grpcwire.go index fe51db64..ecfae802 100644 --- a/third_party/meshnet/daemon/grpcwire/grpcwire.go +++ b/third_party/meshnet/daemon/grpcwire/grpcwire.go @@ -99,8 +99,18 @@ type GRPCWire struct { Originator grpcWireOriginator // create by local host or create on trigger from remote host. This is for debugging. OriginatorIP string // IP address of the host created it. This is for debugging. - StopC chan struct{} // the channel to send stop signal to the receive thread. - mu sync.Mutex + StopC chan struct{} // the channel to send stop signal to the receive thread. + stopOnce sync.Once + mu sync.Mutex +} + +// CloseStopC safely closes the wire's StopC channel at most once. +func (wire *GRPCWire) CloseStopC() { + wire.stopOnce.Do(func() { + if wire.StopC != nil { + close(wire.StopC) + } + }) } type linkKey struct { @@ -188,6 +198,15 @@ func WireDownByUID(namespace string, linkUID int) error { namespace: namespace, linkUID: linkUID, }] + if !ok { + for _, w := range wires.wires { + if w.UID == linkUID && (namespace == "" || w.TopoNamespace == namespace || w.LocalPodNetNS == namespace) { + wire = w + ok = true + break + } + } + } wires.mu.Unlock() if ok { @@ -195,12 +214,8 @@ func WireDownByUID(namespace string, linkUID int) error { defer wire.mu.Unlock() grpcOvrlyLogger.Infof("WireDownByUID: Making wire down from db, %s@%s-%s@%d, peer fid %d, link uid %d", wire.LocalPodName, wire.LocalPodIfaceName, wire.LocalNodeIfaceName, wire.LocalNodeIfaceID, wire.WireIfaceIDOnPeerNode, linkUID) - if wire.IsReady { - if wire.StopC != nil { - close(wire.StopC) - } - wire.IsReady = false - } + wire.CloseStopC() + wire.IsReady = false } else { grpcOvrlyLogger.Infof("WireDownByUID: Did not find entry to make down from db, uid %d, ns %s", linkUID, namespace) @@ -262,12 +277,8 @@ func RemoveWireAcrosAll(wire *GRPCWire, inMem bool) error { // stop the packet receive thread for this pod wire.mu.Lock() - if wire.IsReady { - if wire.StopC != nil { - close(wire.StopC) - } - wire.IsReady = false - } + wire.CloseStopC() + wire.IsReady = false wire.mu.Unlock() // Close and remove the TAP file handle @@ -322,16 +333,16 @@ func RecvFrmLocalPodThread(wire *GRPCWire, locIfNm string) error { if peerIP != "" { _ = streamMgr.GetOrCreateStream(topoNs, peerIP) - defer func() { - wire.mu.Lock() - currIP := wire.PeerNodeIP - currNs := wire.TopoNamespace - wire.mu.Unlock() - if currIP != "" { - streamMgr.ReleaseStream(currNs, currIP) - } - }() } + defer func() { + wire.mu.Lock() + currIP := wire.PeerNodeIP + currNs := wire.TopoNamespace + wire.mu.Unlock() + if currIP != "" { + streamMgr.ReleaseStream(currNs, currIP) + } + }() return forwardPackets(tapFile, nil, wire, locIfNm) } diff --git a/third_party/meshnet/daemon/grpcwire/gwire_map.go b/third_party/meshnet/daemon/grpcwire/gwire_map.go index 03f87d92..708d8501 100644 --- a/third_party/meshnet/daemon/grpcwire/gwire_map.go +++ b/third_party/meshnet/daemon/grpcwire/gwire_map.go @@ -29,19 +29,24 @@ func (w *wireMap) GetHandle(key int64) (*os.File, bool) { return handle, ok } -func (w *wireMap) AddInMem(wire *GRPCWire, handle *os.File) error { - w.mu.Lock() - defer w.mu.Unlock() +func (w *wireMap) cleanupOldWireLocked(wire *GRPCWire) { + lk := linkKey{namespace: wire.LocalPodNetNS, linkUID: wire.UID} + if oldWire, exists := w.wires[lk]; exists && oldWire != wire { + oldWire.CloseStopC() + oldWire.IsReady = false + if oldHandle, ok := w.handles[oldWire.LocalNodeIfaceID]; ok && oldHandle != nil { + _ = oldHandle.Close() + delete(w.handles, oldWire.LocalNodeIfaceID) + } + } for key, oldWire := range w.wires { if oldWire.TopoNamespace == wire.TopoNamespace && oldWire.LocalPodName == wire.LocalPodName && oldWire.UID == wire.UID && oldWire.LocalPodNetNS != wire.LocalPodNetNS { - if oldWire.IsReady { - close(oldWire.StopC) - oldWire.IsReady = false - } + oldWire.CloseStopC() + oldWire.IsReady = false if oldHandle, ok := w.handles[oldWire.LocalNodeIfaceID]; ok && oldHandle != nil { _ = oldHandle.Close() delete(w.handles, oldWire.LocalNodeIfaceID) @@ -49,6 +54,13 @@ func (w *wireMap) AddInMem(wire *GRPCWire, handle *os.File) error { delete(w.wires, key) } } +} + +func (w *wireMap) AddInMem(wire *GRPCWire, handle *os.File) error { + w.mu.Lock() + defer w.mu.Unlock() + + w.cleanupOldWireLocked(wire) w.wires[linkKey{ namespace: wire.LocalPodNetNS, @@ -61,6 +73,7 @@ func (w *wireMap) AddInMem(wire *GRPCWire, handle *os.File) error { func (w *wireMap) AddInMemNDataStore(wire *GRPCWire, handle *os.File) error { w.mu.Lock() + w.cleanupOldWireLocked(wire) w.wires[linkKey{ namespace: wire.LocalPodNetNS, linkUID: wire.UID, diff --git a/third_party/meshnet/daemon/grpcwire/gwire_map_test.go b/third_party/meshnet/daemon/grpcwire/gwire_map_test.go index 6c129689..10981dfc 100644 --- a/third_party/meshnet/daemon/grpcwire/gwire_map_test.go +++ b/third_party/meshnet/daemon/grpcwire/gwire_map_test.go @@ -1,7 +1,10 @@ package grpcwire import ( + "context" "testing" + + mpb "github.com/openconfig/kne/third_party/meshnet/daemon/proto/meshnet/v1beta1" ) func TestAddInMem_StaleWireCleanup(t *testing.T) { @@ -84,3 +87,307 @@ func TestUpdateWireByUID_PeerIPUpdate(t *testing.T) { wires.AtomicDelete(w) } +func TestAddInMemNDataStore_StaleWireCleanup(t *testing.T) { + // Override k8s client interface so K8sStoreGWire doesn't crash on nil client + SetGWireClientInterface(nil) + stopC1 := make(chan struct{}) + w1 := &GRPCWire{ + UID: 303, + TopoNamespace: "default", + LocalPodName: "pod2", + LocalPodNetNS: "/proc/333/ns/net", + IsReady: false, + StopC: stopC1, + } + + wires.AddInMemNDataStore(w1, nil) + + if wire, ok := GetWireByUID("/proc/333/ns/net", 303); !ok || wire != w1 { + t.Fatalf("expected w1 in wires map, got ok=%t", ok) + } + + w2 := &GRPCWire{ + UID: 303, + TopoNamespace: "default", + LocalPodName: "pod2", + LocalPodNetNS: "/proc/444/ns/net", + IsReady: true, + StopC: make(chan struct{}), + } + + wires.AddInMemNDataStore(w2, nil) + + if _, ok := GetWireByUID("/proc/333/ns/net", 303); ok { + t.Fatalf("expected old wire /proc/333/ns/net to be cleaned up by AddInMemNDataStore") + } + + select { + case <-stopC1: + default: + t.Fatalf("expected old wire StopC to be closed even if IsReady was false") + } + + wires.AtomicDelete(w2) +} + +func TestCloseStopC_Idempotent(t *testing.T) { + stopC := make(chan struct{}) + w := &GRPCWire{ + UID: 404, + StopC: stopC, + } + + // Calling CloseStopC multiple times should be safe and idempotent (no panic) + w.CloseStopC() + w.CloseStopC() + w.CloseStopC() + + select { + case <-stopC: + // channel closed as expected + default: + t.Fatalf("expected stopC to be closed") + } +} + +func TestWireDownThenRemove_NoDoubleClosePanic(t *testing.T) { + w := &GRPCWire{ + UID: 505, + TopoNamespace: "default", + LocalPodName: "podX", + LocalPodNetNS: "/proc/505/ns/net", + IsReady: true, + StopC: make(chan struct{}), + } + wires.AddInMem(w, nil) + + // 1. Remote tells local node wire is down + if err := WireDownByUID("/proc/505/ns/net", 505); err != nil { + t.Fatalf("WireDownByUID failed: %v", err) + } + + // 2. Later local pod is destroyed and RemoveWireAcrosAll is called + if err := RemoveWireAcrosAll(w, true); err != nil { + t.Fatalf("RemoveWireAcrosAll failed: %v", err) + } +} + +func TestStreamManager_SendUnregisteredReturnsFalse(t *testing.T) { + // Calling Send on a non-existent stream should return false without creating a leaked stream + key := nodeStreamKey{topoNs: "unregistered-ns", peerIP: "192.0.2.1"} + if streamMgr.Send(key.topoNs, key.peerIP, nil) { + t.Fatalf("expected Send to return false for nil packet") + } + + streamMgr.mu.Lock() + _, exists := streamMgr.streams[key] + streamMgr.mu.Unlock() + + if exists { + t.Fatalf("expected streamMgr.Send not to register unowned stream") + } +} + +func TestNodeStream_StopIdempotent(t *testing.T) { + st := &NodeStream{ + key: nodeStreamKey{topoNs: "test-topo", peerIP: "192.0.2.2"}, + pktChan: make(chan *mpb.Packet, 10), + stopChan: make(chan struct{}), + } + + // Multiple Stop calls should not panic + st.Stop() + st.Stop() + st.Stop() + + select { + case <-st.stopChan: + // success + default: + t.Fatalf("expected stopChan to be closed") + } +} + +func TestCreateGRPCWireLocal_PreservesPeerIPForUpdateWire(t *testing.T) { + stopC := make(chan struct{}) + w := &GRPCWire{ + UID: 601, + TopoNamespace: "default", + LocalPodName: "podLocal", + LocalPodNetNS: "/proc/601/ns/net", + PeerNodeIP: "10.0.0.2", + IsReady: true, + StopC: stopC, + } + wires.AddInMem(w, nil) + defer wires.AtomicDelete(w) + + // Call CreateGRPCWireLocal with new peer IP (peer moved to 10.0.0.3) + resp, err := CreateGRPCWireLocal(context.Background(), &mpb.WireDef{ + LocalPodNetNs: "/proc/601/ns/net", + LinkUid: 601, + PeerNodeIp: "10.0.0.3", + }) + if err != nil || resp == nil || !resp.Response { + t.Fatalf("CreateGRPCWireLocal failed: %v", err) + } + + // PeerNodeIP should remain 10.0.0.2 until UpdateWire is called, ensuring stream migration occurs + if w.PeerNodeIP != "10.0.0.2" { + t.Fatalf("expected PeerNodeIP to remain 10.0.0.2 until UpdateWire, got %s", w.PeerNodeIP) + } + + // Now UpdateWireByUID is called with 10.0.0.3 + updated, ok := UpdateWireByUID("/proc/601/ns/net", 601, 700, "10.0.0.3", make(chan struct{})) + if !ok || updated == nil { + t.Fatalf("UpdateWireByUID failed") + } + if updated.PeerNodeIP != "10.0.0.3" { + t.Fatalf("expected PeerNodeIP to be updated to 10.0.0.3, got %s", updated.PeerNodeIP) + } +} + +func TestWireDownByUID_FallbackByLinkUID(t *testing.T) { + stopC := make(chan struct{}) + w := &GRPCWire{ + UID: 707, + TopoNamespace: "test-ns", + LocalPodName: "podRemote", + LocalPodNetNS: "/proc/707/ns/net", + IsReady: true, + StopC: stopC, + } + wires.AddInMem(w, nil) + defer wires.AtomicDelete(w) + + // Call WireDownByUID using TopoNamespace ("test-ns") instead of container netns ("/proc/707/ns/net") + if err := WireDownByUID("test-ns", 707); err != nil { + t.Fatalf("WireDownByUID failed: %v", err) + } + + if w.IsReady { + t.Fatalf("expected wire to be marked down (IsReady=false)") + } + + select { + case <-stopC: + // success: StopC closed + default: + t.Fatalf("expected StopC to be closed by WireDownByUID fallback") + } +} + +func TestRelocatedPod_StreamRedirection(t *testing.T) { + topoNs := "test-relocate" + oldPeerIP := "192.0.2.10" + newPeerIP := "192.0.2.20" + + stopC := make(chan struct{}) + w := &GRPCWire{ + UID: 808, + TopoNamespace: topoNs, + LocalPodName: "podA", + LocalPodNetNS: "/proc/808/ns/net", + PeerNodeIP: oldPeerIP, + WireIfaceIDOnPeerNode: 10, + IsReady: true, + StopC: stopC, + } + wires.AddInMem(w, nil) + defer wires.AtomicDelete(w) + + // Simulate initial stream acquisition by RecvFrmLocalPodThread + stOld := streamMgr.GetOrCreateStream(topoNs, oldPeerIP) + defer stOld.Stop() + + pkt := &mpb.Packet{RemotIntfId: 10, Frame: []byte{0x01, 0x02}} + + // Outbound send to old peer IP succeeds + if !streamMgr.Send(topoNs, oldPeerIP, pkt) { + t.Fatalf("expected Send to oldPeerIP to succeed before relocation") + } + + // Pod B relocates to newPeerIP: UpdateWireByUID is invoked + updated, ok := UpdateWireByUID("/proc/808/ns/net", 808, 20, newPeerIP, make(chan struct{})) + if !ok || updated == nil { + t.Fatalf("UpdateWireByUID failed") + } + + // 1. PeerNodeIP is updated + if updated.PeerNodeIP != newPeerIP { + t.Fatalf("expected PeerNodeIP to be updated to %s, got %s", newPeerIP, updated.PeerNodeIP) + } + + // 2. WireIfaceIDOnPeerNode is updated to new interface ID + if updated.WireIfaceIDOnPeerNode != 20 { + t.Fatalf("expected WireIfaceIDOnPeerNode to be 20, got %d", updated.WireIfaceIDOnPeerNode) + } + + // 3. New stream is created and accepting packets + if !streamMgr.Send(topoNs, newPeerIP, pkt) { + t.Fatalf("expected Send to newPeerIP to succeed after relocation") + } + + // 4. Old stream was released + streamMgr.mu.Lock() + _, oldStreamStillExists := streamMgr.streams[nodeStreamKey{topoNs: topoNs, peerIP: oldPeerIP}] + streamMgr.mu.Unlock() + + if oldStreamStillExists { + t.Fatalf("expected old stream %s to be released after relocation", oldPeerIP) + } + + // Clean up new stream + streamMgr.ReleaseStream(topoNs, newPeerIP) +} + +func TestPassivePodRestart_SymmetricRecovery(t *testing.T) { + // Node 1 hosts Active Pod A (ID 50) + stopC := make(chan struct{}) + wireA := &GRPCWire{ + UID: 909, + TopoNamespace: "default", + LocalNodeIfaceID: 50, + LocalPodName: "podActive", + LocalPodNetNS: "/proc/podA/ns/net", + PeerNodeIP: "10.0.0.2", + WireIfaceIDOnPeerNode: 100, // Old ID on Node 2 + IsReady: true, + StopC: stopC, + Originator: HOST_CREATED_WIRE, + } + wires.AddInMem(wireA, nil) + defer wires.AtomicDelete(wireA) + + // Passive Pod B on Node 2 restarts and gets new ID 200. + // Node 2 initiates connection to Node 1 via CreateUpdateGRPCWireRemoteTriggered + wireDefFromPassive := &mpb.WireDef{ + LocalPodNetNs: "/proc/podA/ns/net", + LinkUid: 909, + WireIfIdOnPeerNode: 200, // New ID on Node 2 + PeerNodeIp: "10.0.0.2", + TopoNs: "default", + LocalPodName: "podActive", + } + + wire, created, err := CreateUpdateGRPCWireRemoteTriggered(wireDefFromPassive, make(chan struct{})) + if err != nil { + t.Fatalf("CreateUpdateGRPCWireRemoteTriggered failed: %v", err) + } + + if created { + t.Fatalf("expected created=false for existing wire on active pod") + } + + if wire.LocalNodeIfaceID != 50 { + t.Fatalf("expected LocalNodeIfaceID=50, got %d", wire.LocalNodeIfaceID) + } + + if wire.WireIfaceIDOnPeerNode != 200 { + t.Fatalf("expected WireIfaceIDOnPeerNode to be updated to 200, got %d", wire.WireIfaceIDOnPeerNode) + } + + if !wire.IsReady { + t.Fatalf("expected wire to remain ready") + } +} diff --git a/third_party/meshnet/daemon/grpcwire/gwire_rpc_handlers.go b/third_party/meshnet/daemon/grpcwire/gwire_rpc_handlers.go index 648f988f..3d553e5a 100644 --- a/third_party/meshnet/daemon/grpcwire/gwire_rpc_handlers.go +++ b/third_party/meshnet/daemon/grpcwire/gwire_rpc_handlers.go @@ -2,6 +2,7 @@ package grpcwire import ( "context" + "sync" log "github.com/sirupsen/logrus" @@ -9,13 +10,15 @@ import ( "github.com/openconfig/kne/third_party/meshnet/utils/wireutil" ) +var createWireMu sync.Mutex + func CreateGRPCWireLocal(ctx context.Context, wireDef *mpb.WireDef) (*mpb.BoolResponse, error) { + createWireMu.Lock() + defer createWireMu.Unlock() + if wire, ok := GetWireByUID(wireDef.LocalPodNetNs, int(wireDef.LinkUid)); ok && wire != nil { wire.mu.Lock() wire.Originator = HOST_CREATED_WIRE - if wireDef.PeerNodeIp != "" { - wire.PeerNodeIP = wireDef.PeerNodeIp - } wire.mu.Unlock() return &mpb.BoolResponse{Response: true}, nil } @@ -55,6 +58,8 @@ func CreateGRPCWireLocal(ctx context.Context, wireDef *mpb.WireDef) (*mpb.BoolRe // a pod from node A to node B dynamically. // Returns the wire, whether it was freshly created (true) or updated (false), and any error. func CreateUpdateGRPCWireRemoteTriggered(wireDef *mpb.WireDef, stopC chan struct{}) (*GRPCWire, bool, error) { + createWireMu.Lock() + defer createWireMu.Unlock() // If this wire is already created, then only update the already created wire properties like stopC. // This can happen due to a race between the local and remote peer. diff --git a/third_party/meshnet/daemon/grpcwire/stream_manager.go b/third_party/meshnet/daemon/grpcwire/stream_manager.go index c00b2804..70de87d0 100644 --- a/third_party/meshnet/daemon/grpcwire/stream_manager.go +++ b/third_party/meshnet/daemon/grpcwire/stream_manager.go @@ -24,9 +24,17 @@ type NodeStream struct { key nodeStreamKey pktChan chan *mpb.Packet stopChan chan struct{} + stopOnce sync.Once refCount int } +// Stop safely closes the stopChan at most once. +func (s *NodeStream) Stop() { + s.stopOnce.Do(func() { + close(s.stopChan) + }) +} + type nodeStreamManager struct { mu sync.Mutex streams map[nodeStreamKey]*NodeStream @@ -89,7 +97,7 @@ func (m *nodeStreamManager) ReleaseStream(topoNs string, peerIP string) { st.refCount-- if st.refCount <= 0 { - close(st.stopChan) + st.Stop() delete(m.streams, key) } } @@ -109,17 +117,11 @@ func (m *nodeStreamManager) Send(topoNs string, peerIP string, pkt *mpb.Packet) m.mu.Lock() st, ok := m.streams[key] + m.mu.Unlock() + if !ok { - st = &NodeStream{ - key: key, - pktChan: make(chan *mpb.Packet, 10000), - stopChan: make(chan struct{}), - refCount: 1, - } - m.streams[key] = st - go st.run() + return false } - m.mu.Unlock() return st.Send(pkt) } diff --git a/third_party/meshnet/daemon/meshnet/controller.go b/third_party/meshnet/daemon/meshnet/controller.go index 8b3a64cc..da38a168 100644 --- a/third_party/meshnet/daemon/meshnet/controller.go +++ b/third_party/meshnet/daemon/meshnet/controller.go @@ -513,7 +513,7 @@ func (c *TopologyCache) Put(topo *unstructured.Unstructured) { } } - c.topos[key] = topo + c.topos[key] = topo.DeepCopy() // Index new peer dependencies links, _ := parsePodLinks(topo) @@ -557,7 +557,11 @@ func (c *TopologyCache) Get(ns, name string) *unstructured.Unstructured { } c.mu.RLock() defer c.mu.RUnlock() - return c.topos[fmt.Sprintf("%s/%s", ns, name)] + topo := c.topos[fmt.Sprintf("%s/%s", ns, name)] + if topo == nil { + return nil + } + return topo.DeepCopy() } // List returns all cached Topology resources matching the given namespace (or all if namespace is empty or metav1.NamespaceAll). @@ -571,7 +575,7 @@ func (c *TopologyCache) List(ns string) []*unstructured.Unstructured { res := make([]*unstructured.Unstructured, 0, len(c.topos)) for _, topo := range c.topos { if ns == "" || ns == metav1.NamespaceAll || topo.GetNamespace() == ns { - res = append(res, topo) + res = append(res, topo.DeepCopy()) } } return res @@ -944,9 +948,16 @@ func (m *Meshnet) updatePlumbingErrorStatus(ctx context.Context, topo *unstructu return err } + currErr, found, _ := unstructured.NestedString(latestTopo.Object, "status", "plumbing_error") if errMsg == "" { + if !found || currErr == "" { + return nil + } unstructured.RemoveNestedField(latestTopo.Object, "status", "plumbing_error") } else { + if found && currErr == errMsg { + return nil + } if err := unstructured.SetNestedField(latestTopo.Object, errMsg, "status", "plumbing_error"); err != nil { return err } diff --git a/third_party/meshnet/daemon/meshnet/controller_test.go b/third_party/meshnet/daemon/meshnet/controller_test.go index 5c9a1c43..0bed3b2c 100644 --- a/third_party/meshnet/daemon/meshnet/controller_test.go +++ b/third_party/meshnet/daemon/meshnet/controller_test.go @@ -414,6 +414,52 @@ func TestTargetedReconciliation_PeerRestartQueuesDependents(t *testing.T) { } } +func TestTopologyCache_DeepCopyIsolation(t *testing.T) { + cache := NewTopologyCache() + p1 := createFakePodTopology("p1", "default", "10.0.0.1", "/proc/1/ns/net", []string{"p2"}) + cache.Put(p1) + // Mutate the object returned from Get + got := cache.Get("default", "p1") + if got == nil { + t.Fatalf("expected p1 in cache") + } + _ = unstructured.SetNestedField(got.Object, "10.99.99.99", "status", "src_ip") + // Verify cached object was not mutated + got2 := cache.Get("default", "p1") + srcIP, _, _ := unstructured.NestedString(got2.Object, "status", "src_ip") + if srcIP != "10.0.0.1" { + t.Fatalf("expected cached src_ip to remain 10.0.0.1, got %s", srcIP) + } +} +func TestUpdatePlumbingErrorStatus_NoOpWhenUnchanged(t *testing.T) { + InitLogger() + p1 := createFakePodTopology("p1", "default", "10.0.0.1", "/proc/1/ns/net", []string{"p2"}) + fakeClient, err := fakeTopology.NewSimpleClientset(p1) + if err != nil { + t.Fatalf("failed to create fake topology clientset: %v", err) + } + + m := &Meshnet{ + tClient: fakeClient, + topoCache: NewTopologyCache(), + } + m.topoCache.Put(p1) + + // 1. Clearing when already empty should succeed without error + if err := m.updatePlumbingErrorStatus(context.Background(), p1, ""); err != nil { + t.Fatalf("expected nil error on clearing empty error: %v", err) + } + + // 2. Setting an error + if err := m.updatePlumbingErrorStatus(context.Background(), p1, "dial timeout"); err != nil { + t.Fatalf("expected nil error on setting error: %v", err) + } + + // 3. Setting the exact same error again should be a no-op + if err := m.updatePlumbingErrorStatus(context.Background(), p1, "dial timeout"); err != nil { + t.Fatalf("expected nil error on setting duplicate error: %v", err) + } +} diff --git a/third_party/meshnet/daemon/meshnet/meshnet.go b/third_party/meshnet/daemon/meshnet/meshnet.go index d013d13c..ade9f48c 100644 --- a/third_party/meshnet/daemon/meshnet/meshnet.go +++ b/third_party/meshnet/daemon/meshnet/meshnet.go @@ -112,7 +112,7 @@ func New(cfg Config) (*Meshnet, error) { // Otherwise there will be GRPC log for every packet sent as for link type GRPC, GRPC is also the data-plane. This is too // much of log that does not help in debugging and K8S does log rotation very frequently. defaultOpts := []grpc.ServerOption{ - grpc.InitialWindowSize(4 * 1024 * 1024), // 4MB stream window + grpc.InitialWindowSize(4 * 1024 * 1024), // 4MB stream window grpc.InitialConnWindowSize(16 * 1024 * 1024), // 16MB connection window grpc.MaxRecvMsgSize(64 * 1024 * 1024), grpc.MaxSendMsgSize(64 * 1024 * 1024), diff --git a/third_party/meshnet/utils/wireutil/tap.go b/third_party/meshnet/utils/wireutil/tap.go index b0bdde76..21dbf231 100644 --- a/third_party/meshnet/utils/wireutil/tap.go +++ b/third_party/meshnet/utils/wireutil/tap.go @@ -96,4 +96,3 @@ func CreateOrAttachTAP(podNsPath string, ifName string, ipCIDR string) (*os.File return tapFile, nil } - diff --git a/third_party/meshnet/utils/wireutil/veth.go b/third_party/meshnet/utils/wireutil/veth.go index 8add33e5..e5ab7f9e 100644 --- a/third_party/meshnet/utils/wireutil/veth.go +++ b/third_party/meshnet/utils/wireutil/veth.go @@ -68,12 +68,12 @@ func HostVethNames(kubeNs, podName, peerPodName string, linkUID int64) (string, // network namespace (podNsPath). // // Uses deterministic host veth naming (HostVethNames) so that: -// - If the veth pair has not been created yet, it creates the host veth pair, moves the local end -// into podNsPath, and leaves the peer end waiting on the host for the peer pod to claim. -// - If the peer pod already created the host veth pair, it finds the waiting local end on the host -// and moves it into podNsPath. -// - If interrupted or restarted halfway through, it discovers already-moved interfaces and resumes -// idempotently. +// - If the veth pair has not been created yet, it creates the host veth pair, moves the local end +// into podNsPath, and leaves the peer end waiting on the host for the peer pod to claim. +// - If the peer pod already created the host veth pair, it finds the waiting local end on the host +// and moves it into podNsPath. +// - If interrupted or restarted halfway through, it discovers already-moved interfaces and resumes +// idempotently. func ConfigurePodLinks(podNsPath string, links []PodLinkConfig) error { if len(links) == 0 { return nil From a3084c5f2a3958e7008eafa1f04d50cc16d6cc20 Mon Sep 17 00:00:00 2001 From: Kris Raney Date: Fri, 14 Aug 2026 22:37:58 +0000 Subject: [PATCH 12/18] Fix wire cleanup on teardown, prevent dead-wire poisoning, and remove obsolete CNI code 1. Dead-Wire Recovery on WireDownByUID: - Updated WireDownByUID in grpcwire.go to invoke RemoveWireAcrosAll(wire, true) instead of leaving a dead wire in the active map with closed channels and dead reader goroutines. This ensures future peer pod reconnects and reconciliations cleanly re-create TAP handles and reader threads. - Added unit test TestWireDownByUID_CleansUpAndRemovesFromMemory. 2. gRPC Wire Cleanup on Topology CR Deletion: - Added grpcwire.DeletePodWires(ns, name) to the watch.Deleted event handler in controller.go to clean up gRPC wires, reader goroutines, and TAP handles when a local Topology CR is deleted directly. 3. Prevent gRPC Conn Leaks and Timeouts in CNI MakeGRPCChanDown: - Added defer remote.Close() to ensure gRPC connections are closed on exit. - Added a 5-second context timeout to GRPCWireDownRemote RPC calls to avoid hanging CNI deletions when remote peers are unreachable. 4. Clean Up Obsolete Legacy Code: - Removed unused CreatGRPCChan and legacy retry constants from grpcwires-plugin.go, as link creation is now handled by the daemon reconciler. Cross-Node Wire Teardown Resolution (third_party/meshnet/daemon/grpcwire/gwire_rpc_handlers.go): Updated GRPCWireDownRemoteTriggered to check wireDef.LocalPodNetNs first and fallback to wireDef.TopoNs. This ensures remote wire teardowns across nodes correctly match and destroy the remote wire end even when the caller sends its own local container netns path. Added unit test TestGRPCWireDownRemoteTriggered_RemoteNetNSFallback in third_party/meshnet/daemon/grpcwire/gwire_map_test.go. CNI Plugin Cleanup & String Trimming (third_party/meshnet/plugin/meshnet.go): Removed the obsolete and redundant meshnetClient.SkipReverse calls in cmdDel to prevent unnecessary Kubernetes API conflict retries on pod deletion. Trimmed whitespace on interNodeLinkType using strings.TrimSpace and fixed the log typo in SetInterNodeLinkType ("iner" -> "inner"). --- .../meshnet/daemon/grpcwire/grpcwire.go | 14 +- .../meshnet/daemon/grpcwire/gwire_map_test.go | 75 ++++++ .../daemon/grpcwire/gwire_rpc_handlers.go | 14 +- .../meshnet/daemon/meshnet/controller.go | 1 + .../meshnet/plugin/grpcwires-plugin.go | 246 +----------------- third_party/meshnet/plugin/meshnet.go | 18 +- 6 files changed, 95 insertions(+), 273 deletions(-) diff --git a/third_party/meshnet/daemon/grpcwire/grpcwire.go b/third_party/meshnet/daemon/grpcwire/grpcwire.go index ecfae802..818c7b17 100644 --- a/third_party/meshnet/daemon/grpcwire/grpcwire.go +++ b/third_party/meshnet/daemon/grpcwire/grpcwire.go @@ -191,7 +191,7 @@ func UpdateWireByUID(namespace string, linkUID int, peerIntfId int64, peerNodeIP return wire, ok } -// WireDownByUID - stops packet collection from the connected pod +// WireDownByUID stops packet collection and cleans up the wire for the given link UID. func WireDownByUID(namespace string, linkUID int) error { wires.mu.Lock() wire, ok := wires.wires[linkKey{ @@ -210,16 +210,12 @@ func WireDownByUID(namespace string, linkUID int) error { wires.mu.Unlock() if ok { - wire.mu.Lock() - defer wire.mu.Unlock() - grpcOvrlyLogger.Infof("WireDownByUID: Making wire down from db, %s@%s-%s@%d, peer fid %d, link uid %d", + grpcOvrlyLogger.Infof("WireDownByUID: Removing wire from db, %s@%s-%s@%d, peer fid %d, link uid %d", wire.LocalPodName, wire.LocalPodIfaceName, wire.LocalNodeIfaceName, wire.LocalNodeIfaceID, wire.WireIfaceIDOnPeerNode, linkUID) - wire.CloseStopC() - wire.IsReady = false - } else { - grpcOvrlyLogger.Infof("WireDownByUID: Did not find entry to make down from db, uid %d, ns %s", - linkUID, namespace) + return RemoveWireAcrosAll(wire, true) } + grpcOvrlyLogger.Infof("WireDownByUID: Did not find entry to make down from db, uid %d, ns %s", + linkUID, namespace) return nil } diff --git a/third_party/meshnet/daemon/grpcwire/gwire_map_test.go b/third_party/meshnet/daemon/grpcwire/gwire_map_test.go index 10981dfc..66a4aa60 100644 --- a/third_party/meshnet/daemon/grpcwire/gwire_map_test.go +++ b/third_party/meshnet/daemon/grpcwire/gwire_map_test.go @@ -391,3 +391,78 @@ func TestPassivePodRestart_SymmetricRecovery(t *testing.T) { t.Fatalf("expected wire to remain ready") } } + +func TestWireDownByUID_CleansUpAndRemovesFromMemory(t *testing.T) { + stopC := make(chan struct{}) + w := &GRPCWire{ + UID: 950, + TopoNamespace: "default", + LocalPodName: "podDownTest", + LocalPodNetNS: "/proc/950/ns/net", + PeerNodeIP: "10.0.0.2", + IsReady: true, + StopC: stopC, + } + wires.AddInMem(w, nil) + + // WireDownByUID should cleanly remove wire from memory and close StopC + if err := WireDownByUID("/proc/950/ns/net", 950); err != nil { + t.Fatalf("WireDownByUID failed: %v", err) + } + + // 1. Verify StopC closed + select { + case <-stopC: + default: + t.Fatalf("expected stopC to be closed") + } + + // 2. Verify wire is removed from in-memory active map + if _, exists := GetWireByUID("/proc/950/ns/net", 950); exists { + t.Fatalf("expected wire to be removed from in-memory map by WireDownByUID") + } +} + +func TestGRPCWireDownRemoteTriggered_RemoteNetNSFallback(t *testing.T) { + stopC := make(chan struct{}) + w := &GRPCWire{ + UID: 960, + TopoNamespace: "test-topo-ns", + LocalPodName: "podLocal", + LocalPodNetNS: "/proc/nodeB/local/ns/net", + PeerNodeIP: "10.0.0.1", + IsReady: true, + StopC: stopC, + } + wires.AddInMem(w, nil) + defer wires.AtomicDelete(w) + + // Node A sends GRPCWireDownRemoteTriggered with Node A's netns (/proc/nodeA/remote/ns/net) and TopoNs + wireDefFromRemote := &mpb.WireDef{ + LocalPodNetNs: "/proc/nodeA/remote/ns/net", + LinkUid: 960, + TopoNs: "test-topo-ns", + LocalPodName: "podRemote", + } + + if err := GRPCWireDownRemoteTriggered(wireDefFromRemote); err != nil { + t.Fatalf("GRPCWireDownRemoteTriggered failed: %v", err) + } + + // Wire should be marked down and removed + if w.IsReady { + t.Fatalf("expected wire to be marked down") + } + + select { + case <-stopC: + // success: stopC was closed + default: + t.Fatalf("expected stopC to be closed") + } + + if _, exists := GetWireByUID("/proc/nodeB/local/ns/net", 960); exists { + t.Fatalf("expected wire to be removed from memory by GRPCWireDownRemoteTriggered") + } +} + diff --git a/third_party/meshnet/daemon/grpcwire/gwire_rpc_handlers.go b/third_party/meshnet/daemon/grpcwire/gwire_rpc_handlers.go index 3d553e5a..9a214093 100644 --- a/third_party/meshnet/daemon/grpcwire/gwire_rpc_handlers.go +++ b/third_party/meshnet/daemon/grpcwire/gwire_rpc_handlers.go @@ -92,13 +92,11 @@ func CreateUpdateGRPCWireRemoteTriggered(wireDef *mpb.WireDef, stopC chan struct // When the remote peer tells the local node to remove the local end of the grpc-wire info func GRPCWireDownRemoteTriggered(wireDef *mpb.WireDef) error { - - err := WireDownByUID(wireDef.LocalPodNetNs, int(wireDef.LinkUid)) - if err != nil { - grpcOvrlyLogger.Infof("[WIRE-DOWN] Remote end failed in making down wire end in pod %s@%s,. Link uid : %d", - wireDef.LocalPodName, wireDef.IntfNameInPod, wireDef.LinkUid) - return nil + if _, ok := GetWireByUID(wireDef.LocalPodNetNs, int(wireDef.LinkUid)); ok { + return WireDownByUID(wireDef.LocalPodNetNs, int(wireDef.LinkUid)) } - - return nil + if wireDef.TopoNs != "" { + return WireDownByUID(wireDef.TopoNs, int(wireDef.LinkUid)) + } + return WireDownByUID(wireDef.LocalPodNetNs, int(wireDef.LinkUid)) } diff --git a/third_party/meshnet/daemon/meshnet/controller.go b/third_party/meshnet/daemon/meshnet/controller.go index da38a168..3ac94a6e 100644 --- a/third_party/meshnet/daemon/meshnet/controller.go +++ b/third_party/meshnet/daemon/meshnet/controller.go @@ -915,6 +915,7 @@ func (m *Meshnet) RunControllerLoop(ctx context.Context) { // If this was a local pod, clean up its links _ = m.CleanupPodLinks(ctx, topo) + _ = grpcwire.DeletePodWires(ns, name) // Reconcile dependent pods so they update / clean up their link state for _, depKey := range dependents { diff --git a/third_party/meshnet/plugin/grpcwires-plugin.go b/third_party/meshnet/plugin/grpcwires-plugin.go index 3de6f281..134fadf7 100644 --- a/third_party/meshnet/plugin/grpcwires-plugin.go +++ b/third_party/meshnet/plugin/grpcwires-plugin.go @@ -3,256 +3,16 @@ package main import ( "context" "fmt" - "net" "strings" "time" - "github.com/containernetworking/plugins/pkg/ns" mpb "github.com/openconfig/kne/third_party/meshnet/daemon/proto/meshnet/v1beta1" "github.com/openconfig/kne/third_party/meshnet/utils/wireutil" - koko "github.com/redhat-nfvpe/koko/api" log "github.com/sirupsen/logrus" "google.golang.org/grpc" "google.golang.org/grpc/credentials/insecure" ) -const ( - skipStatusRetryInterval = 2 // sec - skipStatusRetryWarnCount = 5 // generate a warning while continuing further - skipStatusRetryCount = skipStatusRetryWarnCount * 4 // how many times to retry -) - -// CreatGRPCChan sets up the local and remote ends of a gRPC wire channel between two pods on different nodes. -func CreatGRPCChan(link *mpb.Link, localPod *mpb.Pod, peerPod *mpb.Pod, localClient mpb.LocalClient, cniArgs *k8sArgs, ctx context.Context) error { - // At this point pods attached to both end of this link are both up. They have got the management IP already. - - if link == nil { - return fmt.Errorf("Add-GRPC[%s]: can't establish grpc channel. link not provided. link:%p", localPod.Name, link) - } - - log.Infof("Add-GRPC[%s]: Setting up grpc-wire:(local-pod:%s:%s@node:%s <----link uid: %d----> remote-pod:%s:%s@node:%s)", - localPod.Name, localPod.Name, link.LocalIntf, localPod.SrcIp, - link.Uid, peerPod.Name, link.PeerIntf, peerPod.SrcIp) - - log.Infof("Add-GRPC[%s]: Checking if we've been skipped for link id %d", localPod.Name, link.Uid) - isSkipped, err := localClient.IsSkipped(ctx, &mpb.SkipQuery{ - Pod: localPod.Name, - Peer: peerPod.Name, - LinkId: link.Uid, - KubeNs: string(cniArgs.K8S_POD_NAMESPACE), - }) - - if err != nil { - log.Errorf("Add-GRPC[%s]: Failed to read skipped status with peer pod %s", localPod.Name, peerPod.Name) - return err - } - - wireDef := mpb.WireDef{ - LocalPodNetNs: localPod.NetNs, - LinkUid: link.Uid, - TopoNs: localPod.KubeNs, - } - // Comparing names to determine higher priority - higherPrio := localPod.Name > peerPod.Name - - if !isSkipped.Response && !higherPrio { - /* If peer POD skipped us (booted before us) or we have a higher priority then we initiate the tunnel. - If peer POD has not skipped us (that means yet to boot or just booted) and it has higher priority - then we do not initiate the grpc tunnel. When the high priority peer pod boots up (or get ready) then - it will take care of grpc tunnel creation. This is needed to avoid the race condition when both - the pods are alive, no one has skipped each other and both of them tries to create the tunnel. In - this situation only high priority pod must create the tunnel and not the low priority one. This will - avoid conflict. */ - - ticker := time.NewTicker(time.Second * skipStatusRetryInterval) - defer ticker.Stop() - - iteration := 1 - for range ticker.C { - // Check if it has created the wire while we were waiting - resp, err := localClient.GRPCWireExists(ctx, &wireDef) - if err != nil { - return fmt.Errorf("Add-GRPC[%s]: could not check grpc wire, %s@%s: %v", localPod.Name, localPod.Name, peerPod.Name, err) - } - if resp.Response { - /* Higher priority pod has created the grpc-link. */ - log.Infof("Add-GRPC[%s]: grpc wire is already created by the remote peer. Local interface id:%d, local pod %s, peer pod %s", localPod, resp.PeerIntfId, localPod.Name, peerPod.Name) - return nil - } - - log.Infof("Add-GRPC[%s]: Retrying to read skipped status for pod %s", localPod.Name, localPod.Name) - isSkipped, err = localClient.IsSkipped(ctx, &mpb.SkipQuery{ - Pod: localPod.Name, - Peer: peerPod.Name, - LinkId: link.Uid, - KubeNs: string(cniArgs.K8S_POD_NAMESPACE), - }) - if err != nil { - log.Errorf("Add-GRPC[%s]: Failed to read skipped status from peer pod %s", localPod.Name, peerPod.Name) - return err - } - - if !isSkipped.Response { - if iteration > skipStatusRetryWarnCount { - log.Warnf("Add-GRPC[%s]: Local pod %s is taking longer time (retry %d) to read skip status, skipped by peer %s.", localPod.Name, localPod.Name, iteration, peerPod.Name) - if iteration == skipStatusRetryCount { - log.Infof("Add-GRPC[%s]: Pod %s is not skipped by higher priority pod %s. Link between %s and %s will be created by higher priority pod.", - localPod.Name, localPod.Name, peerPod.Name, localPod.Name, peerPod.Name) - return nil - } - } - iteration++ - } else { - log.Infof("Add-GRPC[%s]: Local pod %s is skipped by peer %s. So we can create wire now", localPod.Name, localPod.Name, peerPod.Name) - break - } - } // end of for - } - - resp, err := localClient.GRPCWireExists(ctx, &wireDef) - if err != nil { - return fmt.Errorf("Add-GRPC[%s]: could not check grpc wire, %s@%s: %v", localPod.Name, localPod.Name, peerPod.Name, err) - } - if resp.Response { - /* While this pod was busy creating other links or was busy with some other task, the remote - pod had finished creating this grpc-link. */ - log.Infof("Add-GRPC[%s]: grpc wire is already set by the remote peer. Local interface id:%d", localPod.Name, resp.PeerIntfId) - return nil - } - - // Create the local end of the grpc-wire - currNs, err := ns.GetCurrentNS() - if err != nil { - return fmt.Errorf("Add-GRPC[%s]: creating GRPC wire for pod %s : failed to get node ns, err: %v", localPod.Name, localPod.Name, err) - } - - // Build koko's veth struct for the intf to be placed inside the pod - inConIntfNm := link.LocalIntf - inContainerVeth, err := makeVeth(localPod.NetNs, inConIntfNm, link.LocalIp) - if err != nil { - log.Errorf("Add-GRPC[%s]: Could not create vEth for local pod %s:%s, peer pod %s, err %v", localPod.Name, localPod.Name, inConIntfNm, peerPod.Name, err) - return err - } - - respIntfName, err := localClient.GenerateNodeInterfaceName(ctx, &mpb.GenerateNodeInterfaceNameRequest{PodIntfName: link.LocalIntf, PodName: localPod.Name}) - if err != nil { - return fmt.Errorf("Add-GRPC[%s]: could not create node interface for local pod %s, peer pod %s: %v", localPod.Name, err, localPod.Name, peerPod.Name) - } - - hostEndVeth := &koko.VEth{ - LinkName: respIntfName.NodeIntfName, - NsName: currNs.Path()} - - if err = koko.MakeVeth(*inContainerVeth, *hostEndVeth); err != nil { - return fmt.Errorf("Add-GRPC[%s]: creating GRPC wire: failed to create vEth-pair inside pod (%s:%s) and on host (%s). err:%s", - localPod.Name, localPod.Name, inContainerVeth.LinkName, hostEndVeth.LinkName, err) - } - - /* Dial the remote peer to create the remote end of the grpc tunnel. */ - - url := fmt.Sprintf("%s:%d", peerPod.SrcIp, wireutil.GRPCDefaultPort) - url = strings.TrimSpace(url) - remote, err := grpc.Dial(url, grpc.WithTransportCredentials(insecure.NewCredentials())) - if err != nil { - return fmt.Errorf("Add-GRPC[%s]: creating GRPC wire: failed to dial remote gRPC url %s", localPod.Name, url) - } - remoteClient := mpb.NewRemoteClient(remote) - locInf, err := net.InterfaceByName(hostEndVeth.LinkName) - if err != nil { - return fmt.Errorf("Add-GRPC[%s]: could not get interface by name: %v", localPod.Name, err) - } - - wireDefRemot := mpb.WireDef{ - /*WireIfIdOnPeerNode : is the interface id on which a node receives grpc - packets from the remote pod (hosted in a remote node). Through this interface - the remote packets are delivered to the local pod. - - WireIfIdOnPeerNode is the interface that will be used to send packets to the remote pod also. - Packets coming from local pods will be received on this interface, will be encapsulated - in grpc to send it to the peer node (that is hosting the remote pod). - - The remote pod must send packets to this interface for this grpc-wire, to reach - the connected container in this node. For remote pod this must be the destination - interface id to reach the connected pod in this node. From remote pods perspective, - the local interface of this node is the "PeerIntfId" for the remote pod in remote machine */ - WireIfIdOnPeerNode: int64(locInf.Index), - - /* PeerIp: Ip address of the peer machine/node. - For remote pod this must be the IP address on this host. The remote pod must - Transport packets to this pod (over grpc) in this local node. This is the IP - address of the local node which remote node will do a grpc dial, to send - packets over grpc wire. */ - PeerNodeIp: localPod.SrcIp, - - /* We need to tell the remote node, what is the kne specified in container interface name. - We also need to tell to which network namespace the pod in remote node belongs to. */ - IntfNameInPod: link.PeerIntf, - LocalPodNetNs: peerPod.NetNs, - LocalPodName: peerPod.Name, // name of the remote pod - - /*meshnet assigned unique identifier for this link */ - LinkUid: link.Uid, - TopoNs: peerPod.KubeNs, - LocalPodIp: link.PeerIp, - } - - log.Infof("Add-GRPC[%s]: Create GRPC wire: dialing remote node-->%s@%s", localPod.Name, peerPod.Name, url) - creatResp, err := remoteClient.AddGRPCWireRemote(ctx, &wireDefRemot) - if err != nil { - return fmt.Errorf("Add-GRPC[%s]: failed to create grpc tunnel ar remote end:%s err:%v", localPod.Name, url, err) - } else if !creatResp.Response { - return fmt.Errorf("Add-GRPC[%s]: remote end of the grpc-wire (local-pod:%s:%s@node:%s <----link uid: %d----> remote-pod:%s:%s@node:%s) is not up", - localPod.Name, localPod.Name, link.LocalIntf, localPod.SrcIp, - link.Uid, peerPod.Name, link.PeerIntf, peerPod.SrcIp) - } - - /* remote has finished its job. Register local end of the grpc wire with the daemon - and start the packet sending thread. */ - wireDefLocal := mpb.WireDef{ - /*PeerIntfId : this is the interface id (in the remote machine) to which the host/local machine will send grpc - packets for the remote pod. This interface id will be encoded in every packet - sent over this grpc-wire. This interface id is created in the remote machine and - communicated by the remote machine. Availability of this interface id indicates remote - machine is ready to receive packets over this grpc-wire. Remote machine will use this - interface id to pass the packets to the remote pod. */ - WireIfIdOnPeerNode: creatResp.PeerIntfId, - - /* PeerIp : Ip address of the remote node, to which this local node is sending packets over - this grpc-wire. - */ - PeerNodeIp: peerPod.SrcIp, - - /* WireIfNameOnLocalNode : name of the local machine interface, from where packets generated by the local - pod will be picked up and transported over grpc to remote. local meshnet daemon will receive - packets from local pod on this interface. - */ - WireIfNameOnLocalNode: respIntfName.NodeIntfName, - - /*meshnet assigned unique identifier for this link */ - LinkUid: link.Uid, - LocalPodName: localPod.Name, - IntfNameInPod: link.LocalIntf, - LocalPodNetNs: localPod.NetNs, - LocalPodIp: link.LocalIp, - TopoNs: localPod.KubeNs, - } - log.Infof("Add-GRPC[%s]: Creating GRPC wire: adding the local end of the grpc tunnel.", localPod.Name) - r, err := localClient.AddGRPCWireLocal(ctx, &wireDefLocal) - if err != nil { - return fmt.Errorf("Add-GRPC[%s]: failed to create local end of the tunnel %v", localPod.Name, err) - } else if !r.Response { - return fmt.Errorf("Add-GRPC[%s]: local end of the grpc-wire (local-pod:-%s:%s@node:%s <----link uid: %d----> remote-pod:-%s:%s@node:%s) is not up", - localPod.Name, localPod.Name, link.LocalIntf, localPod.SrcIp, - link.Uid, peerPod.Name, link.PeerIntf, peerPod.SrcIp) - } - - log.Infof("Add-GRPC[%s]: Successfully created grpc-wire (local-pod:%s:%s@node:%s:%d <----link uid: %d----> remote-pod:%s:%s@node:%s:%d)", - localPod.Name, localPod.Name, link.LocalIntf, localPod.SrcIp, locInf.Index, - link.Uid, peerPod.Name, link.PeerIntf, peerPod.SrcIp, creatResp.PeerIntfId) - - return nil -} - // MakeGRPCChanDown signals the remote peer node to tear down the remote gRPC wire end when a pod is deleted. func MakeGRPCChanDown(link *mpb.Link, localPod *mpb.Pod, peerPod *mpb.Pod, ctx context.Context) error { if link == nil { @@ -267,6 +27,7 @@ func MakeGRPCChanDown(link *mpb.Link, localPod *mpb.Pod, peerPod *mpb.Pod, ctx c if err != nil { return fmt.Errorf("MakeGRPCChanDown failed to dial remote gRPC url %s", url) } + defer remote.Close() remoteClient := mpb.NewRemoteClient(remote) wireDefRemot := mpb.WireDef{ @@ -282,7 +43,9 @@ func MakeGRPCChanDown(link *mpb.Link, localPod *mpb.Pod, peerPod *mpb.Pod, ctx c } log.Infof("MakeGRPCChanDown: dialing remote node-->%s@%s", peerPod.Name, url) - removeResp, err := remoteClient.GRPCWireDownRemote(ctx, &wireDefRemot) + rpcCtx, rpcCancel := context.WithTimeout(ctx, 5*time.Second) + defer rpcCancel() + removeResp, err := remoteClient.GRPCWireDownRemote(rpcCtx, &wireDefRemot) if err != nil { return fmt.Errorf("MakeGRPCChanDown: GRPC communication error for : %s, err:%v", url, err) } else if !removeResp.Response { @@ -293,3 +56,4 @@ func MakeGRPCChanDown(link *mpb.Link, localPod *mpb.Pod, peerPod *mpb.Pod, ctx c return nil } + diff --git a/third_party/meshnet/plugin/meshnet.go b/third_party/meshnet/plugin/meshnet.go index a5a34e21..6eaf2698 100644 --- a/third_party/meshnet/plugin/meshnet.go +++ b/third_party/meshnet/plugin/meshnet.go @@ -7,6 +7,7 @@ import ( "net" "os" "runtime" + "strings" "time" "github.com/containernetworking/cni/pkg/skel" @@ -393,19 +394,6 @@ func cmdDel(args *skel.CmdArgs) error { // instead of failing, just log the error and move on log.Errorf("Del: Error removing Veth link %s (%s) on pod %s: %v", link.LocalIntf, linkType, localPod.Name, err) } - - // Setting reversed skipped flag so that this pod will try to connect veth pair on restart - log.Infof("Del: Setting skip-reverse flag on peer %s@%s(link id %d) for local interface %s", link.PeerPod, link.PeerIntf, link.Uid, link.LocalIntf) - ok, err := meshnetClient.SkipReverse(ctx, &mpb.SkipQuery{ - Pod: localPod.Name, - Peer: link.PeerPod, - LinkId: link.Uid, - KubeNs: string(cniArgs.K8S_POD_NAMESPACE), - }) - if err != nil || !ok.Response { - log.Errorf("Del: Failed to set skip reversed flag on our peer %s", link.PeerPod) - return err - } } return nil } @@ -418,12 +406,12 @@ func SetInterNodeLinkType() { // via means of file on host (which is read below) containing the value GRPC or VXLAN b, err := os.ReadFile("/etc/cni/net.d/meshnet-inter-node-link-type") if err != nil { - log.Warningf("Could not read iner node link type: %v", err) + log.Warningf("Could not read inner node link type: %v", err) // use the default value return } - interNodeLinkType = string(b) + interNodeLinkType = strings.TrimSpace(string(b)) } // ------------------------------------------------------------------------------------------------- From 5a0999e6ce0b411592ac3b1076d88b9ebab2c7df Mon Sep 17 00:00:00 2001 From: Kris Raney Date: Tue, 18 Aug 2026 00:51:53 +0000 Subject: [PATCH 13/18] K8s APIserver burden improvement MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. Standard Kubernetes Informer Integration (third_party/meshnet/daemon/meshnet/controller.go) • Integrated DynamicSharedInformerFactory in RunControllerLoop. • Cache sync, event streaming, and watch reconnection are now managed by standard client-go Informer infrastructure. Watch event callbacks update topoCache and trigger debounced reconciliation in RAM with 0 unnecessary API server List calls. 2. CLI Initialization Fail-Fast Timeout (topo/node/node.go) • Updated GetCLIConn to apply a 5-second WithTimeoutOps during early boot connection attempts (d.Open()). • When pods enter PodRunning while router OS containers (JunOS, SR Linux, cEOS) are still booting up, d.Open() fails in 5 seconds instead of stalling on prompt-matching timeouts for 30–60 seconds per attempt. Once ready, full operational driver settings are restored. 3. Eliminated Arista Status() Watch Thrashing (topo/node/arista/arista.go & arista_test.go) • Replaced continuous short-lived Watch() stream creation in arista.go Status() with direct single-pod Get() queries via n.Pods(ctx). • Updated arista_test.go to test Get reactors. --- .../meshnet/daemon/meshnet/controller.go | 111 +++++++++++++++--- third_party/meshnet/go.mod | 1 + third_party/meshnet/go.sum | 2 + topo/node/arista/arista.go | 35 +++--- topo/node/arista/arista_test.go | 45 ++++--- topo/node/node.go | 16 ++- 6 files changed, 151 insertions(+), 59 deletions(-) diff --git a/third_party/meshnet/daemon/meshnet/controller.go b/third_party/meshnet/daemon/meshnet/controller.go index 3ac94a6e..ac3f0a14 100644 --- a/third_party/meshnet/daemon/meshnet/controller.go +++ b/third_party/meshnet/daemon/meshnet/controller.go @@ -21,7 +21,11 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/watch" + "k8s.io/client-go/dynamic/dynamicinformer" + "k8s.io/client-go/tools/cache" "k8s.io/client-go/util/retry" + + topologyclientv1 "github.com/openconfig/kne/third_party/meshnet/api/clientset/v1beta1" ) // isNetNSValid returns true if the netns path exists on the host filesystem. @@ -814,9 +818,6 @@ func (m *Meshnet) runReconcileWorker(ctx context.Context) { } } -// RunControllerLoop runs the continuous level-triggered Topology controller in meshnetd. -// It maintains an in-memory topology cache, tracks pod-link dependencies, and coalesces -// incoming events into targeted background reconciliation runs. func (m *Meshnet) RunControllerLoop(ctx context.Context) { mnetdLogger.Infof("Starting Topology controller loop") if m.topoCache == nil { @@ -826,7 +827,98 @@ func (m *Meshnet) RunControllerLoop(ctx context.Context) { m.reconcileQueue = NewReconcileQueue(50 * time.Millisecond) } - // 1. Initial full population of cache from K8s API + go m.runReconcileWorker(ctx) + + if m.GWireDynClient != nil { + factory := dynamicinformer.NewDynamicSharedInformerFactory(m.GWireDynClient, 60*time.Second) + informer := factory.ForResource(topologyclientv1.GVR()) + + informer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ + AddFunc: func(obj interface{}) { + topo, err := toUnstructured(obj) + if err != nil || topo == nil { + return + } + ns := topo.GetNamespace() + name := topo.GetName() + key := fmt.Sprintf("%s/%s", ns, name) + + m.topoCache.Put(topo) + srcIP, _, active := isPodActive(topo) + if active && (m.nodeIP == "" || srcIP == m.nodeIP) { + m.enqueueReconcile(key) + } + for _, depKey := range m.topoCache.GetDependents(ns, name) { + depNS, depName := parseKey(depKey) + if depTopo := m.topoCache.Get(depNS, depName); depTopo != nil { + depSrcIP, _, depActive := isPodActive(depTopo) + if depActive && (m.nodeIP == "" || depSrcIP == m.nodeIP) { + m.enqueueReconcile(depKey) + } + } + } + }, + UpdateFunc: func(oldObj, newObj interface{}) { + topo, err := toUnstructured(newObj) + if err != nil || topo == nil { + return + } + ns := topo.GetNamespace() + name := topo.GetName() + key := fmt.Sprintf("%s/%s", ns, name) + + m.topoCache.Put(topo) + srcIP, _, active := isPodActive(topo) + if active && (m.nodeIP == "" || srcIP == m.nodeIP) { + m.enqueueReconcile(key) + } + for _, depKey := range m.topoCache.GetDependents(ns, name) { + depNS, depName := parseKey(depKey) + if depTopo := m.topoCache.Get(depNS, depName); depTopo != nil { + depSrcIP, _, depActive := isPodActive(depTopo) + if depActive && (m.nodeIP == "" || depSrcIP == m.nodeIP) { + m.enqueueReconcile(depKey) + } + } + } + }, + DeleteFunc: func(obj interface{}) { + topo, err := toUnstructured(obj) + if err != nil || topo == nil { + return + } + ns := topo.GetNamespace() + name := topo.GetName() + + dependents := m.topoCache.GetDependents(ns, name) + m.topoCache.Delete(ns, name) + + _ = m.CleanupPodLinks(ctx, topo) + _ = grpcwire.DeletePodWires(ns, name) + + for _, depKey := range dependents { + depNS, depName := parseKey(depKey) + if depTopo := m.topoCache.Get(depNS, depName); depTopo != nil { + depSrcIP, _, depActive := isPodActive(depTopo) + if depActive && (m.nodeIP == "" || depSrcIP == m.nodeIP) { + m.enqueueReconcile(depKey) + } + } + } + }, + }) + + factory.Start(ctx.Done()) + if cache.WaitForCacheSync(ctx.Done(), informer.Informer().HasSynced) { + mnetdLogger.Infof("Topology informer cache synced successfully") + m.enqueueFullReconcile() + <-ctx.Done() + return + } + mnetdLogger.Warnf("Topology informer cache failed to sync, falling back to manual watch loop") + } + + // Fallback manual watch loop for test environments without GWireDynClient if m.tClient != nil { list, err := m.tClient.Topology(metav1.NamespaceAll).List(ctx, metav1.ListOptions{}) if err == nil && list != nil { @@ -840,8 +932,6 @@ func (m *Meshnet) RunControllerLoop(ctx context.Context) { } } - go m.runReconcileWorker(ctx) - // Trigger initial full reconciliation on startup m.enqueueFullReconcile() @@ -885,17 +975,13 @@ func (m *Meshnet) RunControllerLoop(ctx context.Context) { switch event.Type { case watch.Added, watch.Modified: - // Update cache m.topoCache.Put(topo) - - // Check if this pod is local to this node srcIP, _, active := isPodActive(topo) isLocal := active && (m.nodeIP == "" || srcIP == m.nodeIP) if isLocal { m.enqueueReconcile(key) } - // Find all local dependent pods that have links to this pod dependents := m.topoCache.GetDependents(ns, name) for _, depKey := range dependents { depNS, depName := parseKey(depKey) @@ -909,15 +995,11 @@ func (m *Meshnet) RunControllerLoop(ctx context.Context) { } case watch.Deleted: - // Before removing from cache, get all dependent pods dependents := m.topoCache.GetDependents(ns, name) m.topoCache.Delete(ns, name) - - // If this was a local pod, clean up its links _ = m.CleanupPodLinks(ctx, topo) _ = grpcwire.DeletePodWires(ns, name) - // Reconcile dependent pods so they update / clean up their link state for _, depKey := range dependents { depNS, depName := parseKey(depKey) depTopo := m.topoCache.Get(depNS, depName) @@ -931,7 +1013,6 @@ func (m *Meshnet) RunControllerLoop(ctx context.Context) { } } } - // If watch closed, queue full resync on reconnect m.enqueueFullReconcile() } } diff --git a/third_party/meshnet/go.mod b/third_party/meshnet/go.mod index e51a4e48..47649f4b 100644 --- a/third_party/meshnet/go.mod +++ b/third_party/meshnet/go.mod @@ -54,6 +54,7 @@ require ( github.com/opencontainers/go-digest v1.0.0 // indirect github.com/opencontainers/image-spec v1.1.0 // indirect github.com/pkg/errors v0.9.1 // indirect + github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect github.com/spf13/pflag v1.0.9 // indirect github.com/vishvananda/netns v0.0.5 // indirect github.com/x448/float16 v0.8.4 // indirect diff --git a/third_party/meshnet/go.sum b/third_party/meshnet/go.sum index 990a110a..ed02f1c7 100644 --- a/third_party/meshnet/go.sum +++ b/third_party/meshnet/go.sum @@ -247,6 +247,8 @@ go.opentelemetry.io/proto/otlp v1.9.0 h1:l706jCMITVouPOqEnii2fIAuO3IVGBRPV5ICjce go.opentelemetry.io/proto/otlp v1.9.0/go.mod h1:xE+Cx5E/eEHw+ISFkwPLwCZefwVjY+pqKg1qcK03+/4= go.uber.org/atomic v1.7.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc= go.uber.org/goleak v1.1.10/go.mod h1:8a7PlsEVH3e/a/GLqe5IIrQx6GzcnRmZEufDUTk4A7A= +go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto= +go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE= go.uber.org/multierr v1.6.0/go.mod h1:cdWPpRnG4AhwMwsgIHip0KRBQjJy5kYEpYjJxpXp9iU= go.uber.org/multierr v1.11.0 h1:blXXJkSxSSfBVBlC76pxqeO+LN3aDfLQo+309xJstO0= go.uber.org/multierr v1.11.0/go.mod h1:20+QtiLqy0Nd6FdQB9TLXag12DsQkrbs3htMFfDN80Y= diff --git a/topo/node/arista/arista.go b/topo/node/arista/arista.go index e3c70a55..862529a7 100644 --- a/topo/node/arista/arista.go +++ b/topo/node/arista/arista.go @@ -36,7 +36,6 @@ import ( "google.golang.org/protobuf/proto" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/fields" log "k8s.io/klog/v2" ceos "github.com/aristanetworks/arista-ceoslab-operator/v2/api/v1alpha1" @@ -164,28 +163,26 @@ func (n *Node) Create(ctx context.Context) error { } func (n *Node) Status(ctx context.Context) (node.Status, error) { - w, err := n.KubeClient.CoreV1().Pods(n.Namespace).Watch(ctx, metav1.ListOptions{ - FieldSelector: fields.SelectorFromSet(fields.Set{metav1.ObjectNameField: n.Name()}).String(), - }) + p, err := n.Pods(ctx) if err != nil { + if strings.Contains(err.Error(), "not found") { + return node.StatusUnknown, nil + } return node.StatusFailed, err } - status := node.StatusUnknown - for e := range w.ResultChan() { - p, ok := e.Object.(*corev1.Pod) - if !ok { - continue - } - if p.Status.Phase == corev1.PodPending { - status = node.StatusPending - break - } - if p.Status.Phase == corev1.PodRunning { - status = node.StatusRunning - break - } + if len(p) != 1 { + return node.StatusUnknown, nil + } + switch p[0].Status.Phase { + case corev1.PodPending: + return node.StatusPending, nil + case corev1.PodRunning: + return node.StatusRunning, nil + case corev1.PodFailed: + return node.StatusFailed, nil + default: + return node.StatusUnknown, nil } - return status, nil } func (n *Node) CreateConfig(ctx context.Context) (*corev1.Volume, error) { diff --git a/topo/node/arista/arista_test.go b/topo/node/arista/arista_test.go index d717d6ec..3a99ade8 100644 --- a/topo/node/arista/arista_test.go +++ b/topo/node/arista/arista_test.go @@ -36,6 +36,7 @@ import ( "google.golang.org/protobuf/types/known/anypb" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/watch" "k8s.io/client-go/kubernetes/fake" "k8s.io/client-go/rest" @@ -590,32 +591,28 @@ func TestStatus(t *testing.T) { for _, tt := range tests { t.Run(tt.desc, func(t *testing.T) { name := "pod1" - ki := fake.NewSimpleClientset(&corev1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - }, - }) + ns := "default" + var ki *fake.Clientset + if tt.noPodYet { + ki = fake.NewSimpleClientset() + } else { + ki = fake.NewSimpleClientset(&corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: ns, + }, + Status: corev1.PodStatus{ + Phase: tt.phase, + }, + }) + } - reaction := func(action ktest.Action) (handled bool, ret watch.Interface, err error) { - if tt.cantWatch { - err = errors.New("") - return true, nil, err - } - f := &fakeWatch{} - if !tt.noPodYet { - f.e = []watch.Event{{ - Object: &corev1.Pod{ - Status: corev1.PodStatus{ - Phase: tt.phase, - }, - }, - }} - } - return true, f, nil + if tt.cantWatch { + ki.PrependReactor("get", "pods", func(action ktest.Action) (bool, runtime.Object, error) { + return true, nil, errors.New("failed to get pod") + }) } - ki.PrependWatchReactor("*", reaction) - ns := "default" node := &Node{ Impl: &node.Impl{ KubeClient: ki, @@ -629,7 +626,7 @@ func TestStatus(t *testing.T) { t.Errorf("Status() unexpected err: %s", s) } if s := cmp.Diff(tt.status, status); s != "" { - t.Errorf("New() CEosLabDevice CRDs unexpected diff (-want +got):\n%s", s) + t.Errorf("Status() unexpected diff (-want +got):\n%s", s) } }) } diff --git a/topo/node/node.go b/topo/node/node.go index 114b3ed7..ca1a6c69 100644 --- a/topo/node/node.go +++ b/topo/node/node.go @@ -730,11 +730,14 @@ func (n *Impl) GetCLIConn(platform string, opts []scrapliutil.Option) (*scraplin opts = append(opts, scrapliopts.WithLogger(li)) } + // Fast-fail initial CLI open attempts during early boot (5s timeout per attempt) + openOpts := append([]scrapliutil.Option{scrapliopts.WithTimeoutOps(5 * time.Second)}, opts...) + for { p, err := scrapliplatform.NewPlatform( platform, n.Name(), - opts..., + openOpts..., ) if err != nil { log.Errorf("failed to fetch platform instance for device %s; error: %+v\n", err, n.Name()) @@ -755,6 +758,17 @@ func (n *Impl) GetCLIConn(platform string, opts []scrapliutil.Option) (*scraplin log.V(1).Infof("%s - Cli ready.", n.Name()) + // Re-initialize with full caller options if operation timeout was overridden + pFull, err := scrapliplatform.NewPlatform(platform, n.Name(), opts...) + if err == nil { + if dFull, err := pFull.GetNetworkDriver(); err == nil { + _ = d.Close() + if err = dFull.Open(); err == nil { + return dFull, nil + } + } + } + return d, nil } } From 5f6c914f10afe2551faeda080834f332e1242247 Mon Sep 17 00:00:00 2001 From: Kris Raney Date: Tue, 18 Aug 2026 02:01:04 +0000 Subject: [PATCH 14/18] Fix broken rate default check These variables are initialized to a non-zero default, so the check was never successful --- deploy/deploy.go | 4 ++-- third_party/meshnet/daemon/meshnet/meshnet.go | 4 ++-- topo/topo.go | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/deploy/deploy.go b/deploy/deploy.go index 77c3b1f3..a6e65b1b 100644 --- a/deploy/deploy.go +++ b/deploy/deploy.go @@ -207,10 +207,10 @@ func (d *Deployment) Deploy(ctx context.Context, kubecfg string) (rerr error) { if err != nil { return fmt.Errorf("failed to create k8s config: %w", err) } - if rCfg.QPS == 0 { + if rCfg.QPS < 100 { rCfg.QPS = 100 } - if rCfg.Burst == 0 { + if rCfg.Burst < 200 { rCfg.Burst = 200 } kClient, err := kubernetes.NewForConfig(rCfg) diff --git a/third_party/meshnet/daemon/meshnet/meshnet.go b/third_party/meshnet/daemon/meshnet/meshnet.go index ade9f48c..8d67f19a 100644 --- a/third_party/meshnet/daemon/meshnet/meshnet.go +++ b/third_party/meshnet/daemon/meshnet/meshnet.go @@ -76,10 +76,10 @@ func restConfig() (*rest.Config, error) { return nil, err } } - if rCfg.QPS == 0 { + if rCfg.QPS < 100 { rCfg.QPS = 100 } - if rCfg.Burst == 0 { + if rCfg.Burst < 200 { rCfg.Burst = 200 } return rCfg, nil diff --git a/topo/topo.go b/topo/topo.go index e3525f14..05c91d73 100644 --- a/topo/topo.go +++ b/topo/topo.go @@ -186,10 +186,10 @@ func New(topo *tpb.Topology, opts ...Option) (*Manager, error) { } m.rCfg = rCfg } - if m.rCfg.QPS == 0 { + if m.rCfg.QPS < 100 { m.rCfg.QPS = 100 } - if m.rCfg.Burst == 0 { + if m.rCfg.Burst < 200 { m.rCfg.Burst = 200 } if m.kClient == nil { From 848c365b1aca686bcb1d267a4ea2e2e80a78c84a Mon Sep 17 00:00:00 2001 From: Kris Raney Date: Tue, 18 Aug 2026 14:57:41 +0000 Subject: [PATCH 15/18] Fix codespell --- topo/node/arista/arista.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/topo/node/arista/arista.go b/topo/node/arista/arista.go index 862529a7..fea19833 100644 --- a/topo/node/arista/arista.go +++ b/topo/node/arista/arista.go @@ -419,7 +419,7 @@ func (n *Node) ResetCfg(ctx context.Context) error { } if resp.Failed == nil { - log.Infof("%s - finshed resetting config", n.Name()) + log.Infof("%s - finished resetting config", n.Name()) } return resp.Failed From 4e5e3b8f45c8c8744ae4c9a6c0670a592f870356 Mon Sep 17 00:00:00 2001 From: Kris Raney Date: Tue, 18 Aug 2026 18:43:19 +0000 Subject: [PATCH 16/18] More robust telnet close Don't hang and fail if the close sequence can't be sent (already closed.) Don't return error if the close message can't be sent. --- topo/node/cisco/cisco.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/topo/node/cisco/cisco.go b/topo/node/cisco/cisco.go index 5fb560db..8c8e1578 100644 --- a/topo/node/cisco/cisco.go +++ b/topo/node/cisco/cisco.go @@ -659,7 +659,9 @@ func endTelnet(d *scraplinetwork.Driver) error { // sending ctrl + ] (^]) to end telnet session gracefully. Otherwise, the next connection can be blocked. endTelnet := string(byte(29)) + " quit\n" log.Infof("Closing the connection by sending ctrl+] quit \n") - d.SendCommand(endTelnet) + if _, err := d.Channel.SendInput(endTelnet, scrapliopts.WithTimeoutOps(1*time.Second)); err != nil { + log.V(1).Infof("endTelnet: failed to send telnet quit sequence: %v", err) + } return nil } From f30da53d7f03885dd259feec8ace8dc12ca397ed Mon Sep 17 00:00:00 2001 From: Kris Raney Date: Tue, 18 Aug 2026 20:13:23 +0000 Subject: [PATCH 17/18] Revert change that caused cleanup problems. --- topo/node/node.go | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/topo/node/node.go b/topo/node/node.go index ca1a6c69..ebaa7b95 100644 --- a/topo/node/node.go +++ b/topo/node/node.go @@ -730,14 +730,11 @@ func (n *Impl) GetCLIConn(platform string, opts []scrapliutil.Option) (*scraplin opts = append(opts, scrapliopts.WithLogger(li)) } - // Fast-fail initial CLI open attempts during early boot (5s timeout per attempt) - openOpts := append([]scrapliutil.Option{scrapliopts.WithTimeoutOps(5 * time.Second)}, opts...) - for { p, err := scrapliplatform.NewPlatform( platform, n.Name(), - openOpts..., + opts..., ) if err != nil { log.Errorf("failed to fetch platform instance for device %s; error: %+v\n", err, n.Name()) @@ -757,18 +754,6 @@ func (n *Impl) GetCLIConn(platform string, opts []scrapliutil.Option) (*scraplin } log.V(1).Infof("%s - Cli ready.", n.Name()) - - // Re-initialize with full caller options if operation timeout was overridden - pFull, err := scrapliplatform.NewPlatform(platform, n.Name(), opts...) - if err == nil { - if dFull, err := pFull.GetNetworkDriver(); err == nil { - _ = d.Close() - if err = dFull.Open(); err == nil { - return dFull, nil - } - } - } - return d, nil } } From d413bf6954ddfa26a49d1a94ba3b9965f0f4d6e2 Mon Sep 17 00:00:00 2001 From: Kris Raney Date: Fri, 21 Aug 2026 22:23:07 +0000 Subject: [PATCH 18/18] Address review comments: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. Apply mutex hat idiom in GRPCWire struct (third_party/meshnet/daemon/grpcwire/grpcwire.go): • Reordered GRPCWire fields to place sync.Mutex directly above the mutable fields it synchronizes (IsReady, WireIfaceIDOnPeerNode, PeerNodeIP, StopC). 2. Handle DeletedFinalStateUnknown tombstones (third_party/meshnet/daemon/meshnet/controller.go): • Unwrapped cache.DeletedFinalStateUnknown in toUnstructured and informer DeleteFunc to prevent missed topology deletion events during watch reconnects. 3. Make remote wire teardown non-blocking (third_party/meshnet/daemon/meshnet/controller.go): • Dispatched GRPCWireDownRemote RPCs asynchronously in cleanupRemovedPodLinks to prevent blocking the main reconciliation loop when peer nodes are unreachable. --- .../meshnet/daemon/grpcwire/grpcwire.go | 13 +++--- .../meshnet/daemon/meshnet/controller.go | 41 ++++++++++++------- 2 files changed, 33 insertions(+), 21 deletions(-) diff --git a/third_party/meshnet/daemon/grpcwire/grpcwire.go b/third_party/meshnet/daemon/grpcwire/grpcwire.go index 818c7b17..adcfa7f5 100644 --- a/third_party/meshnet/daemon/grpcwire/grpcwire.go +++ b/third_party/meshnet/daemon/grpcwire/grpcwire.go @@ -91,17 +91,16 @@ type GRPCWire struct { LocalPodIfaceName string // Name the interface which is inside the local pod who will consume packets over this wire. This is for debugging LocalPodNetNS string - /*Peer pod information*/ - WireIfaceIDOnPeerNode int64 // Peer end of the wire interface ID which is present in peer node - PeerNodeIP string // Peer node IP - - IsReady bool // Is this wire ip. Originator grpcWireOriginator // create by local host or create on trigger from remote host. This is for debugging. OriginatorIP string // IP address of the host created it. This is for debugging. - StopC chan struct{} // the channel to send stop signal to the receive thread. stopOnce sync.Once - mu sync.Mutex + + mu sync.Mutex + IsReady bool // Is this wire ready. + WireIfaceIDOnPeerNode int64 // Peer end of the wire interface ID which is present in peer node + PeerNodeIP string // Peer node IP + StopC chan struct{} // the channel to send stop signal to the receive thread. } // CloseStopC safely closes the wire's StopC channel at most once. diff --git a/third_party/meshnet/daemon/meshnet/controller.go b/third_party/meshnet/daemon/meshnet/controller.go index ac3f0a14..df45f27f 100644 --- a/third_party/meshnet/daemon/meshnet/controller.go +++ b/third_party/meshnet/daemon/meshnet/controller.go @@ -63,6 +63,9 @@ func (m *Meshnet) clearPodAliveStatus(ctx context.Context, topo *unstructured.Un // toUnstructured converts any Kubernetes object into *unstructured.Unstructured. func toUnstructured(obj interface{}) (*unstructured.Unstructured, error) { + if tombstone, ok := obj.(cache.DeletedFinalStateUnknown); ok { + obj = tombstone.Obj + } if u, ok := obj.(*unstructured.Unstructured); ok { return u, nil } @@ -173,20 +176,27 @@ func (m *Meshnet) cleanupRemovedPodLinks(ctx context.Context, topo *unstructured wire.UID, wire.LocalPodName, wire.LocalPodIfaceName) if wire.PeerNodeIP != "" && wire.PeerNodeIP != m.nodeIP && wire.PeerNodeIP != "localhost" && wire.PeerNodeIP != "127.0.0.1" { - url := fmt.Sprintf("%s:%d", wire.PeerNodeIP, wireutil.GRPCDefaultPort) - url = strings.TrimSpace(url) - if remoteConn, err := grpc.Dial(url, grpc.WithTransportCredentials(insecure.NewCredentials())); err == nil { - remoteClient := mpb.NewRemoteClient(remoteConn) - rpcCtx, rpcCancel := context.WithTimeout(ctx, 5*time.Second) - _, _ = remoteClient.GRPCWireDownRemote(rpcCtx, &mpb.WireDef{ - TopoNs: wire.TopoNamespace, - LocalPodName: wire.LocalPodName, - LocalPodNetNs: wire.LocalPodNetNS, - LinkUid: int64(wire.UID), - }) - rpcCancel() - remoteConn.Close() - } + peerIP := wire.PeerNodeIP + topoNs := wire.TopoNamespace + podName := wire.LocalPodName + podNetNs := wire.LocalPodNetNS + linkUID := int64(wire.UID) + go func() { + url := fmt.Sprintf("%s:%d", peerIP, wireutil.GRPCDefaultPort) + url = strings.TrimSpace(url) + if remoteConn, err := grpc.Dial(url, grpc.WithTransportCredentials(insecure.NewCredentials())); err == nil { + defer remoteConn.Close() + remoteClient := mpb.NewRemoteClient(remoteConn) + rpcCtx, rpcCancel := context.WithTimeout(context.Background(), 5*time.Second) + defer rpcCancel() + _, _ = remoteClient.GRPCWireDownRemote(rpcCtx, &mpb.WireDef{ + TopoNs: topoNs, + LocalPodName: podName, + LocalPodNetNs: podNetNs, + LinkUid: linkUID, + }) + } + }() } _ = grpcwire.RemoveWireAcrosAll(wire, true) @@ -883,6 +893,9 @@ func (m *Meshnet) RunControllerLoop(ctx context.Context) { } }, DeleteFunc: func(obj interface{}) { + if tombstone, ok := obj.(cache.DeletedFinalStateUnknown); ok { + obj = tombstone.Obj + } topo, err := toUnstructured(obj) if err != nil || topo == nil { return