Skip to content

Commit

Permalink
errcheck, network: always handle errors
Browse files Browse the repository at this point in the history
Signed-off-by: Miguel Duarte Barroso <[email protected]>
  • Loading branch information
maiqueb committed Aug 5, 2022
1 parent 42c4d81 commit e0d667c
Show file tree
Hide file tree
Showing 13 changed files with 87 additions and 29 deletions.
36 changes: 35 additions & 1 deletion nogo_config.json
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,41 @@
"external/": "external doesn't pass errcheck",
"src/": "golang core code does not pass errcheck",
"bazel-out/": "this directory should be off-limits to errcheck",
"pkg/": "KubeVirt pkg does not pass errcheck yet",
"pkg/certificates/": "KubeVirt certificate pkg does not pass errcheck yet",
"pkg/cloud-init/": "KubeVirt cloud-init pkg does not pass errcheck yet",
"pkg/config/": "KubeVirt config pkg does not pass errcheck yet",
"pkg/container-disk/": "KubeVirt container-disk pkg does not pass errcheck yet",
"pkg/controller/": "KubeVirt controller pkg does not pass errcheck yet",
"pkg/downwardmetrics/": "KubeVirt downwardmetrics pkg does not pass errcheck yet",
"pkg/emptydisk/": "KubeVirt emptydisk pkg does not pass errcheck yet",
"pkg/ephemeral-disk/": "KubeVirt ephemeral-disk pkg does not pass errcheck yet",
"pkg/ephemeral-disk-utils/": "KubeVirt ephemeral-disk-utils pkg does not pass errcheck yet",
"pkg/executor/": "KubeVirt executor pkg does not pass errcheck yet",
"pkg/handler-launcher-com/": "KubeVirt handler-launcher-com pkg does not pass errcheck yet",
"pkg/healthz/": "KubeVirt healthz pkg does not pass errcheck yet",
"pkg/hooks/": "KubeVirt hooks pkg does not pass errcheck yet",
"pkg/host-disk/": "KubeVirt host-disk pkg does not pass errcheck yet",
"pkg/hotplug-disk/": "KubeVirt hotplug-disk pkg does not pass errcheck yet",
"pkg/ignition/": "KubeVirt ignition pkg does not pass errcheck yet",
"pkg/inotify-informer/": "KubeVirt inotify-informer pkg does not pass errcheck yet",
"pkg/instancetype/": "KubeVirt instancetype pkg does not pass errcheck yet",
"pkg/monitoring/": "KubeVirt monitoring pkg does not pass errcheck yet",
"pkg/network/setup/": "KubeVirt network/setup pkg does not pass errcheck yet",
"pkg/os/": "KubeVirt os pkg does not pass errcheck yet",
"pkg/rest/": "KubeVirt rest pkg does not pass errcheck yet",
"pkg/safepath/": "KubeVirt safepath pkg does not pass errcheck yet",
"pkg/service/": "KubeVirt service pkg does not pass errcheck yet",
"pkg/testutils/": "KubeVirt test-utils pkg does not pass errcheck yet",
"pkg/unsafepath/": "KubeVirt unsafepath pkg does not pass errcheck yet",
"pkg/util/": "KubeVirt util pkg does not pass errcheck yet",
"pkg/virt-api/": "KubeVirt virt-api pkg does not pass errcheck yet",
"pkg/virt-controller/": "KubeVirt virt-controller pkg does not pass errcheck yet",
"pkg/virt-exportserver/": "KubeVirt virt-exportserver pkg does not pass errcheck yet",
"pkg/virt-handler/": "KubeVirt virt-handler pkg does not pass errcheck yet",
"pkg/virt-launcher/": "KubeVirt virt-launcher pkg does not pass errcheck yet",
"pkg/virt-operator/": "KubeVirt virt-operator pkg does not pass errcheck yet",
"pkg/virtctl/": "KubeVirt virtctl pkg does not pass errcheck yet",
"pkg/watchdog/": "KubeVirt watchdog pkg does not pass errcheck yet",
"cmd/": "KubeVirt binaries do not pass errcheck yet",
"tools/": "KubeVirt tools do not pass errcheck yet",
"tests/console": "tests/console does not pass errcheck yet",
Expand Down
2 changes: 1 addition & 1 deletion pkg/network/cache/cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ var _ = Describe("cache", func() {
}
})

AfterEach(func() { cache.Delete() })
AfterEach(func() { Expect(cache.Delete()).To(Succeed()) })

