diff --git a/pkg/config/v1alpha1/types.go b/pkg/config/v1alpha1/types.go index 1af58a0348..f2226278c6 100644 --- a/pkg/config/v1alpha1/types.go +++ b/pkg/config/v1alpha1/types.go @@ -94,6 +94,10 @@ type ControllerConfigurationSpec struct { // Defaults to 2 minutes if not set. // +optional CacheSyncTimeout *time.Duration `json:"cacheSyncTimeout,omitempty"` + + // RecoverPanic indicates if panics should be recovered. + // +optional + RecoverPanic *bool `json:"recoverPanic,omitempty"` } // ControllerMetrics defines the metrics configs. diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index 4286c135dd..fe7f94fdc1 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -56,7 +56,8 @@ type Options struct { CacheSyncTimeout time.Duration // RecoverPanic indicates whether the panic caused by reconcile should be recovered. - RecoverPanic bool + // Defaults to the Controller.RecoverPanic setting from the Manager if unset. + RecoverPanic *bool } // Controller implements a Kubernetes API. A Controller manages a work queue fed reconcile.Requests @@ -139,6 +140,10 @@ func NewUnmanaged(name string, mgr manager.Manager, options Options) (Controller return nil, err } + if options.RecoverPanic == nil { + options.RecoverPanic = mgr.GetControllerOptions().RecoverPanic + } + // Create controller with dependencies set return &controller.Controller{ Do: options.Reconciler, diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go index 8932cb35ca..b5d816bc28 100644 --- a/pkg/controller/controller_test.go +++ b/pkg/controller/controller_test.go @@ -25,11 +25,14 @@ import ( . "github.com/onsi/gomega" "go.uber.org/goleak" corev1 "k8s.io/api/core/v1" + "k8s.io/utils/pointer" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/config/v1alpha1" "sigs.k8s.io/controller-runtime/pkg/controller" "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/handler" + internalcontroller "sigs.k8s.io/controller-runtime/pkg/internal/controller" "sigs.k8s.io/controller-runtime/pkg/manager" "sigs.k8s.io/controller-runtime/pkg/reconcile" "sigs.k8s.io/controller-runtime/pkg/runtime/inject" @@ -142,6 +145,39 @@ var _ = Describe("controller.Controller", func() { clientTransport.CloseIdleConnections() Eventually(func() error { return goleak.Find(currentGRs) }).Should(Succeed()) }) + + It("should default RecoverPanic from the manager", func() { + m, err := manager.New(cfg, manager.Options{Controller: v1alpha1.ControllerConfigurationSpec{RecoverPanic: pointer.Bool(true)}}) + Expect(err).NotTo(HaveOccurred()) + + c, err := controller.New("new-controller", m, controller.Options{ + Reconciler: reconcile.Func(nil), + }) + Expect(err).NotTo(HaveOccurred()) + + ctrl, ok := c.(*internalcontroller.Controller) + Expect(ok).To(BeTrue()) + + Expect(ctrl.RecoverPanic).NotTo(BeNil()) + Expect(*ctrl.RecoverPanic).To(BeTrue()) + }) + + It("should not override RecoverPanic on the controller", func() { + m, err := manager.New(cfg, manager.Options{Controller: v1alpha1.ControllerConfigurationSpec{RecoverPanic: pointer.Bool(true)}}) + Expect(err).NotTo(HaveOccurred()) + + c, err := controller.New("new-controller", m, controller.Options{ + Reconciler: reconcile.Func(nil), + RecoverPanic: pointer.Bool(false), + }) + Expect(err).NotTo(HaveOccurred()) + + ctrl, ok := c.(*internalcontroller.Controller) + Expect(ok).To(BeTrue()) + + Expect(ctrl.RecoverPanic).NotTo(BeNil()) + Expect(*ctrl.RecoverPanic).To(BeFalse()) + }) }) }) diff --git a/pkg/internal/controller/controller.go b/pkg/internal/controller/controller.go index b67657bf0c..f7734695ce 100644 --- a/pkg/internal/controller/controller.go +++ b/pkg/internal/controller/controller.go @@ -92,7 +92,7 @@ type Controller struct { LogConstructor func(request *reconcile.Request) logr.Logger // RecoverPanic indicates whether the panic caused by reconcile should be recovered. - RecoverPanic bool + RecoverPanic *bool } // watchDescription contains all the information necessary to start a watch. @@ -106,7 +106,7 @@ type watchDescription struct { func (c *Controller) Reconcile(ctx context.Context, req reconcile.Request) (_ reconcile.Result, err error) { defer func() { if r := recover(); r != nil { - if c.RecoverPanic { + if c.RecoverPanic != nil && *c.RecoverPanic { for _, fn := range utilruntime.PanicHandlers { fn(r) } diff --git a/pkg/internal/controller/controller_test.go b/pkg/internal/controller/controller_test.go index 2c7726e2d5..5d3b1c9bea 100644 --- a/pkg/internal/controller/controller_test.go +++ b/pkg/internal/controller/controller_test.go @@ -33,6 +33,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/util/workqueue" + "k8s.io/utils/pointer" "sigs.k8s.io/controller-runtime/pkg/cache" "sigs.k8s.io/controller-runtime/pkg/cache/informertest" "sigs.k8s.io/controller-runtime/pkg/client" @@ -114,7 +115,7 @@ var _ = Describe("controller", func() { defer func() { Expect(recover()).To(BeNil()) }() - ctrl.RecoverPanic = true + ctrl.RecoverPanic = pointer.Bool(true) ctrl.Do = reconcile.Func(func(context.Context, reconcile.Request) (reconcile.Result, error) { var res *reconcile.Result return *res, nil