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 <[email protected]>
zeeke · Aug 21, 2024 · 1183e4f · 1183e4f
1 parent ac35440
commit 1183e4f
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 3 deletions.
diff --git 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
@@ -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() {
@@ -2136,26 +2142,34 @@ 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 {
 		cfg.Spec.EnableOperatorWebhook = flagValue
 		clients.Update(context.TODO(), &cfg)
 		Expect(err).ToNot(HaveOccurred())
 		Expect(cfg.Spec.EnableOperatorWebhook).To(Equal(flagValue))
+		ret = func() {
+			setSriovOperatorSpecFlag(flagName, !flagValue)
+		}
 	}
 
 	if flagValue {
@@ -2171,6 +2185,8 @@ func setSriovOperatorSpecFlag(flagName string, flagValue bool) {
 			}
 		}, 1*time.Minute, 10*time.Second).WithOffset(1).Should(Succeed())
 	}
+
+	return ret
 }
 
 func setOperatorConfigLogLevel(level int) {