Fixing some more PR comments

cmd/sriov-network-operator-config-cleanup/cleanup.go

@@ -3,33 +3,28 @@ package main
 import (
 	"context"
 	"os"
+	"time"
 
 	snolog "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/log"
 	"github.com/spf13/cobra"
-	"k8s.io/client-go/dynamic"
 
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/clientcmd"
 	"sigs.k8s.io/controller-runtime/pkg/log"
 
 	sriovnetworkv1 "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/client/clientset/versioned/typed/sriovnetwork/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
-	"k8s.io/apimachinery/pkg/runtime/schema"
 )
 
 var (
 	namespace string
+	contextTO time.Duration = 5
 )
 
 func init() {
 	rootCmd.Flags().StringVarP(&namespace, "namespace", "n", "", "designated SriovOperatorConfig namespace")
 }
 
-func DynamicClientFor(g schema.GroupVersionKind, obj *unstructured.Unstructured, namespace string) (dynamic.ResourceInterface, error) {
-	return nil, nil
-}
-
 func runCleanupCmd(cmd *cobra.Command, args []string) error {
 	var (
 		config *rest.Config
@@ -57,7 +52,10 @@ func runCleanupCmd(cmd *cobra.Command, args []string) error {
 		setupLog.Error(err, "failed to create 'sriovnetworkv1' clientset")
 	}
 
-	sriovcs.SriovOperatorConfigs(namespace).Delete(context.Background(), "default", metav1.DeleteOptions{})
+	// adding context timeout although client-go Delete should be non-blocking by default
+	ctx, timeoutFunc := context.WithTimeout(context.Background(), time.Second*contextTO)
+	defer timeoutFunc()
+	sriovcs.SriovOperatorConfigs(namespace).Delete(ctx, "default", metav1.DeleteOptions{})
 	if err != nil {
 		setupLog.Error(err, "failed to delete SriovOperatorConfig")
 		return err

cmd/sriov-network-operator-config-cleanup/cleanup_test.go

@@ -2,14 +2,22 @@ package main
 
 import (
 	"context"
+	"sync"
 
+	"github.com/golang/mock/gomock"
 	sriovnetworkv1 "github.com/k8snetworkplumbingwg/sriov-network-operator/api/v1"
+	"github.com/k8snetworkplumbingwg/sriov-network-operator/controllers"
+	"github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/featuregate"
+	mock_platforms "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/platforms/mock"
+	"github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/platforms/openshift"
 	"github.com/k8snetworkplumbingwg/sriov-network-operator/test/util"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	"github.com/spf13/cobra"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/client-go/kubernetes/scheme"
+	ctrl "sigs.k8s.io/controller-runtime"
 )
 
 var (
@@ -25,24 +33,94 @@ var (
 var _ = Describe("cleanup", Ordered, func() {
 
 	defaultSriovOperatorConfig := &sriovnetworkv1.SriovOperatorConfig{
-		ObjectMeta: metav1.ObjectMeta{Name: "default", Namespace: testNamespace},
-		Spec:       defaultSriovOperatorSpec,
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "default",
+			Namespace: testNamespace,
+		},
+		Spec: defaultSriovOperatorSpec,
 	}
 
-	BeforeEach(func() {
-		Expect(k8sClient.Create(context.Background(), defaultSriovOperatorConfig)).NotTo(HaveOccurred())
-		err := util.WaitForNamespacedObject(defaultSriovOperatorConfig, k8sClient, testNamespace, "default", util.RetryInterval, util.APITimeout*3)
-		Expect(err).NotTo(HaveOccurred())
+	BeforeAll(func() {
+
+		By("Create SriovOperatorConfig controller k8s objs")
+		config := defaultSriovOperatorConfig
+		Expect(k8sClient.Create(context.Background(), config)).Should(Succeed())
+
+		somePolicy := &sriovnetworkv1.SriovNetworkNodePolicy{}
+		somePolicy.SetNamespace(testNamespace)
+		somePolicy.SetName("some-policy")
+		somePolicy.Spec = sriovnetworkv1.SriovNetworkNodePolicySpec{
+			NumVfs:       5,
+			NodeSelector: map[string]string{"foo": "bar"},
+			NicSelector:  sriovnetworkv1.SriovNetworkNicSelector{},
+			Priority:     20,
+		}
+		Expect(k8sClient.Create(context.Background(), somePolicy)).ToNot(HaveOccurred())
+		DeferCleanup(func() {
+			err := k8sClient.Delete(context.Background(), somePolicy)
+			Expect(err).ToNot(HaveOccurred())
+		})
+
+		// setup controller manager
+		By("Setup controller manager")
+		k8sManager, err := ctrl.NewManager(cfg, ctrl.Options{
+			Scheme: scheme.Scheme,
+		})
+		Expect(err).ToNot(HaveOccurred())
+
+		t := GinkgoT()
+		mockCtrl := gomock.NewController(t)
+		platformHelper := mock_platforms.NewMockInterface(mockCtrl)
+		platformHelper.EXPECT().GetFlavor().Return(openshift.OpenshiftFlavorDefault).AnyTimes()
+		platformHelper.EXPECT().IsOpenshiftCluster().Return(false).AnyTimes()
+		platformHelper.EXPECT().IsHypershift().Return(false).AnyTimes()
+
+		err = (&controllers.SriovOperatorConfigReconciler{
+			Client:         k8sManager.GetClient(),
+			Scheme:         k8sManager.GetScheme(),
+			PlatformHelper: platformHelper,
+			FeatureGate:    featuregate.New(),
+		}).SetupWithManager(k8sManager)
+		Expect(err).ToNot(HaveOccurred())
+
+		ctx, cancel := context.WithCancel(context.Background())
+
+		wg := sync.WaitGroup{}
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			defer GinkgoRecover()
+			By("Start controller manager")
+			err := k8sManager.Start(ctx)
+			Expect(err).ToNot(HaveOccurred())
+		}()
+
+		DeferCleanup(func() {
+			By("Shut down manager")
+			cancel()
+			wg.Wait()
+		})
 	})
 
 	It("test webhook cleanup flow", func() {
 		cmd := &cobra.Command{}
 		namespace = testNamespace
-		Expect(runCleanupCmd(cmd, []string{})).Should(Succeed())
 
+		// verify that finalizer has been added, by controller, upon object creation
 		config := &sriovnetworkv1.SriovOperatorConfig{}
-		Expect(k8sClient.Get(context.Background(), types.NamespacedName{Name: "default", Namespace: testNamespace}, config)).To(MatchError(
-			ContainSubstring("sriovoperatorconfigs.sriovnetwork.openshift.io \"default\" not found")))
+		Eventually(func() []string {
+			// wait for SriovOperatorConfig flags to get updated
+			err := k8sClient.Get(context.Background(), types.NamespacedName{Name: "default", Namespace: testNamespace}, config)
+			if err != nil {
+				return nil
+			}
+			return config.Finalizers
+		}, util.APITimeout, util.RetryInterval).Should(Equal([]string{sriovnetworkv1.OPERATORCONFIGFINALIZERNAME}))
+
+		Expect(runCleanupCmd(cmd, []string{})).Should(Succeed())
+		config = &sriovnetworkv1.SriovOperatorConfig{}
+		err := util.WaitForNamespacedObjectDeleted(config, k8sClient, testNamespace, "default", util.RetryInterval, util.APITimeout)
+		Expect(err).NotTo(HaveOccurred())
 
 	})
 })

cmd/sriov-network-operator-config-cleanup/main.go

@@ -4,11 +4,10 @@ import (
 	"flag"
 	"os"
 
+	snolog "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/log"
 	"github.com/spf13/cobra"
 	"k8s.io/klog/v2"
 	"sigs.k8s.io/controller-runtime/pkg/log"
-
-	snolog "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/log"
 )
 
 const (
@@ -26,13 +25,12 @@ Example: sriov-network-operator-config-cleanup -n <sriov-operator ns>`,
 	}
 )
 
-func init() {
+func main() {
+
 	klog.InitFlags(nil)
 	snolog.BindFlags(flag.CommandLine)
 	rootCmd.PersistentFlags().AddGoFlagSet(flag.CommandLine)
-}
 
-func main() {
 	if err := rootCmd.Execute(); err != nil {
 		log.Log.Error(err, "Error executing", componentName)
 		os.Exit(1)

cmd/sriov-network-operator-config-cleanup/suite_test.go

@@ -45,6 +45,7 @@ var _ = BeforeSuite(func() {
 
 	// Go to project root directory
 	err := os.Chdir("../..")
+	Expect(err).NotTo(HaveOccurred())
 
 	By("bootstrapping test environment")
 	testEnv = &envtest.Environment{
@@ -58,9 +59,7 @@ var _ = BeforeSuite(func() {
 	Expect(cfg).NotTo(BeNil())
 
 	apiserverDir := testEnv.ControlPlane.GetAPIServer().CertDir
-	println("aprserver path:", apiserverDir)
 	kubecfgPath = findKubecfg(apiserverDir, ".kubecfg")
-	println("got kubecfg:", kubecfgPath)
 	err = os.Setenv("KUBECONFIG", kubecfgPath)
 	Expect(err).NotTo(HaveOccurred())
 

controllers/sriovoperatorconfig_controller.go

@@ -83,8 +83,6 @@ func (r *SriovOperatorConfigReconciler) Reconcile(ctx context.Context, req ctrl.
 	if err != nil {
 		if apierrors.IsNotFound(err) {
 			logger.Info("default SriovOperatorConfig object not found. waiting for creation.")
-
-			err := r.deleteAllWebhooks(ctx)
 			return reconcile.Result{}, err
 		}
 		// Error reading the object - requeue the request.
@@ -96,7 +94,6 @@ func (r *SriovOperatorConfigReconciler) Reconcile(ctx context.Context, req ctrl.
 
 	// examine DeletionTimestamp to determine if object is under deletion
 	if defaultConfig.ObjectMeta.DeletionTimestamp.IsZero() {
-
 		if !sriovnetworkv1.StringInArray(sriovnetworkv1.OPERATORCONFIGFINALIZERNAME, defaultConfig.ObjectMeta.Finalizers) {
 			defaultConfig.ObjectMeta.Finalizers = append(defaultConfig.ObjectMeta.Finalizers, sriovnetworkv1.OPERATORCONFIGFINALIZERNAME)
 			if err := r.Update(ctx, defaultConfig); err != nil {

deployment/sriov-network-operator-chart/templates/pre-delete-webooks.yaml

@@ -22,4 +22,4 @@ spec:
           args:
             - --namespace
             - {{ .Release.Namespace }}
-      restartPolicy: Never    
+      restartPolicy: Never