Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(shutdown): fix crash when shutting down before server and task scheduler have started. #2148

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions pkg/api/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -370,13 +370,18 @@ func (c *Controller) LoadNewConfig(newConfig *config.Config) {

func (c *Controller) Shutdown() {
c.StopBackgroundTasks()
ctx := context.Background()
_ = c.Server.Shutdown(ctx)

if c.Server != nil {
ctx := context.Background()
_ = c.Server.Shutdown(ctx)
}
}

// Will stop scheduler and wait for all tasks to finish their work.
func (c *Controller) StopBackgroundTasks() {
c.taskScheduler.Shutdown()
if c.taskScheduler != nil {
c.taskScheduler.Shutdown()
}
}

func (c *Controller) StartBackgroundTasks() {
Expand Down
4 changes: 0 additions & 4 deletions pkg/cli/server/config_reloader.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
func NewHotReloader(ctlr *api.Controller, filePath string) (*HotReloader, error) {
// creates a new file watcher
watcher, err := fsnotify.NewWatcher()
if err != nil {

Check failure on line 24 in pkg/cli/server/config_reloader.go

View workflow job for this annotation

GitHub Actions / coverage

condition "err != nil" was 8 times false but never true

Check failure on line 24 in pkg/cli/server/config_reloader.go

View workflow job for this annotation

GitHub Actions / coverage

condition "err != nil" was 3 times false but never true
return nil, err
}

Expand All @@ -36,13 +36,11 @@

func signalHandler(ctlr *api.Controller, sigCh chan os.Signal) {
// if signal then shutdown
if sig, ok := <-sigCh; ok {

Check failure on line 39 in pkg/cli/server/config_reloader.go

View workflow job for this annotation

GitHub Actions / coverage

condition "ok" was never evaluated

Check failure on line 39 in pkg/cli/server/config_reloader.go

View workflow job for this annotation

GitHub Actions / coverage

condition "ok" was never evaluated
ctlr.Log.Info().Interface("signal", sig).Msg("received signal")

// gracefully shutdown http server
ctlr.Shutdown() //nolint: contextcheck

close(sigCh)
}
}

Expand All @@ -61,8 +59,6 @@
func (hr *HotReloader) Start() {
done := make(chan bool)

initShutDownRoutine(hr.ctlr)

// run watcher
go func() {
defer hr.watcher.Close()
Expand All @@ -72,13 +68,13 @@
select {
// watch for events
case event := <-hr.watcher.Events:
if event.Op == fsnotify.Write {

Check failure on line 71 in pkg/cli/server/config_reloader.go

View workflow job for this annotation

GitHub Actions / coverage

condition "event.Op == fsnotify.Write" was 6 times false but never true
log.Info().Msg("config file changed, trying to reload config")

newConfig := config.New()

err := LoadConfiguration(newConfig, hr.filePath)
if err != nil {

Check failure on line 77 in pkg/cli/server/config_reloader.go

View workflow job for this annotation

GitHub Actions / coverage

condition "err != nil" was never evaluated
log.Error().Err(err).Msg("failed to reload config, retry writing it.")

continue
Expand All @@ -100,7 +96,7 @@
}
}()

if err := hr.watcher.Add(hr.filePath); err != nil {

Check failure on line 99 in pkg/cli/server/config_reloader.go

View workflow job for this annotation

GitHub Actions / coverage

condition "err != nil" was 8 times false but never true

Check failure on line 99 in pkg/cli/server/config_reloader.go

View workflow job for this annotation

GitHub Actions / coverage

condition "err != nil" was 3 times false but never true
log.Panic().Err(err).Str("config", hr.filePath).Msg("failed to add config file to fsnotity watcher")
}

Expand Down
2 changes: 2 additions & 0 deletions pkg/cli/server/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
Short: "`serve` stores and distributes OCI images",
Long: "`serve` stores and distributes OCI images",
RunE: func(cmd *cobra.Command, args []string) error {
if len(args) > 0 {

Check failure on line 48 in pkg/cli/server/root.go

View workflow job for this annotation

GitHub Actions / coverage

condition "len(args) > 0" was 13 times true but never false
if err := LoadConfiguration(conf, args[0]); err != nil {
return err
}
Expand All @@ -55,7 +55,7 @@

// config reloader
hotReloader, err := NewHotReloader(ctlr, args[0])
if err != nil {

Check failure on line 58 in pkg/cli/server/root.go

View workflow job for this annotation

GitHub Actions / coverage

condition "err != nil" was 8 times false but never true
ctlr.Log.Error().Err(err).Msg("failed to create a new hot reloader")

return err
Expand All @@ -69,6 +69,8 @@
return err
}

initShutDownRoutine(ctlr)

if err := ctlr.Run(); err != nil {
log.Error().Err(err).Msg("failed to start controller, exiting")
}
Expand Down
Loading