7490: use loggerutil to log any datamanager errors.

diff --git a/services/datamanager/datamanager.go b/services/datamanager/datamanager.go
index f3be93c19fbb176b3ce9cefada5c543049ea501d..50213787aabad4cfe17a0cedd81d609c0ee0fc4f 100644 (file)
--- a/services/datamanager/datamanager.go
+++ b/services/datamanager/datamanager.go
@@ -42,21 +42,21 @@ func init() {
 func main() {
        flag.Parse()
        if minutesBetweenRuns == 0 {
-               arv, err := makeArvadosClient()
+               arv, err := arvadosclient.MakeArvadosClient()
                if err != nil {
-                       log.Fatalf("makeArvadosClient: %v", err)
+                       loggerutil.FatalWithMessage(arvLogger, fmt.Sprintf("Error making arvados client: %v", err))
                }
                err = singlerun(arv)
                if err != nil {
-                       log.Fatalf("singlerun: %v", err)
+                       loggerutil.FatalWithMessage(arvLogger, fmt.Sprintf("singlerun: %v", err))
                }
        } else {
                waitTime := time.Minute * time.Duration(minutesBetweenRuns)
                for {
                        log.Println("Beginning Run")
-                       arv, err := makeArvadosClient()
+                       arv, err := arvadosclient.MakeArvadosClient()
                        if err != nil {
-                               log.Fatalf("makeArvadosClient: %v", err)
+                               loggerutil.FatalWithMessage(arvLogger, fmt.Sprintf("Error making arvados client: %v", err))
                        }
                        err = singlerun(arv)
                        if err != nil {
@@ -68,9 +68,7 @@ func main() {
        }
 }
 
-func makeArvadosClient() (arvadosclient.ArvadosClient, error) {
-       return arvadosclient.MakeArvadosClient()
-}
+var arvLogger *logger.Logger
 
 func singlerun(arv arvadosclient.ArvadosClient) error {
        var err error
@@ -80,7 +78,6 @@ func singlerun(arv arvadosclient.ArvadosClient) error {
                return errors.New("Current user is not an admin. Datamanager requires a privileged token.")
        }
 
-       var arvLogger *logger.Logger
        if logEventTypePrefix != "" {
                arvLogger = logger.NewLogger(logger.LoggerParams{
                        Client:          arv,
@@ -111,7 +108,7 @@ func singlerun(arv arvadosclient.ArvadosClient) error {
                return readCollections.Err
        }
 
-       _, err = summary.MaybeWriteData(arvLogger, readCollections, keepServerInfo)
+       err = summary.MaybeWriteData(arvLogger, readCollections, keepServerInfo)
        if err != nil {
                return err
        }

diff --git a/services/datamanager/datamanager_test.go b/services/datamanager/datamanager_test.go
index 4d82b4928b7bec480a33cf7e6347add61cee878e..53fb46b3ef3cb881be6915d9a8f750c0eed52e3b 100644 (file)
--- a/services/datamanager/datamanager_test.go
+++ b/services/datamanager/datamanager_test.go
@@ -31,7 +31,7 @@ func SetupDataManagerTest(t *testing.T) {
        arvadostest.StartKeep(2, false)
 
        var err error
-       arv, err = makeArvadosClient()
+       arv, err = arvadosclient.MakeArvadosClient()
        if err != nil {
                t.Fatalf("Error making arvados client: %s", err)
        }

diff --git a/services/datamanager/keep/keep_test.go b/services/datamanager/keep/keep_test.go
index b5091f2396b1e30aef746e93fa7c31f52b79a817..3ca7b8218ae011915daae8d61451fb147ae85f41 100644 (file)
--- a/services/datamanager/keep/keep_test.go
+++ b/services/datamanager/keep/keep_test.go
@@ -3,6 +3,7 @@ package keep
 import (
        "encoding/json"
        "fmt"
+       "net"
        "net/http"
        "net/http/httptest"
        "net/url"
@@ -152,18 +153,18 @@ type APITestData struct {
 }
 
 func (s *KeepSuite) TestGetKeepServers_UnsupportedServiceType(c *C) {
-       testGetKeepServersFromAPI(c, APITestData{1, "notadisk", 200})
+       testGetKeepServersFromAPI(c, APITestData{1, "notadisk", 200}, "Unsupported service type")
 }
 
 func (s *KeepSuite) TestGetKeepServers_ReceivedTooFewServers(c *C) {
-       testGetKeepServersFromAPI(c, APITestData{2, "disk", 200})
+       testGetKeepServersFromAPI(c, APITestData{2, "disk", 200}, "Did not receive all available keep servers")
 }
 
 func (s *KeepSuite) TestGetKeepServers_ServerError(c *C) {
-       testGetKeepServersFromAPI(c, APITestData{-1, "disk", -1})
+       testGetKeepServersFromAPI(c, APITestData{-1, "disk", -1}, "arvados API server error")
 }
 
-func testGetKeepServersFromAPI(c *C, testData APITestData) {
+func testGetKeepServersFromAPI(c *C, testData APITestData, expectedError string) {
        keepServers := ServiceList{
                ItemsAvailable: testData.numServers,
                KeepServers: []ServerAddress{{
@@ -202,11 +203,8 @@ func testGetKeepServersFromAPI(c *C, testData APITestData) {
        }
 
        _, err := GetKeepServersAndSummarize(params)
-       if testData.numServers > 1 {
-               c.Assert(err, ErrorMatches, ".*Did not receive all available keep servers.*")
-       } else if testData.serverType != "disk" {
-               c.Assert(err, ErrorMatches, ".*Unsupported service type.*")
-       }
+       c.Assert(err, NotNil)
+       c.Assert(err, ErrorMatches, fmt.Sprintf(".*%s.*", expectedError))
 }
 
 type KeepServerTestData struct {
@@ -265,15 +263,15 @@ func testGetKeepServersAndSummarize(c *C, testData KeepServerTestData) {
 
        ksURL, err := url.Parse(ks.URL)
        c.Check(err, IsNil)
-       ksURLParts := strings.Split(ksURL.Host, ":")
-       ksPort, err := strconv.Atoi(ksURLParts[1])
+       ksHost, port, err := net.SplitHostPort(ksURL.Host)
+       ksPort, err := strconv.Atoi(port)
        c.Check(err, IsNil)
 
        servers_list := ServiceList{
                ItemsAvailable: 1,
                KeepServers: []ServerAddress{{
                        SSL:         false,
-                       Host:        strings.Split(ksURL.Host, ":")[0],
+                       Host:        ksHost,
                        Port:        ksPort,
                        UUID:        "abcdefg",
                        ServiceType: "disk",

diff --git a/services/datamanager/summary/file.go b/services/datamanager/summary/file.go
index 7ed9765c13f5a64351796c870fa2b23c49527a3a..63c8adeb745b05b6f8886751cc135b78b0fd8d87 100644 (file)
--- a/services/datamanager/summary/file.go
+++ b/services/datamanager/summary/file.go
@@ -51,13 +51,13 @@ func init() {
 // working with stale data.
 func MaybeWriteData(arvLogger *logger.Logger,
        readCollections collection.ReadCollections,
-       keepServerInfo keep.ReadServers) (bool, error) {
+       keepServerInfo keep.ReadServers) error {
        if WriteDataTo == "" {
-               return false, nil
+               return nil
        }
        summaryFile, err := os.Create(WriteDataTo)
        if err != nil {
-               return false, err
+               return err
        }
        defer summaryFile.Close()
 
@@ -67,10 +67,10 @@ func MaybeWriteData(arvLogger *logger.Logger,
                KeepServerInfo:  keepServerInfo}
        err = enc.Encode(data)
        if err != nil {
-               return false, err
+               return err
        }
        log.Printf("Wrote summary data to: %s", WriteDataTo)
-       return true, nil
+       return nil
 }
 
 // ShouldReadData should not be used outside of development