Skip to content

Commit

Permalink
bug(shutdown): Fix panic when webhooks are disabled
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
zeeke committed Aug 21, 2024
1 parent ac35440 commit 1183e4f
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 3 deletions.
1 change: 1 addition & 0 deletions pkg/utils/shutdown.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{})
Expand Down
22 changes: 19 additions & 3 deletions test/conformance/tests/test_sriov_operator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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() {
Expand Down Expand Up @@ -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 {
Expand All @@ -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) {
Expand Down

0 comments on commit 1183e4f

Please sign in to comment.