[ARVADOS] updated: 1.3.0-795-gdac1fdef3

Git user git at public.curoverse.com
Tue Apr 23 20:29:26 UTC 2019


Summary of changes:
 cmd/arvados-server/cmd.go     |  3 ++-
 lib/config/cmd.go             | 54 +++++++++++++++++++++++++++++++++++++++++++
 lib/config/cmd_test.go        | 28 ++++++++++++++++------
 lib/config/deprecated.go      | 11 +++++----
 lib/config/deprecated_test.go | 16 +++++++------
 lib/config/load.go            | 46 +++++++++++++++++++++++++++++++-----
 lib/config/load_test.go       | 12 ++++++++++
 7 files changed, 144 insertions(+), 26 deletions(-)

       via  dac1fdef3e46dfab1f16da27b4f45613dd9b71e6 (commit)
       via  f7c4c3fa0d3d177982a7ff665325ea082a872c99 (commit)
       via  771897a58ea5fb2980c040d29ef01232abecec81 (commit)
      from  86074c13f4441fa0804e30a1d68781175ba32e0d (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit dac1fdef3e46dfab1f16da27b4f45613dd9b71e6
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Tue Apr 23 16:28:38 2019 -0400

    15003: Add config-check command.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/cmd/arvados-server/cmd.go b/cmd/arvados-server/cmd.go
index cad540c91..983159382 100644
--- a/cmd/arvados-server/cmd.go
+++ b/cmd/arvados-server/cmd.go
@@ -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,
 	})
 )
 
diff --git a/lib/config/cmd.go b/lib/config/cmd.go
index aa3e3bca1..e3419cee5 100644
--- a/lib/config/cmd.go
+++ b/lib/config/cmd.go
@@ -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
+}
diff --git a/lib/config/cmd_test.go b/lib/config/cmd_test.go
index 39dcb4fe6..925bbe648 100644
--- a/lib/config/cmd_test.go
+++ b/lib/config/cmd_test.go
@@ -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.*`)
diff --git a/lib/config/deprecated.go b/lib/config/deprecated.go
index 5d6796ea1..2fc839cf2 100644
--- a/lib/config/deprecated.go
+++ b/lib/config/deprecated.go
@@ -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
diff --git a/lib/config/deprecated_test.go b/lib/config/deprecated_test.go
index bdce5b542..308b0cc35 100644
--- a/lib/config/deprecated_test.go
+++ b/lib/config/deprecated_test.go
@@ -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"
 `)
 }
diff --git a/lib/config/load.go b/lib/config/load.go
index ec5ce636a..644dcfdb8 100644
--- a/lib/config/load.go
+++ b/lib/config/load.go
@@ -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 {

commit f7c4c3fa0d3d177982a7ff665325ea082a872c99
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Tue Apr 23 14:15:42 2019 -0400

    15003: Error out if PostgreSQL connection map is ambiguous.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/lib/config/load.go b/lib/config/load.go
index 87ec741bf..ec5ce636a 100644
--- a/lib/config/load.go
+++ b/lib/config/load.go
@@ -12,6 +12,7 @@ import (
 	"io"
 	"io/ioutil"
 	"os"
+	"strings"
 
 	"git.curoverse.com/arvados.git/sdk/go/arvados"
 	"github.com/ghodss/yaml"
@@ -98,5 +99,23 @@ func Load(rdr io.Reader, log logger) (*arvados.Config, error) {
 	if err != nil {
 		return nil, err
 	}
+	for id, cc := range cfg.Clusters {
+		err = checkKeyConflict(fmt.Sprintf("Clusters.%s.PostgreSQL.Connection", id), cc.PostgreSQL.Connection)
+		if err != nil {
+			return nil, err
+		}
+	}
 	return &cfg, nil
 }
+
+func checkKeyConflict(label string, m map[string]string) error {
+	saw := map[string]bool{}
+	for k := range m {
+		k = strings.ToLower(k)
+		if saw[k] {
+			return fmt.Errorf("%s: multiple keys with tolower(key)==%q (use same case as defaults)", label, k)
+		}
+		saw[k] = true
+	}
+	return nil
+}
diff --git a/lib/config/load_test.go b/lib/config/load_test.go
index 315e021eb..fddf3660d 100644
--- a/lib/config/load_test.go
+++ b/lib/config/load_test.go
@@ -53,6 +53,18 @@ func (s *LoadSuite) TestMultipleClusters(c *check.C) {
 	c.Check(c2.ClusterID, check.Equals, "z2222")
 }
 
+func (s *LoadSuite) TestPostgreSQLKeyConflict(c *check.C) {
+	_, err := Load(bytes.NewBufferString(`
+Clusters:
+ zzzzz:
+  postgresql:
+   connection:
+     dbname: dbname
+     host: host
+`), ctxlog.TestLogger(c))
+	c.Assert(err, check.ErrorMatches, `Clusters.zzzzz.PostgreSQL.Connection: multiple keys with .*"(dbname|host)".*`)
+}
+
 func (s *LoadSuite) TestMovedKeys(c *check.C) {
 	s.checkEquivalent(c, `# config has old keys only
 Clusters:

commit 771897a58ea5fb2980c040d29ef01232abecec81
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Tue Apr 23 14:15:14 2019 -0400

    15003: Add missed error checks.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/lib/config/load.go b/lib/config/load.go
index e9ae5c6d1..87ec741bf 100644
--- a/lib/config/load.go
+++ b/lib/config/load.go
@@ -32,7 +32,6 @@ func LoadFile(path string, log logger) (*arvados.Config, error) {
 }
 
 func Load(rdr io.Reader, log logger) (*arvados.Config, error) {
-	var cfg arvados.Config
 	buf, err := ioutil.ReadAll(rdr)
 	if err != nil {
 		return nil, err
@@ -64,15 +63,23 @@ func Load(rdr io.Reader, log logger) (*arvados.Config, error) {
 		if err != nil {
 			return nil, fmt.Errorf("loading defaults for %s: %s", id, err)
 		}
-		mergo.Merge(&merged, src, mergo.WithOverride)
+		err = mergo.Merge(&merged, src, mergo.WithOverride)
+		if err != nil {
+			return nil, fmt.Errorf("merging defaults for %s: %s", id, err)
+		}
 	}
 	var src map[string]interface{}
 	err = yaml.Unmarshal(buf, &src)
 	if err != nil {
 		return nil, fmt.Errorf("loading config data: %s", err)
 	}
-	mergo.Merge(&merged, src, mergo.WithOverride)
+	err = mergo.Merge(&merged, src, mergo.WithOverride)
+	if err != nil {
+		return nil, fmt.Errorf("merging config data: %s", err)
+	}
 
+	// map[string]interface{} => json => arvados.Config
+	var cfg arvados.Config
 	var errEnc error
 	pr, pw := io.Pipe()
 	go func() {

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list