Skip to content

Commit

Permalink
pkg/start: Fill in deferred HandleCrash
Browse files Browse the repository at this point in the history
Clayton wants these in each goroutine we launch [1].  Obviously
there's no way to reach inside the informer Start()s and add it there.
I'm also adding this to the FIXME comment for rerolling the
auto-update worker goroutines; we'll get those straigtened out in
future work.

Cherry picked from 9c42a92 (openshift#424), around conflicts due to the lack
of Context pivots in 4.5.

[1]: openshift#424
  • Loading branch information
wking committed Aug 27, 2020
1 parent c8af639 commit c8f99b2
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 2 deletions.
2 changes: 1 addition & 1 deletion pkg/autoupdate/autoupdate.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ func (ctrl *Controller) Run(workers int, stopCh <-chan struct{}) error {
}

for i := 0; i < workers; i++ {
// FIXME: actually wait until these complete if the Context is canceled.
// FIXME: actually wait until these complete if the Context is canceled. And possibly add utilruntime.HandleCrash.
go wait.Until(ctrl.worker, time.Second, stopCh)
}

Expand Down
1 change: 0 additions & 1 deletion pkg/cvo/cvo.go
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,6 @@ func loadConfigMapVerifierDataFromUpdate(update *payload.Update, clientBuilder v

// Run runs the cluster version operator until stopCh is completed. Workers is ignored for now.
func (optr *Operator) Run(ctx context.Context, workers int) error {
defer utilruntime.HandleCrash()
defer optr.queue.ShutDown()
stopCh := ctx.Done()
workerStopCh := make(chan struct{})
Expand Down
6 changes: 6 additions & 0 deletions pkg/start/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"github.com/google/uuid"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
"k8s.io/client-go/informers"
"k8s.io/client-go/kubernetes"
"k8s.io/client-go/kubernetes/scheme"
Expand Down Expand Up @@ -147,6 +148,7 @@ func (o *Options) run(ctx context.Context, controllerCtx *Context, lock *resourc
defer func() { signal.Stop(ch) }()
signal.Notify(ch, os.Interrupt, syscall.SIGTERM)
go func() {
defer utilruntime.HandleCrash()
sig := <-ch
klog.Infof("Shutting down due to %s", sig)
runCancel()
Expand All @@ -159,6 +161,7 @@ func (o *Options) run(ctx context.Context, controllerCtx *Context, lock *resourc
if o.ListenAddr != "" {
resultChannelCount++
go func() {
defer utilruntime.HandleCrash()
err := cvo.RunMetrics(postMainContext, shutdownContext, o.ListenAddr)
resultChannel <- asyncResult{name: "metrics server", error: err}
}()
Expand All @@ -172,6 +175,7 @@ func (o *Options) run(ctx context.Context, controllerCtx *Context, lock *resourc

resultChannelCount++
go func() {
defer utilruntime.HandleCrash()
leaderelection.RunOrDie(postMainContext, leaderelection.LeaderElectionConfig{
Lock: lock,
ReleaseOnCancel: true,
Expand All @@ -182,13 +186,15 @@ func (o *Options) run(ctx context.Context, controllerCtx *Context, lock *resourc
OnStartedLeading: func(_ context.Context) { // no need for this passed-through postMainContext, because goroutines we launch inside will use runContext
resultChannelCount++
go func() {
defer utilruntime.HandleCrash()
err := controllerCtx.CVO.Run(runContext, 2)
resultChannel <- asyncResult{name: "main operator", error: err}
}()

if controllerCtx.AutoUpdate != nil {
resultChannelCount++
go func() {
defer utilruntime.HandleCrash()
err := controllerCtx.AutoUpdate.Run(2, runContext.Done())
resultChannel <- asyncResult{name: "auto-update controller", error: err}
}()
Expand Down

0 comments on commit c8f99b2

Please sign in to comment.