From 661398b2ed8a83e7f0c483decc16ddf2bf1105cf Mon Sep 17 00:00:00 2001 From: Ido Heyvi Date: Sun, 20 Oct 2024 13:46:26 +0300 Subject: [PATCH] Adding sriov operator config controller finalizer, to enable generated webhooks objects removal by sriov operator Signed-off-by: Ido Heyvi --- Dockerfile | 2 + Makefile | 2 +- api/v1/helper.go | 11 +- .../manifests/operator-webhook/002-rbac.yaml | 4 - .../operator-webhook/003-webhook.yaml | 4 - bindata/manifests/webhook/002-rbac.yaml | 4 - bindata/manifests/webhook/003-webhook.yaml | 2 - .../cleanup.go | 68 +++++++ .../cleanup_test.go | 48 +++++ .../main.go | 40 +++++ .../suite_test.go | 122 +++++++++++++ cmd/webhook/delete.go | 168 ------------------ controllers/sriovoperatorconfig_controller.go | 114 +++++++----- .../sriovoperatorconfig_controller_test.go | 40 ++++- .../templates/pre-delete-webooks.yaml | 2 +- 15 files changed, 399 insertions(+), 232 deletions(-) create mode 100644 cmd/sriov-network-operator-config-cleanup/cleanup.go create mode 100644 cmd/sriov-network-operator-config-cleanup/cleanup_test.go create mode 100644 cmd/sriov-network-operator-config-cleanup/main.go create mode 100644 cmd/sriov-network-operator-config-cleanup/suite_test.go delete mode 100644 cmd/webhook/delete.go diff --git a/Dockerfile b/Dockerfile index 2b26247e8..7735bef7b 100644 --- a/Dockerfile +++ b/Dockerfile @@ -2,9 +2,11 @@ FROM golang:1.22 AS builder WORKDIR /go/src/github.com/k8snetworkplumbingwg/sriov-network-operator COPY . . RUN make _build-manager BIN_PATH=build/_output/cmd +RUN make _build-sriov-network-operator-config-cleanup BIN_PATH=build/_output/cmd FROM quay.io/centos/centos:stream9 COPY --from=builder /go/src/github.com/k8snetworkplumbingwg/sriov-network-operator/build/_output/cmd/manager /usr/bin/sriov-network-operator +COPY --from=builder /go/src/github.com/k8snetworkplumbingwg/sriov-network-operator/build/_output/cmd/sriov-network-operator-config-cleanup /usr/bin/sriov-network-operator-config-cleanup COPY bindata /bindata ENV OPERATOR_NAME=sriov-network-operator CMD ["/usr/bin/sriov-network-operator"] diff --git a/Makefile b/Makefile index 3718b75bd..310f1dc52 100644 --- a/Makefile +++ b/Makefile @@ -60,7 +60,7 @@ GOLANGCI_LINT_VER = v1.55.2 all: generate lint build -build: manager _build-sriov-network-config-daemon _build-webhook +build: manager _build-sriov-network-config-daemon _build-webhook _build-sriov-network-operator-config-cleanup _build-%: WHAT=$* hack/build-go.sh diff --git a/api/v1/helper.go b/api/v1/helper.go index bfdfbc473..62ea0d2a5 100644 --- a/api/v1/helper.go +++ b/api/v1/helper.go @@ -26,11 +26,12 @@ import ( ) const ( - LASTNETWORKNAMESPACE = "operator.sriovnetwork.openshift.io/last-network-namespace" - NETATTDEFFINALIZERNAME = "netattdef.finalizers.sriovnetwork.openshift.io" - POOLCONFIGFINALIZERNAME = "poolconfig.finalizers.sriovnetwork.openshift.io" - ESwithModeLegacy = "legacy" - ESwithModeSwitchDev = "switchdev" + LASTNETWORKNAMESPACE = "operator.sriovnetwork.openshift.io/last-network-namespace" + NETATTDEFFINALIZERNAME = "netattdef.finalizers.sriovnetwork.openshift.io" + POOLCONFIGFINALIZERNAME = "poolconfig.finalizers.sriovnetwork.openshift.io" + OPERATORCONFIGFINALIZERNAME = "operatorconfig.finalizers.sriovnetwork.openshift.io" + ESwithModeLegacy = "legacy" + ESwithModeSwitchDev = "switchdev" SriovCniStateEnable = "enable" SriovCniStateDisable = "disable" diff --git a/bindata/manifests/operator-webhook/002-rbac.yaml b/bindata/manifests/operator-webhook/002-rbac.yaml index 2fefc95b7..7396c3ca4 100644 --- a/bindata/manifests/operator-webhook/002-rbac.yaml +++ b/bindata/manifests/operator-webhook/002-rbac.yaml @@ -9,8 +9,6 @@ apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole metadata: name: operator-webhook - labels: - created-by: {{ .OperatorGeneratedResourcesLabelSelector | default "sriov-network-operator" }} rules: - apiGroups: - "" @@ -34,8 +32,6 @@ apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRoleBinding metadata: name: operator-webhook-role-binding - labels: - created-by: {{ .OperatorGeneratedResourcesLabelSelector | default "sriov-network-operator" }} roleRef: apiGroup: rbac.authorization.k8s.io kind: ClusterRole diff --git a/bindata/manifests/operator-webhook/003-webhook.yaml b/bindata/manifests/operator-webhook/003-webhook.yaml index e9022c686..9181f1edf 100644 --- a/bindata/manifests/operator-webhook/003-webhook.yaml +++ b/bindata/manifests/operator-webhook/003-webhook.yaml @@ -11,8 +11,6 @@ metadata: # more variables. cert-manager.io/inject-ca-from: {{.Namespace}}/{{.OperatorWebhookSecretName}} {{- end }} - labels: - created-by: {{ .OperatorGeneratedResourcesLabelSelector | default "sriov-network-operator" }} webhooks: - name: operator-webhook.sriovnetwork.openshift.io sideEffects: None @@ -45,8 +43,6 @@ metadata: # more variables. cert-manager.io/inject-ca-from: {{.Namespace}}/{{.OperatorWebhookSecretName}} {{- end }} - labels: - created-by: {{ .OperatorGeneratedResourcesLabelSelector | default "sriov-network-operator" }} webhooks: - name: operator-webhook.sriovnetwork.openshift.io sideEffects: None diff --git a/bindata/manifests/webhook/002-rbac.yaml b/bindata/manifests/webhook/002-rbac.yaml index 0734363af..32affca29 100644 --- a/bindata/manifests/webhook/002-rbac.yaml +++ b/bindata/manifests/webhook/002-rbac.yaml @@ -9,8 +9,6 @@ apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole metadata: name: network-resources-injector - labels: - created-by: {{ .OperatorGeneratedResourcesLabelSelector | default "sriov-network-operator" }} rules: - apiGroups: - k8s.cni.cncf.io @@ -33,8 +31,6 @@ apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRoleBinding metadata: name: network-resources-injector-role-binding - labels: - created-by: {{ .OperatorGeneratedResourcesLabelSelector | default "sriov-network-operator" }} roleRef: apiGroup: rbac.authorization.k8s.io kind: ClusterRole diff --git a/bindata/manifests/webhook/003-webhook.yaml b/bindata/manifests/webhook/003-webhook.yaml index a957b56c3..ead56de0c 100644 --- a/bindata/manifests/webhook/003-webhook.yaml +++ b/bindata/manifests/webhook/003-webhook.yaml @@ -11,8 +11,6 @@ metadata: # more variables. cert-manager.io/inject-ca-from: {{.Namespace}}/{{.InjectorWebhookSecretName}} {{- end }} - labels: - created-by: {{ .OperatorGeneratedResourcesLabelSelector | default "sriov-network-operator" }} webhooks: - name: network-resources-injector-config.k8s.io sideEffects: None diff --git a/cmd/sriov-network-operator-config-cleanup/cleanup.go b/cmd/sriov-network-operator-config-cleanup/cleanup.go new file mode 100644 index 000000000..c6c699555 --- /dev/null +++ b/cmd/sriov-network-operator-config-cleanup/cleanup.go @@ -0,0 +1,68 @@ +package main + +import ( + "context" + "os" + + 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 +) + +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 + 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") + } + + sriovcs, err := sriovnetworkv1.NewForConfig(config) + if err != nil { + setupLog.Error(err, "failed to create 'sriovnetworkv1' clientset") + } + + sriovcs.SriovOperatorConfigs(namespace).Delete(context.Background(), "default", metav1.DeleteOptions{}) + if err != nil { + setupLog.Error(err, "failed to delete SriovOperatorConfig") + return err + } + + return nil + +} diff --git a/cmd/sriov-network-operator-config-cleanup/cleanup_test.go b/cmd/sriov-network-operator-config-cleanup/cleanup_test.go new file mode 100644 index 000000000..96b4f0ba0 --- /dev/null +++ b/cmd/sriov-network-operator-config-cleanup/cleanup_test.go @@ -0,0 +1,48 @@ +package main + +import ( + "context" + + sriovnetworkv1 "github.com/k8snetworkplumbingwg/sriov-network-operator/api/v1" + "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" +) + +var ( + testNamespace string = "sriov-network-operator" + defaultSriovOperatorSpec = sriovnetworkv1.SriovOperatorConfigSpec{ + EnableInjector: true, + EnableOperatorWebhook: true, + LogLevel: 2, + FeatureGates: nil, + } +) + +var _ = Describe("cleanup", Ordered, func() { + + defaultSriovOperatorConfig := &sriovnetworkv1.SriovOperatorConfig{ + 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()) + }) + + It("test webhook cleanup flow", func() { + cmd := &cobra.Command{} + namespace = testNamespace + Expect(runCleanupCmd(cmd, []string{})).Should(Succeed()) + + 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"))) + + }) +}) diff --git a/cmd/sriov-network-operator-config-cleanup/main.go b/cmd/sriov-network-operator-config-cleanup/main.go new file mode 100644 index 000000000..6feae413f --- /dev/null +++ b/cmd/sriov-network-operator-config-cleanup/main.go @@ -0,0 +1,40 @@ +package main + +import ( + "flag" + "os" + + "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 ( + componentName = "sriov-network-operator-config-cleanup" +) + +var ( + rootCmd = &cobra.Command{ + Use: componentName, + Short: "Removes 'default' SriovOperatorConfig", + Long: `Removes 'default' SriovOperatorConfig in order to cleanup non-namespaced objects e.g clusterroles/clusterrolebinding/validating/mutating webhooks + +Example: sriov-network-operator-config-cleanup -n -l `, + RunE: runCleanupCmd, + } +) + +func init() { + 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) + } +} diff --git a/cmd/sriov-network-operator-config-cleanup/suite_test.go b/cmd/sriov-network-operator-config-cleanup/suite_test.go new file mode 100644 index 000000000..9040ef49d --- /dev/null +++ b/cmd/sriov-network-operator-config-cleanup/suite_test.go @@ -0,0 +1,122 @@ +package main + +import ( + "context" + "io/fs" + "os" + "path/filepath" + "testing" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "go.uber.org/zap/zapcore" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/scheme" + "k8s.io/client-go/rest" + logf "sigs.k8s.io/controller-runtime/pkg/log" + + //+kubebuilder:scaffold:imports + sriovnetworkv1 "github.com/k8snetworkplumbingwg/sriov-network-operator/api/v1" + "github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/vars" + "github.com/k8snetworkplumbingwg/sriov-network-operator/test/util" + + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/envtest" + "sigs.k8s.io/controller-runtime/pkg/log/zap" +) + +var ( + k8sClient client.Client + testEnv *envtest.Environment + cfg *rest.Config + kubecfgPath string +) + +var _ = BeforeSuite(func() { + + logf.SetLogger(zap.New( + zap.WriteTo(GinkgoWriter), + zap.UseDevMode(true), + func(o *zap.Options) { + o.TimeEncoder = zapcore.RFC3339NanoTimeEncoder + })) + + // Go to project root directory + err := os.Chdir("../..") + + By("bootstrapping test environment") + testEnv = &envtest.Environment{ + CRDDirectoryPaths: []string{filepath.Join("config", "crd", "bases"), filepath.Join("test", "util", "crds")}, + ErrorIfCRDPathMissing: true, + } + + testEnv.ControlPlane.GetAPIServer().Configure().Set("disable-admission-plugins", "MutatingAdmissionWebhook", "ValidatingAdmissionWebhook") + cfg, err = testEnv.Start() + Expect(err).NotTo(HaveOccurred()) + 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()) + + By("registering schemes") + err = sriovnetworkv1.AddToScheme(scheme.Scheme) + Expect(err).NotTo(HaveOccurred()) + vars.Config = cfg + vars.Scheme = scheme.Scheme + vars.Namespace = testNamespace + + By("creating K8s client") + k8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme}) + Expect(err).NotTo(HaveOccurred()) + Expect(k8sClient).NotTo(BeNil()) + + By("creating default/common k8s objects for tests") + // Create test namespace + ns := &corev1.Namespace{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Name: testNamespace, + }, + Spec: corev1.NamespaceSpec{}, + Status: corev1.NamespaceStatus{}, + } + ctx := context.Background() + Expect(k8sClient.Create(ctx, ns)).Should(Succeed()) +}) + +var _ = AfterSuite(func() { + By("tearing down the test environment") + if testEnv != nil { + Eventually(func() error { + return testEnv.Stop() + }, util.APITimeout, time.Second).ShouldNot(HaveOccurred()) + } +}) + +func findKubecfg(path, ext string) string { + var cfg string + filepath.WalkDir(path, func(s string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + if filepath.Ext(d.Name()) == ext { + cfg = s + } + return nil + }) + return cfg +} + +func TestAPIs(t *testing.T) { + _, reporterConfig := GinkgoConfiguration() + + RegisterFailHandler(Fail) + + RunSpecs(t, "operator-webhook Suite", reporterConfig) +} diff --git a/cmd/webhook/delete.go b/cmd/webhook/delete.go deleted file mode 100644 index 52c84d332..000000000 --- a/cmd/webhook/delete.go +++ /dev/null @@ -1,168 +0,0 @@ -package main - -import ( - "context" - "fmt" - "os" - "sync" - "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" - "k8s.io/apimachinery/pkg/watch" -) - -var ( - // Generated cluster wide resources generated by sriov-network-operator - // These resources will be watched for deletion by runDeleteCmd - gvrMutatingWebhookConfiguration = schema.GroupVersionResource{Group: "admissionregistration.k8s.io", Version: "v1", Resource: "mutatingwebhookconfigurations"} - gvrValidatingWebhookConfiguration = schema.GroupVersionResource{Group: "admissionregistration.k8s.io", Version: "v1", Resource: "validatingwebhookconfigurations"} - gvrClusterRoles = schema.GroupVersionResource{Group: "rbac.authorization.k8s.io", Version: "v1", Resource: "clusterroles"} - gvrClusterRoleBinding = schema.GroupVersionResource{Group: "rbac.authorization.k8s.io", Version: "v1", Resource: "clusterrolebindings"} - - clusterResources = []schema.GroupVersionResource{gvrValidatingWebhookConfiguration, gvrMutatingWebhookConfiguration, gvrClusterRoles, gvrClusterRoleBinding} -) - -var ( - dynamicClient *dynamic.DynamicClient - watcherTO int - namespace string - labelSelector string - wg sync.WaitGroup -) - -var ( - deleteCmd = &cobra.Command{ - Use: "delete", - Short: "Deletes provisioned sriov-operator non-namespaced objects", - Long: "Deletes provisioned sriov-operator non-namespaced objects e.g clusterroles/clusterrolebinding/validating/mutating webhooks", - Run: runDeleteCmd, - } -) - -func init() { - rootCmd.AddCommand(deleteCmd) - deleteCmd.Flags().StringVarP(&namespace, "namespace", "n", "", "designated SriovOperatorConfig namespace") - deleteCmd.Flags().StringVarP(&labelSelector, "label-selector", "l", "", "sriov-network-operator provisioned webhooks specified labels") - deleteCmd.Flags().IntVarP(&watcherTO, "watcher-timeout", "t", 10, "sriov-network-operator watch context defined timeout") -} - -func DynamicClientFor(g schema.GroupVersionKind, obj *unstructured.Unstructured, namespace string) (dynamic.ResourceInterface, error) { - return nil, nil -} - -func runDeleteCmd(cmd *cobra.Command, args []string) { - var ( - config *rest.Config - err error - ) - // init logger - snolog.InitLog() - setupLog := log.Log.WithName("sriov-network-operator-webhook") - - setupLog.Info("Run sriov-network-operator-webhook deletion") - - // 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") - } - // use dynamic client to iterate over 'clusterResources' (multiple objects) - dynamicClient, err = dynamic.NewForConfig(config) - if err != nil { - setupLog.Error(err, "failed to create k8s dynamic client") - } - - sriovcs, err := sriovnetworkv1.NewForConfig(config) - if err != nil { - setupLog.Error(err, "failed to create 'sriovnetworkv1' clientset") - } - - def, err := sriovcs.SriovOperatorConfigs(namespace).Get(context.Background(), "default", metav1.GetOptions{}) - if err != nil { - setupLog.Error(err, "failed to get 'default' SriovOperatorConfig") - os.Exit(1) - } - - def.Spec.EnableInjector = false - def.Spec.EnableOperatorWebhook = false - - out, err := sriovcs.SriovOperatorConfigs(namespace).Update(context.Background(), def, metav1.UpdateOptions{}) - if err != nil { - setupLog.Error(fmt.Errorf("%v. %v", err, out), "failed to update SriovOperatorConfig") - os.Exit(1) - } - - opts := metav1.ListOptions{ - TypeMeta: metav1.TypeMeta{}, - LabelSelector: labelSelector, - FieldSelector: "", - } - - watch := func(rsrc schema.GroupVersionResource) { - dr := dynamicClient.Resource(rsrc) - wh, err := dr.List(context.Background(), opts) - if err != nil { - setupLog.Error(err, "failed to list", rsrc.Resource, "objects") - } - if len(wh.Items) == 0 { - setupLog.V(0).Info("could not find", "webhooks", rsrc.Resource) - wg.Done() - return - } else { - setupLog.V(0).Info("found the following", "resource", rsrc.Resource, "items", len(wh.Items)) - } - - ctx, cancel := context.WithTimeout(context.Background(), time.Duration(watcherTO)*time.Second) - defer cancel() - watcher, err := dr.Watch(ctx, opts) - if err != nil { - setupLog.Error(err, "failed to establish sriov-operator-webhook cluster objects deletion watcher") - - } - - defer watcher.Stop() - for { - select { - case event := <-watcher.ResultChan(): - if event.Type == watch.Deleted { - wh, _ := dr.List(context.Background(), opts) - setupLog.V(6).Info("remainin sriov-network-operator", "cluster objects:", len(wh.Items)) - if len(wh.Items) == 0 { - setupLog.Info("watched objects have been deleted", "successfuly", rsrc.Resource) - wg.Done() - return - } - } - - case <-ctx.Done(): - setupLog.Error(err, "sriov-operator-webhook delete watch timeout for", rsrc.Resource, "occurred") - wg.Done() - return - } - } - } - - wg.Add(len(clusterResources)) - for _, r := range clusterResources { - go watch(r) - } - wg.Wait() - - os.Exit(0) -} diff --git a/controllers/sriovoperatorconfig_controller.go b/controllers/sriovoperatorconfig_controller.go index b44ea38ce..2cce637f3 100644 --- a/controllers/sriovoperatorconfig_controller.go +++ b/controllers/sriovoperatorconfig_controller.go @@ -94,58 +94,89 @@ func (r *SriovOperatorConfigReconciler) Reconcile(ctx context.Context, req ctrl. snolog.SetLogLevel(defaultConfig.Spec.LogLevel) - r.FeatureGate.Init(defaultConfig.Spec.FeatureGates) - logger.Info("enabled featureGates", "featureGates", r.FeatureGate.String()) + // examine DeletionTimestamp to determine if object is under deletion + if defaultConfig.ObjectMeta.DeletionTimestamp.IsZero() { - if !defaultConfig.Spec.EnableInjector { - logger.Info("SR-IOV Network Resource Injector is disabled.") - } + 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 { + return reconcile.Result{}, err + } + } - if !defaultConfig.Spec.EnableOperatorWebhook { - logger.Info("SR-IOV Network Operator Webhook is disabled.") - } + r.FeatureGate.Init(defaultConfig.Spec.FeatureGates) + logger.Info("enabled featureGates", "featureGates", r.FeatureGate.String()) - // Fetch the SriovNetworkNodePolicyList - policyList := &sriovnetworkv1.SriovNetworkNodePolicyList{} - err = r.List(ctx, policyList, &client.ListOptions{}) - if err != nil { - // Error reading the object - requeue the request. - return reconcile.Result{}, err - } - // Sort the policies with priority, higher priority ones is applied later - // We need to use the sort so we always get the policies in the same order - // That is needed so when we create the node Affinity for the sriov-device plugin - // it will remain in the same order and not trigger a pod recreation - sort.Sort(sriovnetworkv1.ByPriority(policyList.Items)) - - // Render and sync webhook objects - if err = r.syncWebhookObjs(ctx, defaultConfig); err != nil { - return reconcile.Result{}, err - } + if !defaultConfig.Spec.EnableInjector { + logger.Info("SR-IOV Network Resource Injector is disabled.") + } - // Sync SriovNetworkConfigDaemon objects - if err = r.syncConfigDaemonSet(ctx, defaultConfig); err != nil { - return reconcile.Result{}, err - } + if !defaultConfig.Spec.EnableOperatorWebhook { + logger.Info("SR-IOV Network Operator Webhook is disabled.") + } - if err = syncPluginDaemonObjs(ctx, r.Client, r.Scheme, defaultConfig, policyList); err != nil { - return reconcile.Result{}, err - } + // Fetch the SriovNetworkNodePolicyList + policyList := &sriovnetworkv1.SriovNetworkNodePolicyList{} + err = r.List(ctx, policyList, &client.ListOptions{}) + if err != nil { + // Error reading the object - requeue the request. + return reconcile.Result{}, err + } + // Sort the policies with priority, higher priority ones is applied later + // We need to use the sort so we always get the policies in the same order + // That is needed so when we create the node Affinity for the sriov-device plugin + // it will remain in the same order and not trigger a pod recreation + sort.Sort(sriovnetworkv1.ByPriority(policyList.Items)) + + // Render and sync webhook objects + if err = r.syncWebhookObjs(ctx, defaultConfig); err != nil { + return reconcile.Result{}, err + } - if err = r.syncMetricsExporter(ctx, defaultConfig); err != nil { - return reconcile.Result{}, err - } + // Sync SriovNetworkConfigDaemon objects + if err = r.syncConfigDaemonSet(ctx, defaultConfig); err != nil { + return reconcile.Result{}, err + } - // For Openshift we need to create the systemd files using a machine config - if vars.ClusterType == consts.ClusterTypeOpenshift { - // TODO: add support for hypershift as today there is no MCO on hypershift clusters - if r.PlatformHelper.IsHypershift() { - return ctrl.Result{}, fmt.Errorf("systemd mode is not supported on hypershift") + if err = syncPluginDaemonObjs(ctx, r.Client, r.Scheme, defaultConfig, policyList); err != nil { + return reconcile.Result{}, err } - if err = r.syncOpenShiftSystemdService(ctx, defaultConfig); err != nil { + if err = r.syncMetricsExporter(ctx, defaultConfig); err != nil { return reconcile.Result{}, err } + + // For Openshift we need to create the systemd files using a machine config + if vars.ClusterType == consts.ClusterTypeOpenshift { + // TODO: add support for hypershift as today there is no MCO on hypershift clusters + if r.PlatformHelper.IsHypershift() { + return ctrl.Result{}, fmt.Errorf("systemd mode is not supported on hypershift") + } + + if err = r.syncOpenShiftSystemdService(ctx, defaultConfig); err != nil { + return reconcile.Result{}, err + } + } + } else { + // The object is being deleted + if sriovnetworkv1.StringInArray(sriovnetworkv1.OPERATORCONFIGFINALIZERNAME, defaultConfig.ObjectMeta.Finalizers) { + // our finalizer is present, so lets handle any external dependency + logger.Info("delete SriovOperatorConfig CR", "Namespace", defaultConfig.Namespace, "Name", defaultConfig.Name) + // make sure webhooks objects are deleted prior of removing finalizer + err := r.deleteAllWebhooks(ctx) + if err != nil { + return reconcile.Result{}, err + } + // remove our finalizer from the list and update it. + var found bool + defaultConfig.ObjectMeta.Finalizers, found = sriovnetworkv1.RemoveString(sriovnetworkv1.OPERATORCONFIGFINALIZERNAME, defaultConfig.ObjectMeta.Finalizers) + if found { + if err := r.Update(ctx, defaultConfig); err != nil { + return reconcile.Result{}, err + } + } + } + return reconcile.Result{}, err } logger.Info("Reconcile SriovOperatorConfig completed successfully") @@ -300,7 +331,6 @@ func (r *SriovOperatorConfigReconciler) syncWebhookObjs(ctx context.Context, dc data.Data["OperatorWebhookCA"] = os.Getenv("ADMISSION_CONTROLLERS_CERTIFICATES_OPERATOR_CA_CRT") data.Data["InjectorWebhookSecretName"] = os.Getenv("ADMISSION_CONTROLLERS_CERTIFICATES_INJECTOR_SECRET_NAME") data.Data["InjectorWebhookCA"] = os.Getenv("ADMISSION_CONTROLLERS_CERTIFICATES_INJECTOR_CA_CRT") - data.Data["OperatorGeneratedResourcesLabelSelector"] = os.Getenv("OPERATOR_GENERATED_RESOURCES_LABEL_SELECTOR") data.Data["ExternalControlPlane"] = false if r.PlatformHelper.IsOpenshiftCluster() { diff --git a/controllers/sriovoperatorconfig_controller_test.go b/controllers/sriovoperatorconfig_controller_test.go index 7f6db3522..47e4fc09d 100644 --- a/controllers/sriovoperatorconfig_controller_test.go +++ b/controllers/sriovoperatorconfig_controller_test.go @@ -102,9 +102,15 @@ var _ = Describe("SriovOperatorConfig controller", Ordered, func() { Context("When is up", func() { BeforeEach(func() { + var err error config := &sriovnetworkv1.SriovOperatorConfig{} - err := util.WaitForNamespacedObject(config, k8sClient, testNamespace, "default", util.RetryInterval, util.APITimeout) + err = util.WaitForNamespacedObject(config, k8sClient, testNamespace, "default", util.RetryInterval, util.APITimeout) Expect(err).NotTo(HaveOccurred()) + // in case controller yet to add object's finalizer (e.g whenever test deferCleanup is creating new 'default' config object) + if len(config.Finalizers) == 0 { + err = util.WaitForNamespacedObject(config, k8sClient, testNamespace, "default", util.RetryInterval, util.APITimeout) + Expect(err).NotTo(HaveOccurred()) + } config.Spec = sriovnetworkv1.SriovOperatorConfigSpec{ EnableInjector: true, EnableOperatorWebhook: true, @@ -240,6 +246,38 @@ var _ = Describe("SriovOperatorConfig controller", Ordered, func() { client.ObjectKey{Name: "network-resources-injector-config"}) }) + It("should add/delete finalizer 'operatorconfig' when SriovOperatorConfig/default is added/deleted", func() { + DeferCleanup(k8sClient.Create, context.Background(), makeDefaultSriovOpConfig()) + + // verify that finalizer has been added 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})) + + err := k8sClient.Delete(context.Background(), &sriovnetworkv1.SriovOperatorConfig{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: "default"}, + }) + Expect(err).NotTo(HaveOccurred()) + + // verify that finalizer has been removed + var empty []string + 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(empty)) + }) + It("should be able to update the node selector of sriov-network-config-daemon", func() { By("specify the configDaemonNodeSelector") nodeSelector := map[string]string{"node-role.kubernetes.io/worker": ""} diff --git a/deployment/sriov-network-operator-chart/templates/pre-delete-webooks.yaml b/deployment/sriov-network-operator-chart/templates/pre-delete-webooks.yaml index 26b72fb4d..501ea6721 100644 --- a/deployment/sriov-network-operator-chart/templates/pre-delete-webooks.yaml +++ b/deployment/sriov-network-operator-chart/templates/pre-delete-webooks.yaml @@ -20,5 +20,5 @@ spec: - --namespace - {{ .Release.Namespace }} - --label-selector - - "created-by={{ .Values.operator.admissionControllers.labelSelector | default (include "sriov-network-operator.fullname" .) }}" + - "kubernetes.io/managed-by={{ .Values.operator.admissionControllers.labelSelector | default (include "sriov-network-operator.fullname" .) }}" restartPolicy: Never \ No newline at end of file