Skip to content

Commit

Permalink
refactor(cmd/ingress): invert signal ctx logic (apache#2139)
Browse files Browse the repository at this point in the history
* refactor(cmd/ingress): invert signal ctx logic

this commit changes the signal handling in cmd/ingress to be wrapped in
a context, and inverts which goroutine runs the controller and
which watches for the context to be cancelled, which allows some
scaffolding (`sync.WaitGroup`) to be removed and now properly handles
the controller exiting with `nil` (as it does when leader election
fails)
  • Loading branch information
acuteaura authored and Revolyssup committed Apr 12, 2024
1 parent 313470e commit 66b88c4
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 28 deletions.
42 changes: 20 additions & 22 deletions cmd/ingress/ingress.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,12 @@
package ingress

import (
"context"
"encoding/json"
"fmt"
"os"
"os/signal"
"strings"
"sync"
"syscall"
"time"

Expand All @@ -42,13 +42,19 @@ func dief(template string, args ...interface{}) {
os.Exit(1)
}

func waitForSignal(stopCh chan struct{}) {
sigCh := make(chan os.Signal, 1)
signal.Notify(sigCh, syscall.SIGINT, syscall.SIGTERM)

sig := <-sigCh
log.Infof("signal %d (%s) received", sig, sig.String())
close(stopCh)
func contextWithSignalCancel(ctx context.Context, signals ...os.Signal) context.Context {
newCtx, cancel := context.WithCancel(ctx)
go func() {
sigCh := make(chan os.Signal, 1)
signal.Notify(sigCh, signals...)

sig := <-sigCh
log.Infof("signal %d (%s) received", sig, sig.String())
signal.Stop(sigCh)
close(sigCh)
cancel()
}()
return newCtx
}

// NewIngressCommand creates the ingress sub command for apisix-ingress-controller.
Expand Down Expand Up @@ -118,8 +124,8 @@ the apisix cluster and others are created`,
dief("failed to initialize logging: %s", err)
}
log.DefaultLogger = logger
log.Info("init apisix ingress controller")

log.Info("init apisix ingress controller")
log.Info("version:\n", version.Long())

// We should make sure that the cfg that's logged out is sanitized.
Expand All @@ -132,25 +138,17 @@ the apisix cluster and others are created`,
}
log.Info("use configuration\n", string(data))

stop := make(chan struct{})
ctx := contextWithSignalCancel(context.Background(), syscall.SIGINT, syscall.SIGTERM)

ingress, err := controller.NewController(cfg)
if err != nil {
dief("failed to create ingress controller: %s", err)
}
wg := sync.WaitGroup{}
wg.Add(1)
go func() {
defer wg.Done()

log.Info("start ingress controller")

if err := ingress.Run(stop); err != nil {
dief("failed to run ingress controller: %s", err)
}
}()
if err := ingress.Run(ctx); err != nil {
dief("failed to run ingress controller: %s", err)
}

waitForSignal(stop)
wg.Wait()
log.Info("apisix ingress controller exited")
},
}
Expand Down
9 changes: 3 additions & 6 deletions pkg/providers/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,13 +144,10 @@ func (c *Controller) Eventf(_ runtime.Object, eventType string, reason string, m
}

// Run launches the controller.
func (c *Controller) Run(stop chan struct{}) error {
rootCtx, rootCancel := context.WithCancel(context.Background())
func (c *Controller) Run(ctx context.Context) error {
rootCtx, rootCancel := context.WithCancel(ctx)
defer rootCancel()
go func() {
<-stop
rootCancel()
}()

c.MetricsCollector.ResetLeader(false)

go func() {
Expand Down

0 comments on commit 66b88c4

Please sign in to comment.