From 68200af27ac67509f15fea09459ff1f0d1b703f6 Mon Sep 17 00:00:00 2001
From: Tom Clegg <tom@curii.com>
Date: Thu, 28 Apr 2022 15:17:05 -0400
Subject: [PATCH] 18947: Refactor keep-balance as arvados-server subcommand.

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@curii.com>
---
 build/run-tests.sh                        |  7 ++++
 cmd/arvados-server/cmd.go                 |  2 ++
 sdk/python/tests/run_test_server.py       |  3 ++
 services/keep-balance/balance.go          |  2 +-
 services/keep-balance/balance_run_test.go |  2 +-
 services/keep-balance/balance_test.go     |  2 +-
 services/keep-balance/block_state.go      |  2 +-
 services/keep-balance/block_state_test.go |  2 +-
 services/keep-balance/change_set.go       |  2 +-
 services/keep-balance/change_set_test.go  |  2 +-
 services/keep-balance/collection.go       |  2 +-
 services/keep-balance/collection_test.go  |  2 +-
 services/keep-balance/integration_test.go |  2 +-
 services/keep-balance/keep_service.go     |  2 +-
 services/keep-balance/main.go             | 39 +++++++----------------
 services/keep-balance/main_test.go        |  6 ++--
 services/keep-balance/metrics.go          |  2 +-
 services/keep-balance/server.go           |  2 +-
 18 files changed, 39 insertions(+), 44 deletions(-)

diff --git a/build/run-tests.sh b/build/run-tests.sh
index d517e7e2c4..0f996f77e9 100755
--- a/build/run-tests.sh
+++ b/build/run-tests.sh
@@ -158,6 +158,8 @@ only_install=
 temp=
 temp_preserve=
 
