[ARVADOS] created: 1.3.0-1285-gfe372a454

Git user git at public.curoverse.com
Wed Jul 10 20:35:58 UTC 2019


        at  fe372a4542a7e2d74ff717118d578cd618a4f27c (commit)


commit fe372a4542a7e2d74ff717118d578cd618a4f27c
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Wed Jul 10 16:10:52 2019 -0400

    13647: Suppress "exit status 1" after config-check diff output.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/lib/config/cmd.go b/lib/config/cmd.go
index b394bbe4a..f78b16c71 100644
--- a/lib/config/cmd.go
+++ b/lib/config/cmd.go
@@ -132,6 +132,7 @@ func (checkCommand) RunCommand(prog string, args []string, stdin io.Reader, stdo
 	if bytes.HasPrefix(diff, []byte("--- ")) {
 		fmt.Fprintln(stdout, "Your configuration is relying on deprecated entries. Suggest making the following changes.")
 		stdout.Write(diff)
+		err = nil
 		return 1
 	} else if len(diff) > 0 {
 		fmt.Fprintf(stderr, "Unexpected diff output:\n%s", diff)

commit 289d1a08301eef358284fc5e416b5569bfc6d5b1
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Wed Jul 10 16:10:50 2019 -0400

    13647: Load one example value from old keepstore config.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/build/run-tests.sh b/build/run-tests.sh
index 3c69ae91d..507ce448d 100755
--- a/build/run-tests.sh
+++ b/build/run-tests.sh
@@ -774,7 +774,7 @@ do_test_once() {
         # before trying "go test". Otherwise, coverage-reporting
         # mode makes Go show the wrong line numbers when reporting
         # compilation errors.
-        go get -ldflags "-X main.version=${ARVADOS_VERSION:-$(git log -n1 --format=%H)-dev}" -t "git.curoverse.com/arvados.git/$1" && \
+        go get -ldflags "-X git.curoverse.com/arvados.git/lib/cmd.version=${ARVADOS_VERSION:-$(git log -n1 --format=%H)-dev}" -t "git.curoverse.com/arvados.git/$1" && \
             cd "$GOPATH/src/git.curoverse.com/arvados.git/$1" && \
             if [[ -n "${testargs[$1]}" ]]
         then
@@ -840,7 +840,7 @@ do_install_once() {
         result=1
     elif [[ "$2" == "go" ]]
     then
-        go get -ldflags "-X main.version=${ARVADOS_VERSION:-$(git log -n1 --format=%H)-dev}" -t "git.curoverse.com/arvados.git/$1"
+        go get -ldflags "-X git.curoverse.com/arvados.git/lib/cmd.version=${ARVADOS_VERSION:-$(git log -n1 --format=%H)-dev}" -t "git.curoverse.com/arvados.git/$1"
     elif [[ "$2" == "pip" ]]
     then
         # $3 can name a path directory for us to use, including trailing
diff --git a/cmd/arvados-server/cmd.go b/cmd/arvados-server/cmd.go
index dd34eff7d..14cd63e46 100644
--- a/cmd/arvados-server/cmd.go
+++ b/cmd/arvados-server/cmd.go
@@ -15,11 +15,10 @@ import (
 )
 
 var (
-	version = "dev"
 	handler = cmd.Multi(map[string]cmd.Handler{
-		"version":   cmd.Version(version),
-		"-version":  cmd.Version(version),
-		"--version": cmd.Version(version),
+		"version":   cmd.Version,
+		"-version":  cmd.Version,
+		"--version": cmd.Version,
 
 		"cloudtest":       cloudtest.Command,
 		"config-check":    config.CheckCommand,
diff --git a/lib/cloud/cloudtest/cmd.go b/lib/cloud/cloudtest/cmd.go
index 35a78695f..9c3fc46c0 100644
--- a/lib/cloud/cloudtest/cmd.go
+++ b/lib/cloud/cloudtest/cmd.go
@@ -63,7 +63,9 @@ func (command) RunCommand(prog string, args []string, stdin io.Reader, stdout, s
 		logger.Info("exiting")
 	}()
 
-	cfg, err := config.LoadFile(*configFile, logger)
+	loader := config.NewLoader(stdin, logger)
+	loader.Path = *configFile
+	cfg, err := loader.Load()
 	if err != nil {
 		return 1
 	}
diff --git a/lib/cmd/cmd.go b/lib/cmd/cmd.go
index 9292ef7e5..24b69f0cc 100644
--- a/lib/cmd/cmd.go
+++ b/lib/cmd/cmd.go
@@ -28,11 +28,18 @@ func (f HandlerFunc) RunCommand(prog string, args []string, stdin io.Reader, std
 	return f(prog, args, stdin, stdout, stderr)
 }
 
-type Version string
+// Version is a Handler that prints the package version (set at build
+// time using -ldflags) and Go runtime version to stdout, and returns
+// 0.
+var Version versionCommand
 
-func (v Version) RunCommand(prog string, args []string, stdin io.Reader, stdout, stderr io.Writer) int {
+var version = "dev"
+
+type versionCommand struct{}
+
+func (versionCommand) RunCommand(prog string, args []string, stdin io.Reader, stdout, stderr io.Writer) int {
 	prog = regexp.MustCompile(` -*version$`).ReplaceAllLiteralString(prog, "")
-	fmt.Fprintf(stdout, "%s %s (%s)\n", prog, v, runtime.Version())
+	fmt.Fprintf(stdout, "%s %s (%s)\n", prog, version, runtime.Version())
 	return 0
 }
 
diff --git a/lib/config/cmd.go b/lib/config/cmd.go
index 0351ad02a..b394bbe4a 100644
--- a/lib/config/cmd.go
+++ b/lib/config/cmd.go
@@ -9,13 +9,12 @@ import (
 	"flag"
 	"fmt"
 	"io"
-	"io/ioutil"
 	"os"
 	"os/exec"
 
-	"git.curoverse.com/arvados.git/sdk/go/arvados"
 	"git.curoverse.com/arvados.git/sdk/go/ctxlog"
 	"github.com/ghodss/yaml"
+	"github.com/sirupsen/logrus"
 )
 
 var DumpCommand dumpCommand
@@ -30,9 +29,15 @@ func (dumpCommand) RunCommand(prog string, args []string, stdin io.Reader, stdou
 		}
 	}()
 
+	loader := &Loader{
+		Stdin:  stdin,
+		Logger: ctxlog.New(stderr, "text", "info"),
+	}
+
 	flags := flag.NewFlagSet("", flag.ContinueOnError)
 	flags.SetOutput(stderr)
-	configFile := flags.String("config", arvados.DefaultConfigFile, "Site configuration `file`")
+	loader.SetupFlags(flags)
+
 	err = flags.Parse(args)
 	if err == flag.ErrHelp {
 		err = nil
@@ -45,8 +50,8 @@ func (dumpCommand) RunCommand(prog string, args []string, stdin io.Reader, stdou
 		flags.Usage()
 		return 2
 	}
-	log := ctxlog.New(stderr, "text", "info")
-	cfg, err := loadFileOrStdin(*configFile, stdin, log)
+
+	cfg, err := loader.Load()
 	if err != nil {
 		return 1
 	}
@@ -67,15 +72,25 @@ type checkCommand struct{}
 
 func (checkCommand) RunCommand(prog string, args []string, stdin io.Reader, stdout, stderr io.Writer) int {
 	var err error
+	var logbuf = &bytes.Buffer{}
 	defer func() {
+		io.Copy(stderr, logbuf)
 		if err != nil {
 			fmt.Fprintf(stderr, "%s\n", err)
 		}
 	}()
 
+	logger := logrus.New()
+	logger.Out = logbuf
+	loader := &Loader{
+		Stdin:  stdin,
+		Logger: logger,
+	}
+
 	flags := flag.NewFlagSet("", flag.ContinueOnError)
 	flags.SetOutput(stderr)
-	configFile := flags.String("config", arvados.DefaultConfigFile, "Site configuration `file`")
+	loader.SetupFlags(flags)
+
 	err = flags.Parse(args)
 	if err == flag.ErrHelp {
 		err = nil
@@ -88,21 +103,14 @@ func (checkCommand) RunCommand(prog string, args []string, stdin io.Reader, stdo
 		flags.Usage()
 		return 2
 	}
-	log := &plainLogger{w: stderr}
-	var buf []byte
-	if *configFile == "-" {
-		buf, err = ioutil.ReadAll(stdin)
-	} else {
-		buf, err = ioutil.ReadFile(*configFile)
-	}
-	if err != nil {
-		return 1
-	}
-	withoutDepr, err := load(bytes.NewBuffer(buf), log, false)
+
+	loader.SkipDeprecated = true
+	withoutDepr, err := loader.Load()
 	if err != nil {
 		return 1
 	}
-	withDepr, err := load(bytes.NewBuffer(buf), nil, true)
+	loader.SkipDeprecated = false
+	withDepr, err := loader.Load()
 	if err != nil {
 		return 1
 	}
@@ -131,36 +139,7 @@ func (checkCommand) RunCommand(prog string, args []string, stdin io.Reader, stdo
 	} else if err != nil {
 		return 1
 	}
-	if log.used {
-		return 1
-	}
-	return 0
-}
-
-type plainLogger struct {
-	w    io.Writer
-	used bool
-}
-
-func (pl *plainLogger) Warnf(format string, args ...interface{}) {
-	pl.used = true
-	fmt.Fprintf(pl.w, format+"\n", args...)
-}
-
-var DumpDefaultsCommand defaultsCommand
-
-type defaultsCommand struct{}
-
-func (defaultsCommand) RunCommand(prog string, args []string, stdin io.Reader, stdout, stderr io.Writer) int {
-	var err error
-	defer func() {
-		if err != nil {
-			fmt.Fprintf(stderr, "%s\n", err)
-		}
-	}()
-
-	_, err = stdout.Write(DefaultYAML)
-	if err != nil {
+	if logbuf.Len() > 0 {
 		return 1
 	}
 	return 0
diff --git a/lib/config/cmd_test.go b/lib/config/cmd_test.go
index f2915a039..938267ff1 100644
--- a/lib/config/cmd_test.go
+++ b/lib/config/cmd_test.go
@@ -6,6 +6,9 @@ package config
 
 import (
 	"bytes"
+	"io"
+	"io/ioutil"
+	"os"
 
 	"git.curoverse.com/arvados.git/lib/cmd"
 	check "gopkg.in/check.v1"
@@ -62,6 +65,26 @@ Clusters:
 	c.Check(stdout.String(), check.Matches, `(?ms).*\n\- +.*MaxItemsPerResponse: 1000\n\+ +MaxItemsPerResponse: 1234\n.*`)
 }
 
+func (s *CommandSuite) TestCheckOldKeepstoreConfigFile(c *check.C) {
+	f, err := ioutil.TempFile("", "")
+	c.Assert(err, check.IsNil)
+	defer os.Remove(f.Name())
+
+	io.WriteString(f, "Debug: true\n")
+
+	var stdout, stderr bytes.Buffer
+	in := `
+Clusters:
+ z1234:
+  SystemLogs:
+    LogLevel: info
+`
+	code := CheckCommand.RunCommand("arvados config-check", []string{"-config", "-", "-legacy-keepstore-config", f.Name()}, bytes.NewBufferString(in), &stdout, &stderr)
+	c.Check(code, check.Equals, 1)
+	c.Check(stdout.String(), check.Matches, `(?ms).*\n\- +.*LogLevel: info\n\+ +LogLevel: debug\n.*`)
+	c.Check(stderr.String(), check.Matches, `.*you should remove the legacy keepstore config file.*\n`)
+}
+
 func (s *CommandSuite) TestCheckUnknownKey(c *check.C) {
 	var stdout, stderr bytes.Buffer
 	in := `
@@ -80,10 +103,10 @@ Clusters:
 	code := CheckCommand.RunCommand("arvados config-check", []string{"-config", "-"}, bytes.NewBufferString(in), &stdout, &stderr)
 	c.Log(stderr.String())
 	c.Check(code, check.Equals, 1)
-	c.Check(stderr.String(), check.Matches, `(?ms).*deprecated or unknown config entry: Clusters.z1234.Bogus1\n.*`)
-	c.Check(stderr.String(), check.Matches, `(?ms).*deprecated or unknown config entry: Clusters.z1234.BogusSection\n.*`)
-	c.Check(stderr.String(), check.Matches, `(?ms).*deprecated or unknown config entry: Clusters.z1234.API.Bogus3\n.*`)
-	c.Check(stderr.String(), check.Matches, `(?ms).*unexpected object in config entry: Clusters.z1234.PostgreSQL.ConnectionPool\n.*`)
+	c.Check(stderr.String(), check.Matches, `(?ms).*deprecated or unknown config entry: Clusters.z1234.Bogus1"\n.*`)
+	c.Check(stderr.String(), check.Matches, `(?ms).*deprecated or unknown config entry: Clusters.z1234.BogusSection"\n.*`)
+	c.Check(stderr.String(), check.Matches, `(?ms).*deprecated or unknown config entry: Clusters.z1234.API.Bogus3"\n.*`)
+	c.Check(stderr.String(), check.Matches, `(?ms).*unexpected object in config entry: Clusters.z1234.PostgreSQL.ConnectionPool"\n.*`)
 }
 
 func (s *CommandSuite) TestDumpFormatting(c *check.C) {
diff --git a/lib/config/deprecated.go b/lib/config/deprecated.go
index 8ffa2a583..93e3b805c 100644
--- a/lib/config/deprecated.go
+++ b/lib/config/deprecated.go
@@ -6,6 +6,7 @@ package config
 
 import (
 	"fmt"
+	"io/ioutil"
 	"os"
 	"strings"
 
@@ -47,9 +48,9 @@ type systemServiceInstance struct {
 	Insecure bool
 }
 
-func applyDeprecatedConfig(cfg *arvados.Config, configdata []byte, log logger) error {
+func (ldr *Loader) applyDeprecatedConfig(cfg *arvados.Config) error {
 	var dc deprecatedConfig
-	err := yaml.Unmarshal(configdata, &dc)
+	err := yaml.Unmarshal(ldr.configdata, &dc)
 	if err != nil {
 		return err
 	}
@@ -65,8 +66,8 @@ func applyDeprecatedConfig(cfg *arvados.Config, configdata []byte, log logger) e
 		for name, np := range dcluster.NodeProfiles {
 			if name == "*" || name == os.Getenv("ARVADOS_NODE_PROFILE") || name == hostname {
 				name = "localhost"
-			} else if log != nil {
-				log.Warnf("overriding Clusters.%s.Services using Clusters.%s.NodeProfiles.%s (guessing %q is a hostname)", id, id, name, name)
+			} else if ldr.Logger != nil {
+				ldr.Logger.Warnf("overriding Clusters.%s.Services using Clusters.%s.NodeProfiles.%s (guessing %q is a hostname)", id, id, name, name)
 			}
 			applyDeprecatedNodeProfile(name, np.RailsAPI, &cluster.Services.RailsAPI)
 			applyDeprecatedNodeProfile(name, np.Controller, &cluster.Services.Controller)
@@ -100,3 +101,43 @@ func applyDeprecatedNodeProfile(hostname string, ssi systemServiceInstance, svc
 	}
 	svc.InternalURLs[arvados.URL{Scheme: scheme, Host: host}] = arvados.ServiceInstance{}
 }
+
+type oldKeepstoreConfig struct {
+	Debug *bool
+}
+
+// update config using values from an old-style keepstore config file.
+func (ldr *Loader) loadOldKeepstoreConfig(cfg *arvados.Config) error {
+	path := ldr.KeepstorePath
+	if path == "" {
+		return nil
+	}
+	buf, err := ioutil.ReadFile(path)
+	if os.IsNotExist(err) && path == defaultKeepstoreConfigPath {
+		return nil
+	} else if err != nil {
+		return err
+	} else {
+		ldr.Logger.Warnf("you should remove the legacy keepstore config file (%s) after migrating all config keys to the cluster configuration file (%s)", path, ldr.Path)
+	}
+	cluster, err := cfg.GetCluster("")
+	if err != nil {
+		return err
+	}
+
+	var oc oldKeepstoreConfig
+	err = yaml.Unmarshal(buf, &oc)
+	if err != nil {
+		return fmt.Errorf("%s: %s", path, err)
+	}
+
+	if v := oc.Debug; v == nil {
+	} else if *v && cluster.SystemLogs.LogLevel != "debug" {
+		cluster.SystemLogs.LogLevel = "debug"
+	} else if !*v && cluster.SystemLogs.LogLevel != "info" {
+		cluster.SystemLogs.LogLevel = "info"
+	}
+
+	cfg.Clusters[cluster.ClusterID] = *cluster
+	return nil
+}
diff --git a/lib/config/export_test.go b/lib/config/export_test.go
index 581e54cdc..9e745c802 100644
--- a/lib/config/export_test.go
+++ b/lib/config/export_test.go
@@ -19,7 +19,7 @@ type ExportSuite struct{}
 
 func (s *ExportSuite) TestExport(c *check.C) {
 	confdata := bytes.Replace(DefaultYAML, []byte("SAMPLE"), []byte("testkey"), -1)
-	cfg, err := Load(bytes.NewBuffer(confdata), ctxlog.TestLogger(c))
+	cfg, err := NewLoader(bytes.NewBuffer(confdata), ctxlog.TestLogger(c)).Load()
 	c.Assert(err, check.IsNil)
 	cluster := cfg.Clusters["xxxxx"]
 	cluster.ManagementToken = "abcdefg"
diff --git a/lib/config/load.go b/lib/config/load.go
index a0c769537..a3c8aed05 100644
--- a/lib/config/load.go
+++ b/lib/config/load.go
@@ -8,6 +8,7 @@ import (
 	"bytes"
 	"encoding/json"
 	"errors"
+	"flag"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -17,37 +18,64 @@ import (
 	"git.curoverse.com/arvados.git/sdk/go/arvados"
 	"github.com/ghodss/yaml"
 	"github.com/imdario/mergo"
+	"github.com/sirupsen/logrus"
 )
 
-type logger interface {
-	Warnf(string, ...interface{})
+const (
+	defaultKeepstoreConfigPath = "/etc/arvados/keepstore.yml"
+)
+
+type Loader struct {
+	Stdin          io.Reader
+	Logger         logrus.FieldLogger
+	SkipDeprecated bool
+
+	Path          string
+	KeepstorePath string
+
+	configdata []byte
 }
 
-func loadFileOrStdin(path string, stdin io.Reader, log logger) (*arvados.Config, error) {
-	if path == "-" {
-		return load(stdin, log, true)
-	} else {
-		return LoadFile(path, log)
-	}
+func NewLoader(stdin io.Reader, logger logrus.FieldLogger) *Loader {
+	ldr := &Loader{Stdin: stdin, Logger: logger}
+	ldr.SetupFlags(flag.NewFlagSet("", flag.ContinueOnError))
+	ldr.Path = "-"
+	return ldr
 }
 
-func LoadFile(path string, log logger) (*arvados.Config, error) {
+// SetupFlags configures a flagset so arguments like -config X can be
+// used to change the loader's Path fields.
+//
+//	ldr := NewLoader(os.Stdin, logrus.New())
+//	flagset := flag.NewFlagSet("", flag.ContinueOnError)
+//	ldr.SetupFlags(flagset)
+//	// ldr.Path == "/etc/arvados/config.yml"
+//	flagset.Parse([]string{"-config", "/tmp/c.yaml"})
+//	// ldr.Path == "/tmp/c.yaml"
+func (ldr *Loader) SetupFlags(flagset *flag.FlagSet) {
+	flagset.StringVar(&ldr.Path, "config", arvados.DefaultConfigFile, "Site configuration `file`")
+	flagset.StringVar(&ldr.KeepstorePath, "legacy-keepstore-config", defaultKeepstoreConfigPath, "Legacy keepstore configuration `file`")
+}
+
+func (ldr *Loader) loadBytes(path string) ([]byte, error) {
+	if path == "-" {
+		return ioutil.ReadAll(ldr.Stdin)
+	}
 	f, err := os.Open(path)
 	if err != nil {
 		return nil, err
 	}
 	defer f.Close()
-	return Load(f, log)
-}
-
-func Load(rdr io.Reader, log logger) (*arvados.Config, error) {
-	return load(rdr, log, true)
+	return ioutil.ReadAll(f)
 }
 
-func load(rdr io.Reader, log logger, useDeprecated bool) (*arvados.Config, error) {
-	buf, err := ioutil.ReadAll(rdr)
-	if err != nil {
-		return nil, err
+func (ldr *Loader) Load() (*arvados.Config, error) {
+	if ldr.configdata == nil {
+		buf, err := ldr.loadBytes(ldr.Path)
+		if err != nil {
+			return nil, err
+		}
+		ldr.configdata = buf
 	}
 
 	// Load the config into a dummy map to get the cluster ID
@@ -57,7 +85,7 @@ func load(rdr io.Reader, log logger, useDeprecated bool) (*arvados.Config, error
 	var dummy struct {
 		Clusters map[string]struct{}
 	}
-	err = yaml.Unmarshal(buf, &dummy)
+	err := yaml.Unmarshal(ldr.configdata, &dummy)
 	if err != nil {
 		return nil, err
 	}
@@ -82,11 +110,11 @@ func load(rdr io.Reader, log logger, useDeprecated bool) (*arvados.Config, error
 		}
 	}
 	var src map[string]interface{}
-	err = yaml.Unmarshal(buf, &src)
+	err = yaml.Unmarshal(ldr.configdata, &src)
 	if err != nil {
 		return nil, fmt.Errorf("loading config data: %s", err)
 	}
-	logExtraKeys(log, merged, src, "")
+	ldr.logExtraKeys(merged, src, "")
 	removeSampleKeys(merged)
 	err = mergo.Merge(&merged, src, mergo.WithOverride)
 	if err != nil {
@@ -109,11 +137,18 @@ func load(rdr io.Reader, log logger, useDeprecated bool) (*arvados.Config, error
 		return nil, fmt.Errorf("transcoding config data: %s", err)
 	}
 
-	if useDeprecated {
-		err = applyDeprecatedConfig(&cfg, buf, log)
+	if !ldr.SkipDeprecated {
+		err = ldr.applyDeprecatedConfig(&cfg)
 		if err != nil {
 			return nil, err
 		}
+		for _, err := range []error{
+			ldr.loadOldKeepstoreConfig(&cfg),
+		} {
+			if err != nil {
+				return nil, err
+			}
+		}
 	}
 
 	// Check for known mistakes
@@ -147,8 +182,8 @@ func removeSampleKeys(m map[string]interface{}) {
 	}
 }
 
-func logExtraKeys(log logger, expected, supplied map[string]interface{}, prefix string) {
-	if log == nil {
+func (ldr *Loader) logExtraKeys(expected, supplied map[string]interface{}, prefix string) {
+	if ldr.Logger == nil {
 		return
 	}
 	allowed := map[string]interface{}{}
@@ -160,7 +195,7 @@ func logExtraKeys(log logger, expected, supplied map[string]interface{}, prefix
 		if !ok && expected["SAMPLE"] != nil {
 			vexp = expected["SAMPLE"]
 		} else if !ok {
-			log.Warnf("deprecated or unknown config entry: %s%s", prefix, k)
+			ldr.Logger.Warnf("deprecated or unknown config entry: %s%s", prefix, k)
 			continue
 		}
 		if vsupp, ok := vsupp.(map[string]interface{}); !ok {
@@ -168,9 +203,9 @@ func logExtraKeys(log logger, expected, supplied map[string]interface{}, prefix
 			// will be caught elsewhere; see TestBadType.
 			continue
 		} else if vexp, ok := vexp.(map[string]interface{}); !ok {
-			log.Warnf("unexpected object in config entry: %s%s", prefix, k)
+			ldr.Logger.Warnf("unexpected object in config entry: %s%s", prefix, k)
 		} else {
-			logExtraKeys(log, vexp, vsupp, prefix+k+".")
+			ldr.logExtraKeys(vexp, vsupp, prefix+k+".")
 		}
 	}
 }
diff --git a/lib/config/load_test.go b/lib/config/load_test.go
index 6b014476b..0ef149dd8 100644
--- a/lib/config/load_test.go
+++ b/lib/config/load_test.go
@@ -29,13 +29,13 @@ var _ = check.Suite(&LoadSuite{})
 type LoadSuite struct{}
 
 func (s *LoadSuite) TestEmpty(c *check.C) {
-	cfg, err := Load(&bytes.Buffer{}, ctxlog.TestLogger(c))
+	cfg, err := NewLoader(&bytes.Buffer{}, ctxlog.TestLogger(c)).Load()
 	c.Check(cfg, check.IsNil)
 	c.Assert(err, check.ErrorMatches, `config does not define any clusters`)
 }
 
 func (s *LoadSuite) TestNoConfigs(c *check.C) {
-	cfg, err := Load(bytes.NewBufferString(`Clusters: {"z1111": {}}`), ctxlog.TestLogger(c))
+	cfg, err := NewLoader(bytes.NewBufferString(`Clusters: {"z1111": {}}`), ctxlog.TestLogger(c)).Load()
 	c.Assert(err, check.IsNil)
 	c.Assert(cfg.Clusters, check.HasLen, 1)
 	cc, err := cfg.GetCluster("z1111")
@@ -50,7 +50,7 @@ func (s *LoadSuite) TestSampleKeys(c *check.C) {
 		`{"Clusters":{"z1111":{}}}`,
 		`{"Clusters":{"z1111":{"InstanceTypes":{"Foo":{"RAM": "12345M"}}}}}`,
 	} {
-		cfg, err := Load(bytes.NewBufferString(yaml), ctxlog.TestLogger(c))
+		cfg, err := NewLoader(bytes.NewBufferString(yaml), ctxlog.TestLogger(c)).Load()
 		c.Assert(err, check.IsNil)
 		cc, err := cfg.GetCluster("z1111")
 		_, hasSample := cc.InstanceTypes["SAMPLE"]
@@ -63,7 +63,7 @@ func (s *LoadSuite) TestSampleKeys(c *check.C) {
 }
 
 func (s *LoadSuite) TestMultipleClusters(c *check.C) {
-	cfg, err := Load(bytes.NewBufferString(`{"Clusters":{"z1111":{},"z2222":{}}}`), ctxlog.TestLogger(c))
+	cfg, err := NewLoader(bytes.NewBufferString(`{"Clusters":{"z1111":{},"z2222":{}}}`), ctxlog.TestLogger(c)).Load()
 	c.Assert(err, check.IsNil)
 	c1, err := cfg.GetCluster("z1111")
 	c.Assert(err, check.IsNil)
@@ -77,7 +77,7 @@ func (s *LoadSuite) TestDeprecatedOrUnknownWarning(c *check.C) {
 	var logbuf bytes.Buffer
 	logger := logrus.New()
 	logger.Out = &logbuf
-	_, err := Load(bytes.NewBufferString(`
+	_, err := NewLoader(bytes.NewBufferString(`
 Clusters:
   zzzzz:
     postgresql: {}
@@ -88,7 +88,7 @@ Clusters:
         Host: z2222.arvadosapi.com
         Proxy: true
         BadKey: badValue
-`), logger)
+`), logger).Load()
 	c.Assert(err, check.IsNil)
 	logs := strings.Split(strings.TrimSuffix(logbuf.String(), "\n"), "\n")
 	for _, log := range logs {
@@ -103,7 +103,9 @@ func (s *LoadSuite) TestNoUnrecognizedKeysInDefaultConfig(c *check.C) {
 	logger.Out = &logbuf
 	var supplied map[string]interface{}
 	yaml.Unmarshal(DefaultYAML, &supplied)
-	cfg, err := Load(bytes.NewBuffer(DefaultYAML), logger)
+
+	loader := NewLoader(bytes.NewBuffer(DefaultYAML), logger)
+	cfg, err := loader.Load()
 	c.Assert(err, check.IsNil)
 	var loaded map[string]interface{}
 	buf, err := yaml.Marshal(cfg)
@@ -111,7 +113,7 @@ func (s *LoadSuite) TestNoUnrecognizedKeysInDefaultConfig(c *check.C) {
 	err = yaml.Unmarshal(buf, &loaded)
 	c.Assert(err, check.IsNil)
 
-	logExtraKeys(logger, loaded, supplied, "")
+	loader.logExtraKeys(loaded, supplied, "")
 	c.Check(logbuf.String(), check.Equals, "")
 }
 
@@ -119,25 +121,25 @@ func (s *LoadSuite) TestNoWarningsForDumpedConfig(c *check.C) {
 	var logbuf bytes.Buffer
 	logger := logrus.New()
 	logger.Out = &logbuf
-	cfg, err := Load(bytes.NewBufferString(`{"Clusters":{"zzzzz":{}}}`), logger)
+	cfg, err := NewLoader(bytes.NewBufferString(`{"Clusters":{"zzzzz":{}}}`), logger).Load()
 	c.Assert(err, check.IsNil)
 	yaml, err := yaml.Marshal(cfg)
 	c.Assert(err, check.IsNil)
-	cfgDumped, err := Load(bytes.NewBuffer(yaml), logger)
+	cfgDumped, err := NewLoader(bytes.NewBuffer(yaml), logger).Load()
 	c.Assert(err, check.IsNil)
 	c.Check(cfg, check.DeepEquals, cfgDumped)
 	c.Check(logbuf.String(), check.Equals, "")
 }
 
 func (s *LoadSuite) TestPostgreSQLKeyConflict(c *check.C) {
-	_, err := Load(bytes.NewBufferString(`
+	_, err := NewLoader(bytes.NewBufferString(`
 Clusters:
  zzzzz:
   postgresql:
    connection:
      DBName: dbname
      Host: host
-`), ctxlog.TestLogger(c))
+`), ctxlog.TestLogger(c)).Load()
 	c.Check(err, check.ErrorMatches, `Clusters.zzzzz.PostgreSQL.Connection: multiple entries for "(dbname|host)".*`)
 }
 
@@ -169,7 +171,7 @@ Clusters:
 `,
 	} {
 		c.Log(data)
-		v, err := Load(bytes.NewBufferString(data), ctxlog.TestLogger(c))
+		v, err := NewLoader(bytes.NewBufferString(data), ctxlog.TestLogger(c)).Load()
 		if v != nil {
 			c.Logf("%#v", v.Clusters["zzzzz"].PostgreSQL.ConnectionPool)
 		}
@@ -210,9 +212,9 @@ Clusters:
 }
 
 func (s *LoadSuite) checkEquivalent(c *check.C, goty, expectedy string) {
-	got, err := Load(bytes.NewBufferString(goty), ctxlog.TestLogger(c))
+	got, err := NewLoader(bytes.NewBufferString(goty), ctxlog.TestLogger(c)).Load()
 	c.Assert(err, check.IsNil)
-	expected, err := Load(bytes.NewBufferString(expectedy), ctxlog.TestLogger(c))
+	expected, err := NewLoader(bytes.NewBufferString(expectedy), ctxlog.TestLogger(c)).Load()
 	c.Assert(err, check.IsNil)
 	if !c.Check(got, check.DeepEquals, expected) {
 		cmd := exec.Command("diff", "-u", "--label", "expected", "--label", "got", "/dev/fd/3", "/dev/fd/4")
diff --git a/lib/service/cmd.go b/lib/service/cmd.go
index 603f48890..b6737bc55 100644
--- a/lib/service/cmd.go
+++ b/lib/service/cmd.go
@@ -10,7 +10,6 @@ import (
 	"flag"
 	"fmt"
 	"io"
-	"io/ioutil"
 	"net"
 	"net/http"
 	"net/url"
@@ -62,20 +61,25 @@ func (c *command) RunCommand(prog string, args []string, stdin io.Reader, stdout
 			log.WithError(err).Info("exiting")
 		}
 	}()
+
 	flags := flag.NewFlagSet("", flag.ContinueOnError)
 	flags.SetOutput(stderr)
-	configFile := flags.String("config", arvados.DefaultConfigFile, "Site configuration `file`")
+
+	loader := config.NewLoader(stdin, log)
+	loader.SetupFlags(flags)
+	versionFlag := flags.Bool("version", false, "Write version information to stdout and exit 0")
+
 	err = flags.Parse(args)
 	if err == flag.ErrHelp {
 		err = nil
 		return 0
 	} else if err != nil {
 		return 2
+	} else if *versionFlag {
+		return cmd.Version.RunCommand(prog, args, stdin, stdout, stderr)
 	}
-	// Logged warnings are discarded for now: the config template
-	// is incomplete, which causes extra warnings about keys that
-	// are really OK.
-	cfg, err := config.LoadFile(*configFile, ctxlog.New(ioutil.Discard, "json", "error"))
+
+	cfg, err := loader.Load()
 	if err != nil {
 		return 1
 	}

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list