Merge pull request #597 from adrianchiris/test-fixes

Test fixes and enhancements

.github/workflows/test.yml

@@ -46,6 +46,9 @@ jobs:
     - name: test controllers on opensfhit
       run: CLUSTER_TYPE=openshift make test-controllers
 
+    - name: test controllers on kubernetes
+      run: CLUSTER_TYPE=kubernetes make test-controllers
+
     - name: test bindata/scripts
       run: make test-bindata-scripts
 

Makefile

@@ -30,6 +30,7 @@ export WATCH_NAMESPACE?=openshift-sriov-network-operator
 export GOFLAGS+=-mod=vendor
 export GO111MODULE=on
 PKGS=$(shell go list ./... | grep -v -E '/vendor/|/test|/examples')
+TESTPKGS?=./...
 
 # go source files, ignore vendor directory
 SRC = $(shell find . -type f -name '*.go' -not -path "./vendor/*")
@@ -56,7 +57,7 @@ GOLANGCI_LINT_VER = v1.55.2
 
 .PHONY: all build clean gendeepcopy test test-e2e test-e2e-k8s run image fmt sync-manifests test-e2e-conformance manifests update-codegen
 
-all: generate vet build
+all: generate lint build
 
 build: manager _build-sriov-network-config-daemon _build-webhook
 
@@ -76,14 +77,14 @@ image: ; $(info Building images...)
 	$(IMAGE_BUILDER) build -f $(DOCKERFILE_WEBHOOK) -t $(WEBHOOK_IMAGE_TAG) $(CURPATH) $(IMAGE_BUILD_OPTS)
 
 # Run tests
-test: generate vet manifests envtest
-	KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir=/tmp -p path)" HOME="$(shell pwd)" go test ./... -coverprofile cover.out -v
+test: generate lint manifests envtest
+	KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir=/tmp -p path)" HOME="$(shell pwd)" go test -coverprofile cover.out -v ${TESTPKGS}
 
 # Build manager binary
-manager: generate vet _build-manager
+manager: generate _build-manager
 
 # Run against the configured Kubernetes cluster in ~/.kube/config
-run: vet skopeo install
+run: skopeo install
 	hack/run-locally.sh
 
 # Install CRDs into a cluster
@@ -132,10 +133,6 @@ fmt: ## Go fmt your code
 fmt-code:
 	go fmt ./...
 
-# Run go vet against code
-vet:
-	go vet ./...
-
 # Generate code
 generate: controller-gen
 	$(CONTROLLER_GEN) object:headerFile="hack/boilerplate.go.txt" paths="./..."
@@ -205,7 +202,7 @@ redeploy-operator-virtual-cluster:
 test-e2e-validation-only:
 	SUITE=./test/validation ./hack/run-e2e-conformance.sh	
 
-test-e2e: generate vet manifests skopeo envtest
+test-e2e: generate manifests skopeo envtest
 	KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir=/tmp -p path)"; source hack/env.sh; HOME="$(shell pwd)" go test ./test/e2e/... -timeout 60m -coverprofile cover.out -v
 
 test-e2e-k8s: export NAMESPACE=sriov-network-operator
@@ -214,14 +211,9 @@ test-e2e-k8s: test-e2e
 test-bindata-scripts: fakechroot
 	fakechroot ./test/scripts/enable-kargs_test.sh
 
-test-%: generate vet manifests envtest
+test-%: generate manifests envtest
 	KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir=/tmp -p path)" HOME="$(shell pwd)" go test ./$*/... -coverprofile cover-$*.out -coverpkg ./... -v
 
-# deploy-setup-k8s: export NAMESPACE=sriov-network-operator
-# deploy-setup-k8s: export ADMISSION_CONTROLLERS_ENABLED=false
-# deploy-setup-k8s: export CNI_BIN_PATH=/opt/cni/bin
-# test-e2e-k8s: test-e2e
-
 GOCOVMERGE = $(BIN_DIR)/gocovmerge
 gocovmerge: ## Download gocovmerge locally if necessary.
 	$(call go-install-tool,$(GOCOVMERGE),github.com/shabbyrobe/gocovmerge/cmd/gocovmerge@latest)

controllers/sriovnetworkpoolconfig_controller_test.go

