From 644fcf2a4cb2194d1e3e8bc20be2f80690fd0693 Mon Sep 17 00:00:00 2001 From: Andrea Panattoni Date: Thu, 19 Sep 2024 08:39:09 +0200 Subject: [PATCH] Delete webhooks when SriovOperatorConfig is deleted When a user deletes the default SriovOperatorConfig resource and tries to recreate it afterwards, the operator webhooks returns the error: ``` Error from server (InternalError): error when creating "/tmp/opconfig.yml": Internal error occurred: failed calling webhook "operator-webhook.sriovnetwork.openshift.io": failed to call webhook: Post "https://operator-webhook-service.openshift-sriov-network-operator.svc:443/validating-custom-resource?timeout=10s": service "operator-webhook-service" not found ``` as the webhook configuration is still present, while the Service and the DaemonSet has been deleted. Delete all the webhook configurations when the user deletes the default SriovOperatorConfig Signed-off-by: Andrea Panattoni --- controllers/sriovoperatorconfig_controller.go | 32 +++++++++- .../sriovoperatorconfig_controller_test.go | 61 ++++++++++++++++--- 2 files changed, 83 insertions(+), 10 deletions(-) diff --git a/controllers/sriovoperatorconfig_controller.go b/controllers/sriovoperatorconfig_controller.go index 1121b623f..377ebd2de 100644 --- a/controllers/sriovoperatorconfig_controller.go +++ b/controllers/sriovoperatorconfig_controller.go @@ -18,6 +18,7 @@ package controllers import ( "context" + "errors" "fmt" "os" "sort" @@ -28,6 +29,7 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" uns "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" kscheme "k8s.io/client-go/kubernetes/scheme" ctrl "sigs.k8s.io/controller-runtime" @@ -81,7 +83,9 @@ 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.") - return reconcile.Result{}, nil + + err := r.deleteAllWebhooks(ctx) + return reconcile.Result{}, err } // Error reading the object - requeue the request. logger.Error(err, "Failed to get default SriovOperatorConfig object") @@ -457,3 +461,29 @@ func (r SriovOperatorConfigReconciler) setLabelInsideObject(ctx context.Context, return nil } + +func (r SriovOperatorConfigReconciler) deleteAllWebhooks(ctx context.Context) error { + var err error + obj := &uns.Unstructured{} + obj.SetGroupVersionKind(schema.GroupVersionKind{Group: "admissionregistration.k8s.io", Kind: "MutatingWebhookConfiguration", Version: "v1"}) + obj.SetName(consts.OperatorWebHookName) + err = errors.Join( + err, r.deleteWebhookObject(ctx, obj), + ) + + obj = &uns.Unstructured{} + obj.SetGroupVersionKind(schema.GroupVersionKind{Group: "admissionregistration.k8s.io", Kind: "ValidatingWebhookConfiguration", Version: "v1"}) + obj.SetName(consts.OperatorWebHookName) + err = errors.Join( + err, r.deleteWebhookObject(ctx, obj), + ) + + obj = &uns.Unstructured{} + obj.SetGroupVersionKind(schema.GroupVersionKind{Group: "admissionregistration.k8s.io", Kind: "MutatingWebhookConfiguration", Version: "v1"}) + obj.SetName(consts.InjectorWebHookName) + err = errors.Join( + err, r.deleteWebhookObject(ctx, obj), + ) + + return err +} diff --git a/controllers/sriovoperatorconfig_controller_test.go b/controllers/sriovoperatorconfig_controller_test.go index 6a98925eb..7f6db3522 100644 --- a/controllers/sriovoperatorconfig_controller_test.go +++ b/controllers/sriovoperatorconfig_controller_test.go @@ -6,6 +6,7 @@ import ( "os" "strings" "sync" + "time" admv1 "k8s.io/api/admissionregistration/v1" appsv1 "k8s.io/api/apps/v1" @@ -38,15 +39,7 @@ var _ = Describe("SriovOperatorConfig controller", Ordered, func() { BeforeAll(func() { By("Create SriovOperatorConfig controller k8s objs") - config := &sriovnetworkv1.SriovOperatorConfig{} - config.SetNamespace(testNamespace) - config.SetName(consts.DefaultConfigName) - config.Spec = sriovnetworkv1.SriovOperatorConfigSpec{ - EnableInjector: true, - EnableOperatorWebhook: true, - ConfigDaemonNodeSelector: map[string]string{}, - LogLevel: 2, - } + config := makeDefaultSriovOpConfig() Expect(k8sClient.Create(context.Background(), config)).Should(Succeed()) DeferCleanup(func() { err := k8sClient.Delete(context.Background(), config) @@ -224,6 +217,29 @@ var _ = Describe("SriovOperatorConfig controller", Ordered, func() { Expect(err).NotTo(HaveOccurred()) }) + // Namespaced resources are deleted via the `.ObjectMeta.OwnerReference` field. That logic can't be tested here because testenv doesn't have built-in controllers + // (See https://book.kubebuilder.io/reference/envtest#testing-considerations). Since Service and DaemonSet are deleted when default/SriovOperatorConfig is no longer + // present, it's important that webhook configurations are deleted as well. + It("should delete the webhooks when SriovOperatorConfig/default is deleted", func() { + DeferCleanup(k8sClient.Create, context.Background(), makeDefaultSriovOpConfig()) + + err := k8sClient.Delete(context.Background(), &sriovnetworkv1.SriovOperatorConfig{ + ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: "default"}, + }) + Expect(err).NotTo(HaveOccurred()) + + assertResourceDoesNotExist( + schema.GroupVersionKind{Group: "admissionregistration.k8s.io", Kind: "MutatingWebhookConfiguration", Version: "v1"}, + client.ObjectKey{Name: "sriov-operator-webhook-config"}) + assertResourceDoesNotExist( + schema.GroupVersionKind{Group: "admissionregistration.k8s.io", Kind: "ValidatingWebhookConfiguration", Version: "v1"}, + client.ObjectKey{Name: "sriov-operator-webhook-config"}) + + assertResourceDoesNotExist( + schema.GroupVersionKind{Group: "admissionregistration.k8s.io", Kind: "MutatingWebhookConfiguration", Version: "v1"}, + client.ObjectKey{Name: "network-resources-injector-config"}) + }) + 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": ""} @@ -517,6 +533,19 @@ var _ = Describe("SriovOperatorConfig controller", Ordered, func() { }) }) +func makeDefaultSriovOpConfig() *sriovnetworkv1.SriovOperatorConfig { + config := &sriovnetworkv1.SriovOperatorConfig{} + config.SetNamespace(testNamespace) + config.SetName(consts.DefaultConfigName) + config.Spec = sriovnetworkv1.SriovOperatorConfigSpec{ + EnableInjector: true, + EnableOperatorWebhook: true, + ConfigDaemonNodeSelector: map[string]string{}, + LogLevel: 2, + } + return config +} + func assertResourceExists(gvk schema.GroupVersionKind, key client.ObjectKey) { u := &unstructured.Unstructured{} u.SetGroupVersionKind(gvk) @@ -524,6 +553,20 @@ func assertResourceExists(gvk schema.GroupVersionKind, key client.ObjectKey) { Expect(err).NotTo(HaveOccurred()) } +func assertResourceDoesNotExist(gvk schema.GroupVersionKind, key client.ObjectKey) { + Eventually(func(g Gomega) { + u := &unstructured.Unstructured{} + u.SetGroupVersionKind(gvk) + err := k8sClient.Get(context.Background(), key, u) + g.Expect(err).To(HaveOccurred()) + g.Expect(errors.IsNotFound(err)).To(BeTrue()) + }). + WithOffset(1). + WithPolling(100*time.Millisecond). + WithTimeout(2*time.Second). + Should(Succeed(), "Resource type[%s] name[%s] still present in the cluster", gvk.String(), key.String()) +} + func updateConfigDaemonNodeSelector(newValue map[string]string) func() { config := &sriovnetworkv1.SriovOperatorConfig{} err := k8sClient.Get(context.Background(), types.NamespacedName{Namespace: testNamespace, Name: "default"}, config)