From f521b1e52bce044926c43f8ca7c31dc2e6952959 Mon Sep 17 00:00:00 2001 From: Andrea Panattoni Date: Wed, 21 Aug 2024 11:24:21 +0200 Subject: [PATCH] bug(shutdown): Fix panic when webhooks are disabled When webooks are disabled, shutdown procedure produces the following panic error: ``` 2024-08-13T12:45:04.971685297Z INFO shutdown utils/shutdown.go:22 Done clearing finalizers on exit 2024-08-13T12:45:04.971713179Z INFO shutdown utils/shutdown.go:23 Seting webhook failure policies to Ignore on exit 2024-08-13T12:45:04.978386488Z ERROR shutdown utils/shutdown.go:64 Error getting webhook {"error": "validatingwebhookconfigurations.admissionregistration.k8s.io \"sriov-operator-webhook-config\" not found"} panic: runtime error: index out of range [0] with length 0 goroutine 1 [running]: github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/utils.updateValidatingWebhook(0x37d7788?) /go/src/github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/utils/shutdown.go:75 +0x198 github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/utils.updateWebhooks() /go/src/github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/utils/shutdown.go:64 +0xa5 github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/utils.Shutdown() /go/src/github.com/k8snetworkplumbingwg/sriov-network-operator/pkg/utils/shutdown.go:23 +0x14 main.main() /go/src/github.com/k8snetworkplumbingwg/sriov-network-operator/main.go:296 +0x1e6a ``` Fix the panic error and add an end2end test case to cover it. Signed-off-by: Andrea Panattoni --- pkg/utils/shutdown.go | 1 + test/conformance/tests/test_sriov_operator.go | 22 ++++++++++++++++--- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/pkg/utils/shutdown.go b/pkg/utils/shutdown.go index 208e2073c..f8f9618d4 100644 --- a/pkg/utils/shutdown.go +++ b/pkg/utils/shutdown.go @@ -71,6 +71,7 @@ func updateValidatingWebhook(c *kubernetes.Clientset) { webhook, err := validatingWebhookClient.Get(context.TODO(), consts.OperatorWebHookName, metav1.GetOptions{}) if err != nil { shutdownLog.Error(err, "Error getting webhook") + return } webhook.Webhooks[0].FailurePolicy = &failurePolicyIgnore _, err = validatingWebhookClient.Update(context.TODO(), webhook, metav1.UpdateOptions{}) diff --git a/test/conformance/tests/test_sriov_operator.go b/test/conformance/tests/test_sriov_operator.go index 63a5df259..740777138 100644 --- a/test/conformance/tests/test_sriov_operator.go +++ b/test/conformance/tests/test_sriov_operator.go @@ -241,7 +241,10 @@ var _ = Describe("[sriov] operator", func() { }) }) - It("should gracefully restart quickly", func() { + DescribeTable("should gracefully restart quickly", func(webookEnabled bool) { + DeferCleanup(setSriovOperatorSpecFlag(operatorNetworkInjectorFlag, webookEnabled)) + DeferCleanup(setSriovOperatorSpecFlag(operatorWebhookFlag, webookEnabled)) + // This test case ensure leader election process runs smoothly when the operator's pod is restarted. oldLease, err := clients.CoordinationV1Interface.Leases(operatorNamespace).Get(context.Background(), consts.LeaderElectionID, metav1.GetOptions{}) if k8serrors.IsNotFound(err) { @@ -268,7 +271,10 @@ var _ = Describe("[sriov] operator", func() { g.Expect(newLease.Spec.HolderIdentity).ToNot(Equal(oldLease.Spec.HolderIdentity)) }, 30*time.Second, 5*time.Second).Should(Succeed()) - }) + }, + Entry("webhooks enabled", true), + Entry("webhooks disabled", true), + ) Context("SriovNetworkMetricsExporter", func() { BeforeEach(func() { @@ -2029,19 +2035,24 @@ func defaultFilterPolicy(policy sriovv1.SriovNetworkNodePolicy) bool { return policy.Spec.DeviceType == "netdevice" } -func setSriovOperatorSpecFlag(flagName string, flagValue bool) { +func setSriovOperatorSpecFlag(flagName string, flagValue bool) func() { cfg := sriovv1.SriovOperatorConfig{} err := clients.Get(context.TODO(), runtimeclient.ObjectKey{ Name: "default", Namespace: operatorNamespace, }, &cfg) + ret := func() {} + Expect(err).ToNot(HaveOccurred()) if flagName == operatorNetworkInjectorFlag && cfg.Spec.EnableInjector != flagValue { cfg.Spec.EnableInjector = flagValue err = clients.Update(context.TODO(), &cfg) Expect(err).ToNot(HaveOccurred()) Expect(cfg.Spec.EnableInjector).To(Equal(flagValue)) + ret = func() { + setSriovOperatorSpecFlag(flagName, !flagValue) + } } if flagName == operatorWebhookFlag && cfg.Spec.EnableOperatorWebhook != flagValue { @@ -2049,6 +2060,9 @@ func setSriovOperatorSpecFlag(flagName string, flagValue bool) { clients.Update(context.TODO(), &cfg) Expect(err).ToNot(HaveOccurred()) Expect(cfg.Spec.EnableOperatorWebhook).To(Equal(flagValue)) + ret = func() { + setSriovOperatorSpecFlag(flagName, !flagValue) + } } if flagValue { @@ -2064,6 +2078,8 @@ func setSriovOperatorSpecFlag(flagName string, flagValue bool) { } }, 1*time.Minute, 10*time.Second).WithOffset(1).Should(Succeed()) } + + return ret } func setOperatorConfigLogLevel(level int) {