Skip to content

Commit

Permalink
Delete webhooks when SriovOperatorConfig is deleted
Browse files Browse the repository at this point in the history
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 <apanatto@redhat.com>
  • Loading branch information
zeeke committed Sep 20, 2024
1 parent 60c6404 commit 644fcf2
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 10 deletions.
32 changes: 31 additions & 1 deletion controllers/sriovoperatorconfig_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package controllers

import (
"context"
"errors"
"fmt"
"os"
"sort"
Expand All @@ -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"
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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
}
61 changes: 52 additions & 9 deletions controllers/sriovoperatorconfig_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"os"
"strings"
"sync"
"time"

admv1 "k8s.io/api/admissionregistration/v1"
appsv1 "k8s.io/api/apps/v1"
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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": ""}
Expand Down Expand Up @@ -517,13 +533,40 @@ 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)
err := k8sClient.Get(context.Background(), key, u)
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)
Expand Down

0 comments on commit 644fcf2

Please sign in to comment.