16306: Fix inability to shutdown passenger processes.

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@curii.com>

diff --git a/lib/boot/cert.go b/lib/boot/cert.go
index 8f6339e63e59d006b20cd0ef76dcaaf5b8b5750e..b2b8c896c2866bceeb399475d5aa7e6cc2dea75c 100644 (file)
--- a/lib/boot/cert.go
+++ b/lib/boot/cert.go
@@ -39,17 +39,17 @@ func (createCertificates) Run(ctx context.Context, fail func(error), super *Supe
        }
 
        // Generate root key
-       err := super.RunProgram(ctx, super.tempdir, nil, nil, "openssl", "genrsa", "-out", "rootCA.key", "4096")
+       err := super.RunProgram(ctx, super.tempdir, runOptions{}, "openssl", "genrsa", "-out", "rootCA.key", "4096")
        if err != nil {
                return err
        }
        // Generate a self-signed root certificate
-       err = super.RunProgram(ctx, super.tempdir, nil, nil, "openssl", "req", "-x509", "-new", "-nodes", "-key", "rootCA.key", "-sha256", "-days", "3650", "-out", "rootCA.crt", "-subj", "/C=US/ST=MA/O=Example Org/CN=localhost")
+       err = super.RunProgram(ctx, super.tempdir, runOptions{}, "openssl", "req", "-x509", "-new", "-nodes", "-key", "rootCA.key", "-sha256", "-days", "3650", "-out", "rootCA.crt", "-subj", "/C=US/ST=MA/O=Example Org/CN=localhost")
        if err != nil {
                return err
        }
        // Generate server key
-       err = super.RunProgram(ctx, super.tempdir, nil, nil, "openssl", "genrsa", "-out", "server.key", "2048")
+       err = super.RunProgram(ctx, super.tempdir, runOptions{}, "openssl", "genrsa", "-out", "server.key", "2048")
        if err != nil {
                return err
        }
@@ -63,12 +63,12 @@ func (createCertificates) Run(ctx context.Context, fail func(error), super *Supe
                return err
        }
        // Generate signing request
-       err = super.RunProgram(ctx, super.tempdir, nil, nil, "openssl", "req", "-new", "-sha256", "-key", "server.key", "-subj", "/C=US/ST=MA/O=Example Org/CN=localhost", "-reqexts", "SAN", "-config", "server.cfg", "-out", "server.csr")
+       err = super.RunProgram(ctx, super.tempdir, runOptions{}, "openssl", "req", "-new", "-sha256", "-key", "server.key", "-subj", "/C=US/ST=MA/O=Example Org/CN=localhost", "-reqexts", "SAN", "-config", "server.cfg", "-out", "server.csr")
        if err != nil {
                return err
        }
        // Sign certificate
-       err = super.RunProgram(ctx, super.tempdir, nil, nil, "openssl", "x509", "-req", "-in", "server.csr", "-CA", "rootCA.crt", "-CAkey", "rootCA.key", "-CAcreateserial", "-out", "server.crt", "-extfile", "server.cfg", "-extensions", "SAN", "-days", "3650", "-sha256")
+       err = super.RunProgram(ctx, super.tempdir, runOptions{}, "openssl", "x509", "-req", "-in", "server.csr", "-CA", "rootCA.crt", "-CAkey", "rootCA.key", "-CAcreateserial", "-out", "server.crt", "-extfile", "server.cfg", "-extensions", "SAN", "-days", "3650", "-sha256")
        if err != nil {
                return err
        }

