Skip to content

Commit

Permalink
Recover panic for controller reconcile and webhook handler
Browse files Browse the repository at this point in the history
Signed-off-by: FillZpp <FillZpp.pub@gmail.com>
  • Loading branch information
FillZpp committed Aug 19, 2021
1 parent 75a1187 commit 9a2a972
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 2 deletions.
4 changes: 4 additions & 0 deletions pkg/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@ type Options struct {
// CacheSyncTimeout refers to the time limit set to wait for syncing caches.
// Defaults to 2 minutes if not set.
CacheSyncTimeout time.Duration

// RecoverPanic indicates whether the panic caused by reconcile should be recovered.
RecoverPanic bool
}

// Controller implements a Kubernetes API. A Controller manages a work queue fed reconcile.Requests
Expand Down Expand Up @@ -133,5 +136,6 @@ func NewUnmanaged(name string, mgr manager.Manager, options Options) (Controller
SetFields: mgr.SetFields,
Name: name,
Log: options.Log.WithName("controller").WithName(name),
RecoverPanic: options.RecoverPanic,
}, nil
}
17 changes: 15 additions & 2 deletions pkg/internal/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,9 @@ type Controller struct {

// Log is used to log messages to users during reconciliation, or for example when a watch is started.
Log logr.Logger

// RecoverPanic indicates whether the panic caused by reconcile should be recovered.
RecoverPanic bool
}

// watchDescription contains all the information necessary to start a watch.
Expand All @@ -95,7 +98,17 @@ type watchDescription struct {
}

// Reconcile implements reconcile.Reconciler.
func (c *Controller) Reconcile(ctx context.Context, req reconcile.Request) (reconcile.Result, error) {
func (c *Controller) Reconcile(ctx context.Context, req reconcile.Request) (_ reconcile.Result, err error) {
if c.RecoverPanic {
defer func() {
if r := recover(); r != nil {
for _, fn := range utilruntime.PanicHandlers {
fn(r)
}
err = fmt.Errorf("panic: %v [recovered]", r)
}
}()
}
log := c.Log.WithValues("name", req.Name, "namespace", req.Namespace)
ctx = logf.IntoContext(ctx, log)
return c.Do.Reconcile(ctx, req)
Expand Down Expand Up @@ -295,7 +308,7 @@ func (c *Controller) reconcileHandler(ctx context.Context, obj interface{}) {

// RunInformersAndControllers the syncHandler, passing it the Namespace/Name string of the
// resource to be synced.
result, err := c.Do.Reconcile(ctx, req)
result, err := c.Reconcile(ctx, req)
switch {
case err != nil:
c.Queue.AddRateLimited(req)
Expand Down
33 changes: 33 additions & 0 deletions pkg/internal/controller/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,39 @@ var _ = Describe("controller", func() {
Expect(err).NotTo(HaveOccurred())
Expect(result).To(Equal(reconcile.Result{Requeue: true}))
})

It("should not recover panic if RecoverPanic is false by default", func() {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()

defer func() {
Expect(recover()).ShouldNot(BeNil())
}()
ctrl.Do = reconcile.Func(func(context.Context, reconcile.Request) (reconcile.Result, error) {
var res *reconcile.Result
return *res, nil
})
_, _ = ctrl.Reconcile(ctx,
reconcile.Request{NamespacedName: types.NamespacedName{Namespace: "foo", Name: "bar"}})
})

It("should recover panic if RecoverPanic is true", func() {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()

defer func() {
Expect(recover()).To(BeNil())
}()
ctrl.RecoverPanic = true
ctrl.Do = reconcile.Func(func(context.Context, reconcile.Request) (reconcile.Result, error) {
var res *reconcile.Result
return *res, nil
})
_, err := ctrl.Reconcile(ctx,
reconcile.Request{NamespacedName: types.NamespacedName{Namespace: "foo", Name: "bar"}})
Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring("[recovered]"))
})
})

Describe("Start", func() {
Expand Down

0 comments on commit 9a2a972

Please sign in to comment.