+ignore_sigint=
+
 clear_temp() {
     if [[ -z "$temp" ]]; then
         # we did not even get as far as making a temp dir
@@ -473,6 +475,10 @@ stop_services() {
 }
 
 interrupt() {
+    if [[ -n "$ignore_sigint" ]]; then
+        echo >&2 "ignored SIGINT"
+        return
+    fi
     failures+=("($(basename $0) interrupted)")
     exit_cleanly
 }
@@ -1216,6 +1222,7 @@ else
     setnextcmd
     HISTFILE="$WORKSPACE/tmp/.history"
     history -r
+    ignore_sigint=1
     while read -p 'What next? ' -e -i "$nextcmd" nextcmd; do
         history -s "$nextcmd"
         history -w
diff --git a/cmd/arvados-server/cmd.go b/cmd/arvados-server/cmd.go
index e4bd39002a..c9a91c8721 100644
--- a/cmd/arvados-server/cmd.go
+++ b/cmd/arvados-server/cmd.go
@@ -22,6 +22,7 @@ import (
 	"git.arvados.org/arvados.git/lib/lsf"
 	"git.arvados.org/arvados.git/lib/recovercollection"
 	"git.arvados.org/arvados.git/services/githttpd"
+	keepbalance "git.arvados.org/arvados.git/services/keep-balance"
 	keepweb "git.arvados.org/arvados.git/services/keep-web"
 	"git.arvados.org/arvados.git/services/keepproxy"
 	"git.arvados.org/arvados.git/services/keepstore"
@@ -46,6 +47,7 @@ var (
 		"git-httpd":          githttpd.Command,
 		"install":            install.Command,
 		"init":               install.InitCommand,
+		"keep-balance":       keepbalance.Command,
 		"keep-web":           keepweb.Command,
 		"keepproxy":          keepproxy.Command,
 		"keepstore":          keepstore.Command,
diff --git a/sdk/python/tests/run_test_server.py b/sdk/python/tests/run_test_server.py
index 74722b256e..6f591b74a5 100644
--- a/sdk/python/tests/run_test_server.py
+++ b/sdk/python/tests/run_test_server.py
@@ -957,6 +957,9 @@ if __name__ == "__main__":
               format(args.action, actions),
               file=sys.stderr)
         sys.exit(1)
+    # Create a new process group so our child processes don't exit on
+    # ^C in run-tests.sh interactive mode.
+    os.setpgid(0, 0)
     if args.action == 'start':
         stop(force=('ARVADOS_TEST_API_HOST' not in os.environ))
         run(leave_running_atexit=True)
diff --git a/services/keep-balance/balance.go b/services/keep-balance/balance.go
index eb6f580f43..1dedb409a4 100644
--- a/services/keep-balance/balance.go
+++ b/services/keep-balance/balance.go
@@ -2,7 +2,7 @@
 //
 // SPDX-License-Identifier: AGPL-3.0
 
-package main
+package keepbalance
 
 import (
 	"bytes"
diff --git a/services/keep-balance/balance_run_test.go b/services/keep-balance/balance_run_test.go
index 0d1b6b5912..2db7bea173 100644
--- a/services/keep-balance/balance_run_test.go
+++ b/services/keep-balance/balance_run_test.go
@@ -2,7 +2,7 @@
 //
 // SPDX-License-Identifier: AGPL-3.0
 
-package main
+package keepbalance
 
 import (
 	"bytes"
diff --git a/services/keep-balance/balance_test.go b/services/keep-balance/balance_test.go
index df04145b9d..6626609b57 100644
--- a/services/keep-balance/balance_test.go
+++ b/services/keep-balance/balance_test.go
@@ -2,7 +2,7 @@
 //
 // SPDX-License-Identifier: AGPL-3.0
 
-package main
+package keepbalance
 
 import (
 	"crypto/md5"
diff --git a/services/keep-balance/block_state.go b/services/keep-balance/block_state.go
index e30b4ff794..07c9952f90 100644
--- a/services/keep-balance/block_state.go
+++ b/services/keep-balance/block_state.go
@@ -2,7 +2,7 @@
 //
 // SPDX-License-Identifier: AGPL-3.0
 
-package main
+package keepbalance
 
 import (
 	"sync"
diff --git a/services/keep-balance/block_state_test.go b/services/keep-balance/block_state_test.go
index aaf2c18e29..8a58be288f 100644
--- a/services/keep-balance/block_state_test.go
+++ b/services/keep-balance/block_state_test.go
@@ -2,7 +2,7 @@
 //
 // SPDX-License-Identifier: AGPL-3.0
 
-package main
+package keepbalance
 
 import (
 	"time"
diff --git a/services/keep-balance/change_set.go b/services/keep-balance/change_set.go
index 85d03d409b..8e0ba028ac 100644
--- a/services/keep-balance/change_set.go
+++ b/services/keep-balance/change_set.go
@@ -2,7 +2,7 @@
 //
 // SPDX-License-Identifier: AGPL-3.0
 
-package main
+package keepbalance
 
 import (
 	"encoding/json"
diff --git a/services/keep-balance/change_set_test.go b/services/keep-balance/change_set_test.go
index baf7ab6dc4..5474d29fb5 100644
--- a/services/keep-balance/change_set_test.go
+++ b/services/keep-balance/change_set_test.go
@@ -2,7 +2,7 @@
 //
 // SPDX-License-Identifier: AGPL-3.0
 
-package main
+package keepbalance
 
 import (
 	"encoding/json"
diff --git a/services/keep-balance/collection.go b/services/keep-balance/collection.go
index 1e1e51abe7..ccb01bdd10 100644
--- a/services/keep-balance/collection.go
+++ b/services/keep-balance/collection.go
@@ -2,7 +2,7 @@
 //
 // SPDX-License-Identifier: AGPL-3.0
 
-package main
+package keepbalance
 
 import (
 	"context"
diff --git a/services/keep-balance/collection_test.go b/services/keep-balance/collection_test.go
index f749bad6ad..ca67d1617d 100644
--- a/services/keep-balance/collection_test.go
+++ b/services/keep-balance/collection_test.go
@@ -2,7 +2,7 @@
 //
 // SPDX-License-Identifier: AGPL-3.0
 
-package main
+package keepbalance
 
 import (
 	"context"
diff --git a/services/keep-balance/integration_test.go b/services/keep-balance/integration_test.go
index a6cc328104..3cfb5cdeda 100644
--- a/services/keep-balance/integration_test.go
+++ b/services/keep-balance/integration_test.go
@@ -2,7 +2,7 @@
 //
 // SPDX-License-Identifier: AGPL-3.0
 
-package main
+package keepbalance
 
 import (
 	"bytes"
diff --git a/services/keep-balance/keep_service.go b/services/keep-balance/keep_service.go
index 17f8418f62..f50fbfd946 100644
--- a/services/keep-balance/keep_service.go
+++ b/services/keep-balance/keep_service.go
@@ -2,7 +2,7 @@
 //
 // SPDX-License-Identifier: AGPL-3.0
 
-package main
+package keepbalance
 
 import (
 	"context"
diff --git a/services/keep-balance/main.go b/services/keep-balance/main.go
index 8a95d389c8..f0b0df5bd3 100644
--- a/services/keep-balance/main.go
+++ b/services/keep-balance/main.go
@@ -2,16 +2,15 @@
 //
 // SPDX-License-Identifier: AGPL-3.0
 
-package main
+package keepbalance
 
 import (
+	"bytes"
 	"context"
 	"flag"
 	"fmt"
 	"io"
-	"net/http"
 	_ "net/http/pprof"
-	"os"
 
 	"git.arvados.org/arvados.git/lib/cmd"
 	"git.arvados.org/arvados.git/lib/config"
@@ -22,16 +21,13 @@ import (
 	"github.com/jmoiron/sqlx"
 	_ "github.com/lib/pq"
 	"github.com/prometheus/client_golang/prometheus"
-	"github.com/sirupsen/logrus"
 )
 
-func main() {
-	os.Exit(runCommand(os.Args[0], os.Args[1:], os.Stdin, os.Stdout, os.Stderr))
-}
+type command struct{}
 
-func runCommand(prog string, args []string, stdin io.Reader, stdout, stderr io.Writer) int {
-	logger := ctxlog.FromContext(context.Background())
+var Command = command{}
 
+func (command) RunCommand(prog string, args []string, stdin io.Reader, stdout, stderr io.Writer) int {
 	var options RunOptions
 	flags := flag.NewFlagSet(prog, flag.ContinueOnError)
 	flags.BoolVar(&options.Once, "once", false,
@@ -42,34 +38,21 @@ func runCommand(prog string, args []string, stdin io.Reader, stdout, stderr io.W
 		"send trash requests (delete unreferenced old blocks, and excess replicas of overreplicated blocks)")
 	flags.BoolVar(&options.CommitConfirmedFields, "commit-confirmed-fields", true,
 		"update collection fields (replicas_confirmed, storage_classes_confirmed, etc.)")
-	dumpFlag := flags.Bool("dump", false, "dump details for each block to stdout")
-	pprofAddr := flags.String("pprof", "", "serve Go profile data at `[addr]:port`")
-	// "show version" is implemented by service.Command, so we
-	// don't need the var here -- we just need the -version flag
+	// These options are implemented by service.Command, so we
+	// don't need the vars here -- we just need the flags
 	// to pass flags.Parse().
+	flags.Bool("dump", false, "dump details for each block to stdout")
+	flags.String("pprof", "", "serve Go profile data at `[addr]:port`")
 	flags.Bool("version", false, "Write version information to stdout and exit 0")
 
-	if *pprofAddr != "" {
-		go func() {
-			logrus.Println(http.ListenAndServe(*pprofAddr, nil))
-		}()
-	}
-
-	loader := config.NewLoader(os.Stdin, logger)
+	logger := ctxlog.New(stderr, "json", "info")
+	loader := config.NewLoader(&bytes.Buffer{}, logger)
 	loader.SetupFlags(flags)
-
 	munged := loader.MungeLegacyConfigArgs(logger, args, "-legacy-keepbalance-config")
 	if ok, code := cmd.ParseFlags(flags, prog, munged, "", stderr); !ok {
 		return code
 	}
 
-	if *dumpFlag {
-		dumper := logrus.New()
-		dumper.Out = os.Stdout
-		dumper.Formatter = &logrus.TextFormatter{}
-		options.Dumper = dumper
-	}
-
 	// Drop our custom args that would be rejected by the generic
 	// service.Command
 	args = nil
diff --git a/services/keep-balance/main_test.go b/services/keep-balance/main_test.go
index 820f352166..26002eca98 100644
--- a/services/keep-balance/main_test.go
+++ b/services/keep-balance/main_test.go
@@ -2,7 +2,7 @@
 //
 // SPDX-License-Identifier: AGPL-3.0
 
-package main
+package keepbalance
 
 import (
 	"bytes"
@@ -25,7 +25,7 @@ type mainSuite struct{}
 
 func (s *mainSuite) TestVersionFlag(c *check.C) {
 	var stdout, stderr bytes.Buffer
-	runCommand("keep-balance", []string{"-version"}, nil, &stdout, &stderr)
+	Command.RunCommand("keep-balance", []string{"-version"}, nil, &stdout, &stderr)
 	c.Check(stderr.String(), check.Equals, "")
 	c.Log(stdout.String())
 	c.Check(stdout.String(), check.Matches, `keep-balance.*\(go1.*\)\n`)
@@ -51,7 +51,7 @@ func (s *mainSuite) TestHTTPServer(c *check.C) {
 	c.Assert(err, check.IsNil)
 
 	var stdout bytes.Buffer
-	go runCommand("keep-balance", []string{"-config", "-"}, bytes.NewBuffer(config), &stdout, &stdout)
+	go Command.RunCommand("keep-balance", []string{"-config", "-"}, bytes.NewBuffer(config), &stdout, &stdout)
 	done := make(chan struct{})
 	go func() {
 		defer close(done)
diff --git a/services/keep-balance/metrics.go b/services/keep-balance/metrics.go
index ce1b1811cc..4683b67b98 100644
--- a/services/keep-balance/metrics.go
+++ b/services/keep-balance/metrics.go
@@ -2,7 +2,7 @@
 //
 // SPDX-License-Identifier: AGPL-3.0
 
-package main
+package keepbalance
 
 import (
 	"fmt"
diff --git a/services/keep-balance/server.go b/services/keep-balance/server.go
index 5299b96c1c..e485f5b206 100644
--- a/services/keep-balance/server.go
+++ b/services/keep-balance/server.go
@@ -2,7 +2,7 @@
 //
 // SPDX-License-Identifier: AGPL-3.0
 
-package main
+package keepbalance
 
 import (
 	"net/http"
-- 
2.30.2