Skip to content

Commit

Permalink
Merge pull request #1299 from abhat/fix_ut_golang_img
Browse files Browse the repository at this point in the history
OCPBUGS-2464: Fix the golang image used for unit-tests
  • Loading branch information
openshift-merge-robot committed Oct 18, 2022
2 parents 11a028b + 567dac1 commit 54f32e8
Show file tree
Hide file tree
Showing 11 changed files with 59 additions and 27 deletions.
11 changes: 9 additions & 2 deletions go-controller/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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

Expand All @@ -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
Expand Down
3 changes: 2 additions & 1 deletion go-controller/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
8 changes: 6 additions & 2 deletions go-controller/hack/test-go.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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:-}
Expand All @@ -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
Expand Down
16 changes: 8 additions & 8 deletions go-controller/hybrid-overlay/pkg/controller/node_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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()

Expand Down Expand Up @@ -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"
Expand Down
1 change: 1 addition & 0 deletions go-controller/pkg/cni/OCP_HACKS.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
//go:build linux
// +build linux

package cni
Expand Down
2 changes: 1 addition & 1 deletion go-controller/pkg/factory/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions go-controller/pkg/node/OCP_HACKS.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
//go:build linux
// +build linux

package node
Expand Down
14 changes: 7 additions & 7 deletions go-controller/pkg/node/gateway_init_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})

Expand Down Expand Up @@ -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)
})
})
Expand Down Expand Up @@ -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)
})
})
Expand Down Expand Up @@ -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)
})
})
Expand Down
10 changes: 5 additions & 5 deletions go-controller/pkg/node/management-port_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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{
Expand All @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down
3 changes: 2 additions & 1 deletion go-controller/pkg/node/management-port_test.go
Original file line number Diff line number Diff line change
@@ -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() {
Expand Down
17 changes: 17 additions & 0 deletions go-controller/pkg/testing/testing.go
Original file line number Diff line number Diff line change
@@ -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
Expand Down

0 comments on commit 54f32e8

Please sign in to comment.