[ARVADOS] updated: 7ed2f9d875d69b1494372c0cb790b18187dbacf2

git at public.curoverse.com git at public.curoverse.com
Wed Dec 2 12:22:18 EST 2015


Summary of changes:
 sdk/go/crunchrunner/upload.go                      |  4 +-
 sdk/go/keepclient/discover.go                      | 13 +++---
 sdk/go/keepclient/keepclient.go                    |  5 ++-
 sdk/go/keepclient/keepclient_test.go               | 48 ++++++++++++++++++++-
 sdk/go/keepclient/support.go                       | 10 ++---
 .../arvados/v1/keep_services_controller.rb         |  2 +-
 .../arvados/v1/keep_services_controller_test.rb    | 16 ++++++-
 services/datamanager/datamanager_test.go           | 50 ++++++++++++++++++++--
 services/datamanager/keep/keep.go                  | 29 ++++++++++---
 services/datamanager/keep/keep_test.go             |  2 +-
 services/keepproxy/keepproxy_test.go               |  3 +-
 services/keepstore/keepstore.go                    |  1 -
 services/keepstore/pull_worker_integration_test.go |  1 -
 13 files changed, 148 insertions(+), 36 deletions(-)

       via  7ed2f9d875d69b1494372c0cb790b18187dbacf2 (commit)
       via  e2197875b3fa58b235268a86170fec582c1a7f59 (commit)
       via  7413300e85b60d0533ec117b2175cb67fd9f10e3 (commit)
       via  d491de72bb56e7ca09570d4cc7cca02e217c5bf5 (commit)
       via  b17b1ad22bd3cf518eb1b3657f62768b60c7db25 (commit)
       via  a2aae98d0dc2550f4b5d2055769c23e9406b8860 (commit)
       via  035edcae46056689b814107d13f1ffbd34c594ef (commit)
      from  96a3d13e88c67f6beef17877f15d97a03c63b525 (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 7ed2f9d875d69b1494372c0cb790b18187dbacf2
Merge: 96a3d13 e219787
Author: radhika <radhika at curoverse.com>
Date:   Wed Dec 2 12:22:00 2015 -0500

    closes #7710
    Merge branch '7710-keepclient-all-service-types'


commit e2197875b3fa58b235268a86170fec582c1a7f59
Author: radhika <radhika at curoverse.com>
Date:   Wed Dec 2 11:58:39 2015 -0500

    7710: text updates around the -service-type argument.

diff --git a/services/datamanager/keep/keep.go b/services/datamanager/keep/keep.go
index d52191c..ea28ffa 100644
--- a/services/datamanager/keep/keep.go
+++ b/services/datamanager/keep/keep.go
@@ -77,13 +77,13 @@ type ServiceList struct {
 	KeepServers    []ServerAddress `json:"items"`
 }
 
-var supportedServiceType string
+var serviceType string
 
 func init() {
-	flag.StringVar(&supportedServiceType,
+	flag.StringVar(&serviceType,
 		"service-type",
 		"disk",
-		"Supported keepservice type. Default is disk.")
+		"Operate only on keep_services with the specified service_type, ignoring all others.")
 }
 
 // String
@@ -131,19 +131,17 @@ func GetKeepServers(params GetKeepServersParams) (results ReadServers, err error
 		return
 	}
 
-	// Ignore any services that are of type other than the "supportedServiceType".
-	// If no services of the "supportedServiceType" are found, raise an error.
-	var indexableKeepServers []ServerAddress
+	var keepServers []ServerAddress
 	for _, server := range sdkResponse.KeepServers {
-		if server.ServiceType == supportedServiceType {
-			indexableKeepServers = append(indexableKeepServers, server)
+		if server.ServiceType == serviceType {
+			keepServers = append(keepServers, server)
 		} else {
-			log.Printf("Ignore unsupported service type: %v", server.ServiceType)
+			log.Printf("Skipping keep_service %q because its service_type %q does not match -service-type=%q", server, server.ServiceType, serviceType)
 		}
 	}
 
-	if len(indexableKeepServers) == 0 {
-		return results, fmt.Errorf("Found no keepservices with the supported type %v", supportedServiceType)
+	if len(keepServers) == 0 {
+		return results, fmt.Errorf("Found no keepservices with the service type %v", serviceType)
 	}
 
 	if params.Logger != nil {
@@ -152,7 +150,7 @@ func GetKeepServers(params GetKeepServersParams) (results ReadServers, err error
 			keepInfo["num_keep_servers_available"] = sdkResponse.ItemsAvailable
 			keepInfo["num_keep_servers_received"] = len(sdkResponse.KeepServers)
 			keepInfo["keep_servers"] = sdkResponse.KeepServers
-			keepInfo["indexable_keep_servers"] = indexableKeepServers
+			keepInfo["indexable_keep_servers"] = keepServers
 		})
 	}
 
@@ -162,7 +160,7 @@ func GetKeepServers(params GetKeepServersParams) (results ReadServers, err error
 		return results, fmt.Errorf("Did not receive all available keep servers: %+v", sdkResponse)
 	}
 
-	results.KeepServerIndexToAddress = indexableKeepServers
+	results.KeepServerIndexToAddress = keepServers
 	results.KeepServerAddressToIndex = make(map[ServerAddress]int)
 	for i, address := range results.KeepServerIndexToAddress {
 		results.KeepServerAddressToIndex[address] = i
diff --git a/services/datamanager/keep/keep_test.go b/services/datamanager/keep/keep_test.go
index 2286453..21a37f0 100644
--- a/services/datamanager/keep/keep_test.go
+++ b/services/datamanager/keep/keep_test.go
@@ -101,7 +101,7 @@ type APITestData struct {
 }
 
 func (s *KeepSuite) TestGetKeepServers_UnsupportedServiceType(c *C) {
-	testGetKeepServersFromAPI(c, APITestData{1, "notadisk", 200}, "Found no keepservices with the supported type disk")
+	testGetKeepServersFromAPI(c, APITestData{1, "notadisk", 200}, "Found no keepservices with the service type disk")
 }
 
 func (s *KeepSuite) TestGetKeepServers_ReceivedTooFewServers(c *C) {

commit 7413300e85b60d0533ec117b2175cb67fd9f10e3
Author: radhika <radhika at curoverse.com>
Date:   Tue Dec 1 13:33:55 2015 -0500

    7710: update crunchrunner.upload.go to compile; this was broken after the keep-web updates; also, add this package to gostuff in run-tests.sh

diff --git a/sdk/go/crunchrunner/upload.go b/sdk/go/crunchrunner/upload.go
index 4ced0ce..06a6678 100644
--- a/sdk/go/crunchrunner/upload.go
+++ b/sdk/go/crunchrunner/upload.go
@@ -130,7 +130,7 @@ func (m *ManifestWriter) WalkFunc(path string, info os.FileInfo, err error) erro
 
 	stream.offset += count
 
-	stream.ManifestStream.Files = append(stream.ManifestStream.Files,
+	stream.ManifestStream.FileTokens = append(stream.ManifestStream.FileTokens,
 		fmt.Sprintf("%v:%v:%v", fileStart, count, fn))
 
 	return nil
@@ -189,7 +189,7 @@ func (m *ManifestWriter) ManifestText() string {
 			buf.WriteString(" ")
 			buf.WriteString(b)
 		}
-		for _, f := range v.Files {
+		for _, f := range v.FileTokens {
 			buf.WriteString(" ")
 			f = strings.Replace(f, " ", "\\040", -1)
 			f = strings.Replace(f, "\n", "", -1)

commit d491de72bb56e7ca09570d4cc7cca02e217c5bf5
Author: radhika <radhika at curoverse.com>
Date:   Tue Dec 1 09:26:13 2015 -0500

    7710: update serviceType argument handling to ignore any keepservices that are of a different type. Add a datamanager test with one extra unsupported-typed keepserver, which should hence be ignored by datamanager.

diff --git a/services/datamanager/datamanager_test.go b/services/datamanager/datamanager_test.go
index 4203936..75950d1 100644
--- a/services/datamanager/datamanager_test.go
+++ b/services/datamanager/datamanager_test.go
@@ -624,15 +624,26 @@ func createMultiStreamBlockCollection(t *testing.T, data string, numStreams, num
 // Also, create stray block and backdate it.
 // After datamanager run: expect blocks from the collection, but not the stray block.
 func TestManifestWithMultipleStreamsAndBlocks(t *testing.T) {
+	testManifestWithMultipleStreamsAndBlocks(t, 100, 10, "")
+}
+
+// Same test as TestManifestWithMultipleStreamsAndBlocks with an additional
+// keepstore of a service type other than "disk". Only the "disk" type services
+// will be indexed by datamanager and hence should work the same way.
+func TestManifestWithMultipleStreamsAndBlocks_WithOneUnsupportedKeepServer(t *testing.T) {
+	testManifestWithMultipleStreamsAndBlocks(t, 2, 2, "testblobstore")
+}
+
+func testManifestWithMultipleStreamsAndBlocks(t *testing.T, numStreams, numBlocks int, createExtraKeepServerWithType string) {
 	defer TearDownDataManagerTest(t)
 	SetupDataManagerTest(t)
 
 	// create collection whose blocks will be backdated
-	collectionWithOldBlocks, oldBlocks := createMultiStreamBlockCollection(t, "old block", 100, 10)
+	collectionWithOldBlocks, oldBlocks := createMultiStreamBlockCollection(t, "old block", numStreams, numBlocks)
 	if collectionWithOldBlocks == "" {
-		t.Fatalf("Failed to create collection with 1000 blocks")
+		t.Fatalf("Failed to create collection with %d blocks", numStreams*numBlocks)
 	}
-	if len(oldBlocks) != 1000 {
+	if len(oldBlocks) != numStreams*numBlocks {
 		t.Fatalf("Not all blocks are created: expected %v, found %v", 1000, len(oldBlocks))
 	}
 
@@ -649,9 +660,41 @@ func TestManifestWithMultipleStreamsAndBlocks(t *testing.T) {
 	// also backdate the stray old block
 	backdateBlocks(t, []string{strayOldBlock})
 
+	// If requested, create an extra keepserver with the given type
+	// This should be ignored during indexing and hence not change the datamanager outcome
+	var extraKeepServerUUID string
+	if createExtraKeepServerWithType != "" {
+		extraKeepServerUUID = addExtraKeepServer(t, createExtraKeepServerWithType)
+		defer deleteExtraKeepServer(extraKeepServerUUID)
+	}
+
 	// run datamanager
 	dataManagerSingleRun(t)
 
 	// verify that strayOldBlock is not to be found, but the collections blocks are still there
 	verifyBlocks(t, []string{strayOldBlock}, oldBlocks, 2)
 }
+
+// Add one more keepstore with the given service type
+func addExtraKeepServer(t *testing.T, serviceType string) string {
+	defer switchToken(arvadostest.AdminToken)()
+
+	extraKeepService := make(arvadosclient.Dict)
+	err := arv.Create("keep_services",
+		arvadosclient.Dict{"keep_service": arvadosclient.Dict{
+			"service_host":     "localhost",
+			"service_port":     "21321",
+			"service_ssl_flag": false,
+			"service_type":     serviceType}},
+		&extraKeepService)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	return extraKeepService["uuid"].(string)
+}
+
+func deleteExtraKeepServer(uuid string) {
+	defer switchToken(arvadostest.AdminToken)()
+	arv.Delete("keep_services", uuid, nil, nil)
+}
diff --git a/services/datamanager/keep/keep.go b/services/datamanager/keep/keep.go
index c165120..d52191c 100644
--- a/services/datamanager/keep/keep.go
+++ b/services/datamanager/keep/keep.go
@@ -77,10 +77,10 @@ type ServiceList struct {
 	KeepServers    []ServerAddress `json:"items"`
 }
 
-var serviceType string
+var supportedServiceType string
 
 func init() {
-	flag.StringVar(&serviceType,
+	flag.StringVar(&supportedServiceType,
 		"service-type",
 		"disk",
 		"Supported keepservice type. Default is disk.")
@@ -131,18 +131,19 @@ func GetKeepServers(params GetKeepServersParams) (results ReadServers, err error
 		return
 	}
 
-	// Currently, only "disk" types are supported. Stop if any other service types are found.
-	foundSupportedServieType := false
+	// Ignore any services that are of type other than the "supportedServiceType".
+	// If no services of the "supportedServiceType" are found, raise an error.
+	var indexableKeepServers []ServerAddress
 	for _, server := range sdkResponse.KeepServers {
-		if server.ServiceType == serviceType {
-			foundSupportedServieType = true
+		if server.ServiceType == supportedServiceType {
+			indexableKeepServers = append(indexableKeepServers, server)
 		} else {
 			log.Printf("Ignore unsupported service type: %v", server.ServiceType)
 		}
 	}
 
-	if !foundSupportedServieType {
-		return results, fmt.Errorf("Found no keepservices with the supported type %v", serviceType)
+	if len(indexableKeepServers) == 0 {
+		return results, fmt.Errorf("Found no keepservices with the supported type %v", supportedServiceType)
 	}
 
 	if params.Logger != nil {
@@ -151,6 +152,7 @@ func GetKeepServers(params GetKeepServersParams) (results ReadServers, err error
 			keepInfo["num_keep_servers_available"] = sdkResponse.ItemsAvailable
 			keepInfo["num_keep_servers_received"] = len(sdkResponse.KeepServers)
 			keepInfo["keep_servers"] = sdkResponse.KeepServers
+			keepInfo["indexable_keep_servers"] = indexableKeepServers
 		})
 	}
 
@@ -160,7 +162,7 @@ func GetKeepServers(params GetKeepServersParams) (results ReadServers, err error
 		return results, fmt.Errorf("Did not receive all available keep servers: %+v", sdkResponse)
 	}
 
-	results.KeepServerIndexToAddress = sdkResponse.KeepServers
+	results.KeepServerIndexToAddress = indexableKeepServers
 	results.KeepServerAddressToIndex = make(map[ServerAddress]int)
 	for i, address := range results.KeepServerIndexToAddress {
 		results.KeepServerAddressToIndex[address] = i
@@ -170,7 +172,7 @@ func GetKeepServers(params GetKeepServersParams) (results ReadServers, err error
 
 	// Send off all the index requests concurrently
 	responseChan := make(chan ServerResponse)
-	for _, keepServer := range sdkResponse.KeepServers {
+	for _, keepServer := range results.KeepServerIndexToAddress {
 		// The above keepsServer variable is reused for each iteration, so
 		// it would be shared across all goroutines. This would result in
 		// us querying one server n times instead of n different servers
@@ -188,7 +190,7 @@ func GetKeepServers(params GetKeepServersParams) (results ReadServers, err error
 	results.BlockToServers = make(map[blockdigest.DigestWithSize][]BlockServerInfo)
 
 	// Read all the responses
-	for i := range sdkResponse.KeepServers {
+	for i := range results.KeepServerIndexToAddress {
 		_ = i // Here to prevent go from complaining.
 		response := <-responseChan
 

commit b17b1ad22bd3cf518eb1b3657f62768b60c7db25
Author: radhika <radhika at curoverse.com>
Date:   Mon Nov 30 18:09:57 2015 -0500

    7710: Remove KeepClient.Using_proxy and all it's references since we are no longer using it.
    Update datamanager to use a configured supported service type instead of hardcoding "disk" type.

diff --git a/sdk/go/keepclient/discover.go b/sdk/go/keepclient/discover.go
index 1095085..099c56f 100644
--- a/sdk/go/keepclient/discover.go
+++ b/sdk/go/keepclient/discover.go
@@ -84,7 +84,6 @@ func (this *KeepClient) loadKeepServers(list svcList) error {
 
 	// replicasPerService is 1 for disks; unknown or unlimited otherwise
 	this.replicasPerService = 1
-	this.Using_proxy = false
 
 	for _, service := range list.Items {
 		scheme := "http"
@@ -100,10 +99,6 @@ func (this *KeepClient) loadKeepServers(list svcList) error {
 		listed[url] = true
 
 		localRoots[service.Uuid] = url
-		if service.SvcType == "proxy" {
-			this.Using_proxy = true
-		}
-
 		if service.ReadOnly == false {
 			writableLocalRoots[service.Uuid] = url
 			if service.SvcType != "disk" {
diff --git a/sdk/go/keepclient/keepclient.go b/sdk/go/keepclient/keepclient.go
index ddb502c..f15a6b2 100644
--- a/sdk/go/keepclient/keepclient.go
+++ b/sdk/go/keepclient/keepclient.go
@@ -68,7 +68,6 @@ const X_Keep_Replicas_Stored = "X-Keep-Replicas-Stored"
 type KeepClient struct {
 	Arvados            *arvadosclient.ArvadosClient
 	Want_replicas      int
-	Using_proxy        bool
 	localRoots         *map[string]string
 	writableLocalRoots *map[string]string
 	gatewayRoots       *map[string]string
@@ -104,7 +103,6 @@ func New(arv *arvadosclient.ArvadosClient) *KeepClient {
 	kc := &KeepClient{
 		Arvados:       arv,
 		Want_replicas: defaultReplicationLevel,
-		Using_proxy:   false,
 		Client: &http.Client{Transport: &http.Transport{
 			TLSClientConfig: &tls.Config{InsecureSkipVerify: arv.ApiInsecure}}},
 		Retries: 2,
diff --git a/sdk/go/keepclient/keepclient_test.go b/sdk/go/keepclient/keepclient_test.go
index 2c77e68..87b9b1d 100644
--- a/sdk/go/keepclient/keepclient_test.go
+++ b/sdk/go/keepclient/keepclient_test.go
@@ -895,7 +895,6 @@ func (s *StandaloneSuite) TestPutProxy(c *C) {
 	kc, _ := MakeKeepClient(&arv)
 
 	kc.Want_replicas = 2
-	kc.Using_proxy = true
 	arv.ApiToken = "abc123"
 	localRoots := make(map[string]string)
 	writableLocalRoots := make(map[string]string)
@@ -928,7 +927,6 @@ func (s *StandaloneSuite) TestPutProxyInsufficientReplicas(c *C) {
 	kc, _ := MakeKeepClient(&arv)
 
 	kc.Want_replicas = 3
-	kc.Using_proxy = true
 	arv.ApiToken = "abc123"
 	localRoots := make(map[string]string)
 	writableLocalRoots := make(map[string]string)
@@ -1271,21 +1269,21 @@ func (s *ServerRequiredSuite) TestMakeKeepClientWithNonDiskTypeService(c *C) {
 	for _, root := range kc.LocalRoots() {
 		c.Check(root, Matches, "http://localhost:\\d+")
 	}
-	c.Assert(kc.LocalRoots()[blobKeepService["uuid"].(string)], NotNil)
+	c.Assert(kc.LocalRoots()[blobKeepService["uuid"].(string)], Not(Equals), "")
 
 	// verify kc.GatewayRoots
 	c.Check(len(kc.GatewayRoots()), Equals, 3)
 	for _, root := range kc.GatewayRoots() {
 		c.Check(root, Matches, "http://localhost:\\d+")
 	}
-	c.Assert(kc.GatewayRoots()[blobKeepService["uuid"].(string)], NotNil)
+	c.Assert(kc.GatewayRoots()[blobKeepService["uuid"].(string)], Not(Equals), "")
 
 	// verify kc.WritableLocalRoots
 	c.Check(len(kc.WritableLocalRoots()), Equals, 3)
 	for _, root := range kc.WritableLocalRoots() {
 		c.Check(root, Matches, "http://localhost:\\d+")
 	}
-	c.Assert(kc.WritableLocalRoots()[blobKeepService["uuid"].(string)], NotNil)
+	c.Assert(kc.WritableLocalRoots()[blobKeepService["uuid"].(string)], Not(Equals), "")
 
 	c.Assert(kc.replicasPerService, Equals, 0)
 	c.Assert(kc.foundNonDiskSvc, Equals, true)
diff --git a/services/datamanager/datamanager_test.go b/services/datamanager/datamanager_test.go
index 28faf98..4203936 100644
--- a/services/datamanager/datamanager_test.go
+++ b/services/datamanager/datamanager_test.go
@@ -41,7 +41,6 @@ func SetupDataManagerTest(t *testing.T) {
 	keepClient = &keepclient.KeepClient{
 		Arvados:       &arv,
 		Want_replicas: 2,
-		Using_proxy:   true,
 		Client:        &http.Client{},
 	}
 
diff --git a/services/datamanager/keep/keep.go b/services/datamanager/keep/keep.go
index 7da74b1..c165120 100644
--- a/services/datamanager/keep/keep.go
+++ b/services/datamanager/keep/keep.go
@@ -6,6 +6,7 @@ import (
 	"bufio"
 	"encoding/json"
 	"errors"
+	"flag"
 	"fmt"
 	"git.curoverse.com/arvados.git/sdk/go/arvadosclient"
 	"git.curoverse.com/arvados.git/sdk/go/blockdigest"
@@ -76,6 +77,15 @@ type ServiceList struct {
 	KeepServers    []ServerAddress `json:"items"`
 }
 
+var serviceType string
+
+func init() {
+	flag.StringVar(&serviceType,
+		"service-type",
+		"disk",
+		"Supported keepservice type. Default is disk.")
+}
+
 // String
 // TODO(misha): Change this to include the UUID as well.
 func (s ServerAddress) String() string {
@@ -122,12 +132,19 @@ func GetKeepServers(params GetKeepServersParams) (results ReadServers, err error
 	}
 
 	// Currently, only "disk" types are supported. Stop if any other service types are found.
+	foundSupportedServieType := false
 	for _, server := range sdkResponse.KeepServers {
-		if server.ServiceType != "disk" {
-			return results, fmt.Errorf("Unsupported service type %q found for: %v", server.ServiceType, server)
+		if server.ServiceType == serviceType {
+			foundSupportedServieType = true
+		} else {
+			log.Printf("Ignore unsupported service type: %v", server.ServiceType)
 		}
 	}
 
+	if !foundSupportedServieType {
+		return results, fmt.Errorf("Found no keepservices with the supported type %v", serviceType)
+	}
+
 	if params.Logger != nil {
 		params.Logger.Update(func(p map[string]interface{}, e map[string]interface{}) {
 			keepInfo := logger.GetOrCreateMap(p, "keep_info")
diff --git a/services/datamanager/keep/keep_test.go b/services/datamanager/keep/keep_test.go
index 0691e64..2286453 100644
--- a/services/datamanager/keep/keep_test.go
+++ b/services/datamanager/keep/keep_test.go
@@ -101,7 +101,7 @@ type APITestData struct {
 }
 
 func (s *KeepSuite) TestGetKeepServers_UnsupportedServiceType(c *C) {
-	testGetKeepServersFromAPI(c, APITestData{1, "notadisk", 200}, "Unsupported service type")
+	testGetKeepServersFromAPI(c, APITestData{1, "notadisk", 200}, "Found no keepservices with the supported type disk")
 }
 
 func (s *KeepSuite) TestGetKeepServers_ReceivedTooFewServers(c *C) {
diff --git a/services/keepproxy/keepproxy_test.go b/services/keepproxy/keepproxy_test.go
index 7c5b362..56c9bff 100644
--- a/services/keepproxy/keepproxy_test.go
+++ b/services/keepproxy/keepproxy_test.go
@@ -105,7 +105,6 @@ func runProxy(c *C, args []string, bogusClientToken bool) *keepclient.KeepClient
 	}
 	kc.SetServiceRoots(sr, sr, sr)
 	kc.Arvados.External = true
-	kc.Using_proxy = true
 
 	return kc
 }
@@ -474,7 +473,7 @@ func (s *NoKeepServerSuite) TestAskGetNoKeepServerError(c *C) {
 	defer closeListener()
 
 	hash := fmt.Sprintf("%x", md5.Sum([]byte("foo")))
-	for _, f := range []func()error {
+	for _, f := range []func() error{
 		func() error {
 			_, _, err := kc.Ask(hash)
 			return err
diff --git a/services/keepstore/keepstore.go b/services/keepstore/keepstore.go
index 7525441..96a887f 100644
--- a/services/keepstore/keepstore.go
+++ b/services/keepstore/keepstore.go
@@ -300,7 +300,6 @@ func main() {
 	keepClient := &keepclient.KeepClient{
 		Arvados:       nil,
 		Want_replicas: 1,
-		Using_proxy:   true,
 		Client:        &http.Client{},
 	}
 
diff --git a/services/keepstore/pull_worker_integration_test.go b/services/keepstore/pull_worker_integration_test.go
index 52b59ec..e0613a2 100644
--- a/services/keepstore/pull_worker_integration_test.go
+++ b/services/keepstore/pull_worker_integration_test.go
@@ -39,7 +39,6 @@ func SetupPullWorkerIntegrationTest(t *testing.T, testData PullWorkIntegrationTe
 	keepClient = &keepclient.KeepClient{
 		Arvados:       &arv,
 		Want_replicas: 1,
-		Using_proxy:   true,
 		Client:        &http.Client{},
 	}
 

commit a2aae98d0dc2550f4b5d2055769c23e9406b8860
Author: Tom Clegg <tom at curoverse.com>
Date:   Mon Nov 30 15:28:19 2015 -0500

    7710: Add keep_services#accessible test.

diff --git a/services/api/test/functional/arvados/v1/keep_services_controller_test.rb b/services/api/test/functional/arvados/v1/keep_services_controller_test.rb
index bfa138d..1375d4c 100644
--- a/services/api/test/functional/arvados/v1/keep_services_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/keep_services_controller_test.rb
@@ -2,7 +2,7 @@ require 'test_helper'
 
 class Arvados::V1::KeepServicesControllerTest < ActionController::TestCase
 
-  test "search keep_services by service_port with < query" do
+  test "search by service_port with < query" do
     authorize_with :active
     get :index, {
       filters: [['service_port', '<', 25107]]
@@ -11,7 +11,7 @@ class Arvados::V1::KeepServicesControllerTest < ActionController::TestCase
     assert_equal false, assigns(:objects).any?
   end
 
-  test "search keep_disks by service_port with >= query" do
+  test "search by service_port with >= query" do
     authorize_with :active
     get :index, {
       filters: [['service_port', '>=', 25107]]
@@ -20,4 +20,16 @@ class Arvados::V1::KeepServicesControllerTest < ActionController::TestCase
     assert_equal true, assigns(:objects).any?
   end
 
+  [:admin, :active, :inactive, :anonymous].each do |u|
+    test "accessible to #{u} user" do
+      authorize_with u
+      get :accessible
+      assert_response :success
+      assert_not_empty json_response['items']
+      json_response['items'].each do |ks|
+        assert_not_equal ks['service_type'], 'proxy'
+      end
+    end
+  end
+
 end

commit 035edcae46056689b814107d13f1ffbd34c594ef
Author: radhika <radhika at curoverse.com>
Date:   Fri Nov 27 17:13:17 2015 -0500

    7710: improve keepclient to use non-disk timeouts when any non-disk typed keepservices are found; previously, this was done for proxy typed keepservices only.

diff --git a/sdk/go/keepclient/discover.go b/sdk/go/keepclient/discover.go
index 650c2b5..1095085 100644
--- a/sdk/go/keepclient/discover.go
+++ b/sdk/go/keepclient/discover.go
@@ -111,6 +111,10 @@ func (this *KeepClient) loadKeepServers(list svcList) error {
 			}
 		}
 
+		if service.SvcType != "disk" {
+			this.foundNonDiskSvc = true
+		}
+
 		// Gateway services are only used when specified by
 		// UUID, so there's nothing to gain by filtering them
 		// by service type. Including all accessible services
@@ -119,8 +123,8 @@ func (this *KeepClient) loadKeepServers(list svcList) error {
 		gatewayRoots[service.Uuid] = url
 	}
 
-	if this.Using_proxy {
-		this.setClientSettingsProxy()
+	if this.foundNonDiskSvc {
+		this.setClientSettingsNonDisk()
 	} else {
 		this.setClientSettingsDisk()
 	}
diff --git a/sdk/go/keepclient/keepclient.go b/sdk/go/keepclient/keepclient.go
index 0e6fadc..ddb502c 100644
--- a/sdk/go/keepclient/keepclient.go
+++ b/sdk/go/keepclient/keepclient.go
@@ -78,6 +78,9 @@ type KeepClient struct {
 
 	// set to 1 if all writable services are of disk type, otherwise 0
 	replicasPerService int
+
+	// Any non-disk typed services found in the list of keepservers?
+	foundNonDiskSvc bool
 }
 
 // MakeKeepClient creates a new KeepClient by contacting the API server to discover Keep servers.
diff --git a/sdk/go/keepclient/keepclient_test.go b/sdk/go/keepclient/keepclient_test.go
index 4615ebc..2c77e68 100644
--- a/sdk/go/keepclient/keepclient_test.go
+++ b/sdk/go/keepclient/keepclient_test.go
@@ -16,6 +16,7 @@ import (
 	"os"
 	"strings"
 	"testing"
+	"time"
 )
 
 // Gocheck boilerplate
@@ -1245,3 +1246,48 @@ func (s *StandaloneSuite) TestPutBRetry(c *C) {
 	c.Check(hash, Equals, "")
 	c.Check(replicas, Equals, 2)
 }
+
+func (s *ServerRequiredSuite) TestMakeKeepClientWithNonDiskTypeService(c *C) {
+	arv, err := arvadosclient.MakeArvadosClient()
+	c.Assert(err, Equals, nil)
+
+	// Add an additional "testblobstore" keepservice
+	blobKeepService := make(arvadosclient.Dict)
+	err = arv.Create("keep_services",
+		arvadosclient.Dict{"keep_service": arvadosclient.Dict{
+			"service_host": "localhost",
+			"service_port": "21321",
+			"service_type": "testblobstore"}},
+		&blobKeepService)
+	c.Assert(err, Equals, nil)
+	defer func() { arv.Delete("keep_services", blobKeepService["uuid"].(string), nil, nil) }()
+
+	// Make a keepclient and ensure that the testblobstore is included
+	kc, err := MakeKeepClient(&arv)
+	c.Assert(err, Equals, nil)
+
+	// verify kc.LocalRoots
+	c.Check(len(kc.LocalRoots()), Equals, 3)
+	for _, root := range kc.LocalRoots() {
+		c.Check(root, Matches, "http://localhost:\\d+")
+	}
+	c.Assert(kc.LocalRoots()[blobKeepService["uuid"].(string)], NotNil)
+
+	// verify kc.GatewayRoots
+	c.Check(len(kc.GatewayRoots()), Equals, 3)
+	for _, root := range kc.GatewayRoots() {
+		c.Check(root, Matches, "http://localhost:\\d+")
+	}
+	c.Assert(kc.GatewayRoots()[blobKeepService["uuid"].(string)], NotNil)
+
+	// verify kc.WritableLocalRoots
+	c.Check(len(kc.WritableLocalRoots()), Equals, 3)
+	for _, root := range kc.WritableLocalRoots() {
+		c.Check(root, Matches, "http://localhost:\\d+")
+	}
+	c.Assert(kc.WritableLocalRoots()[blobKeepService["uuid"].(string)], NotNil)
+
+	c.Assert(kc.replicasPerService, Equals, 0)
+	c.Assert(kc.foundNonDiskSvc, Equals, true)
+	c.Assert(kc.Client.Timeout, Equals, 300*time.Second)
+}
diff --git a/sdk/go/keepclient/support.go b/sdk/go/keepclient/support.go
index 90ac3bc..b904b09 100644
--- a/sdk/go/keepclient/support.go
+++ b/sdk/go/keepclient/support.go
@@ -34,9 +34,9 @@ func Md5String(s string) string {
 	return fmt.Sprintf("%x", md5.Sum([]byte(s)))
 }
 
-// Set timeouts apply when connecting to keepproxy services (assumed to be over
-// the Internet).
-func (this *KeepClient) setClientSettingsProxy() {
+// Set timeouts applicable when connecting to non-disk services
+// (assumed to be over the Internet).
+func (this *KeepClient) setClientSettingsNonDisk() {
 	if this.Client.Timeout == 0 {
 		// Maximum time to wait for a complete response
 		this.Client.Timeout = 300 * time.Second
@@ -58,8 +58,8 @@ func (this *KeepClient) setClientSettingsProxy() {
 	}
 }
 
-// Set timeouts apply when connecting to keepstore services directly (assumed
-// to be on the local network).
+// Set timeouts applicable when connecting to keepstore services directly
+// (assumed to be on the local network).
 func (this *KeepClient) setClientSettingsDisk() {
 	if this.Client.Timeout == 0 {
 		// Maximum time to wait for a complete response
diff --git a/services/api/app/controllers/arvados/v1/keep_services_controller.rb b/services/api/app/controllers/arvados/v1/keep_services_controller.rb
index fc2ee93..d2a512b 100644
--- a/services/api/app/controllers/arvados/v1/keep_services_controller.rb
+++ b/services/api/app/controllers/arvados/v1/keep_services_controller.rb
@@ -13,7 +13,7 @@ class Arvados::V1::KeepServicesController < ApplicationController
     if request.headers['X-External-Client'] == '1'
       @objects = model_class.where('service_type=?', 'proxy')
     else
-      @objects = model_class.where('service_type=?', 'disk')
+      @objects = model_class.where(model_class.arel_table[:service_type].not_eq('proxy'))
     end
     render_list
   end

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list