From a86b3fad3511b0a36c7dfc6814f18ab1c216ef59 Mon Sep 17 00:00:00 2001 From: Aaron U'Ren Date: Tue, 13 Apr 2021 15:29:11 -0500 Subject: [PATCH] fact: handle errors from Close() explicitely --- .../proxy/network_services_controller.go | 24 +++++++++---------- pkg/controllers/routing/pbr.go | 4 +++- pkg/controllers/routing/utils.go | 4 ++-- pkg/utils/utils.go | 7 ++++++ 4 files changed, 24 insertions(+), 15 deletions(-) diff --git a/pkg/controllers/proxy/network_services_controller.go b/pkg/controllers/proxy/network_services_controller.go index f9b4304a7..963d72309 100644 --- a/pkg/controllers/proxy/network_services_controller.go +++ b/pkg/controllers/proxy/network_services_controller.go @@ -958,20 +958,20 @@ func (ln *linuxNetworking) prepareEndpointForDsr(containerID string, endpointIP if err != nil { return errors.New("Failed to get namespace due to " + err.Error()) } - defer hostNetworkNamespaceHandle.Close() + defer utils.CloseCloserDisregardError(&hostNetworkNamespaceHandle) activeNetworkNamespaceHandle, err = netns.Get() if err != nil { return errors.New("Failed to get namespace due to " + err.Error()) } klog.V(1).Infof("Current network namespace before netns.Set: " + activeNetworkNamespaceHandle.String()) - activeNetworkNamespaceHandle.Close() + defer utils.CloseCloserDisregardError(&activeNetworkNamespaceHandle) dockerClient, err := client.NewClientWithOpts(client.FromEnv) if err != nil { return errors.New("Failed to get docker client due to " + err.Error()) } - defer dockerClient.Close() + defer utils.CloseCloserDisregardError(dockerClient) containerSpec, err := dockerClient.ContainerInspect(context.Background(), containerID) if err != nil { @@ -983,7 +983,7 @@ func (ln *linuxNetworking) prepareEndpointForDsr(containerID string, endpointIP if err != nil { return errors.New("Failed to get endpoint namespace due to " + err.Error()) } - defer endpointNamespaceHandle.Close() + defer utils.CloseCloserDisregardError(&endpointNamespaceHandle) err = netns.Set(endpointNamespaceHandle) if err != nil { @@ -995,7 +995,7 @@ func (ln *linuxNetworking) prepareEndpointForDsr(containerID string, endpointIP return errors.New("Failed to get activeNetworkNamespace due to " + err.Error()) } klog.V(2).Infof("Current network namespace after netns. Set to container network namespace: " + activeNetworkNamespaceHandle.String()) - activeNetworkNamespaceHandle.Close() + _ = activeNetworkNamespaceHandle.Close() // create a ipip tunnel interface inside the endpoint container tunIf, err := netlink.LinkByName(KubeTunnelIf) @@ -1085,7 +1085,7 @@ func (ln *linuxNetworking) prepareEndpointForDsr(containerID string, endpointIP return errors.New("Failed to get activeNetworkNamespace handle due to " + err.Error()) } klog.Infof("Current network namespace after revert namespace to host network namespace: " + activeNetworkNamespaceHandle.String()) - activeNetworkNamespaceHandle.Close() + _ = activeNetworkNamespaceHandle.Close() return nil } @@ -1110,7 +1110,7 @@ func (ln *linuxNetworking) prepareEndpointForDsrWithCRI(runtimeEndpoint, contain return errors.New("failed to get host namespace due to " + err.Error()) } klog.V(1).Infof("current network namespace before netns.Set: " + hostNetworkNamespaceHandle.String()) - defer hostNetworkNamespaceHandle.Close() + defer utils.CloseCloserDisregardError(&hostNetworkNamespaceHandle) rs, err := cri.NewRemoteRuntimeService(runtimeEndpoint, cri.DefaultConnectionTimeout) if err != nil { @@ -1130,7 +1130,7 @@ func (ln *linuxNetworking) prepareEndpointForDsrWithCRI(runtimeEndpoint, contain if err != nil { return fmt.Errorf("failed to get endpoint namespace (containerID=%s, pid=%d, error=%s)", containerID, pid, err) } - defer endpointNamespaceHandle.Close() + defer utils.CloseCloserDisregardError(&endpointNamespaceHandle) err = netns.Set(endpointNamespaceHandle) if err != nil { @@ -1142,7 +1142,7 @@ func (ln *linuxNetworking) prepareEndpointForDsrWithCRI(runtimeEndpoint, contain return errors.New("failed to get activeNetworkNamespace due to " + err.Error()) } klog.V(2).Infof("Current network namespace after netns. Set to container network namespace: " + activeNetworkNamespaceHandle.String()) - activeNetworkNamespaceHandle.Close() + _ = activeNetworkNamespaceHandle.Close() // TODO: fix boilerplate `netns.Set(hostNetworkNamespaceHandle)` code. Need a robust // way to switch back to old namespace, pretty much all things will go wrong if we dont switch back @@ -1235,7 +1235,7 @@ func (ln *linuxNetworking) prepareEndpointForDsrWithCRI(runtimeEndpoint, contain return errors.New("Failed to get activeNetworkNamespace handle due to " + err.Error()) } klog.Infof("Current network namespace after revert namespace to host network namespace: " + activeNetworkNamespaceHandle.String()) - activeNetworkNamespaceHandle.Close() + _ = activeNetworkNamespaceHandle.Close() return nil } @@ -2063,7 +2063,7 @@ func (ln *linuxNetworking) setupPolicyRoutingForDSR() error { if err != nil { return errors.New("Failed to setup policy routing required for DSR due to " + err.Error()) } - defer f.Close() + defer utils.CloseCloserDisregardError(f) if _, err = f.WriteString(customDSRRouteTableID + " " + customDSRRouteTableName + "\n"); err != nil { return errors.New("Failed to setup policy routing required for DSR due to " + err.Error()) } @@ -2094,7 +2094,7 @@ func (ln *linuxNetworking) setupRoutesForExternalIPForDSR(serviceInfoMap service if err != nil { return errors.New("Failed setup external ip routing table required for DSR due to " + err.Error()) } - defer f.Close() + defer utils.CloseCloserDisregardError(f) if _, err = f.WriteString(externalIPRouteTableID + " " + externalIPRouteTableName + "\n"); err != nil { return errors.New("Failed setup external ip routing table required for DSR due to " + err.Error()) } diff --git a/pkg/controllers/routing/pbr.go b/pkg/controllers/routing/pbr.go index c5e3f1391..7528f9438 100644 --- a/pkg/controllers/routing/pbr.go +++ b/pkg/controllers/routing/pbr.go @@ -6,6 +6,8 @@ import ( "os" "os/exec" "strings" + + "github.com/cloudnativelabs/kube-router/pkg/utils" ) // setup a custom routing table that will be used for policy based routing to ensure traffic originating @@ -64,7 +66,7 @@ func rtTablesAdd(tableNumber, tableName string) error { if err != nil { return fmt.Errorf("failed to open: %s", err.Error()) } - defer f.Close() + defer utils.CloseCloserDisregardError(f) if _, err = f.WriteString(tableNumber + " " + tableName + "\n"); err != nil { return fmt.Errorf("failed to write: %s", err.Error()) } diff --git a/pkg/controllers/routing/utils.go b/pkg/controllers/routing/utils.go index ad803f36a..9dc9e7650 100644 --- a/pkg/controllers/routing/utils.go +++ b/pkg/controllers/routing/utils.go @@ -65,7 +65,7 @@ func ipv4IsEnabled() bool { if err != nil { return false } - l.Close() + _ = l.Close() return true } @@ -82,7 +82,7 @@ func ipv6IsEnabled() bool { if err != nil { return false } - l.Close() + _ = l.Close() return true } diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index 64d3af411..7a5857719 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -1,6 +1,7 @@ package utils import ( + "io" "sync" ) @@ -41,3 +42,9 @@ func (b *Broadcaster) Notify(instance interface{}) { go listener.OnUpdate(instance) } } + +// CloseCloserDisregardError it is a common need throughout kube-router's code base to need close a closer in defer +// statements, this allows an action like that to pass a linter as well as describe its intention well +func CloseCloserDisregardError(handler io.Closer) { + _ = handler.Close() +}