14807: Move "fallback to ARVADOS_*" logic out to lib/service.

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg@veritasgenetics.com>

diff --git a/lib/dispatchcloud/cmd.go b/lib/dispatchcloud/cmd.go
index f8143ac8ce5f50a06a4bdd39ed363bc6699c5563..22ceb8aebe787ae79c1274cc0c714bc39df04640 100644 (file)
--- a/lib/dispatchcloud/cmd.go
+++ b/lib/dispatchcloud/cmd.go
@@ -6,6 +6,7 @@ package dispatchcloud
 
 import (
        "context"
+       "fmt"
 
        "git.curoverse.com/arvados.git/lib/cmd"
        "git.curoverse.com/arvados.git/lib/service"
@@ -14,10 +15,15 @@ import (
 
 var Command cmd.Handler = service.Command(arvados.ServiceNameDispatchCloud, newHandler)
 
-func newHandler(ctx context.Context, cluster *arvados.Cluster, _ *arvados.NodeProfile, token string) service.Handler {
+func newHandler(ctx context.Context, cluster *arvados.Cluster, np *arvados.NodeProfile, token string) service.Handler {
+       ac, err := arvados.NewClientFromConfig(cluster)
+       if err != nil {
+               return service.ErrorHandler(ctx, cluster, np, fmt.Errorf("error initializing client from cluster config: %s", err))
+       }
        d := &dispatcher{
                Cluster:   cluster,
                Context:   ctx,
+               ArvClient: ac,
                AuthToken: token,
        }
        go d.Start()

diff --git a/lib/dispatchcloud/dispatcher.go b/lib/dispatchcloud/dispatcher.go
index 147b0c015ce00dc1bf7ba62bd3575335d6921ee2..71ff9c784e958fa7927cb3ca57214593d74eecd7 100644 (file)
--- a/lib/dispatchcloud/dispatcher.go
+++ b/lib/dispatchcloud/dispatcher.go
@@ -46,6 +46,7 @@ type pool interface {
 type dispatcher struct {
        Cluster       *arvados.Cluster
        Context       context.Context
+       ArvClient     *arvados.Client
        AuthToken     string
        InstanceSetID cloud.InstanceSetID
 
@@ -111,20 +112,15 @@ func (disp *dispatcher) setup() {
 func (disp *dispatcher) initialize() {
        disp.logger = ctxlog.FromContext(disp.Context)
 
-       arvClient, err := arvados.NewClientFromConfig(disp.Cluster)
-       if err != nil {
-               disp.logger.WithError(err).Warn("error initializing client from cluster config, falling back to ARVADOS_API_HOST(_INSECURE) environment variables")
-               arvClient = arvados.NewClientFromEnv()
-       }
-       arvClient.AuthToken = disp.AuthToken
+       disp.ArvClient.AuthToken = disp.AuthToken
 
        if disp.InstanceSetID == "" {
-               if strings.HasPrefix(arvClient.AuthToken, "v2/") {
-                       disp.InstanceSetID = cloud.InstanceSetID(strings.Split(arvClient.AuthToken, "/")[1])
+               if strings.HasPrefix(disp.AuthToken, "v2/") {
+                       disp.InstanceSetID = cloud.InstanceSetID(strings.Split(disp.AuthToken, "/")[1])
                } else {
                        // Use some other string unique to this token
                        // that doesn't reveal the token itself.
-                       disp.InstanceSetID = cloud.InstanceSetID(fmt.Sprintf("%x", md5.Sum([]byte(arvClient.AuthToken))))
+                       disp.InstanceSetID = cloud.InstanceSetID(fmt.Sprintf("%x", md5.Sum([]byte(disp.AuthToken))))
                }
        }
        disp.stop = make(chan struct{}, 1)
@@ -142,8 +138,8 @@ func (disp *dispatcher) initialize() {
        }
        disp.instanceSet = instanceSet
        disp.reg = prometheus.NewRegistry()
-       disp.pool = worker.NewPool(disp.logger, arvClient, disp.reg, disp.instanceSet, disp.newExecutor, disp.sshKey.PublicKey(), disp.Cluster)
-       disp.queue = container.NewQueue(disp.logger, disp.reg, disp.typeChooser, arvClient)
+       disp.pool = worker.NewPool(disp.logger, disp.ArvClient, disp.reg, disp.instanceSet, disp.newExecutor, disp.sshKey.PublicKey(), disp.Cluster)
+       disp.queue = container.NewQueue(disp.logger, disp.reg, disp.typeChooser, disp.ArvClient)
 
        if disp.Cluster.ManagementToken == "" {
                disp.httpHandler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

diff --git a/lib/dispatchcloud/dispatcher_test.go b/lib/dispatchcloud/dispatcher_test.go
index d7e841e7329d021e81c1fe7bcb1f6085a46dbdd2..00157b75c649226880898c802973e9cd03a82173 100644 (file)
--- a/lib/dispatchcloud/dispatcher_test.go
+++ b/lib/dispatchcloud/dispatcher_test.go
@@ -83,14 +83,17 @@ func (s *DispatcherSuite) SetUpTest(c *check.C) {
                        },
                },
                Services: arvados.Services{
-                       DispatchCloud: arvados.Service{InternalURLs: map[arvados.URL]arvados.ServiceInstance{
-                               arvados.URL{Scheme: "https", Host: os.Getenv("ARVADOS_API_HOST")}: {},
-                       }},
+                       Controller: arvados.Service{ExternalURL: arvados.URL{Scheme: "https", Host: os.Getenv("ARVADOS_API_HOST")}},
                },
        }
+
+       arvClient, err := arvados.NewClientFromConfig(s.cluster)
+       c.Check(err, check.IsNil)
+
        s.disp = &dispatcher{
                Cluster:   s.cluster,
                Context:   s.ctx,
+               ArvClient: arvClient,
                AuthToken: arvadostest.AdminToken,
        }
        // Test cases can modify s.cluster before calling

diff --git a/lib/service/cmd.go b/lib/service/cmd.go
index 2d3fb90259da90d3d42eab2747e5b2efa6967d91..e853da943222aa2182b01f41d12ebb3cbec5193a 100644 (file)
--- a/lib/service/cmd.go
+++ b/lib/service/cmd.go
@@ -11,6 +11,7 @@ import (
        "fmt"
        "io"
        "net/http"
+       "net/url"
        "os"
 
        "git.curoverse.com/arvados.git/lib/cmd"
@@ -95,16 +96,24 @@ func (c *command) RunCommand(prog string, args []string, stdin io.Reader, stdout
                return 1
        }
 
-       // Currently all components use SystemRootToken if configured,
-       // otherwise ARVADOS_API_TOKEN. In future, per-process tokens
-       // will be generated/obtained here.
-       token := cluster.SystemRootToken
-       if token == "" {
+       if cluster.SystemRootToken == "" {
                log.Warn("SystemRootToken missing from cluster config, falling back to ARVADOS_API_TOKEN environment variable")
-               token = os.Getenv("ARVADOS_API_TOKEN")
+               cluster.SystemRootToken = os.Getenv("ARVADOS_API_TOKEN")
+       }
+       if cluster.Services.Controller.ExternalURL.Host == "" {
+               log.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
+               }
        }
 
-       handler := c.newHandler(ctx, cluster, profile, token)
+       handler := c.newHandler(ctx, cluster, profile, cluster.SystemRootToken)
        if err = handler.CheckHealth(); err != nil {
                return 1
        }

diff --git a/lib/service/error.go b/lib/service/error.go
new file mode 100644 (file)
index 0000000..8955210
--- /dev/null
+++ b/lib/service/error.go
@@ -0,0 +1,38 @@
+// Copyright (C) The Arvados Authors. All rights reserved.
+//
+// SPDX-License-Identifier: Apache-2.0
+
+package service
+
+import (
+       "context"
+       "net/http"
+
+       "git.curoverse.com/arvados.git/sdk/go/arvados"
+       "git.curoverse.com/arvados.git/sdk/go/ctxlog"
+       "github.com/sirupsen/logrus"
+)
+
+// ErrorHandler returns a Handler that reports itself as unhealthy and
+// responds 500 to all requests.  ErrorHandler itself logs the given
+// error once, and the handler logs it again for each incoming
+// request.
+func ErrorHandler(ctx context.Context, _ *arvados.Cluster, _ *arvados.NodeProfile, err error) Handler {
+       logger := ctxlog.FromContext(ctx)
+       logger.WithError(err).Error("unhealthy service")
+       return errorHandler{err, logger}
+}
+
+type errorHandler struct {
+       err    error
+       logger logrus.FieldLogger
+}
+
+func (eh errorHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
+       eh.logger.WithError(eh.err).Error("unhealthy service")
+       http.Error(w, "", http.StatusInternalServerError)
+}
+
+func (eh errorHandler) CheckHealth() error {
+       return eh.err
+}