@@ -13,12 +13,18 @@ import (
 	mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1"
 
 	sriovnetworkv1 "github.com/k8snetworkplumbingwg/sriov-network-operator/api/v1"
+	"github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/consts"
 	constants "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/consts"
+	"github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/vars"
 )
 
 var _ = Describe("Operator", func() {
 	Context("When is up", func() {
 		It("should be able to create machine config for MachineConfigPool specified in sriov pool config", func() {
+			if vars.ClusterType != consts.ClusterTypeOpenshift {
+				Skip("test should only be executed with openshift cluster type")
+			}
+
 			config := &sriovnetworkv1.SriovNetworkPoolConfig{}
 			config.SetNamespace(testNamespace)
 			config.SetName("ovs-hw-offload-config")

controllers/suite_test.go

@@ -67,7 +67,7 @@ const (
 	interval = time.Millisecond * 250
 )
 
-var _ = BeforeSuite(func(done Done) {
+var _ = BeforeSuite(func() {
 	logf.SetLogger(zap.New(
 		zap.WriteTo(GinkgoWriter),
 		zap.UseDevMode(true),
@@ -213,15 +213,16 @@ var _ = BeforeSuite(func(done Done) {
 	poolConfig.SetName(constants.DefaultConfigName)
 	poolConfig.Spec = sriovnetworkv1.SriovNetworkPoolConfigSpec{}
 	Expect(k8sClient.Create(context.TODO(), poolConfig)).Should(Succeed())
-	close(done)
 })
 
 var _ = AfterSuite(func() {
 	By("tearing down the test environment")
 	cancel()
-	Eventually(func() error {
-		return testEnv.Stop()
-	}, timeout, time.Second).ShouldNot(HaveOccurred())
+	if testEnv != nil {
+		Eventually(func() error {
+			return testEnv.Stop()
+		}, timeout, time.Second).ShouldNot(HaveOccurred())
+	}
 })
 
 func TestAPIs(t *testing.T) {

pkg/webhook/validate_test.go

@@ -10,8 +10,6 @@ import (
 
 	. "github.com/onsi/gomega"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/client-go/kubernetes"
-	"sigs.k8s.io/controller-runtime/pkg/envtest"
 
 	. "github.com/k8snetworkplumbingwg/sriov-network-operator/api/v1"
 	constants "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/consts"
@@ -1044,13 +1042,8 @@ func TestValidatePolicyForNodeStateWithInvalidDevice(t *testing.T) {
 		},
 	}
 	g := NewGomegaWithT(t)
-	var testEnv = &envtest.Environment{}
 
-	cfg, err := testEnv.Start()
-	g.Expect(err).ToNot(HaveOccurred())
-	g.Expect(cfg).ToNot(BeNil())
-	kubeclient = kubernetes.NewForConfigOrDie(cfg)
-	_, err = validatePolicyForNodeState(policy, state, NewNode())
+	_, err := validatePolicyForNodeState(policy, state, NewNode())
 	g.Expect(err).NotTo(HaveOccurred())
 }
 

test/conformance/tests/init.go

@@ -18,4 +18,7 @@ func init() {
 	}
 
 	clients = testclient.New("")
+	if clients == nil {
+		panic("failed package init, failed to create ClientSet")
+	}
 }

test/e2e/e2e_tests_suite_test.go

@@ -13,7 +13,7 @@ import (
 
 	"k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/client-go/rest"
-	"k8s.io/utils/pointer"
+	"k8s.io/utils/ptr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/envtest"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
@@ -55,7 +55,7 @@ func TestSriovTests(t *testing.T) {
 
 var sriovIface *sriovnetworkv1.InterfaceExt
 
-var _ = BeforeSuite(func(done Done) {
+var _ = BeforeSuite(func() {
 	logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true)))
 
 	// Go to project root directory
@@ -64,7 +64,7 @@ var _ = BeforeSuite(func(done Done) {
 	By("bootstrapping test environment")
 	testEnv = &envtest.Environment{
 		CRDDirectoryPaths:  []string{filepath.Join("config", "crd", "bases"), filepath.Join("test", "util", "crds")},
-		UseExistingCluster: pointer.BoolPtr(true),
+		UseExistingCluster: ptr.To[bool](true),
 	}
 
 	var err error
@@ -102,8 +102,6 @@ var _ = BeforeSuite(func(done Done) {
 		doneNetNsSet = make(chan error, 1)
 		go netns.SetPfVfLinkNetNs(testPciDev, testNsPath, devPollInterval, quitNetNsSet, doneNetNsSet)
 	}
-
-	close(done)
 })
 
 var _ = AfterSuite(func() {

test/e2e/sriovoperatornodepolicy_test.go

@@ -28,6 +28,8 @@ var _ = Describe("Operator", func() {
 
 	execute.BeforeAll(func() {
 		clients := testclient.New("")
+		Expect(clients).ToNot(BeNil())
+
 		Eventually(func() *cluster.EnabledNodes {
 			sriovInfos, _ = cluster.DiscoverSriov(clients, testNamespace)
 			return sriovInfos

test/util/clean/clean.go

@@ -20,6 +20,10 @@ func All() error {
 		operatorNamespace = "openshift-sriov-network-operator"
 	}
 	clients := client.New("")
+	if clients == nil {
+		return fmt.Errorf("failed to create ClientSet")
+	}
+
 	if RestoreNodeDrainState {
 		err := cluster.SetDisableNodeDrainState(clients, operatorNamespace, false)
 		if err != nil {

test/validation/tests/test_validation.go

@@ -24,15 +24,6 @@ var (
 	operatorNamespace string
 )
 
-func init() {
-	operatorNamespace = os.Getenv("OPERATOR_NAMESPACE")
-	if operatorNamespace == "" {
-		operatorNamespace = "openshift-sriov-network-operator"
-	}
-
-	clients = testclient.New("")
-}
-
 const (
 	sriovOperatorDeploymentName = "sriov-network-operator"
 	// SriovNetworkNodePolicies contains the name of the sriov network node policies CRD
@@ -45,6 +36,16 @@ const (
 	sriovOperatorConfigs = "sriovoperatorconfigs.sriovnetwork.openshift.io"
 )
 
+var _ = BeforeSuite(func() {
+	operatorNamespace = os.Getenv("OPERATOR_NAMESPACE")
+	if operatorNamespace == "" {
+		operatorNamespace = "openshift-sriov-network-operator"
+	}
+
+	clients = testclient.New("")
+	Expect(clients).ToNot(BeNil())
+})
+
 var _ = Describe("validation", func() {
 
 	Context("sriov", func() {