From 424a5d7744df4a0f626ef3bb0dcabadfebc409b3 Mon Sep 17 00:00:00 2001 From: Nino Kodabande Date: Tue, 12 Nov 2024 18:19:33 -0800 Subject: [PATCH 1/4] Fixes incompatible log format Some of the logs outputs were encountering `*fmt.wrapError` type `!w` in the logs. This change addresses the wrong format output. Also, makes the logs in watcher_linux.go to match the rest of log formats. Signed-off-by: Nino Kodabande --- src/go/guestagent/pkg/iptables/iptables.go | 4 +-- src/go/guestagent/pkg/kube/watcher_linux.go | 37 ++++++++++----------- 2 files changed, 20 insertions(+), 21 deletions(-) diff --git a/src/go/guestagent/pkg/iptables/iptables.go b/src/go/guestagent/pkg/iptables/iptables.go index f2a5f90e791..d05318fac10 100644 --- a/src/go/guestagent/pkg/iptables/iptables.go +++ b/src/go/guestagent/pkg/iptables/iptables.go @@ -99,9 +99,9 @@ func ForwardPorts(ctx context.Context, tracker tracker.Tracker, updateInterval t } name := entryToString(p) if err := tracker.Add(generateID(name), portMap); err != nil { - log.Errorf("failed to listen %q: %w", name, err) + log.Errorf("failed to listen %s: %s", name, err) } else { - log.Infof("opened listener for %q", name) + log.Infof("opened listener for %s", name) } } } diff --git a/src/go/guestagent/pkg/kube/watcher_linux.go b/src/go/guestagent/pkg/kube/watcher_linux.go index b8ff465c013..85406fa97c4 100644 --- a/src/go/guestagent/pkg/kube/watcher_linux.go +++ b/src/go/guestagent/pkg/kube/watcher_linux.go @@ -154,13 +154,12 @@ func WatchForServices( case event := <-eventCh: if event.deleted { if err := portTracker.Remove(string(event.UID)); err != nil { - log.Errorw("failed to delete a port from tracker", log.Fields{ - "error": err, - "UID": event.UID, - "ports": event.portMapping, - "namespace": event.namespace, - "name": event.name, - }) + log.Errorf("failed to delete port mapping: %v from tracker UID: %v namespace: %s name: %s failed: %s", + event.portMapping, + event.UID, + event.namespace, + event.name, + err) } else { log.Debugf("kubernetes service: port mapping deleted %s/%s:%v", event.namespace, event.name, event.portMapping) @@ -168,22 +167,22 @@ func WatchForServices( } else { portMapping, err := createPortMapping(event.portMapping, k8sServiceListenerIP) if err != nil { - log.Errorw("failed to create port mapping", log.Fields{ - "error": err, - "ports": event.portMapping, - "namespace": event.namespace, - "name": event.name, - }) + log.Errorf("failed to create port mapping: %v from tracker UID: %v namespace: %s name: %s failed: %s", + event.portMapping, + event.UID, + event.namespace, + event.name, + err) continue } if err := portTracker.Add(string(event.UID), portMapping); err != nil { - log.Errorw("failed to add port mapping", log.Fields{ - "error": err, - "ports": event.portMapping, - "namespace": event.namespace, - "name": event.name, - }) + log.Errorf("failed to add port mapping: %v from tracker UID: %v namespace: %s name: %s failed: %s", + event.portMapping, + event.UID, + event.namespace, + event.name, + err) } else { log.Debugf("kubernetes service: port mapping added %s/%s:%v", event.namespace, event.name, event.portMapping) From eb9b5d41f178bbdbf47fff2e4a9de0a8eab021e9 Mon Sep 17 00:00:00 2001 From: Nino Kodabande Date: Tue, 12 Nov 2024 18:32:44 -0800 Subject: [PATCH 2/4] Rename vtunnel.go to forwarder.go Vtunnel is no longer used, therefore, renaming the file to forwarder.go Signed-off-by: Nino Kodabande --- src/go/guestagent/pkg/forwarder/{vtunnel.go => forwarder.go} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/go/guestagent/pkg/forwarder/{vtunnel.go => forwarder.go} (100%) diff --git a/src/go/guestagent/pkg/forwarder/vtunnel.go b/src/go/guestagent/pkg/forwarder/forwarder.go similarity index 100% rename from src/go/guestagent/pkg/forwarder/vtunnel.go rename to src/go/guestagent/pkg/forwarder/forwarder.go From 6f4b7b41627016d6ac5a802ed833f98af5362989 Mon Sep 17 00:00:00 2001 From: Nino Kodabande Date: Tue, 12 Nov 2024 19:06:44 -0800 Subject: [PATCH 3/4] Fix inefficiencies in API Tracker Previously, the portStorage.Add and WSLProxy.Send functions were called with an empty portMapping. This change adds a check for the length of portMapping before calling these functions, making sure they are only invoked when necessary. Signed-off-by: Nino Kodabande --- src/go/guestagent/pkg/tracker/apitracker.go | 43 ++++++++++++--------- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/src/go/guestagent/pkg/tracker/apitracker.go b/src/go/guestagent/pkg/tracker/apitracker.go index 15c723d5ad4..f3565f33034 100644 --- a/src/go/guestagent/pkg/tracker/apitracker.go +++ b/src/go/guestagent/pkg/tracker/apitracker.go @@ -116,19 +116,22 @@ func (a *APITracker) Add(containerID string, portMap nat.PortMap) error { tmpPortBinding = append(tmpPortBinding, portBinding) } - successfullyForwarded[portProto] = tmpPortBinding - } - - a.portStorage.add(containerID, successfullyForwarded) - portMapping := guestagentTypes.PortMapping{ - Remove: false, - Ports: successfullyForwarded, + if len(tmpPortBinding) != 0 { + successfullyForwarded[portProto] = tmpPortBinding + } } - log.Debugf("forwarding to wsl-proxy to add port mapping: %+v", portMapping) - err := a.wslProxyForwarder.Send(portMapping) - if err != nil { - return fmt.Errorf("sending port mappings to wsl proxy error: %w", err) + if len(successfullyForwarded) != 0 { + a.portStorage.add(containerID, successfullyForwarded) + portMapping := guestagentTypes.PortMapping{ + Remove: false, + Ports: successfullyForwarded, + } + log.Debugf("forwarding to wsl-proxy to add port mapping: %+v", portMapping) + err := a.wslProxyForwarder.Send(portMapping) + if err != nil { + return fmt.Errorf("sending port mappings to wsl proxy error: %w", err) + } } if len(errs) != 0 { @@ -176,14 +179,16 @@ func (a *APITracker) Remove(containerID string) error { } } - portMapping := guestagentTypes.PortMapping{ - Remove: true, - Ports: portMap, - } - log.Debugf("forwarding to wsl-proxy to remove port mapping: %+v", portMapping) - err := a.wslProxyForwarder.Send(portMapping) - if err != nil { - return fmt.Errorf("sending port mappings to wsl proxy error: %w", err) + if len(portMap) != 0 { + portMapping := guestagentTypes.PortMapping{ + Remove: true, + Ports: portMap, + } + log.Debugf("forwarding to wsl-proxy to remove port mapping: %+v", portMapping) + err := a.wslProxyForwarder.Send(portMapping) + if err != nil { + return fmt.Errorf("sending port mappings to wsl proxy error: %w", err) + } } if len(errs) != 0 { From b2abce08c00392e3c74731afb8b26c3ad779a14c Mon Sep 17 00:00:00 2001 From: Nino Kodabande Date: Tue, 12 Nov 2024 19:20:18 -0800 Subject: [PATCH 4/4] Don't recreate iptables loopback rule When portTracker.Add fails, we should simply continue without attempting to recreate the loopback iptables rule. Signed-off-by: Nino Kodabande --- src/go/guestagent/pkg/docker/events.go | 1 + 1 file changed, 1 insertion(+) diff --git a/src/go/guestagent/pkg/docker/events.go b/src/go/guestagent/pkg/docker/events.go index ac275d519cb..9e7efaf64f2 100644 --- a/src/go/guestagent/pkg/docker/events.go +++ b/src/go/guestagent/pkg/docker/events.go @@ -158,6 +158,7 @@ func (e *EventMonitor) initializeRunningContainers(ctx context.Context) error { if err := e.portTracker.Add(container.ID, portMap); err != nil { log.Errorf("registering already running containers failed: %v", err) + continue } for _, netSettings := range container.NetworkSettings.Networks {