14714: Tests use cluster config
authorEric Biagiotti <ebiagiotti@veritasgenetics.com>
Wed, 25 Sep 2019 17:51:07 +0000 (13:51 -0400)
committerEric Biagiotti <ebiagiotti@veritasgenetics.com>
Thu, 26 Sep 2019 18:18:47 +0000 (14:18 -0400)
Arvados-DCO-1.1-Signed-off-by: Eric Biagiotti <ebiagiotti@veritasgenetics.com>

services/keep-balance/balance_run_test.go
services/keep-balance/collection_test.go
services/keep-balance/integration_test.go
services/keep-balance/main_test.go [deleted file]
services/keep-balance/server.go

index db530bc4926de88502132730f35c816ec3cf92b6..1478e6e2effb042f20206853c489b4d162fae1a4 100644 (file)
@@ -5,6 +5,7 @@
 package main
 
 import (
+       "context"
        "encoding/json"
        "fmt"
        "io"
@@ -16,7 +17,9 @@ import (
        "sync"
        "time"
 
+       "git.curoverse.com/arvados.git/lib/config"
        "git.curoverse.com/arvados.git/sdk/go/arvados"
+       "git.curoverse.com/arvados.git/sdk/go/arvadostest"
        "github.com/sirupsen/logrus"
        check "gopkg.in/check.v1"
 )
@@ -303,7 +306,21 @@ func (s *stubServer) serveKeepstorePull() *reqTracker {
 
 type runSuite struct {
        stub   stubServer
-       config Config
+       config *arvados.Cluster
+       client *arvados.Client
+}
+
+func (s *runSuite) newServer(options *RunOptions) *Server {
+       srv := &Server{
+               Cluster:    s.config,
+               ArvClient:  s.client,
+               RunOptions: *options,
+               Metrics:    newMetrics(),
+               Logger:     options.Logger,
+               Dumper:     options.Dumper,
+       }
+       srv.init(context.Background())
+       return srv
 }
 
 // make a log.Logger that writes to the current test's c.Log().
@@ -330,14 +347,19 @@ func (s *runSuite) logger(c *check.C) *logrus.Logger {
 }
 
 func (s *runSuite) SetUpTest(c *check.C) {
-       s.config = Config{
-               Client: arvados.Client{
-                       AuthToken: "xyzzy",
-                       APIHost:   "zzzzz.arvadosapi.com",
-                       Client:    s.stub.Start()},
-               KeepServiceTypes: []string{"disk"},
-               RunPeriod:        arvados.Duration(time.Second),
-       }
+       cfg, err := config.NewLoader(nil, nil).Load()
+       c.Assert(err, check.Equals, nil)
+       s.config, err = cfg.GetCluster("")
+       c.Assert(err, check.Equals, nil)
+
+       s.config.Collections.BalancePeriod = arvados.Duration(time.Second)
+       arvadostest.SetServiceURL(&s.config.Services.Keepbalance, "http://localhost:/")
+
+       s.client = &arvados.Client{
+               AuthToken: "xyzzy",
+               APIHost:   "zzzzz.arvadosapi.com",
+               Client:    s.stub.Start()}
+
        s.stub.serveDiscoveryDoc()
        s.stub.logf = c.Logf
 }
@@ -359,35 +381,13 @@ func (s *runSuite) TestRefuseZeroCollections(c *check.C) {
        s.stub.serveKeepstoreIndexFoo4Bar1()
        trashReqs := s.stub.serveKeepstoreTrash()
        pullReqs := s.stub.serveKeepstorePull()
-       srv, err := NewServer(s.config, opts)
-       c.Assert(err, check.IsNil)
-       _, err = srv.Run()
+       srv := s.newServer(&opts)
+       _, err := srv.runOnce()
        c.Check(err, check.ErrorMatches, "received zero collections")
        c.Check(trashReqs.Count(), check.Equals, 4)
        c.Check(pullReqs.Count(), check.Equals, 0)
 }
 
-func (s *runSuite) TestServiceTypes(c *check.C) {
-       opts := RunOptions{
-               CommitPulls: true,
-               CommitTrash: true,
-               Logger:      s.logger(c),
-       }
-       s.config.KeepServiceTypes = []string{"unlisted-type"}
-       s.stub.serveCurrentUserAdmin()
-       s.stub.serveFooBarFileCollections()
-       s.stub.serveKeepServices(stubServices)
-       s.stub.serveKeepstoreMounts()
-       indexReqs := s.stub.serveKeepstoreIndexFoo4Bar1()
-       trashReqs := s.stub.serveKeepstoreTrash()
-       srv, err := NewServer(s.config, opts)
-       c.Assert(err, check.IsNil)
-       _, err = srv.Run()
-       c.Check(err, check.IsNil)
-       c.Check(indexReqs.Count(), check.Equals, 0)
-       c.Check(trashReqs.Count(), check.Equals, 0)
-}
-
 func (s *runSuite) TestRefuseNonAdmin(c *check.C) {
        opts := RunOptions{
                CommitPulls: true,
@@ -400,9 +400,8 @@ func (s *runSuite) TestRefuseNonAdmin(c *check.C) {
        s.stub.serveKeepstoreMounts()
        trashReqs := s.stub.serveKeepstoreTrash()
        pullReqs := s.stub.serveKeepstorePull()
-       srv, err := NewServer(s.config, opts)
-       c.Assert(err, check.IsNil)
-       _, err = srv.Run()
+       srv := s.newServer(&opts)
+       _, err := srv.runOnce()
        c.Check(err, check.ErrorMatches, "current user .* is not .* admin user")
        c.Check(trashReqs.Count(), check.Equals, 0)
        c.Check(pullReqs.Count(), check.Equals, 0)
@@ -421,9 +420,8 @@ func (s *runSuite) TestDetectSkippedCollections(c *check.C) {
        s.stub.serveKeepstoreIndexFoo4Bar1()
        trashReqs := s.stub.serveKeepstoreTrash()
        pullReqs := s.stub.serveKeepstorePull()
-       srv, err := NewServer(s.config, opts)
-       c.Assert(err, check.IsNil)
-       _, err = srv.Run()
+       srv := s.newServer(&opts)
+       _, err := srv.runOnce()
        c.Check(err, check.ErrorMatches, `Retrieved 2 collections with modtime <= .* but server now reports there are 3 collections.*`)
        c.Check(trashReqs.Count(), check.Equals, 4)
        c.Check(pullReqs.Count(), check.Equals, 0)
@@ -432,7 +430,7 @@ func (s *runSuite) TestDetectSkippedCollections(c *check.C) {
 func (s *runSuite) TestWriteLostBlocks(c *check.C) {
        lostf, err := ioutil.TempFile("", "keep-balance-lost-blocks-test-")
        c.Assert(err, check.IsNil)
-       s.config.LostBlocksFile = lostf.Name()
+       s.config.Collections.BlobMissingReport = lostf.Name()
        defer os.Remove(lostf.Name())
        opts := RunOptions{
                CommitPulls: true,
@@ -446,9 +444,9 @@ func (s *runSuite) TestWriteLostBlocks(c *check.C) {
        s.stub.serveKeepstoreIndexFoo1()
        s.stub.serveKeepstoreTrash()
        s.stub.serveKeepstorePull()
-       srv, err := NewServer(s.config, opts)
+       srv := s.newServer(&opts)
        c.Assert(err, check.IsNil)
-       _, err = srv.Run()
+       _, err = srv.runOnce()
        c.Check(err, check.IsNil)
        lost, err := ioutil.ReadFile(lostf.Name())
        c.Assert(err, check.IsNil)
@@ -468,9 +466,8 @@ func (s *runSuite) TestDryRun(c *check.C) {
        s.stub.serveKeepstoreIndexFoo4Bar1()
        trashReqs := s.stub.serveKeepstoreTrash()
        pullReqs := s.stub.serveKeepstorePull()
-       srv, err := NewServer(s.config, opts)
-       c.Assert(err, check.IsNil)
-       bal, err := srv.Run()
+       srv := s.newServer(&opts)
+       bal, err := srv.runOnce()
        c.Check(err, check.IsNil)
        for _, req := range collReqs.reqs {
                c.Check(req.Form.Get("include_trash"), check.Equals, "true")
@@ -486,10 +483,9 @@ func (s *runSuite) TestDryRun(c *check.C) {
 func (s *runSuite) TestCommit(c *check.C) {
        lostf, err := ioutil.TempFile("", "keep-balance-lost-blocks-test-")
        c.Assert(err, check.IsNil)
-       s.config.LostBlocksFile = lostf.Name()
+       s.config.Collections.BlobMissingReport = lostf.Name()
        defer os.Remove(lostf.Name())
 
-       s.config.Listen = ":"
        s.config.ManagementToken = "xyzzy"
        opts := RunOptions{
                CommitPulls: true,
@@ -504,9 +500,8 @@ func (s *runSuite) TestCommit(c *check.C) {
        s.stub.serveKeepstoreIndexFoo4Bar1()
        trashReqs := s.stub.serveKeepstoreTrash()
        pullReqs := s.stub.serveKeepstorePull()
-       srv, err := NewServer(s.config, opts)
-       c.Assert(err, check.IsNil)
-       bal, err := srv.Run()
+       srv := s.newServer(&opts)
+       bal, err := srv.runOnce()
        c.Check(err, check.IsNil)
        c.Check(trashReqs.Count(), check.Equals, 8)
        c.Check(pullReqs.Count(), check.Equals, 4)
@@ -529,7 +524,6 @@ func (s *runSuite) TestCommit(c *check.C) {
 }
 
 func (s *runSuite) TestRunForever(c *check.C) {
-       s.config.Listen = ":"
        s.config.ManagementToken = "xyzzy"
        opts := RunOptions{
                CommitPulls: true,
@@ -546,13 +540,12 @@ func (s *runSuite) TestRunForever(c *check.C) {
        pullReqs := s.stub.serveKeepstorePull()
 
        stop := make(chan interface{})
-       s.config.RunPeriod = arvados.Duration(time.Millisecond)
-       srv, err := NewServer(s.config, opts)
-       c.Assert(err, check.IsNil)
+       s.config.Collections.BalancePeriod = arvados.Duration(time.Millisecond)
+       srv := s.newServer(&opts)
 
        done := make(chan bool)
        go func() {
-               srv.RunForever(stop)
+               srv.runForever(stop)
                close(done)
        }()
 
@@ -571,13 +564,16 @@ func (s *runSuite) TestRunForever(c *check.C) {
 }
 
 func (s *runSuite) getMetrics(c *check.C, srv *Server) string {
-       resp, err := http.Get("http://" + srv.listening + "/metrics")
-       c.Assert(err, check.IsNil)
-       c.Check(resp.StatusCode, check.Equals, http.StatusUnauthorized)
+       req := httptest.NewRequest("GET", "/metrics", nil)
+       resp := httptest.NewRecorder()
+       srv.ServeHTTP(resp, req)
+       c.Check(resp.Code, check.Equals, http.StatusUnauthorized)
+
+       req = httptest.NewRequest("GET", "/metrics?api_token=xyzzy", nil)
+       resp = httptest.NewRecorder()
+       srv.ServeHTTP(resp, req)
+       c.Check(resp.Code, check.Equals, http.StatusOK)
 
-       resp, err = http.Get("http://" + srv.listening + "/metrics?api_token=xyzzy")
-       c.Assert(err, check.IsNil)
-       c.Check(resp.StatusCode, check.Equals, http.StatusOK)
        buf, err := ioutil.ReadAll(resp.Body)
        c.Check(err, check.IsNil)
        return string(buf)
index 6aaf07abae395241fdbd5f26be8ae111f14aac1f..a2200e1db90a4ddf69fd65112c432df9bbcba2c6 100644 (file)
@@ -29,7 +29,7 @@ func (s *integrationSuite) TestIdenticalTimestamps(c *check.C) {
                        longestStreak := 0
                        var lastMod time.Time
                        sawUUID := make(map[string]bool)
-                       err := EachCollection(&s.config.Client, pageSize, func(c arvados.Collection) error {
+                       err := EachCollection(s.client, pageSize, func(c arvados.Collection) error {
                                if c.ModifiedAt == nil {
                                        return nil
                                }
index a79779c7dc8f9fdb5eb7316a74c28fb614d9da52..b50b6caf5a8affdd0a34feb5d660f938cbb42fb3 100644 (file)
@@ -11,6 +11,7 @@ import (
        "testing"
        "time"
 
+       "git.curoverse.com/arvados.git/lib/config"
        "git.curoverse.com/arvados.git/sdk/go/arvados"
        "git.curoverse.com/arvados.git/sdk/go/arvadosclient"
        "git.curoverse.com/arvados.git/sdk/go/arvadostest"
@@ -22,7 +23,8 @@ import (
 var _ = check.Suite(&integrationSuite{})
 
 type integrationSuite struct {
-       config     Config
+       config     *arvados.Cluster
+       client     *arvados.Client
        keepClient *keepclient.KeepClient
 }
 
@@ -59,14 +61,16 @@ func (s *integrationSuite) TearDownSuite(c *check.C) {
 }
 
 func (s *integrationSuite) SetUpTest(c *check.C) {
-       s.config = Config{
-               Client: arvados.Client{
-                       APIHost:   os.Getenv("ARVADOS_API_HOST"),
-                       AuthToken: arvadostest.DataManagerToken,
-                       Insecure:  true,
-               },
-               KeepServiceTypes: []string{"disk"},
-               RunPeriod:        arvados.Duration(time.Second),
+       cfg, err := config.NewLoader(nil, nil).Load()
+       c.Assert(err, check.Equals, nil)
+       s.config, err = cfg.GetCluster("")
+       c.Assert(err, check.Equals, nil)
+       s.config.Collections.BalancePeriod = arvados.Duration(time.Second)
+
+       s.client = &arvados.Client{
+               APIHost:   os.Getenv("ARVADOS_API_HOST"),
+               AuthToken: arvadostest.DataManagerToken,
+               Insecure:  true,
        }
 }
 
@@ -86,7 +90,7 @@ func (s *integrationSuite) TestBalanceAPIFixtures(c *check.C) {
                        Logger:  logger,
                        Metrics: newMetrics(),
                }
-               nextOpts, err := bal.Run(s.config, opts)
+               nextOpts, err := bal.Run(s.client, s.config, opts)
                c.Check(err, check.IsNil)
                c.Check(nextOpts.SafeRendezvousState, check.Not(check.Equals), "")
                c.Check(nextOpts.CommitPulls, check.Equals, true)
diff --git a/services/keep-balance/main_test.go b/services/keep-balance/main_test.go
deleted file mode 100644 (file)
index a280434..0000000
+++ /dev/null
@@ -1,46 +0,0 @@
-// Copyright (C) The Arvados Authors. All rights reserved.
-//
-// SPDX-License-Identifier: AGPL-3.0
-
-package main
-
-import (
-       "time"
-
-       "github.com/ghodss/yaml"
-       check "gopkg.in/check.v1"
-)
-
-var _ = check.Suite(&mainSuite{})
-
-type mainSuite struct{}
-
-func (s *mainSuite) TestExampleJSON(c *check.C) {
-       var config Config
-       c.Check(yaml.Unmarshal(exampleConfigFile, &config), check.IsNil)
-       c.Check(config.KeepServiceTypes, check.DeepEquals, []string{"disk"})
-       c.Check(config.Client.AuthToken, check.Equals, "xyzzy")
-       c.Check(time.Duration(config.RunPeriod), check.Equals, 600*time.Second)
-}
-
-func (s *mainSuite) TestConfigJSONWithKeepServiceList(c *check.C) {
-       var config Config
-       c.Check(yaml.Unmarshal([]byte(`{
-                   "Client": {
-                       "APIHost": "zzzzz.arvadosapi.com:443",
-                       "AuthToken": "xyzzy",
-                       "Insecure": false
-                   },
-                   "KeepServiceList": {
-                       "items": [
-                           {"uuid":"zzzzz-bi64l-abcdefghijklmno", "service_type":"disk", "service_host":"a.zzzzz.arvadosapi.com", "service_port":12345},
-                           {"uuid":"zzzzz-bi64l-bcdefghijklmnop", "service_type":"blob", "service_host":"b.zzzzz.arvadosapi.com", "service_port":12345}
-                       ]
-                   },
-                   "RunPeriod": "600s"
-               }`), &config), check.IsNil)
-       c.Assert(len(config.KeepServiceList.Items), check.Equals, 2)
-       c.Check(config.KeepServiceList.Items[0].UUID, check.Equals, "zzzzz-bi64l-abcdefghijklmno")
-       c.Check(config.KeepServiceList.Items[0].ServicePort, check.Equals, 12345)
-       c.Check(config.Client.AuthToken, check.Equals, "xyzzy")
-}
index 0f4bb7176ae58044027a947cb821ba8db5ff9cdc..23e597c89e63e384aef6ab9dff4ea297f74e76c3 100644 (file)
@@ -13,7 +13,10 @@ import (
        "time"
 
        "git.curoverse.com/arvados.git/sdk/go/arvados"
+       "git.curoverse.com/arvados.git/sdk/go/auth"
        "git.curoverse.com/arvados.git/sdk/go/ctxlog"
+       "github.com/julienschmidt/httprouter"
+       "github.com/prometheus/client_golang/prometheus/promhttp"
        "github.com/sirupsen/logrus"
 )
 
@@ -39,16 +42,22 @@ type RunOptions struct {
 }
 
 type Server struct {
-       http.Handler
        Cluster    *arvados.Cluster
        ArvClient  *arvados.Client
        RunOptions RunOptions
        Metrics    *metrics
 
+       httpHandler http.Handler
+
        Logger logrus.FieldLogger
        Dumper logrus.FieldLogger
 }
 
+// ServeHTTP implements service.Handler.
+func (srv *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) {
+       srv.httpHandler.ServeHTTP(w, r)
+}
+
 // CheckHealth implements service.Handler.
 func (srv *Server) CheckHealth() error {
        return nil
@@ -56,16 +65,11 @@ func (srv *Server) CheckHealth() error {
 
 // Start sets up and runs the balancer.
 func (srv *Server) Start(ctx context.Context) {
-       if srv.RunOptions.Logger == nil {
-               srv.RunOptions.Logger = ctxlog.FromContext(ctx)
-       }
-
-       srv.Logger = srv.RunOptions.Logger
-       srv.Dumper = srv.RunOptions.Dumper
+       srv.init(ctx)
 
        var err error
        if srv.RunOptions.Once {
-               _, err = srv.run()
+               _, err = srv.runOnce()
        } else {
                err = srv.runForever(nil)
        }
@@ -74,7 +78,30 @@ func (srv *Server) Start(ctx context.Context) {
        }
 }
 
-func (srv *Server) run() (*Balancer, error) {
+func (srv *Server) init(ctx context.Context) {
+       if srv.RunOptions.Logger == nil {
+               srv.RunOptions.Logger = ctxlog.FromContext(ctx)
+       }
+
+       srv.Logger = srv.RunOptions.Logger
+       srv.Dumper = srv.RunOptions.Dumper
+
+       if srv.Cluster.ManagementToken == "" {
+               srv.httpHandler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+                       http.Error(w, "Management API authentication is not configured", http.StatusForbidden)
+               })
+       } else {
+               mux := httprouter.New()
+               metricsH := promhttp.HandlerFor(srv.Metrics.reg, promhttp.HandlerOpts{
+                       ErrorLog: srv.Logger,
+               })
+               mux.Handler("GET", "/metrics", metricsH)
+               mux.Handler("GET", "/metrics.json", metricsH)
+               srv.httpHandler = auth.RequireLiteralToken(srv.Cluster.ManagementToken, mux)
+       }
+}
+
+func (srv *Server) runOnce() (*Balancer, error) {
        bal := &Balancer{
                Logger:         srv.Logger,
                Dumper:         srv.Dumper,
@@ -106,7 +133,7 @@ func (srv *Server) runForever(stop <-chan interface{}) error {
                        logger.Print("=======  Consider using -commit-pulls and -commit-trash flags.")
                }
 
-               _, err := srv.run()
+               _, err := srv.runOnce()
                if err != nil {
                        logger.Print("run failed: ", err)
                } else {