15003: Add config-check command.
authorTom Clegg <tclegg@veritasgenetics.com>
Tue, 23 Apr 2019 20:28:38 +0000 (16:28 -0400)
committerTom Clegg <tclegg@veritasgenetics.com>
Tue, 23 Apr 2019 20:28:38 +0000 (16:28 -0400)
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg@veritasgenetics.com>

cmd/arvados-server/cmd.go
lib/config/cmd.go
lib/config/cmd_test.go
lib/config/deprecated.go
lib/config/deprecated_test.go
lib/config/load.go

index cad540c91b4123eb9534dd03fac4b577ca7d2bd1..983159382297dab0a5d95fbf1f35f440fc015720 100644 (file)
@@ -20,9 +20,10 @@ var (
                "-version":  cmd.Version(version),
                "--version": cmd.Version(version),
 
+               "config-check":   config.CheckCommand,
+               "config-dump":    config.DumpCommand,
                "controller":     controller.Command,
                "dispatch-cloud": dispatchcloud.Command,
-               "dump-config":    config.DumpCommand,
        })
 )
 
index aa3e3bca1267f18f65f5cfe8436c0bf879927709..e3419cee54e1d64689ddab47b2495203b8c6763c 100644 (file)
@@ -5,8 +5,12 @@
 package config
 
 import (
+       "bytes"
        "fmt"
        "io"
+       "io/ioutil"
+       "os"
+       "os/exec"
 
        "git.curoverse.com/arvados.git/lib/cmd"
        "git.curoverse.com/arvados.git/sdk/go/ctxlog"
@@ -43,3 +47,53 @@ func (dumpCommand) RunCommand(prog string, args []string, stdin io.Reader, stdou
        }
        return 0
 }
+
+var CheckCommand cmd.Handler = checkCommand{}
+
+type checkCommand struct{}
+
+func (checkCommand) 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)
+               }
+       }()
+       if len(args) != 0 {
+               err = fmt.Errorf("usage: %s <config-src.yaml && echo 'no changes needed'", prog)
+               return 2
+       }
+       log := ctxlog.New(stderr, "text", "info")
+       buf, err := ioutil.ReadAll(stdin)
+       if err != nil {
+               return 1
+       }
+       withoutDepr, err := load(bytes.NewBuffer(buf), log, false)
+       if err != nil {
+               return 1
+       }
+       withDepr, err := load(bytes.NewBuffer(buf), log, true)
+       if err != nil {
+               return 1
+       }
+       cmd := exec.Command("diff", "-u", "--label", "without-deprecated-configs", "--label", "relying-on-deprecated-configs", "/dev/fd/3", "/dev/fd/4")
+       for _, obj := range []interface{}{withoutDepr, withDepr} {
+               y, _ := yaml.Marshal(obj)
+               pr, pw, err := os.Pipe()
+               if err != nil {
+                       return 1
+               }
+               defer pr.Close()
+               go func() {
+                       io.Copy(pw, bytes.NewBuffer(y))
+                       pw.Close()
+               }()
+               cmd.ExtraFiles = append(cmd.ExtraFiles, pr)
+       }
+       diff, err := cmd.CombinedOutput()
+       if err == nil {
+               return 0
+       }
+       _, err = stdout.Write(diff)
+       return 1
+}
index 39dcb4fe6bbc149ab9397e1ca6b2a6e48e81cb63..925bbe6483facc9c7791c598bdf4fbe5983eb12c 100644 (file)
@@ -16,29 +16,43 @@ type CommandSuite struct{}
 
 func (s *CommandSuite) TestBadArg(c *check.C) {
        var stderr bytes.Buffer
-       code := DumpCommand.RunCommand("arvados dump-config", []string{"-badarg"}, bytes.NewBuffer(nil), bytes.NewBuffer(nil), &stderr)
+       code := DumpCommand.RunCommand("arvados config-dump", []string{"-badarg"}, bytes.NewBuffer(nil), bytes.NewBuffer(nil), &stderr)
        c.Check(code, check.Equals, 2)
        c.Check(stderr.String(), check.Matches, `(?ms)usage: .*`)
 }
 
 func (s *CommandSuite) TestEmptyInput(c *check.C) {
        var stdout, stderr bytes.Buffer
-       code := DumpCommand.RunCommand("arvados dump-config", nil, &bytes.Buffer{}, &stdout, &stderr)
+       code := DumpCommand.RunCommand("arvados config-dump", nil, &bytes.Buffer{}, &stdout, &stderr)
        c.Check(code, check.Equals, 1)
        c.Check(stderr.String(), check.Matches, `config does not define any clusters\n`)
 }
 
