[ARVADOS] updated: 1.3.0-1657-g22cff6bb3

Git user git at public.curoverse.com
Fri Sep 27 18:16:22 UTC 2019


Summary of changes:
 lib/config/cmd_test.go                  |  6 ++++++
 lib/config/deprecated.go                | 28 ++++++++++++++++++++++++++++
 lib/config/deprecated_keepstore.go      |  4 ++--
 lib/config/deprecated_keepstore_test.go |  4 ++++
 lib/config/load.go                      |  1 +
 lib/config/load_test.go                 |  6 ++++++
 lib/service/cmd.go                      | 18 ------------------
 7 files changed, 47 insertions(+), 20 deletions(-)

       via  22cff6bb354d6980c33a0f471d5860f968e853a6 (commit)
      from  110580d079cdb0b0a773ecf1671c1f97f1736cc6 (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 22cff6bb354d6980c33a0f471d5860f968e853a6
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Fri Sep 27 14:12:22 2019 -0400

    13647: Check ARVADOS_* env vars when loading config.
    
    Previously they were only checked when starting up a service, so they
    weren't noticed by config-dump etc., and couldn't be used by the
    keepstore config migration when loading the keep_services endpoint.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/lib/config/cmd_test.go b/lib/config/cmd_test.go
index 019d5edd0..fb1cba38b 100644
--- a/lib/config/cmd_test.go
+++ b/lib/config/cmd_test.go
@@ -24,6 +24,12 @@ var (
 
 type CommandSuite struct{}
 
+func (s *CommandSuite) SetUpSuite(c *check.C) {
+	os.Unsetenv("ARVADOS_API_HOST")
+	os.Unsetenv("ARVADOS_API_HOST_INSECURE")
+	os.Unsetenv("ARVADOS_API_TOKEN")
+}
+
 func (s *CommandSuite) TestBadArg(c *check.C) {
 	var stderr bytes.Buffer
 	code := DumpCommand.RunCommand("arvados config-dump", []string{"-badarg"}, bytes.NewBuffer(nil), bytes.NewBuffer(nil), &stderr)
diff --git a/lib/config/deprecated.go b/lib/config/deprecated.go
index 0a030fb04..d0e61dbca 100644
--- a/lib/config/deprecated.go
+++ b/lib/config/deprecated.go
@@ -7,6 +7,7 @@ package config
 import (
 	"fmt"
 	"io/ioutil"
+	"net/url"
 	"os"
 	"strings"
 
@@ -474,3 +475,30 @@ func (ldr *Loader) loadOldGitHttpdConfig(cfg *arvados.Config) error {
 	cfg.Clusters[cluster.ClusterID] = *cluster
 	return nil
 }
+
+func (ldr *Loader) loadOldEnvironmentVariables(cfg *arvados.Config) error {
+	if os.Getenv("ARVADOS_API_TOKEN") == "" && os.Getenv("ARVADOS_API_HOST") == "" {
+		return nil
+	}
+	cluster, err := cfg.GetCluster("")
+	if err != nil {
+		return err
+	}
+	if tok := os.Getenv("ARVADOS_API_TOKEN"); tok != "" && cluster.SystemRootToken == "" {
+		ldr.Logger.Warn("SystemRootToken missing from cluster config, falling back to ARVADOS_API_TOKEN environment variable")
+		cluster.SystemRootToken = tok
+	}
+	if apihost := os.Getenv("ARVADOS_API_HOST"); apihost != "" && cluster.Services.Controller.ExternalURL.Host == "" {
+		ldr.Logger.Warn("Services.Controller.ExternalURL missing from cluster config, falling back to ARVADOS_API_HOST(_INSECURE) environment variables")
+		u, err := url.Parse("https://" + apihost)
+		if err != nil {
+			return fmt.Errorf("cannot parse ARVADOS_API_HOST: %s", err)
+		}
+		cluster.Services.Controller.ExternalURL = arvados.URL(*u)
+		if i := os.Getenv("ARVADOS_API_HOST_INSECURE"); i != "" && i != "0" {
+			cluster.TLS.Insecure = true
+		}
+	}
+	cfg.Clusters[cluster.ClusterID] = *cluster
+	return nil
+}
diff --git a/lib/config/deprecated_keepstore.go b/lib/config/deprecated_keepstore.go
index a7d41e985..29019296d 100644
--- a/lib/config/deprecated_keepstore.go
+++ b/lib/config/deprecated_keepstore.go
@@ -584,7 +584,7 @@ func keepServiceIsMe(ks arvados.KeepService, hostname string, listen string) boo
 // been warned about in loadOldKeepstoreConfig() -- i.e., unmigrated
 // keepstore hosts other than the present host, and obsolete content
 // in the keep_services table.
-func (ldr *Loader) checkPendingKeepstoreMigrations(cluster arvados.Cluster) error {
+func (ldr *Loader) checkPendingKeepstoreMigrations(cluster *arvados.Cluster) error {
 	if cluster.Services.Controller.ExternalURL.String() == "" {
 		ldr.Logger.Debug("Services.Controller.ExternalURL not configured -- skipping check for pending keepstore config migrations")
 		return nil
@@ -593,7 +593,7 @@ func (ldr *Loader) checkPendingKeepstoreMigrations(cluster arvados.Cluster) erro
 		ldr.Logger.Debug("(Loader).SkipAPICalls == true -- skipping check for pending keepstore config migrations")
 		return nil
 	}
-	client, err := arvados.NewClientFromConfig(&cluster)
+	client, err := arvados.NewClientFromConfig(cluster)
 	if err != nil {
 		return err
 	}
diff --git a/lib/config/deprecated_keepstore_test.go b/lib/config/deprecated_keepstore_test.go
index d1028e8bd..62f3f1e3f 100644
--- a/lib/config/deprecated_keepstore_test.go
+++ b/lib/config/deprecated_keepstore_test.go
@@ -30,6 +30,10 @@ type KeepstoreMigrationSuite struct {
 var _ = check.Suite(&KeepstoreMigrationSuite{})
 
 func (s *KeepstoreMigrationSuite) SetUpSuite(c *check.C) {
+	os.Setenv("ARVADOS_API_HOST", os.Getenv("ARVADOS_TEST_API_HOST"))
+	os.Setenv("ARVADOS_API_HOST_INSECURE", "1")
+	os.Setenv("ARVADOS_API_TOKEN", arvadostest.AdminToken)
+
 	// We don't need the keepstore servers, but we do need
 	// keep_services listings that point to localhost, rather than
 	// the apiserver fixtures that point to fictional hosts
diff --git a/lib/config/load.go b/lib/config/load.go
index d9ee97b51..61d80a3c5 100644
--- a/lib/config/load.go
+++ b/lib/config/load.go
@@ -245,6 +245,7 @@ func (ldr *Loader) Load() (*arvados.Config, error) {
 		// * no primary config was loaded, and this is the
 		// legacy config file for the current component
 		for _, err := range []error{
+			ldr.loadOldEnvironmentVariables(&cfg),
 			ldr.loadOldKeepstoreConfig(&cfg),
 			ldr.loadOldKeepWebConfig(&cfg),
 			ldr.loadOldCrunchDispatchSlurmConfig(&cfg),
diff --git a/lib/config/load_test.go b/lib/config/load_test.go
index 17e0af7ba..2e521ab09 100644
--- a/lib/config/load_test.go
+++ b/lib/config/load_test.go
@@ -46,6 +46,12 @@ func testLoader(c *check.C, configdata string, logdst io.Writer) *Loader {
 
 type LoadSuite struct{}
 
+func (s *LoadSuite) SetUpSuite(c *check.C) {
+	os.Unsetenv("ARVADOS_API_HOST")
+	os.Unsetenv("ARVADOS_API_HOST_INSECURE")
+	os.Unsetenv("ARVADOS_API_TOKEN")
+}
+
 func (s *LoadSuite) TestEmpty(c *check.C) {
 	cfg, err := testLoader(c, "", nil).Load()
 	c.Check(cfg, check.IsNil)
diff --git a/lib/service/cmd.go b/lib/service/cmd.go
index d5619b87b..ddff5f47a 100644
--- a/lib/service/cmd.go
+++ b/lib/service/cmd.go
@@ -12,7 +12,6 @@ import (
 	"io"
 	"net"
 	"net/http"
-	"net/url"
 	"os"
 	"strings"
 
@@ -110,23 +109,6 @@ func (c *command) RunCommand(prog string, args []string, stdin io.Reader, stdout
 	}
 	ctx = context.WithValue(ctx, contextKeyURL{}, listenURL)
 
-	if cluster.SystemRootToken == "" {
-		logger.Warn("SystemRootToken missing from cluster config, falling back to ARVADOS_API_TOKEN environment variable")
-		cluster.SystemRootToken = os.Getenv("ARVADOS_API_TOKEN")
-	}
-	if cluster.Services.Controller.ExternalURL.Host == "" {
-		logger.Warn("Services.Controller.ExternalURL missing from cluster config, falling back to ARVADOS_API_HOST(_INSECURE) environment variables")
-		u, err := url.Parse("https://" + os.Getenv("ARVADOS_API_HOST"))
-		if err != nil {
-			err = fmt.Errorf("ARVADOS_API_HOST: %s", err)
-			return 1
-		}
-		cluster.Services.Controller.ExternalURL = arvados.URL(*u)
-		if i := os.Getenv("ARVADOS_API_HOST_INSECURE"); i != "" && i != "0" {
-			cluster.TLS.Insecure = true
-		}
-	}
-
 	reg := prometheus.NewRegistry()
 	handler := c.newHandler(ctx, cluster, cluster.SystemRootToken, reg)
 	if err = handler.CheckHealth(); err != nil {

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list