Skip to content

Commit

Permalink
fact: handle errors from Close() explicitely
Browse files Browse the repository at this point in the history
  • Loading branch information
aauren committed Apr 14, 2021
1 parent 57ddac3 commit a86b3fa
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 15 deletions.
24 changes: 12 additions & 12 deletions pkg/controllers/proxy/network_services_controller.go
Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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)
Expand Down Expand Up @@ -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
}

Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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())
}
Expand Down Expand Up @@ -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())
}
Expand Down
4 changes: 3 additions & 1 deletion pkg/controllers/routing/pbr.go
Expand Up @@ -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
Expand Down Expand Up @@ -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())
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/controllers/routing/utils.go
Expand Up @@ -65,7 +65,7 @@ func ipv4IsEnabled() bool {
if err != nil {
return false
}
l.Close()
_ = l.Close()

return true
}
Expand All @@ -82,7 +82,7 @@ func ipv6IsEnabled() bool {
if err != nil {
return false
}
l.Close()
_ = l.Close()

return true
}
Expand Down
7 changes: 7 additions & 0 deletions pkg/utils/utils.go
@@ -1,6 +1,7 @@
package utils

import (
"io"
"sync"
)

Expand Down Expand Up @@ -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()
}

0 comments on commit a86b3fa

Please sign in to comment.