From 902c3317440f81077ea561b621c8d46dcb689013 Mon Sep 17 00:00:00 2001 From: radhika Date: Tue, 8 Sep 2015 20:48:22 -0400 Subject: [PATCH] 6260: update datamanager.go to return error than Fatalf in some cases; add additional tests to test those errors raised. --- services/datamanager/datamanager.go | 31 ++++++++++---- services/datamanager/datamanager_test.go | 53 ++++++++++++++++++------ 2 files changed, 63 insertions(+), 21 deletions(-) diff --git a/services/datamanager/datamanager.go b/services/datamanager/datamanager.go index 8f3109362b..9cfd2ee5d7 100644 --- 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 e876fa4b25..2ab1a48159 100644 --- 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") + } +} -- 2.30.2