From ef719082771e7b82139fc27a597212ed4d553318 Mon Sep 17 00:00:00 2001 From: Dave Protasowski Date: Wed, 29 Jan 2025 19:04:05 -0500 Subject: [PATCH 1/4] If the IP is on the lo device already don't fail --- pkg/loadbalancer/address_other.go | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/pkg/loadbalancer/address_other.go b/pkg/loadbalancer/address_other.go index c652eea3..1e65a694 100644 --- a/pkg/loadbalancer/address_other.go +++ b/pkg/loadbalancer/address_other.go @@ -2,11 +2,21 @@ package loadbalancer -import "os/exec" +import ( + "bytes" + "errors" + "os/exec" +) func AddIPToLocalInterface(ip string) error { - err := exec.Command("ip", "addr", "add", ip, "dev", "lo").Run() - if err != nil { + outBytes, err := exec.Command("ip", "addr", "add", ip, "dev", "lo").CombinedOutput() + + var exerr *exec.ExitError + if errors.As(err, &exerr) && bytes.ContainsAny(outBytes, "File exists") { + // unsure why but `Run()` doesn't capture stderr properly + // if 'File exists' is in the output, then the lo device has the IP already + return nil + } else if err != nil { return err } return nil From be2387700071000b44b2cc29459cb0c4fe01fabf Mon Sep 17 00:00:00 2001 From: Dave Protasowski Date: Thu, 30 Jan 2025 14:16:01 -0500 Subject: [PATCH 2/4] don't fail if we're unable to add/remove IP to lo interface Signed-off-by: Dave Protasowski --- pkg/loadbalancer/address_darwin.go | 19 ++++++------------- pkg/loadbalancer/address_other.go | 26 ++++++-------------------- pkg/loadbalancer/address_windows.go | 18 ++++++------------ pkg/loadbalancer/tunnel.go | 8 ++++---- 4 files changed, 22 insertions(+), 49 deletions(-) diff --git a/pkg/loadbalancer/address_darwin.go b/pkg/loadbalancer/address_darwin.go index bdeff96e..6885a82d 100644 --- a/pkg/loadbalancer/address_darwin.go +++ b/pkg/loadbalancer/address_darwin.go @@ -6,21 +6,14 @@ import ( "os/exec" ) -func AddIPToLocalInterface(ip string) error { +func AddIPToLocalInterface(ip string) (string, error) { // TODO: IPv6 - err := exec.Command("ifconfig", "lo0", "alias", ip, "netmask", "255.255.255.255").Run() - if err != nil { - return err - } - return nil + output, err := exec.Command("ifconfig", "lo0", "alias", ip, "netmask", "255.255.255.255").CombinedOutput() + return string(output), err } -func RemoveIPFromLocalInterface(ip string) error { +func RemoveIPFromLocalInterface(ip string) (string, error) { // delete the IP address - err := exec.Command("ifconfig", "lo0", "-alias", ip).Run() - if err != nil { - return err - } - return nil - + output, err := exec.Command("ifconfig", "lo0", "-alias", ip).CombinedOutput() + return string(output), err } diff --git a/pkg/loadbalancer/address_other.go b/pkg/loadbalancer/address_other.go index 1e65a694..7e489c46 100644 --- a/pkg/loadbalancer/address_other.go +++ b/pkg/loadbalancer/address_other.go @@ -3,29 +3,15 @@ package loadbalancer import ( - "bytes" - "errors" "os/exec" ) -func AddIPToLocalInterface(ip string) error { - outBytes, err := exec.Command("ip", "addr", "add", ip, "dev", "lo").CombinedOutput() - - var exerr *exec.ExitError - if errors.As(err, &exerr) && bytes.ContainsAny(outBytes, "File exists") { - // unsure why but `Run()` doesn't capture stderr properly - // if 'File exists' is in the output, then the lo device has the IP already - return nil - } else if err != nil { - return err - } - return nil +func AddIPToLocalInterface(ip string) (string, error) { + output, err := exec.Command("ip", "addr", "add", ip, "dev", "lo").CombinedOutput() + return string(output), err } -func RemoveIPFromLocalInterface(ip string) error { - err := exec.Command("ip", "addr", "del", ip, "dev", "lo").Run() - if err != nil { - return err - } - return nil +func RemoveIPFromLocalInterface(ip string) (string, error) { + output, err := exec.Command("ip", "addr", "del", ip, "dev", "lo").CombinedOutput() + return string(output), err } diff --git a/pkg/loadbalancer/address_windows.go b/pkg/loadbalancer/address_windows.go index fabd69a0..e166ea7f 100644 --- a/pkg/loadbalancer/address_windows.go +++ b/pkg/loadbalancer/address_windows.go @@ -6,18 +6,12 @@ import ( "os/exec" ) -func AddIPToLocalInterface(ip string) error { - err := exec.Command("netsh", "interface", "ip", "add", "address", "loopback", ip, "255.255.255.255").Run() - if err != nil { - return err - } - return nil +func AddIPToLocalInterface(ip string) (string, error) { + output, err := exec.Command("netsh", "interface", "ip", "add", "address", "loopback", ip, "255.255.255.255").CombinedOutput() + return string(output), err } -func RemoveIPFromLocalInterface(ip string) error { - err := exec.Command("netsh", "interface", "ip", "delete", "address", "loopback", ip, "255.255.255.255").Run() - if err != nil { - return err - } - return nil +func RemoveIPFromLocalInterface(ip string) (string, error) { + output, err := exec.Command("netsh", "interface", "ip", "delete", "address", "loopback", ip, "255.255.255.255").CombinedOutput() + return string(output), err } diff --git a/pkg/loadbalancer/tunnel.go b/pkg/loadbalancer/tunnel.go index 729b8aeb..191c6475 100644 --- a/pkg/loadbalancer/tunnel.go +++ b/pkg/loadbalancer/tunnel.go @@ -39,9 +39,9 @@ func (t *tunnelManager) setupTunnels(containerName string) error { } klog.V(0).Infof("setting IPv4 address %s associated to container %s", ipv4, containerName) - err = AddIPToLocalInterface(ipv4) + output, err := AddIPToLocalInterface(ipv4) if err != nil { - return err + klog.V(4).Infof("error adding IP to local interface: %v - %s", err, output) } // create tunnel from the ip:svcport to the localhost:portmap @@ -83,9 +83,9 @@ func (t *tunnelManager) removeTunnels(containerName string) error { } klog.V(0).Infof("Removing IPv4 address %s associated to local interface", tunnelIP) - err := RemoveIPFromLocalInterface(tunnelIP) + output, err := RemoveIPFromLocalInterface(tunnelIP) if err != nil { - return err + klog.V(4).Infof("error removing IP from local interface: %v - %s", err, output) } return nil } From 768e95c3ed1a7049478beb6424f3660997d891f6 Mon Sep 17 00:00:00 2001 From: Dave Protasowski Date: Thu, 30 Jan 2025 14:41:30 -0500 Subject: [PATCH 3/4] wrap error and bubble up - print log when setting up tunnel fails --- pkg/loadbalancer/server.go | 2 +- pkg/loadbalancer/tunnel.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/loadbalancer/server.go b/pkg/loadbalancer/server.go index 1b410b83..99c88423 100644 --- a/pkg/loadbalancer/server.go +++ b/pkg/loadbalancer/server.go @@ -121,7 +121,7 @@ func (s *Server) EnsureLoadBalancer(ctx context.Context, clusterName string, ser klog.V(2).Infof("updating loadbalancer tunnels on userspace") err = s.tunnelManager.setupTunnels(loadBalancerName(clusterName, service)) if err != nil { - return nil, err + klog.V(2).ErrorS(err, "error setting up tunnels") } } diff --git a/pkg/loadbalancer/tunnel.go b/pkg/loadbalancer/tunnel.go index 191c6475..f7aaad35 100644 --- a/pkg/loadbalancer/tunnel.go +++ b/pkg/loadbalancer/tunnel.go @@ -41,7 +41,7 @@ func (t *tunnelManager) setupTunnels(containerName string) error { klog.V(0).Infof("setting IPv4 address %s associated to container %s", ipv4, containerName) output, err := AddIPToLocalInterface(ipv4) if err != nil { - klog.V(4).Infof("error adding IP to local interface: %v - %s", err, output) + return fmt.Errorf("error adding IP to local interface: %w - %s", err, output) } // create tunnel from the ip:svcport to the localhost:portmap @@ -85,7 +85,7 @@ func (t *tunnelManager) removeTunnels(containerName string) error { klog.V(0).Infof("Removing IPv4 address %s associated to local interface", tunnelIP) output, err := RemoveIPFromLocalInterface(tunnelIP) if err != nil { - klog.V(4).Infof("error removing IP from local interface: %v - %s", err, output) + return fmt.Errorf("error removing IP from local interface: %w - %s", err, output) } return nil } From e9d2742c52d08d4989f27f013d79f6246ba1a344 Mon Sep 17 00:00:00 2001 From: Antonio Ojea Date: Thu, 30 Jan 2025 20:38:20 +0000 Subject: [PATCH 4/4] Update pkg/loadbalancer/server.go --- pkg/loadbalancer/server.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/loadbalancer/server.go b/pkg/loadbalancer/server.go index 99c88423..e761323b 100644 --- a/pkg/loadbalancer/server.go +++ b/pkg/loadbalancer/server.go @@ -121,7 +121,7 @@ func (s *Server) EnsureLoadBalancer(ctx context.Context, clusterName string, ser klog.V(2).Infof("updating loadbalancer tunnels on userspace") err = s.tunnelManager.setupTunnels(loadBalancerName(clusterName, service)) if err != nil { - klog.V(2).ErrorS(err, "error setting up tunnels") + klog.ErrorS(err, "error setting up tunnels") } }