From: Tom Clegg <tom@tomclegg.ca>
Date: Thu, 20 Feb 2020 19:24:30 +0000 (-0500)
Subject: 15954: Don't return from Stop() until child processes end.
X-Git-Tag: 2.1.0~273^2~42
X-Git-Url: https://git.arvados.org/arvados.git/commitdiff_plain/71b3aadcbf0d8a3c0b283fc27a17773951d417c7

15954: Don't return from Stop() until child processes end.

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@tomclegg.ca>
---

diff --git a/lib/boot/cmd.go b/lib/boot/cmd.go
index b11cb89e43..5f5bb1ee77 100644
--- a/lib/boot/cmd.go
+++ b/lib/boot/cmd.go
@@ -97,7 +97,8 @@ func (bootCommand) RunCommand(prog string, args []string, stdin io.Reader, stdou
 	defer boot.Stop()
 	if url, ok := boot.WaitReady(); ok {
 		fmt.Fprintln(stdout, url)
-		<-ctx.Done() // wait for signal
+		// Wait for signal/crash + orderly shutdown
+		<-boot.done
 		return 0
 	} else {
 		return 1
@@ -121,6 +122,7 @@ type Booter struct {
 	done          chan struct{}
 	healthChecker *health.Aggregator
 	tasksReady    map[string]chan bool
+	waitShutdown  sync.WaitGroup
 
 	tempdir    string
 	configfile string
@@ -288,6 +290,7 @@ func (boot *Booter) run(cfg *arvados.Config) error {
 	boot.healthChecker = &health.Aggregator{Cluster: boot.cluster}
 	<-boot.ctx.Done()
 	boot.logger.Info("shutting down")
+	boot.waitShutdown.Wait()
 	return boot.ctx.Err()
 }
 
@@ -315,9 +318,12 @@ func (boot *Booter) Stop() {
 }
 
 func (boot *Booter) WaitReady() (*arvados.URL, bool) {
+	ticker := time.NewTicker(time.Second)
+	defer ticker.Stop()
 	for waiting := true; waiting; {
-		time.Sleep(time.Second)
-		if boot.ctx.Err() != nil {
+		select {
+		case <-ticker.C:
+		case <-boot.ctx.Done():
 			return nil, false
 		}
 		if boot.healthChecker == nil {
diff --git a/lib/boot/nginx.go b/lib/boot/nginx.go
index 5c1954c838..a06d3a7008 100644
--- a/lib/boot/nginx.go
+++ b/lib/boot/nginx.go
@@ -77,7 +77,9 @@ func (runNginx) Run(ctx context.Context, fail func(error), boot *Booter) error {
 			}
 		}
 	}
+	boot.waitShutdown.Add(1)
 	go func() {
+		defer boot.waitShutdown.Done()
 		fail(boot.RunProgram(ctx, ".", nil, nil, nginx,
 			"-g", "error_log stderr info;",
 			"-g", "pid "+filepath.Join(boot.tempdir, "nginx.pid")+";",
diff --git a/lib/boot/passenger.go b/lib/boot/passenger.go
index 822e737cb9..21834dab26 100644
--- a/lib/boot/passenger.go
+++ b/lib/boot/passenger.go
@@ -90,7 +90,9 @@ func (runner runPassenger) Run(ctx context.Context, fail func(error), boot *Boot
 	if err != nil {
 		return fmt.Errorf("bug: no InternalURLs for component %q: %v", runner, runner.svc.InternalURLs)
 	}
+	boot.waitShutdown.Add(1)
 	go func() {
+		defer boot.waitShutdown.Done()
 		err = boot.RunProgram(ctx, runner.src, nil, nil, "bundle", "exec",
 			"passenger", "start",
 			"-p", port,
diff --git a/lib/boot/postgresql.go b/lib/boot/postgresql.go
index 48e24ffaec..ed5aecd09f 100644
--- a/lib/boot/postgresql.go
+++ b/lib/boot/postgresql.go
@@ -56,7 +56,9 @@ func (runPostgreSQL) Run(ctx context.Context, fail func(error), boot *Booter) er
 
 	port := boot.cluster.PostgreSQL.Connection["port"]
 
+	boot.waitShutdown.Add(1)
 	go func() {
+		defer boot.waitShutdown.Done()
 		fail(boot.RunProgram(ctx, boot.tempdir, nil, nil, filepath.Join(bindir, "postgres"),
 			"-l",          // enable ssl
 			"-D", datadir, // data dir
diff --git a/lib/boot/service.go b/lib/boot/service.go
index 6edf78b3ce..4b35e13768 100644
--- a/lib/boot/service.go
+++ b/lib/boot/service.go
@@ -45,7 +45,10 @@ func (runner runGoProgram) String() string {
 
 func (runner runGoProgram) Run(ctx context.Context, fail func(error), boot *Booter) error {
 	boot.wait(ctx, runner.depends...)
-	boot.RunProgram(ctx, runner.src, nil, nil, "go", "install")
+	err := boot.RunProgram(ctx, runner.src, nil, nil, "go", "install")
+	if err != nil {
+		return err
+	}
 	if ctx.Err() != nil {
 		return ctx.Err()
 	}
@@ -54,13 +57,17 @@ func (runner runGoProgram) Run(ctx context.Context, fail func(error), boot *Boot
 		// Run one for each URL
 		for u := range runner.svc.InternalURLs {
 			u := u
+			boot.waitShutdown.Add(1)
 			go func() {
+				defer boot.waitShutdown.Done()
 				fail(boot.RunProgram(ctx, boot.tempdir, nil, []string{"ARVADOS_SERVICE_INTERNAL_URL=" + u.String()}, basename))
 			}()
 		}
 	} else {
 		// Just run one
+		boot.waitShutdown.Add(1)
 		go func() {
+			defer boot.waitShutdown.Done()
 			fail(boot.RunProgram(ctx, boot.tempdir, nil, nil, basename))
 		}()
 	}