[ARVADOS] updated: 1.3.0-1619-g97e49cd63

Git user git at public.curoverse.com
Wed Sep 25 17:51:11 UTC 2019


Summary of changes:
 services/keep-balance/balance_run_test.go | 118 +++++++++++++++---------------
 services/keep-balance/collection_test.go  |   2 +-
 services/keep-balance/integration_test.go |  24 +++---
 services/keep-balance/main_test.go        |  46 ------------
 services/keep-balance/server.go           |  47 +++++++++---
 5 files changed, 109 insertions(+), 128 deletions(-)
 delete mode 100644 services/keep-balance/main_test.go

       via  97e49cd63b00855a659b76c65ea5122968cf8635 (commit)
      from  0f5693feaf2761a46186c3d43e7af294b433c039 (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 97e49cd63b00855a659b76c65ea5122968cf8635
Author: Eric Biagiotti <ebiagiotti at veritasgenetics.com>
Date:   Wed Sep 25 13:51:07 2019 -0400

    14714: Tests use cluster config
    
    Arvados-DCO-1.1-Signed-off-by: Eric Biagiotti <ebiagiotti at veritasgenetics.com>

diff --git a/services/keep-balance/balance_run_test.go b/services/keep-balance/balance_run_test.go
index db530bc49..1478e6e2e 100644
--- a/services/keep-balance/balance_run_test.go
+++ b/services/keep-balance/balance_run_test.go
@@ -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)
diff --git a/services/keep-balance/collection_test.go b/services/keep-balance/collection_test.go
index 6aaf07aba..a2200e1db 100644
--- a/services/keep-balance/collection_test.go
+++ b/services/keep-balance/collection_test.go
@@ -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
 				}
diff --git a/services/keep-balance/integration_test.go b/services/keep-balance/integration_test.go
index a79779c7d..b50b6caf5 100644
--- a/services/keep-balance/integration_test.go
+++ b/services/keep-balance/integration_test.go
@@ -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
index a2804344b..000000000
--- a/services/keep-balance/main_test.go
+++ /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")
-}
diff --git a/services/keep-balance/server.go b/services/keep-balance/server.go
index 0f4bb7176..23e597c89 100644
--- a/services/keep-balance/server.go
+++ b/services/keep-balance/server.go
@@ -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 {

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list