6260: update datamanager.go to return error than Fatalf in some cases; add additional...

diff --git a/services/datamanager/datamanager.go b/services/datamanager/datamanager.go
index 8f3109362b7ee3def6ebc32c9755f937348fb81e..9cfd2ee5d71891fd6dfb1ff99f496d314abcdf7c 100644 (file)
--- a/services/datamanager/datamanager.go
+++ b/services/datamanager/datamanager.go
@@ -3,6 +3,7 @@
 package main
 
 import (
+       "errors"
        "flag"
        "fmt"
        "git.curoverse.com/arvados.git/sdk/go/arvadosclient"
@@ -41,7 +42,7 @@ func init() {
 func main() {
        flag.Parse()
        if minutesBetweenRuns == 0 {
-               err := singlerun()
+               err := singlerun(makeArvadosClient())
                if err != nil {
                        log.Fatalf("Got an error: %v", err)
                }
@@ -49,7 +50,7 @@ func main() {
                waitTime := time.Minute * time.Duration(minutesBetweenRuns)
                for {
                        log.Println("Beginning Run")
-                       err := singlerun()
+                       err := singlerun(makeArvadosClient())
                        if err != nil {
                                log.Printf("Got an error: %v", err)
                        }
@@ -59,16 +60,24 @@ func main() {
        }
 }
 
-func singlerun() error {
+func makeArvadosClient() arvadosclient.ArvadosClient {
        arv, err := arvadosclient.MakeArvadosClient()
        if err != nil {
-               log.Fatalf("Error setting up arvados client %s", err.Error())
+               log.Fatalf("Error setting up arvados client: %s", err)
        }
+       return arv
+}
+
+var dataManagerToken string
 
+func singlerun(arv arvadosclient.ArvadosClient) error {
+       var err error
        if is_admin, err := util.UserIsAdmin(arv); err != nil {
-               log.Fatalf("Error querying current arvados user %s", err.Error())
+               log.Printf("Error querying current arvados user %s", err.Error())
+               return err
        } else if !is_admin {
-               log.Fatalf("Current user is not an admin. Datamanager can only be run by admins.")
+               log.Printf("Current user is not an admin. Datamanager can only be run by admins.")
+               return errors.New("Current user is not an admin. Datamanager can only be run by admins.")
        }
 
        var arvLogger *logger.Logger
@@ -85,13 +94,17 @@ func singlerun() error {
        }
 
        // Verify that datamanager token belongs to an admin user
-       dataManagerToken := keep.GetDataManagerToken(arvLogger)
+       if dataManagerToken == "" {
+               dataManagerToken = keep.GetDataManagerToken(arvLogger)
+       }
        origArvToken := arv.ApiToken
        arv.ApiToken = dataManagerToken
        if is_admin, err := util.UserIsAdmin(arv); err != nil {
-               log.Fatalf("Error querying arvados user for data manager token %s", err.Error())
+               log.Printf("Error querying arvados user for data manager token %s", err.Error())
+               return err
        } else if !is_admin {
-               log.Fatalf("Datamanager token does not belong to an admin user.")
+               log.Printf("Datamanager token does not belong to an admin user.")
+               return errors.New("Datamanager token does not belong to an admin user.")
        }
        arv.ApiToken = origArvToken
 

diff --git a/services/datamanager/datamanager_test.go b/services/datamanager/datamanager_test.go
index e876fa4b25eb309dc0af3debae9212550a9dca86..2ab1a481598042708d71bffba455140ab85e66e8 100644 (file)
--- a/services/datamanager/datamanager_test.go
+++ b/services/datamanager/datamanager_test.go
@@ -8,7 +8,6 @@ import (
        "git.curoverse.com/arvados.git/sdk/go/keepclient"
        "git.curoverse.com/arvados.git/services/datamanager/keep"
        "io/ioutil"
-       "log"
        "net/http"
        "os"
        "os/exec"
@@ -18,6 +17,8 @@ import (
        "time"
 )
 
+const ACTIVE_USER_TOKEN = "3kg6k6lzmp9kj5cpkcoxie963cmvjahbt2fod9zru30k1jqdmi"
+
 var arv arvadosclient.ArvadosClient
 var keepClient *keepclient.KeepClient
 var keepServers []string
@@ -30,12 +31,7 @@ func SetupDataManagerTest(t *testing.T) {
        arvadostest.StartAPI()
        arvadostest.StartKeep()
 
-       // make arvadosclient
-       var err error
-       arv, err = arvadosclient.MakeArvadosClient()
-       if err != nil {
-               t.Fatalf("Error setting up arvados client: %s", err)
-       }
+       arv = makeArvadosClient()
 
        // keep client
        keepClient = &keepclient.KeepClient{
@@ -164,7 +160,7 @@ func deleteCollection(t *testing.T, uuid string) {
 }
 
 func dataManagerSingleRun(t *testing.T) {
-       err := singlerun()
+       err := singlerun(arv)
        if err != nil {
                t.Fatalf("Error during singlerun %s", err)
        }
@@ -352,7 +348,6 @@ Also create some collections and delete some of them.
 Verify block indexes.
 */
 func TestPutAndGetBlocks(t *testing.T) {
-       log.Print("TestPutAndGetBlocks start")
        defer TearDownDataManagerTest(t)
        SetupDataManagerTest(t)
 
@@ -481,13 +476,11 @@ func TestPutAndGetBlocks(t *testing.T) {
 }
 
 func TestDatamanagerSingleRunRepeatedly(t *testing.T) {
-       log.Print("TestDatamanagerSingleRunRepeatedly start")
-
        defer TearDownDataManagerTest(t)
        SetupDataManagerTest(t)
 
        for i := 0; i < 10; i++ {
-               err := singlerun()
+               err := singlerun(arv)
                if err != nil {
                        t.Fatalf("Got an error during datamanager singlerun: %v", err)
                }
@@ -521,3 +514,39 @@ func TestGetStatusRepeatedly(t *testing.T) {
                }
        }
 }
+
+func TestRunDatamanagerWithBogusServer(t *testing.T) {
+       defer TearDownDataManagerTest(t)
+       SetupDataManagerTest(t)
+
+       arv.ApiServer = "bogus-server"
+
+       err := singlerun(arv)
+       if err == nil {
+               t.Fatalf("Expected error during singlerun with bogus server")
+       }
+}
+
+func TestRunDatamanagerAsNonAdminUser(t *testing.T) {
+       defer TearDownDataManagerTest(t)
+       SetupDataManagerTest(t)
+
+       arv.ApiToken = ACTIVE_USER_TOKEN
+
+       err := singlerun(arv)
+       if err == nil {
+               t.Fatalf("Expected error during singlerun as non-admin user")
+       }
+}
+
+func TestRunDatamanagerWithNonAdminDataManagerToken(t *testing.T) {
+       defer TearDownDataManagerTest(t)
+       SetupDataManagerTest(t)
+
+       dataManagerToken = ACTIVE_USER_TOKEN
+
+       err := singlerun(arv)
+       if err == nil {
+               t.Fatalf("Expected error during singlerun with non-admin user token as datamanager token")
+       }
+}