diff --git a/lib/boot/nginx.go b/lib/boot/nginx.go
index d49fabe5673d8b7fe78b0e0875a34bcf94658ccf..d14d0515201b0b3237ea56059101d78695e37769 100644 (file)
--- a/lib/boot/nginx.go
+++ b/lib/boot/nginx.go
@@ -120,7 +120,7 @@ func (runNginx) Run(ctx context.Context, fail func(error), super *Supervisor) er
        super.waitShutdown.Add(1)
        go func() {
                defer super.waitShutdown.Done()
-               fail(super.RunProgram(ctx, ".", nil, nil, nginx, args...))
+               fail(super.RunProgram(ctx, ".", runOptions{}, nginx, args...))
        }()
        // Choose one of the ports where Nginx should listen, and wait
        // here until we can connect. If ExternalURL is https://foo (with no port) then we connect to "foo:https"

diff --git a/lib/boot/passenger.go b/lib/boot/passenger.go
index 03464aaf7cf470d9ba46d921ac748292fda112f6..4a605e35afb3c1a16346b74af0bba24522c1d720 100644 (file)
--- a/lib/boot/passenger.go
+++ b/lib/boot/passenger.go
@@ -50,32 +50,32 @@ func (runner installPassenger) Run(ctx context.Context, fail func(error), super
        defer passengerInstallMutex.Unlock()
 
        var buf bytes.Buffer
-       err = super.RunProgram(ctx, runner.src, &buf, nil, "gem", "list", "--details", "bundler")
+       err = super.RunProgram(ctx, runner.src, runOptions{output: &buf}, "gem", "list", "--details", "bundler")
        if err != nil {
                return err
        }
        for _, version := range []string{"1.16.6", "1.17.3", "2.0.2"} {
                if !strings.Contains(buf.String(), "("+version+")") {
-                       err = super.RunProgram(ctx, runner.src, nil, nil, "gem", "install", "--user", "--conservative", "--no-document", "bundler:1.16.6", "bundler:1.17.3", "bundler:2.0.2")
+                       err = super.RunProgram(ctx, runner.src, runOptions{}, "gem", "install", "--user", "--conservative", "--no-document", "bundler:1.16.6", "bundler:1.17.3", "bundler:2.0.2")
                        if err != nil {
                                return err
                        }
                        break
                }
        }
-       err = super.RunProgram(ctx, runner.src, nil, nil, "bundle", "install", "--jobs", "4", "--path", filepath.Join(os.Getenv("HOME"), ".gem"))
+       err = super.RunProgram(ctx, runner.src, runOptions{}, "bundle", "install", "--jobs", "4", "--path", filepath.Join(os.Getenv("HOME"), ".gem"))
        if err != nil {
                return err
        }
-       err = super.RunProgram(ctx, runner.src, nil, nil, "bundle", "exec", "passenger-config", "build-native-support")
+       err = super.RunProgram(ctx, runner.src, runOptions{}, "bundle", "exec", "passenger-config", "build-native-support")
        if err != nil {
                return err
        }
-       err = super.RunProgram(ctx, runner.src, nil, nil, "bundle", "exec", "passenger-config", "install-standalone-runtime")
+       err = super.RunProgram(ctx, runner.src, runOptions{}, "bundle", "exec", "passenger-config", "install-standalone-runtime")
        if err != nil {
                return err
        }
-       err = super.RunProgram(ctx, runner.src, nil, nil, "bundle", "exec", "passenger-config", "validate-install")
+       err = super.RunProgram(ctx, runner.src, runOptions{}, "bundle", "exec", "passenger-config", "validate-install")
        if err != nil && !strings.Contains(err.Error(), "exit status 2") {
                // Exit code 2 indicates there were warnings (like
                // "other passenger installations have been detected",
@@ -139,8 +139,14 @@ func (runner runPassenger) Run(ctx context.Context, fail func(error), super *Sup
                        "--no-install-runtime",
                        "--pid-file", filepath.Join(super.wwwtempdir, "passenger."+strings.Replace(appdir, "/", "_", -1)+".pid"),
                }
+               opts := runOptions{
+                       env: append([]string{
+                               "HOME=/var/www",
+                               "TMPDIR=" + super.wwwtempdir,
+                       }, railsEnv...),
+               }
                if super.ClusterType == "production" {
-                       cmdline = append([]string{"sudo", "-u", "www-data", "-E", "HOME=/var/www", "PATH=/var/lib/arvados/bin:" + os.Getenv("PATH"), "/var/lib/arvados/bin/bundle"}, cmdline[1:]...)
+                       opts.user = "www-data"
                } else {
                        // This would be desirable in the production
                        // case too, but it fails with sudo because
@@ -149,8 +155,7 @@ func (runner runPassenger) Run(ctx context.Context, fail func(error), super *Sup
                        // failed (13: Permission denied)"
                        cmdline = append(cmdline, "--log-file", "/dev/stderr")
                }
-               env := append([]string{"TMPDIR=" + super.wwwtempdir}, railsEnv...)
-               err = super.RunProgram(ctx, appdir, nil, env, cmdline[0], cmdline[1:]...)
+               err = super.RunProgram(ctx, appdir, opts, cmdline[0], cmdline[1:]...)
                fail(err)
        }()
        return nil

diff --git a/lib/boot/postgresql.go b/lib/boot/postgresql.go
index e45c4e16869f47043cc3637afe1f14ca7d57c553..daa0414a3ce84d5a66186cf07c5c3ece9687b940 100644 (file)
--- a/lib/boot/postgresql.go
+++ b/lib/boot/postgresql.go
@@ -48,7 +48,7 @@ func (runPostgreSQL) Run(ctx context.Context, fail func(error), super *Superviso
        }
 
        buf := bytes.NewBuffer(nil)
-       err = super.RunProgram(ctx, super.tempdir, buf, nil, "pg_config", "--bindir")
+       err = super.RunProgram(ctx, super.tempdir, runOptions{output: buf}, "pg_config", "--bindir")
        if err != nil {
                return err
        }
@@ -93,17 +93,17 @@ func (runPostgreSQL) Run(ctx context.Context, fail func(error), super *Superviso
                args = append([]string{"postgres", prog}, args...)
                prog = "setuidgid"
        }
-       err = super.RunProgram(ctx, super.tempdir, nil, nil, prog, args...)
+       err = super.RunProgram(ctx, super.tempdir, runOptions{}, prog, args...)
        if err != nil {
                return err
        }
 
-       err = super.RunProgram(ctx, super.tempdir, nil, nil, "cp", "server.crt", "server.key", datadir)
+       err = super.RunProgram(ctx, super.tempdir, runOptions{}, "cp", "server.crt", "server.key", datadir)
        if err != nil {
                return err
        }
        if iamroot {
-               err = super.RunProgram(ctx, super.tempdir, nil, nil, "chown", "postgres", datadir+"/server.crt", datadir+"/server.key")
+               err = super.RunProgram(ctx, super.tempdir, runOptions{}, "chown", "postgres", datadir+"/server.crt", datadir+"/server.key")
                if err != nil {
                        return err
                }
@@ -124,7 +124,7 @@ func (runPostgreSQL) Run(ctx context.Context, fail func(error), super *Superviso
                        args = append([]string{"postgres", prog}, args...)
                        prog = "setuidgid"
                }
-               fail(super.RunProgram(ctx, super.tempdir, nil, nil, prog, args...))
+               fail(super.RunProgram(ctx, super.tempdir, runOptions{}, prog, args...))
        }()
 
        for {

diff --git a/lib/boot/seed.go b/lib/boot/seed.go
index 1f6cb764e070af369252cd75a16b12f2666fa0fa..bd1e942658e9f50fba873d3de4f3a1c971dd54dc 100644 (file)
--- a/lib/boot/seed.go
+++ b/lib/boot/seed.go
@@ -23,11 +23,11 @@ func (seedDatabase) Run(ctx context.Context, fail func(error), super *Supervisor
        if super.ClusterType == "production" {
                return nil
        }
-       err = super.RunProgram(ctx, "services/api", nil, railsEnv, "bundle", "exec", "rake", "db:setup")
+       err = super.RunProgram(ctx, "services/api", runOptions{env: railsEnv}, "bundle", "exec", "rake", "db:setup")
        if err != nil {
                return err
        }
-       err = super.RunProgram(ctx, "services/api", nil, railsEnv, "bundle", "exec", "./script/get_anonymous_user_token.rb")
+       err = super.RunProgram(ctx, "services/api", runOptions{env: railsEnv}, "bundle", "exec", "./script/get_anonymous_user_token.rb")
        if err != nil {
                return err
        }

diff --git a/lib/boot/service.go b/lib/boot/service.go
index 77fdc98be038a465b1f240bba1a92e252f8a2f5f..090e852446f7c3270f50c94a7ac88870d162a38e 100644 (file)
--- a/lib/boot/service.go
+++ b/lib/boot/service.go
@@ -31,7 +31,7 @@ func (runner runServiceCommand) String() string {
 
 func (runner runServiceCommand) Run(ctx context.Context, fail func(error), super *Supervisor) error {
        binfile := filepath.Join(super.bindir, "arvados-server")
-       err := super.RunProgram(ctx, super.bindir, nil, nil, binfile, "-version")
+       err := super.RunProgram(ctx, super.bindir, runOptions{}, binfile, "-version")
        if err != nil {
                return err
        }
@@ -46,7 +46,7 @@ func (runner runServiceCommand) Run(ctx context.Context, fail func(error), super
                super.waitShutdown.Add(1)
                go func() {
                        defer super.waitShutdown.Done()
-                       fail(super.RunProgram(ctx, super.tempdir, nil, []string{"ARVADOS_SERVICE_INTERNAL_URL=" + u.String()}, binfile, runner.name, "-config", super.configfile))
+                       fail(super.RunProgram(ctx, super.tempdir, runOptions{env: []string{"ARVADOS_SERVICE_INTERNAL_URL=" + u.String()}}, binfile, runner.name, "-config", super.configfile))
                }()
        }
        return nil
@@ -77,7 +77,7 @@ func (runner runGoProgram) Run(ctx context.Context, fail func(error), super *Sup
                return ctx.Err()
        }
 
-       err = super.RunProgram(ctx, super.tempdir, nil, nil, binfile, "-version")
+       err = super.RunProgram(ctx, super.tempdir, runOptions{}, binfile, "-version")
        if err != nil {
                return err
        }
@@ -93,7 +93,7 @@ func (runner runGoProgram) Run(ctx context.Context, fail func(error), super *Sup
                super.waitShutdown.Add(1)
                go func() {
                        defer super.waitShutdown.Done()
-                       fail(super.RunProgram(ctx, super.tempdir, nil, []string{"ARVADOS_SERVICE_INTERNAL_URL=" + u.String()}, binfile))
+                       fail(super.RunProgram(ctx, super.tempdir, runOptions{env: []string{"ARVADOS_SERVICE_INTERNAL_URL=" + u.String()}}, binfile))
                }()
        }
        return nil

diff --git a/lib/boot/supervisor.go b/lib/boot/supervisor.go
index 7dba9b5dcf9ae35e1712a817a805d4361cf71da5..5e88775e58b49ce49f868c552ebc3aeb70c7169c 100644 (file)
--- a/lib/boot/supervisor.go
+++ b/lib/boot/supervisor.go
@@ -21,6 +21,7 @@ import (
        "os/user"
        "path/filepath"
        "reflect"
+       "strconv"
        "strings"
        "sync"
        "syscall"
@@ -206,13 +207,13 @@ func (super *Supervisor) run(cfg *arvados.Config) error {
        } else if super.SourceVersion == "" {
                // Find current source tree version.
                var buf bytes.Buffer
-               err = super.RunProgram(super.ctx, ".", &buf, nil, "git", "diff", "--shortstat")
+               err = super.RunProgram(super.ctx, ".", runOptions{output: &buf}, "git", "diff", "--shortstat")
                if err != nil {
                        return err
                }
                dirty := buf.Len() > 0
                buf.Reset()
-               err = super.RunProgram(super.ctx, ".", &buf, nil, "git", "log", "-n1", "--format=%H")
+               err = super.RunProgram(super.ctx, ".", runOptions{output: &buf}, "git", "log", "-n1", "--format=%H")
                if err != nil {
                        return err
                }
@@ -407,7 +408,7 @@ func (super *Supervisor) installGoProgram(ctx context.Context, srcpath string) (
        if super.ClusterType == "production" {
                return binfile, nil
        }
-       err := super.RunProgram(ctx, filepath.Join(super.SourcePath, srcpath), nil, []string{"GOBIN=" + super.bindir}, "go", "install", "-ldflags", "-X git.arvados.org/arvados.git/lib/cmd.version="+super.SourceVersion+" -X main.version="+super.SourceVersion)
+       err := super.RunProgram(ctx, filepath.Join(super.SourcePath, srcpath), runOptions{env: []string{"GOBIN=" + super.bindir}}, "go", "install", "-ldflags", "-X git.arvados.org/arvados.git/lib/cmd.version="+super.SourceVersion+" -X main.version="+super.SourceVersion)
        return binfile, err
 }
 
@@ -470,6 +471,12 @@ func (super *Supervisor) lookPath(prog string) string {
        return prog
 }
 
+type runOptions struct {
+       output io.Writer // attach stdout
+       env    []string  // add/replace environment variables
+       user   string    // run as specified user
+}
+
 // RunProgram runs prog with args, using dir as working directory. If ctx is
 // cancelled while the child is running, RunProgram terminates the child, waits
 // for it to exit, then returns.
@@ -478,7 +485,7 @@ func (super *Supervisor) lookPath(prog string) string {
 //
 // Child's stdout will be written to output if non-nil, otherwise the
 // boot command's stderr.
-func (super *Supervisor) RunProgram(ctx context.Context, dir string, output io.Writer, env []string, prog string, args ...string) error {
+func (super *Supervisor) RunProgram(ctx context.Context, dir string, opts runOptions, prog string, args ...string) error {
        cmdline := fmt.Sprintf("%s", append([]string{prog}, args...))
        super.logger.WithField("command", cmdline).WithField("dir", dir).Info("executing")
 
@@ -531,10 +538,10 @@ func (super *Supervisor) RunProgram(ctx context.Context, dir string, output io.W
        }()
        copiers.Add(1)
        go func() {
-               if output == nil {
+               if opts.output == nil {
                        io.Copy(logwriter, stdout)
                } else {
-                       io.Copy(output, stdout)
+                       io.Copy(opts.output, stdout)
                }
                copiers.Done()
        }()
@@ -544,10 +551,25 @@ func (super *Supervisor) RunProgram(ctx context.Context, dir string, output io.W
        } else {
                cmd.Dir = filepath.Join(super.SourcePath, dir)
        }
-       env = append([]string(nil), env...)
+       env := append([]string(nil), opts.env...)
        env = append(env, super.environ...)
        cmd.Env = dedupEnv(env)
 
+       if opts.user != "" {
+               u, err := user.Lookup(opts.user)
+               if err != nil {
+                       return fmt.Errorf("user.Lookup(%q): %w", opts.user, err)
+               }
+               uid, _ := strconv.Atoi(u.Uid)
+               gid, _ := strconv.Atoi(u.Gid)
+               cmd.SysProcAttr = &syscall.SysProcAttr{
+                       Credential: &syscall.Credential{
+                               Uid: uint32(uid),
+                               Gid: uint32(gid),
+                       },
+               }
+       }
+
        exited := false
        defer func() { exited = true }()
        go func() {