From 3c73545ff58102dbb8c70628599d442552e83ea3 Mon Sep 17 00:00:00 2001 From: Aniket Bhat Date: Fri, 5 Aug 2022 10:34:06 -0400 Subject: [PATCH 1/3] Fix gofmt errors on release-4.10 branch There were errors in gofmt and utest with release-4.10 branch This commit fixes the gofmt errors. Signed-off-by: Aniket Bhat --- go-controller/pkg/cni/OCP_HACKS.go | 1 + go-controller/pkg/factory/types.go | 2 +- go-controller/pkg/node/OCP_HACKS.go | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/go-controller/pkg/cni/OCP_HACKS.go b/go-controller/pkg/cni/OCP_HACKS.go index ea8fd121e9..04e9248884 100644 --- a/go-controller/pkg/cni/OCP_HACKS.go +++ b/go-controller/pkg/cni/OCP_HACKS.go @@ -1,3 +1,4 @@ +//go:build linux // +build linux package cni diff --git a/go-controller/pkg/factory/types.go b/go-controller/pkg/factory/types.go index 1753b6689a..592167f80d 100644 --- a/go-controller/pkg/factory/types.go +++ b/go-controller/pkg/factory/types.go @@ -42,7 +42,7 @@ type NodeWatchFactory interface { AddPodHandler(handlerFuncs cache.ResourceEventHandler, processExisting func([]interface{})) *Handler RemovePodHandler(handler *Handler) - AddNamespaceHandler(handlerFuncs cache.ResourceEventHandler, processExisting func([]interface{})) (*Handler) + AddNamespaceHandler(handlerFuncs cache.ResourceEventHandler, processExisting func([]interface{})) *Handler RemoveNamespaceHandler(handler *Handler) NodeInformer() cache.SharedIndexInformer diff --git a/go-controller/pkg/node/OCP_HACKS.go b/go-controller/pkg/node/OCP_HACKS.go index 44c1c64173..bedb1cb34b 100644 --- a/go-controller/pkg/node/OCP_HACKS.go +++ b/go-controller/pkg/node/OCP_HACKS.go @@ -1,3 +1,4 @@ +//go:build linux // +build linux package node From 6cbbda45b3c3e64f964171954c69bf5565b756aa Mon Sep 17 00:00:00 2001 From: Tim Rozet Date: Mon, 18 Apr 2022 16:55:17 -0400 Subject: [PATCH 2/3] Allow make check without root Skips tests that require sudo or special capabilities to execute. To run, use "make check NOROOT=TRUE" Signed-off-by: Tim Rozet (cherry picked from commit f72b33d2b01d92fe549ace1494e1dde48c6b9852) --- go-controller/Makefile | 9 ++++++++- go-controller/hack/test-go.sh | 8 ++++++-- .../pkg/controller/node_linux_test.go | 16 ++++++++-------- .../pkg/node/gateway_init_linux_test.go | 14 +++++++------- .../pkg/node/management-port_linux_test.go | 10 +++++----- go-controller/pkg/node/management-port_test.go | 3 ++- go-controller/pkg/testing/testing.go | 17 +++++++++++++++++ 7 files changed, 53 insertions(+), 24 deletions(-) diff --git a/go-controller/Makefile b/go-controller/Makefile index ebc63fb160..606a878c3c 100644 --- a/go-controller/Makefile +++ b/go-controller/Makefile @@ -20,7 +20,14 @@ else CONTAINER_RUNTIME=docker endif CONTAINER_RUNNABLE ?= $(shell $(CONTAINER_RUNTIME) -v > /dev/null 2>&1; echo $$?) + OVN_VERSION ?= v21.09.1 +ifeq ($(NOROOT),TRUE) +C_ARGS = -e NOROOT=TRUE +else +C_ARGS = --cap-add=NET_ADMIN --cap-add=SYS_ADMIN +endif +export NOROOT .PHONY: all build check test @@ -41,7 +48,7 @@ windows: # tests in a container. Refer: https://www.projectatomic.io/blog/2016/03/dwalsh_selinux_containers/ for additional context check test: ifeq ($(CONTAINER_RUNNABLE), 0) - $(CONTAINER_RUNTIME) run -it --rm --security-opt label=disable --cap-add=NET_ADMIN --cap-add=SYS_ADMIN -v $(shell dirname $(PWD)):/go/src/github.com/ovn-org/ovn-kubernetes -w /go/src/github.com/ovn-org/ovn-kubernetes/go-controller -e COVERALLS=${COVERALLS} -e GINKGO_FOCUS="${GINKGO_FOCUS}" $(GO_DOCKER_IMG) sh -c "RACE=1 DOCKER_TEST=1 COVERALLS=${COVERALLS} PKGS="${PKGS}" hack/test-go.sh focus \"${GINKGO_FOCUS}\" " + $(CONTAINER_RUNTIME) run -it --rm --security-opt label=disable ${C_ARGS} -v $(shell dirname $(PWD)):/go/src/github.com/ovn-org/ovn-kubernetes -w /go/src/github.com/ovn-org/ovn-kubernetes/go-controller -e COVERALLS=${COVERALLS} -e GINKGO_FOCUS="${GINKGO_FOCUS}" $(GO_DOCKER_IMG) sh -c "RACE=1 DOCKER_TEST=1 COVERALLS=${COVERALLS} PKGS="${PKGS}" hack/test-go.sh focus \"${GINKGO_FOCUS}\" " else RACE=1 hack/test-go.sh endif diff --git a/go-controller/hack/test-go.sh b/go-controller/hack/test-go.sh index 9c42d3e9b8..aef1321852 100755 --- a/go-controller/hack/test-go.sh +++ b/go-controller/hack/test-go.sh @@ -36,7 +36,11 @@ function testrun { go test -mod=vendor -covermode set -c "${pkg}" -o "${testfile}" fi args="" - go_test="sudo ${testfile}" + if [ "$NOROOT" = "TRUE" ]; then + go_test="${testfile}" + else + go_test="sudo ${testfile}" + fi fi if [[ -n "$gingko_focus" ]]; then local ginkgoargs=${ginkgo_focus:-} @@ -57,7 +61,7 @@ function testrun { ${go_test} -test.v ${args} ${ginkgoargs} 2>&1 } -# These packages requires root for network namespace maniuplation in unit tests +# These packages requires root for network namespace manipulation in unit tests root_pkgs=("github.com/ovn-org/ovn-kubernetes/go-controller/pkg/node" "github.com/ovn-org/ovn-kubernetes/go-controller/hybrid-overlay/pkg/controller") i=0 diff --git a/go-controller/hybrid-overlay/pkg/controller/node_linux_test.go b/go-controller/hybrid-overlay/pkg/controller/node_linux_test.go index fa65509d65..e7d64f0fda 100644 --- a/go-controller/hybrid-overlay/pkg/controller/node_linux_test.go +++ b/go-controller/hybrid-overlay/pkg/controller/node_linux_test.go @@ -226,7 +226,7 @@ var _ = Describe("Hybrid Overlay Node Linux Operations", func() { Expect(testutils.UnmountNS(netns)).To(Succeed()) }) - It("does not set up tunnels for non-hybrid-overlay nodes without annotations", func() { + ovntest.OnSupportedPlatformsIt("does not set up tunnels for non-hybrid-overlay nodes without annotations", func() { app.Action = func(ctx *cli.Context) error { const ( node1Name string = "node1" @@ -273,7 +273,7 @@ var _ = Describe("Hybrid Overlay Node Linux Operations", func() { appRun(app, netns) }) - It("does not set up tunnels for non-hybrid-overlay nodes with subnet annotations", func() { + ovntest.OnSupportedPlatformsIt("does not set up tunnels for non-hybrid-overlay nodes with subnet annotations", func() { app.Action = func(ctx *cli.Context) error { const ( node1Name string = "node1" @@ -322,7 +322,7 @@ var _ = Describe("Hybrid Overlay Node Linux Operations", func() { appRun(app, netns) }) - It("sets up local node hybrid overlay bridge", func() { + ovntest.OnSupportedPlatformsIt("sets up local node hybrid overlay bridge", func() { app.Action = func(ctx *cli.Context) error { const ( thisDrMAC string = "22:33:44:55:66:77" @@ -366,7 +366,7 @@ var _ = Describe("Hybrid Overlay Node Linux Operations", func() { } appRun(app, netns) }) - It("sets up local linux pod", func() { + ovntest.OnSupportedPlatformsIt("sets up local linux pod", func() { app.Action = func(ctx *cli.Context) error { const ( thisDrMAC string = "22:33:44:55:66:77" @@ -418,7 +418,7 @@ var _ = Describe("Hybrid Overlay Node Linux Operations", func() { appRun(app, netns) }) - It("sets up tunnels for Windows nodes", func() { + ovntest.OnSupportedPlatformsIt("sets up tunnels for Windows nodes", func() { app.Action = func(ctx *cli.Context) error { const ( node1Name string = "node1" @@ -474,7 +474,7 @@ var _ = Describe("Hybrid Overlay Node Linux Operations", func() { appRun(app, netns) }) - It("removes stale node flows on initial sync", func() { + ovntest.OnSupportedPlatformsIt("removes stale node flows on initial sync", func() { app.Action = func(ctx *cli.Context) error { const ( node1Name string = "node1" @@ -526,7 +526,7 @@ var _ = Describe("Hybrid Overlay Node Linux Operations", func() { appRun(app, netns) }) - It("removes stale pod flows on initial sync", func() { + ovntest.OnSupportedPlatformsIt("removes stale pod flows on initial sync", func() { app.Action = func(ctx *cli.Context) error { fakeClient := fake.NewSimpleClientset() @@ -581,7 +581,7 @@ var _ = Describe("Hybrid Overlay Node Linux Operations", func() { appRun(app, netns) }) - It("sets up local pod flows", func() { + ovntest.OnSupportedPlatformsIt("sets up local pod flows", func() { app.Action = func(ctx *cli.Context) error { const ( pod1IP string = "1.2.3.5" diff --git a/go-controller/pkg/node/gateway_init_linux_test.go b/go-controller/pkg/node/gateway_init_linux_test.go index 08e15130c4..9df70d5f14 100644 --- a/go-controller/pkg/node/gateway_init_linux_test.go +++ b/go-controller/pkg/node/gateway_init_linux_test.go @@ -938,7 +938,7 @@ var _ = Describe("Gateway Init Operations", func() { Expect(err).NotTo(HaveOccurred()) }) - It("newLocalGateway sets up a local interface gateway", func() { + ovntest.OnSupportedPlatformsIt("newLocalGateway sets up a local interface gateway", func() { localGatewayInterfaceTest(app, testNS, eth0Name, eth0MAC, eth0IP, eth0GWIP, eth0CIDR) }) @@ -987,20 +987,20 @@ var _ = Describe("Gateway Init Operations", func() { Expect(err).NotTo(HaveOccurred()) }) - It("sets up a shared interface gateway", func() { + ovntest.OnSupportedPlatformsIt("sets up a shared interface gateway", func() { shareGatewayInterfaceTest(app, testNS, eth0Name, eth0MAC, eth0IP, eth0GWIP, eth0CIDR, 0) }) - It("sets up a shared interface gateway with tagged VLAN", func() { + ovntest.OnSupportedPlatformsIt("sets up a shared interface gateway with tagged VLAN", func() { shareGatewayInterfaceTest(app, testNS, eth0Name, eth0MAC, eth0IP, eth0GWIP, eth0CIDR, 3000) }) config.Gateway.Interface = eth0Name - It("sets up a shared interface gateway with predetermined gateway interface", func() { + ovntest.OnSupportedPlatformsIt("sets up a shared interface gateway with predetermined gateway interface", func() { shareGatewayInterfaceTest(app, testNS, eth0Name, eth0MAC, eth0IP, eth0GWIP, eth0CIDR, 0) }) - It("sets up a shared interface gateway with tagged VLAN + predetermined gateway interface", func() { + ovntest.OnSupportedPlatformsIt("sets up a shared interface gateway with tagged VLAN + predetermined gateway interface", func() { shareGatewayInterfaceTest(app, testNS, eth0Name, eth0MAC, eth0IP, eth0GWIP, eth0CIDR, 3000) }) }) @@ -1071,7 +1071,7 @@ var _ = Describe("Gateway Operations DPU", func() { Expect(err).NotTo(HaveOccurred()) }) - It("sets up a shared interface gateway DPU", func() { + ovntest.OnSupportedPlatformsIt("sets up a shared interface gateway DPU", func() { shareGatewayInterfaceDPUTest(app, testNS, brphys, hostMAC, hostCIDR) }) }) @@ -1114,7 +1114,7 @@ var _ = Describe("Gateway Operations DPU", func() { Expect(err).NotTo(HaveOccurred()) }) - It("sets up a shared interface gateway DPU host", func() { + ovntest.OnSupportedPlatformsIt("sets up a shared interface gateway DPU host", func() { shareGatewayInterfaceDPUHostTest(app, testNS, uplinkName, hostIP, gwIP) }) }) diff --git a/go-controller/pkg/node/management-port_linux_test.go b/go-controller/pkg/node/management-port_linux_test.go index 84dd6d0eea..0290360b02 100644 --- a/go-controller/pkg/node/management-port_linux_test.go +++ b/go-controller/pkg/node/management-port_linux_test.go @@ -449,7 +449,7 @@ var _ = Describe("Management Port Operations", func() { Expect(err).NotTo(HaveOccurred()) }) - It("sets up the management port for IPv4 clusters", func() { + ovntest.OnSupportedPlatformsIt("sets up the management port for IPv4 clusters", func() { app.Action = func(ctx *cli.Context) error { testManagementPort(ctx, fexec, testNS, []managementPortTestConfig{ @@ -473,7 +473,7 @@ var _ = Describe("Management Port Operations", func() { Expect(err).NotTo(HaveOccurred()) }) - It("sets up the management port for IPv6 clusters", func() { + ovntest.OnSupportedPlatformsIt("sets up the management port for IPv6 clusters", func() { app.Action = func(ctx *cli.Context) error { testManagementPort(ctx, fexec, testNS, []managementPortTestConfig{ @@ -499,7 +499,7 @@ var _ = Describe("Management Port Operations", func() { Expect(err).NotTo(HaveOccurred()) }) - It("sets up the management port for dual-stack clusters", func() { + ovntest.OnSupportedPlatformsIt("sets up the management port for dual-stack clusters", func() { app.Action = func(ctx *cli.Context) error { testManagementPort(ctx, fexec, testNS, []managementPortTestConfig{ @@ -550,7 +550,7 @@ var _ = Describe("Management Port Operations", func() { Expect(err).NotTo(HaveOccurred()) }) - It("sets up the management port for IPv4 dpu clusters", func() { + ovntest.OnSupportedPlatformsIt("sets up the management port for IPv4 dpu clusters", func() { app.Action = func(ctx *cli.Context) error { testManagementPortDPU(ctx, fexec, testNS, []managementPortTestConfig{ @@ -591,7 +591,7 @@ var _ = Describe("Management Port Operations", func() { Expect(err).NotTo(HaveOccurred()) }) - It("sets up the management port for IPv4 dpu-host clusters", func() { + ovntest.OnSupportedPlatformsIt("sets up the management port for IPv4 dpu-host clusters", func() { app.Action = func(ctx *cli.Context) error { testManagementPortDPUHost(ctx, fexec, testNS, []managementPortTestConfig{ diff --git a/go-controller/pkg/node/management-port_test.go b/go-controller/pkg/node/management-port_test.go index d873471e11..aa37794136 100644 --- a/go-controller/pkg/node/management-port_test.go +++ b/go-controller/pkg/node/management-port_test.go @@ -1,11 +1,12 @@ package node import ( + "reflect" + . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/config" "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/types" - "reflect" ) var _ = Describe("Mananagement port tests", func() { diff --git a/go-controller/pkg/testing/testing.go b/go-controller/pkg/testing/testing.go index 722a61c0fb..7045bab170 100644 --- a/go-controller/pkg/testing/testing.go +++ b/go-controller/pkg/testing/testing.go @@ -1,10 +1,27 @@ package testing import ( + "os" + + "github.com/onsi/ginkgo" "github.com/onsi/gomega/format" + kruntime "k8s.io/apimachinery/pkg/util/runtime" ) +// OnSupportedPlatformsIt is a wrapper around ginkgo.It to determine if running +// the test is applicable for the current test environment. This is used to skip +// tests that are unable to execute in certain environments. Such as those without +// root or cap_net_admin privileges +func OnSupportedPlatformsIt(description string, f interface{}) { + if os.Getenv("NOROOT") != "TRUE" { + ginkgo.It(description, f) + } else { + defer ginkgo.GinkgoRecover() + ginkgo.Skip(description) + } +} + func init() { // Gomega's default string diff behavior makes it impossible to figure // out what fake command is failing, so turn it off From 567dac1d107de5e44c868dfcdbc5681cb3d41ef6 Mon Sep 17 00:00:00 2001 From: Aniket Bhat Date: Thu, 29 Sep 2022 16:45:50 -0400 Subject: [PATCH 3/3] Update golang version and prometheus client dependency for utests --- go-controller/Makefile | 2 +- go-controller/go.mod | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/go-controller/Makefile b/go-controller/Makefile index 606a878c3c..b9ee03ad1b 100644 --- a/go-controller/Makefile +++ b/go-controller/Makefile @@ -9,7 +9,7 @@ PKGS ?= GOPATH ?= $(shell go env GOPATH) TEST_REPORT_DIR?=$(CURDIR)/_artifacts export TEST_REPORT_DIR -GO_VERSION ?= 1.16.3 +GO_VERSION ?= 1.17.6 GO_DOCKER_IMG = quay.io/giantswarm/golang:${GO_VERSION} # CONTAINER_RUNNABLE determines if the tests can be run inside a container. It checks to see if # podman/docker is installed on the system. diff --git a/go-controller/go.mod b/go-controller/go.mod index 77b8f301f9..44e08df629 100644 --- a/go-controller/go.mod +++ b/go-controller/go.mod @@ -13,9 +13,9 @@ require ( github.com/coreos/go-iptables v0.4.5 github.com/google/uuid v1.2.0 github.com/gorilla/mux v1.8.0 + github.com/j-keck/arping v1.0.2 github.com/juju/errors v0.0.0-20200330140219-3fe23663418f // indirect github.com/juju/testing v0.0.0-20200706033705-4c23f9c453cd // indirect - github.com/j-keck/arping v1.0.2 github.com/k8snetworkplumbingwg/network-attachment-definition-client v0.0.0-20200626054723-37f83d1996bc github.com/matttproud/golang_protobuf_extensions v1.0.2-0.20181231171920-c182affec369 // indirect github.com/miekg/dns v1.1.31 @@ -27,6 +27,7 @@ require ( github.com/ovn-org/libovsdb v0.6.1-0.20220603151050-98c0bad3cff1 github.com/pkg/errors v0.9.1 github.com/prometheus/client_golang v1.11.0 + github.com/prometheus/client_model v0.2.0 // indirect github.com/spf13/afero v1.4.1 github.com/stretchr/testify v1.7.0 github.com/urfave/cli/v2 v2.2.0