12260: Add -config arg. Drop non-resolvable hostname support.

Change check key to svctype+http://host:port/path.

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

diff --git a/sdk/go/arvados/config.go b/sdk/go/arvados/config.go
index d7bae38080f2a983e75f8364bf3b9c1aad8f6ece..84e66b39a88e62cf5d1819b3ce232b3043212ffe 100644 (file)
--- a/sdk/go/arvados/config.go
+++ b/sdk/go/arvados/config.go
@@ -3,20 +3,21 @@ package arvados
 import (
        "fmt"
        "os"
-       "strings"
 
        "git.curoverse.com/arvados.git/sdk/go/config"
 )
 
+const DefaultConfigFile = "/etc/arvados/config.yml"
+
 type Config struct {
        Clusters map[string]Cluster
 }
 
 // GetConfig returns the current system config, loading it from
-// /etc if needed.
-func GetConfig() (*Config, error) {
+// configFile if needed.
+func GetConfig(configFile string) (*Config, error) {
        var cfg Config
-       err := config.LoadFile(&cfg, "/etc/arvados/config.yml")
+       err := config.LoadFile(&cfg, configFile)
        return &cfg, err
 }
 
@@ -63,14 +64,8 @@ func (cc *Cluster) GetThisSystemNode() (*SystemNode, error) {
 // error is returned if the appropriate configuration can't be
 // determined (e.g., this does not appear to be a system node).
 func (cc *Cluster) GetSystemNode(node string) (*SystemNode, error) {
-       // Generally node is "a.b.ca", use the first of {"a.b.ca",
-       // "a.b", "a"} that has an entry in SystemNodes.
-       labels := strings.Split(node, ".")
-       for j := len(labels); j > 0; j-- {
-               hostpart := strings.Join(labels[:j], ".")
-               if cfg, ok := cc.SystemNodes[hostpart]; ok {
-                       return &cfg, nil
-               }
+       if cfg, ok := cc.SystemNodes[node]; ok {
+               return &cfg, nil
        }
        // If node is not listed, but "*" gives a default system node
        // config, use the default config.

diff --git a/sdk/go/health/aggregator.go b/sdk/go/health/aggregator.go
index 1ba1ee3c51a3c8e0059bb617fac8c08dc4a5e21d..e881db8827ba278e9c7409b6448ab57941317707 100644 (file)
--- a/sdk/go/health/aggregator.go
+++ b/sdk/go/health/aggregator.go
@@ -49,15 +49,6 @@ func (agg *Aggregator) ServeHTTP(resp http.ResponseWriter, req *http.Request) {
 
        resp.Header().Set("Content-Type", "application/json")
 
-       if agg.Config == nil {
-               cfg, err := arvados.GetConfig()
-               if err != nil {
-                       err = fmt.Errorf("arvados.GetConfig(): %s", err)
-                       sendErr(http.StatusInternalServerError, err)
-                       return
-               }
-               agg.Config = cfg
-       }
        cluster, err := agg.Config.GetCluster("")
        if err != nil {
                err = fmt.Errorf("arvados.GetCluster(): %s", err)
@@ -119,11 +110,20 @@ func (agg *Aggregator) ClusterHealth(cluster *arvados.Cluster) ClusterHealthResp
                        wg.Add(1)
                        go func(node string) {
                                defer wg.Done()
-                               pingResp := agg.ping(node, addr, cluster)
+                               var pingResp CheckResponse
+                               url, err := agg.pingURL(node, addr)
+                               if err != nil {
+                                       pingResp = CheckResponse{
+                                               Health: "ERROR",
+                                               Error:  err.Error(),
+                                       }
+                               } else {
+                                       pingResp = agg.ping(url, cluster)
+                               }
 
                                mtx.Lock()
                                defer mtx.Unlock()
-                               resp.Checks[node+"/"+svc+"/_health/ping"] = pingResp
+                               resp.Checks[svc+"+"+url] = pingResp
                                svHealth := resp.Services[svc]
                                if pingResp.OK() {
                                        svHealth.N++
@@ -148,7 +148,12 @@ func (agg *Aggregator) ClusterHealth(cluster *arvados.Cluster) ClusterHealthResp
        return resp
 }
 
-func (agg *Aggregator) ping(node, addr string, cluster *arvados.Cluster) (result CheckResponse) {
+func (agg *Aggregator) pingURL(node, addr string) (string, error) {
+       _, port, err := net.SplitHostPort(addr)
+       return "http://" + node + ":" + port + "/_health/ping", err
+}
+
+func (agg *Aggregator) ping(url string, cluster *arvados.Cluster) (result CheckResponse) {
        t0 := time.Now()
 
        var err error
@@ -159,11 +164,7 @@ func (agg *Aggregator) ping(node, addr string, cluster *arvados.Cluster) (result
                }
        }()
 
-       _, port, err := net.SplitHostPort(addr)
-       if err != nil {
-               return
-       }
-       req, err := http.NewRequest("GET", "http://"+node+":"+port+"/_health/ping", nil)
+       req, err := http.NewRequest("GET", url, nil)
        if err != nil {
                return
        }

diff --git a/sdk/go/health/aggregator_test.go b/sdk/go/health/aggregator_test.go
index b66671b1e16f1c0839f9aa1165b1b042f6eb7c91..048886af3019229925dbe36b1e76b70b5c88cfd5 100644 (file)
--- a/sdk/go/health/aggregator_test.go
+++ b/sdk/go/health/aggregator_test.go
@@ -2,6 +2,8 @@ package health
 
 import (
        "encoding/json"
+       "fmt"
+       "net"
        "net/http"
        "net/http/httptest"
        "strings"
@@ -102,12 +104,13 @@ func (*healthyHandler) ServeHTTP(resp http.ResponseWriter, req *http.Request) {
 func (s *AggregatorSuite) TestHealthy(c *check.C) {
        srv, listen := s.stubServer(&healthyHandler{})
        defer srv.Close()
+       _, port, _ := net.SplitHostPort(listen)
        s.handler.Config.Clusters["zzzzz"].SystemNodes["localhost"] = arvados.SystemNode{
                Keepstore: arvados.Keepstore{Listen: listen},
        }
        s.handler.ServeHTTP(s.resp, s.req)
        resp := s.checkOK(c)
-       ep := resp.Checks["localhost/keepstore/_health/ping"]
+       ep := resp.Checks[fmt.Sprintf("keepstore+http://localhost:%d/_health/ping", port)]
        c.Check(ep.Health, check.Equals, "OK")
        c.Check(ep.Status, check.Equals, 200)
 }
@@ -115,8 +118,10 @@ func (s *AggregatorSuite) TestHealthy(c *check.C) {
 func (s *AggregatorSuite) TestHealthyAndUnhealthy(c *check.C) {
        srvH, listenH := s.stubServer(&healthyHandler{})
        defer srvH.Close()
+       _, portH, _ := net.SplitHostPort(listenH)
        srvU, listenU := s.stubServer(&unhealthyHandler{})
        defer srvU.Close()
+       _, portU, _ := net.SplitHostPort(listenU)
        s.handler.Config.Clusters["zzzzz"].SystemNodes["localhost"] = arvados.SystemNode{
                Keepstore: arvados.Keepstore{Listen: listenH},
        }
@@ -125,10 +130,10 @@ func (s *AggregatorSuite) TestHealthyAndUnhealthy(c *check.C) {
        }
        s.handler.ServeHTTP(s.resp, s.req)
        resp := s.checkUnhealthy(c)
-       ep := resp.Checks["localhost/keepstore/_health/ping"]
+       ep := resp.Checks[fmt.Sprintf("keepstore+http://localhost:%d/_health/ping", portH)]
        c.Check(ep.Health, check.Equals, "OK")
        c.Check(ep.Status, check.Equals, 200)
-       ep = resp.Checks["127.0.0.1/keepstore/_health/ping"]
+       ep = resp.Checks[fmt.Sprintf("keepstore+http://127.0.0.1:%d/_health/ping", portU)]
        c.Check(ep.Health, check.Equals, "ERROR")
        c.Check(ep.Status, check.Equals, 200)
 }

diff --git a/services/health/main.go b/services/health/main.go
index 3e089fa5a229966dd79f410bbc3e56605f26b4bd..b6358deefcf8d333971435b2a9ede4021bc68f54 100644 (file)
--- a/services/health/main.go
+++ b/services/health/main.go
@@ -1,6 +1,7 @@
 package main
 
 import (
+       "flag"
        "net/http"
 
        "git.curoverse.com/arvados.git/sdk/go/arvados"
@@ -10,10 +11,13 @@ import (
 )
 
 func main() {
+       configFile := flag.String("config", arvados.DefaultConfigFile, "`path` to arvados configuration file")
+       flag.Parse()
+
        log.SetFormatter(&log.JSONFormatter{
                TimestampFormat: "2006-01-02T15:04:05.000000000Z07:00",
        })
-       cfg, err := arvados.GetConfig()
+       cfg, err := arvados.GetConfig(*configFile)
        if err != nil {
                log.Fatal(err)
        }
@@ -26,11 +30,18 @@ func main() {
                log.Fatal(err)
        }
 
+       log := log.WithField("Service", "Health")
        srv := &httpserver.Server{
                Addr: nodeCfg.Health.Listen,
                Server: http.Server{
                        Handler: &health.Aggregator{
                                Config: cfg,
+                               Log: func(req *http.Request, err error) {
+                                       log.WithField("RemoteAddr", req.RemoteAddr).
+                                               WithField("Path", req.URL.Path).
+                                               WithError(err).
+                                               Info("HTTP request")
+                               },
                        },
                },
        }