-func (s *CommandSuite) TestLogDeprecatedKeys(c *check.C) {
+func (s *CommandSuite) TestCheckNoDeprecatedKeys(c *check.C) {
        var stdout, stderr bytes.Buffer
        in := `
 Clusters:
  z1234:
-  RequestLimits:
+  API:
     MaxItemsPerResponse: 1234
 `
-       code := DumpCommand.RunCommand("arvados dump-config", nil, bytes.NewBufferString(in), &stdout, &stderr)
+       code := CheckCommand.RunCommand("arvados config-check", nil, bytes.NewBufferString(in), &stdout, &stderr)
        c.Check(code, check.Equals, 0)
-       c.Check(stderr.String(), check.Matches, `(?ms).*overriding Clusters.z1234.API.MaxItemsPerResponse .* = 1234.*`)
+       c.Check(stdout.String(), check.Equals, "")
+       c.Check(stderr.String(), check.Equals, "")
+}
+
+func (s *CommandSuite) TestCheckDeprecatedKeys(c *check.C) {
+       var stdout, stderr bytes.Buffer
+       in := `
+Clusters:
+ z1234:
+  RequestLimits:
+    MaxItemsPerResponse: 1234
+`
+       code := CheckCommand.RunCommand("arvados config-check", nil, bytes.NewBufferString(in), &stdout, &stderr)
+       c.Check(code, check.Equals, 1)
+       c.Check(stdout.String(), check.Matches, `(?ms).*API:\n\- +.*MaxItemsPerResponse: 1000\n\+ +MaxItemsPerResponse: 1234\n.*`)
 }
 
 func (s *CommandSuite) TestUnknownKey(c *check.C) {
@@ -49,7 +63,7 @@ Clusters:
   UnknownKey: foobar
   ManagementToken: secret
 `
-       code := DumpCommand.RunCommand("arvados dump-config", nil, bytes.NewBufferString(in), &stdout, &stderr)
+       code := DumpCommand.RunCommand("arvados config-dump", nil, bytes.NewBufferString(in), &stdout, &stderr)
        c.Check(code, check.Equals, 0)
        c.Check(stderr.String(), check.Equals, "")
        c.Check(stdout.String(), check.Matches, `(?ms)Clusters:\n  z1234:\n.*`)
index 5d6796ea1623a02a8439f75c7aadff1516c9796a..2fc839cf281650bada1b16907229584309119d0f 100644 (file)
@@ -44,17 +44,18 @@ 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 {
-                               applyDeprecatedNodeProfile(hostname, np.RailsAPI, &cluster.Services.RailsAPI)
-                               applyDeprecatedNodeProfile(hostname, np.Controller, &cluster.Services.Controller)
-                               applyDeprecatedNodeProfile(hostname, np.DispatchCloud, &cluster.Services.DispatchCloud)
+                               name = "localhost"
+                       } else {
+                               log.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)
+                       applyDeprecatedNodeProfile(name, np.DispatchCloud, &cluster.Services.DispatchCloud)
                }
                if dst, n := &cluster.API.MaxItemsPerResponse, dcluster.RequestLimits.MaxItemsPerResponse; n != nil && *n != *dst {
-                       log.Warnf("overriding Clusters.%s.API.MaxItemsPerResponse with deprecated config RequestLimits.MultiClusterRequestConcurrency = %d", id, *n)
                        *dst = *n
                }
                if dst, n := &cluster.API.MaxRequestAmplification, dcluster.RequestLimits.MultiClusterRequestConcurrency; n != nil && *n != *dst {
-                       log.Warnf("overriding Clusters.%s.API.MaxRequestAmplification with deprecated config RequestLimits.MultiClusterRequestConcurrency = %d", id, *n)
                        *dst = *n
                }
                cfg.Clusters[id] = cluster
index bdce5b5428244c23e62ed34bc1cc975340429dcd..308b0cc359e92b1d79e2d60502d2069eed3d5102 100644 (file)
@@ -18,34 +18,36 @@ Clusters:
  z1111:
   NodeProfiles:
    "*":
-    arvados-dispatch-cloud:
-     listen: ":9006"
     arvados-controller:
      listen: ":9004"
    `+hostname+`:
     arvados-api-server:
      listen: ":8000"
+   dispatch-host:
+    arvados-dispatch-cloud:
+     listen: ":9006"
 `, `
 Clusters:
  z1111:
   Services:
    RailsAPI:
     InternalURLs:
-     "http://`+hostname+`:8000": {}
+     "http://localhost:8000": {}
    Controller:
     InternalURLs:
-     "http://`+hostname+`:9004": {}
+     "http://localhost:9004": {}
    DispatchCloud:
     InternalURLs:
-     "http://`+hostname+`:9006": {}
+     "http://dispatch-host:9006": {}
   NodeProfiles:
    "*":
-    arvados-dispatch-cloud:
-     listen: ":9006"
     arvados-controller:
      listen: ":9004"
    `+hostname+`:
     arvados-api-server:
      listen: ":8000"
+   dispatch-host:
+    arvados-dispatch-cloud:
+     listen: ":9006"
 `)
 }
index ec5ce636a0eb520115be4d8301f7ea4adfd95ebd..644dcfdb8c70d8138876e0adc79ba4b162cb8ae9 100644 (file)
@@ -33,6 +33,10 @@ func LoadFile(path string, log logger) (*arvados.Config, error) {
 }
 
 func Load(rdr io.Reader, log logger) (*arvados.Config, error) {
+       return load(rdr, log, true)
+}
+
+func load(rdr io.Reader, log logger, useDeprecated bool) (*arvados.Config, error) {
        buf, err := ioutil.ReadAll(rdr)
        if err != nil {
                return nil, err
@@ -95,10 +99,14 @@ func Load(rdr io.Reader, log logger) (*arvados.Config, error) {
                return nil, fmt.Errorf("transcoding config data: %s", err)
        }
 
-       err = applyDeprecatedConfig(&cfg, buf, log)
-       if err != nil {
-               return nil, err
+       if useDeprecated {
+               err = applyDeprecatedConfig(&cfg, buf, log)
+               if err != nil {
+                       return nil, err
+               }
        }
+
+       // Check for known mistakes
        for id, cc := range cfg.Clusters {
                err = checkKeyConflict(fmt.Sprintf("Clusters.%s.PostgreSQL.Connection", id), cc.PostgreSQL.Connection)
                if err != nil {