Fixing some more PR comments

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

@@ -2,67 +2,75 @@ 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"
+	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/log"
 
 	sriovnetworkv1 "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/client/clientset/versioned/typed/sriovnetwork/v1"
+	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
-	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/watch"
 )
 
 var (
 	namespace string
+	watchTO   int
 )
 
 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
+	rootCmd.Flags().IntVarP(&watchTO, "watch-timeout", "w", 10, "sriov-operator config post-delete watch timeout ")
 }
 
 func runCleanupCmd(cmd *cobra.Command, args []string) error {
-	var (
-		config *rest.Config
-		err    error
-	)
+
 	// init logger
 	snolog.InitLog()
 	setupLog := log.Log.WithName("sriov-network-operator-config-cleanup")
-
 	setupLog.Info("Run sriov-network-operator-config-cleanup")
 
-	// creates the in-cluster config
-	kubeconfig := os.Getenv("KUBECONFIG")
-	if kubeconfig != "" {
-		config, err = clientcmd.BuildConfigFromFlags("", kubeconfig)
-	} else {
-		config, err = rest.InClusterConfig()
-	}
-	if err != nil {
-		setupLog.Error(err, "failed initialization k8s rest config")
-	}
+	// adding context timeout although client-go Delete should be non-blocking by default
+	ctx, timeoutFunc := context.WithTimeout(context.Background(), time.Second*time.Duration(watchTO))
+	defer timeoutFunc()
 
-	sriovcs, err := sriovnetworkv1.NewForConfig(config)
+	restConfig := ctrl.GetConfigOrDie()
+	sriovcs, err := sriovnetworkv1.NewForConfig(restConfig)
 	if err != nil {
 		setupLog.Error(err, "failed to create 'sriovnetworkv1' clientset")
 	}
 
-	sriovcs.SriovOperatorConfigs(namespace).Delete(context.Background(), "default", metav1.DeleteOptions{})
+	err = sriovcs.SriovOperatorConfigs(namespace).Delete(context.Background(), "default", metav1.DeleteOptions{})
 	if err != nil {
+		if errors.IsNotFound(err) {
+			return nil
+		}
 		setupLog.Error(err, "failed to delete SriovOperatorConfig")
 		return err
 	}
 
-	return nil
+	// watching 'default' config deletion with context timeout, in case sriov-operator fails to delete 'default' config
+	watcher, err := sriovcs.SriovOperatorConfigs(namespace).Watch(ctx, metav1.ListOptions{Watch: true})
+	defer watcher.Stop()
+	if err != nil {
+		setupLog.Error(err, "failed creating 'default' SriovOperatorConfig object watcher")
+		return err
+	}
+	for {
+		select {
+		case event := <-watcher.ResultChan():
+			if event.Type == watch.Deleted {
+				setupLog.Info("'default' SriovOperatorConfig is deleted")
+				return nil
+			}
+
+		case <-ctx.Done():
+			err = ctx.Err()
+			setupLog.Error(err, "timeout has occurred for 'default' SriovOperatorConfig deletion")
+			return err
+		}
+	}
 
 }

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

@@ -2,17 +2,34 @@ 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"
+	"sigs.k8s.io/controller-runtime/pkg/manager"
 )
 
+type configController struct {
+	k8sManager manager.Manager
+	ctx        context.Context
+	cancel     context.CancelFunc
+	wg         *sync.WaitGroup
+}
+
 var (
+	controller               *configController
 	testNamespace            string = "sriov-network-operator"
 	defaultSriovOperatorSpec        = sriovnetworkv1.SriovOperatorConfigSpec{
 		EnableInjector:        true,
@@ -24,25 +41,138 @@ var (
 
 var _ = Describe("cleanup", Ordered, func() {
 
-	defaultSriovOperatorConfig := &sriovnetworkv1.SriovOperatorConfig{
-		ObjectMeta: metav1.ObjectMeta{Name: "default", Namespace: testNamespace},
-		Spec:       defaultSriovOperatorSpec,
-	}
+	BeforeAll(func() {
+		By("Create SriovOperatorConfig controller k8s objs")
+		config := getDefaultSriovOperatorConfig()
+		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())
+		})
+
+		controller = newConfigController()
 
-	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())
 	})
 
 	It("test webhook cleanup flow", func() {
+		controller.start()
+		defer controller.stop()
+
 		cmd := &cobra.Command{}
 		namespace = testNamespace
+		// verify that finalizer has been added, by controller, upon object creation
+		config := &sriovnetworkv1.SriovOperatorConfig{}
+		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())
 
-		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")))
+	})
 
+	It("test 'default' config cleanup timeout", func() {
+		// in this test case sriov-operator controller has been scaled down.
+		// we are testing returned ctx timeout error, for not being able to delete 'default' config object
+		config := getDefaultSriovOperatorConfig()
+		config.Finalizers = []string{sriovnetworkv1.OPERATORCONFIGFINALIZERNAME}
+		Expect(k8sClient.Create(context.Background(), config)).Should(Succeed())
+
+		cmd := &cobra.Command{}
+		namespace = testNamespace
+		// verify that finalizer has been added, by controller, upon object creation
+		config = &sriovnetworkv1.SriovOperatorConfig{}
+		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}))
+
+		watchTO = 1
+		err := runCleanupCmd(cmd, []string{})
+		Expect(err.Error()).To(ContainSubstring("context deadline exceeded"))
 	})
 })
+
+func getDefaultSriovOperatorConfig() *sriovnetworkv1.SriovOperatorConfig {
+	return &sriovnetworkv1.SriovOperatorConfig{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "default",
+			Namespace: testNamespace,
+		},
+		Spec: defaultSriovOperatorSpec,
+	}
+}
+
+func newConfigController() *configController {
+	// 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{}
+	controller = &configController{
+		k8sManager: k8sManager,
+		ctx:        ctx,
+		cancel:     cancel,
+		wg:         &wg,
+	}
+
+	return controller
+}
+
+func (c *configController) start() {
+
+	c.wg.Add(1)
+	go func() {
+		defer c.wg.Done()
+		defer GinkgoRecover()
+		By("Start controller manager")
+		err := c.k8sManager.Start(c.ctx)
+		Expect(err).ToNot(HaveOccurred())
+	}()
+}
+
+func (c *configController) stop() {
+	c.cancel()
+	c.wg.Wait()
+}

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,15 +25,14 @@ 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)
+		log.Log.Error(err, "Error executing sriov-network-operator-config-cleanup")
 		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())