It("should return os.ErrNotExist if no cache entry exists", func() {
var newData data
Expand Down
2 changes: 1 addition & 1 deletion pkg/network/cache/domaininterface_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ var _ = Describe("DomainInterfaceCache", func() {
BeforeEach(dutils.MockDefaultOwnershipManager)

AfterEach(func() {
cacheCreator.New("").Delete()
Expect(cacheCreator.New("").Delete()).To(Succeed())
})

It("should return os.ErrNotExist if no cache entry exists", func() {
Expand Down
2 changes: 1 addition & 1 deletion pkg/network/cache/podinterface_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ var _ = Describe("Pod Interface", func() {
}
})

AfterEach(func() { cacheCreator.New("").Delete() })
AfterEach(func() { Expect(cacheCreator.New("").Delete()).To(Succeed()) })

It("should return os.ErrNotExist if no cache entry exists", func() {
_, err := podIfaceCache.Read()
Expand Down
2 changes: 1 addition & 1 deletion pkg/network/dhcp/bridge_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ var _ = Describe("Bridge DHCP configurator", func() {
})

AfterEach(func() {
cacheCreator.New("").Delete()
Expect(cacheCreator.New("").Delete()).To(Succeed())
})

Context("Generate", func() {
Expand Down
8 changes: 7 additions & 1 deletion pkg/network/dhcp/configurator.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ import (
"fmt"
"os"

"kubevirt.io/client-go/log"

v1 "kubevirt.io/api/core/v1"

"kubevirt.io/kubevirt/pkg/network/cache"
Expand Down Expand Up @@ -95,7 +97,11 @@ func (d *configurator) EnsureDHCPServerStarted(podInterfaceName string, dhcpConf
if err != nil {
return fmt.Errorf("failed to create dhcp started file %s: %s", dhcpStartedFile, err)
}
newFile.Close()

if err := newFile.Close(); err != nil {
log.Log.Warningf(
"failed to close the DHCP readiness file descriptor %d: %v", int(newFile.Fd()), err)
}
}
return nil
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/network/dhcp/configurator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ var _ = Describe("DHCP configurator", func() {
})

AfterEach(func() {
cacheCreator.New("").Delete()
Expect(cacheCreator.New("").Delete()).To(Succeed())
Expect(os.RemoveAll(fakeDhcpStartedDir)).To(Succeed())
})

Expand Down
8 changes: 7 additions & 1 deletion pkg/network/dhcp/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,14 +86,20 @@ func SingleClientDHCPServer(
if err != nil {
return err
}
defer l.Close()
defer closeDHCPServerIgnoringError(l)
err = dhcp.Serve(l, handler)
if err != nil {
return err
}
return nil
}

func closeDHCPServerIgnoringError(l ServeIfConn) {
if err := l.Close(); err != nil {
log.Log.Warningf("failed to close DHCP server connection: %v", err)
}
}

func prepareDHCPOptions(
clientMask net.IPMask,
routerIP net.IP,
Expand Down
2 changes: 1 addition & 1 deletion pkg/network/dhcp/server/socket_listener.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func NewUDP4FilterListener(interfaceName, laddr string) (c ServeIfConn, e error)
}
defer func() {
if e != nil {
l.Close()
closeDHCPServerIgnoringError(l)
}
}()
p := ipv4.NewPacketConn(l)
Expand Down
2 changes: 1 addition & 1 deletion pkg/network/domainspec/generators_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ var _ = Describe("Pod Network", func() {
})

AfterEach(func() {
os.RemoveAll(tmpDir)
Expect(os.RemoveAll(tmpDir)).To(Succeed())
})

Context("on successful setup", func() {
Expand Down
10 changes: 9 additions & 1 deletion pkg/network/link/ethtool.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import (
"fmt"
"syscall"
"unsafe"

"kubevirt.io/client-go/log"
)

const (
Expand Down Expand Up @@ -45,7 +47,7 @@ func EthtoolTXOff(name string) error {
if err != nil {
return err
}
defer syscall.Close(socket)
defer closeSocketIgnoringError(socket)

// Request current value
value := EthtoolValue{Cmd: ETHTOOL_GTXCSUM}
Expand All @@ -62,3 +64,9 @@ func EthtoolTXOff(name string) error {
value = EthtoolValue{ETHTOOL_STXCSUM, 0}
return ioctlEthtool(socket, uintptr(unsafe.Pointer(&request))) // #nosec Used for a RawSyscall
}

func closeSocketIgnoringError(fd int) {
if err := syscall.Close(fd); err != nil {
log.Log.Warningf("failed to close socket file descriptor %d: %v", fd, err)
}
}
2 changes: 1 addition & 1 deletion pkg/network/setup/network_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ var _ = Describe("VMNetworkConfigurator", func() {
baseCacheCreator tempCacheCreator
)
AfterEach(func() {
baseCacheCreator.New("").Delete()
Expect(baseCacheCreator.New("").Delete()).To(Succeed())
})
Context("interface configuration", func() {

Expand Down
38 changes: 21 additions & 17 deletions pkg/network/setup/podnic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ var _ = Describe("podNIC", func() {
mockDHCPConfigurator = dhcp.NewMockConfigurator(ctrl)
})
AfterEach(func() {
baseCacheCreator.New("").Delete()
Expect(baseCacheCreator.New("").Delete()).To(Succeed())
})
When("reading networking configuration succeed", func() {
var (
Expand Down Expand Up @@ -128,18 +128,22 @@ var _ = Describe("podNIC", func() {
api.NewDefaulter(runtime.GOARCH).SetObjectDefaults_Domain(domain)
podnic, err = newPhase2PodNICWithMocks(vmi)
Expect(err).ToNot(HaveOccurred())
cache.WriteDHCPInterfaceCache(
podnic.cacheCreator,
getPIDString(podnic.launcherPID),
podnic.podInterfaceName,
&cache.DHCPConfig{Name: podnic.podInterfaceName},
)
cache.WriteDomainInterfaceCache(
podnic.cacheCreator,
getPIDString(podnic.launcherPID),
podnic.vmiSpecIface.Name,
&domain.Spec.Devices.Interfaces[0],
)
Expect(
cache.WriteDHCPInterfaceCache(
podnic.cacheCreator,
getPIDString(podnic.launcherPID),
podnic.podInterfaceName,
&cache.DHCPConfig{Name: podnic.podInterfaceName},
),
).To(Succeed())
Expect(
cache.WriteDomainInterfaceCache(
podnic.cacheCreator,
getPIDString(podnic.launcherPID),
podnic.vmiSpecIface.Name,
&domain.Spec.Devices.Interfaces[0],
),
).To(Succeed())
})
Context("and starting the DHCP server fails", func() {
BeforeEach(func() {
Expand All @@ -150,7 +154,7 @@ var _ = Describe("podNIC", func() {
}
})
It("phase2 should panic", func() {
Expect(func() { podnic.PlugPhase2(domain) }).To(Panic())
Expect(func() { _ = podnic.PlugPhase2(domain) }).To(Panic())
})
})
Context("and starting the DHCP server succeed", func() {
Expand Down Expand Up @@ -247,7 +251,7 @@ var _ = Describe("podNIC", func() {
Expect(err).ToNot(HaveOccurred())
pid := 1
podnic.launcherPID = &pid
podnic.setState(cache.PodIfaceNetworkPreparationPending)
Expect(podnic.setState(cache.PodIfaceNetworkPreparationPending)).To(Succeed())
})

BeforeEach(func() {
Expand Down Expand Up @@ -286,7 +290,7 @@ var _ = Describe("podNIC", func() {
vmi := newVMIMasqueradeInterface("testnamespace", "testVmName", masqueradeCidr, masqueradeIpv6Cidr)
podnic, err = newPhase1PodNICWithMocks(vmi)
Expect(err).ToNot(HaveOccurred())
podnic.setState(cache.PodIfaceNetworkPreparationStarted)
Expect(podnic.setState(cache.PodIfaceNetworkPreparationStarted)).To(Succeed())
})
Context("and phase1 is called", func() {
It("should fail with CriticalNetworkError", func() {
Expand All @@ -305,7 +309,7 @@ var _ = Describe("podNIC", func() {
vmi := newVMIMasqueradeInterface("testnamespace", "testVmName", masqueradeCidr, masqueradeIpv6Cidr)
podnic, err = newPhase1PodNICWithMocks(vmi)
Expect(err).ToNot(HaveOccurred())
podnic.setState(cache.PodIfaceNetworkPreparationFinished)
Expect(podnic.setState(cache.PodIfaceNetworkPreparationFinished)).To(Succeed())
})
Context("and phase1 is called", func() {
It("should successfully return without calling infra configurator", func() {
Expand Down

0 comments on commit e0d667c

Please sign in to comment.