From: Tom Clegg Date: Mon, 4 Jan 2021 20:26:46 +0000 (-0500) Subject: 16306: Fix inability to shutdown passenger processes. X-Git-Tag: 2.2.0~141^2~21 X-Git-Url: https://git.arvados.org/arvados.git/commitdiff_plain/d71d4273d1f3d0b6381efafa649b81c6b4107cf1 16306: Fix inability to shutdown passenger processes. Arvados-DCO-1.1-Signed-off-by: Tom Clegg --- diff --git a/lib/boot/cert.go b/lib/boot/cert.go index 8f6339e63e..b2b8c896c2 100644 --- 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 d49fabe567..d14d051520 100644 --- 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 03464aaf7c..4a605e35af 100644 --- 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 e45c4e1686..daa0414a3c 100644 --- 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 1f6cb764e0..bd1e942658 100644 --- 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 77fdc98be0..090e852446 100644 --- 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 7dba9b5dcf..5e88775e58 100644 --- 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() {