[ARVADOS] created: 2.1.0-1255-gd32d37c6b

Git user git at public.arvados.org
Fri Aug 27 14:16:17 UTC 2021


        at  d32d37c6b05a1fa292b814e0ab0fae2792be309d (commit)


commit d32d37c6b05a1fa292b814e0ab0fae2792be309d
Author: Lucas Di Pentima <lucas.dipentima at curii.com>
Date:   Thu Aug 26 18:02:01 2021 -0300

    17696: Moves default storage classes loading to keepclient.New().
    
    Also adds integration test that proves that a collection and its keepclient
    use the same default storage classes list.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima at curii.com>

diff --git a/lib/controller/integration_test.go b/lib/controller/integration_test.go
index 685144205..70c2e9e43 100644
--- a/lib/controller/integration_test.go
+++ b/lib/controller/integration_test.go
@@ -150,6 +150,16 @@ func (s *IntegrationSuite) TearDownSuite(c *check.C) {
 	}
 }
 
+func (s *IntegrationSuite) TestDefaultStorageClassesOnCollections(c *check.C) {
+	conn := s.testClusters["z1111"].Conn()
+	rootctx, _, _ := s.testClusters["z1111"].RootClients()
+	userctx, _, kc, _ := s.testClusters["z1111"].UserClients(rootctx, c, conn, s.oidcprovider.AuthEmail, true)
+	c.Assert(len(kc.DefaultStorageClasses) > 0, check.Equals, true)
+	coll, err := conn.CollectionCreate(userctx, arvados.CreateOptions{})
+	c.Assert(err, check.IsNil)
+	c.Assert(coll.StorageClassesDesired, check.DeepEquals, kc.DefaultStorageClasses)
+}
+
 func (s *IntegrationSuite) TestGetCollectionByPDH(c *check.C) {
 	conn1 := s.testClusters["z1111"].Conn()
 	rootctx1, _, _ := s.testClusters["z1111"].RootClients()
@@ -676,6 +686,7 @@ func (s *IntegrationSuite) TestListUsers(c *check.C) {
 	for _, user := range lst.Items {
 		if user.Username == "" {
 			nullUsername = true
+			break
 		}
 	}
 	c.Assert(nullUsername, check.Equals, true)
diff --git a/sdk/go/keepclient/keepclient.go b/sdk/go/keepclient/keepclient.go
index 4baebbd8a..68ac886dd 100644
--- a/sdk/go/keepclient/keepclient.go
+++ b/sdk/go/keepclient/keepclient.go
@@ -141,10 +141,6 @@ func (kc *KeepClient) loadDefaultClasses() error {
 // use.
 func MakeKeepClient(arv *arvadosclient.ArvadosClient) (*KeepClient, error) {
 	kc := New(arv)
-	err := kc.loadDefaultClasses()
-	if err != nil {
-		DebugPrintf("DEBUG: Unable to load the default storage classes cluster config")
-	}
 	return kc, kc.discoverServices()
 }
 
@@ -159,11 +155,16 @@ func New(arv *arvadosclient.ArvadosClient) *KeepClient {
 			defaultReplicationLevel = int(v)
 		}
 	}
-	return &KeepClient{
+	kc := &KeepClient{
 		Arvados:       arv,
 		Want_replicas: defaultReplicationLevel,
 		Retries:       2,
 	}
+	err = kc.loadDefaultClasses()
+	if err != nil {
+		DebugPrintf("DEBUG: Unable to load the default storage classes cluster config")
+	}
+	return kc
 }
 
 // PutHR puts a block given the block hash, a reader, and the number of bytes
diff --git a/sdk/go/keepclient/keepclient_test.go b/sdk/go/keepclient/keepclient_test.go
index ed2cee645..9000b2f50 100644
--- a/sdk/go/keepclient/keepclient_test.go
+++ b/sdk/go/keepclient/keepclient_test.go
@@ -84,8 +84,7 @@ func (s *ServerRequiredSuite) TestDefaultStorageClasses(c *C) {
 	c.Assert(cc, NotNil)
 	c.Assert(cc.(map[string]interface{})["default"], NotNil)
 
-	kc, err := MakeKeepClient(arv)
-	c.Assert(err, IsNil)
+	kc := New(arv)
 	c.Assert(kc.DefaultStorageClasses, DeepEquals, []string{"default"})
 }
 

commit e0cb9fc0fc616f966f8480aea57f21ebcc5ce918
Author: Lucas Di Pentima <lucas.dipentima at curii.com>
Date:   Mon Aug 23 13:08:35 2021 -0300

    17696: Adds default storage classes config loading to KeepClient, with tests.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima at curii.com>

diff --git a/sdk/go/keepclient/keepclient.go b/sdk/go/keepclient/keepclient.go
index 3bc6f4afc..4baebbd8a 100644
--- a/sdk/go/keepclient/keepclient.go
+++ b/sdk/go/keepclient/keepclient.go
@@ -97,17 +97,18 @@ type HTTPClient interface {
 
 // KeepClient holds information about Arvados and Keep servers.
 type KeepClient struct {
-	Arvados            *arvadosclient.ArvadosClient
-	Want_replicas      int
-	localRoots         map[string]string
-	writableLocalRoots map[string]string
-	gatewayRoots       map[string]string
-	lock               sync.RWMutex
-	HTTPClient         HTTPClient
-	Retries            int
-	BlockCache         *BlockCache
-	RequestID          string
-	StorageClasses     []string
+	Arvados               *arvadosclient.ArvadosClient
+	Want_replicas         int
+	localRoots            map[string]string
+	writableLocalRoots    map[string]string
+	gatewayRoots          map[string]string
+	lock                  sync.RWMutex
+	HTTPClient            HTTPClient
+	Retries               int
+	BlockCache            *BlockCache
+	RequestID             string
+	StorageClasses        []string
+	DefaultStorageClasses []string // Set by cluster's exported config
 
 	// set to 1 if all writable services are of disk type, otherwise 0
 	replicasPerService int
@@ -119,11 +120,31 @@ type KeepClient struct {
 	disableDiscovery bool
 }
 
-// MakeKeepClient creates a new KeepClient, calls
+func (kc *KeepClient) loadDefaultClasses() error {
+	scData, err := kc.Arvados.ClusterConfig("StorageClasses")
+	if err != nil {
+		return err
+	}
+	classes := scData.(map[string]interface{})
+	for scName := range classes {
+		scConf, _ := classes[scName].(map[string]interface{})
+		isDefault, ok := scConf["Default"].(bool)
+		if ok && isDefault {
+			kc.DefaultStorageClasses = append(kc.DefaultStorageClasses, scName)
+		}
+	}
+	return nil
+}
+
+// MakeKeepClient creates a new KeepClient, loads default storage classes, calls
 // DiscoverKeepServices(), and returns when the client is ready to
 // use.
 func MakeKeepClient(arv *arvadosclient.ArvadosClient) (*KeepClient, error) {
 	kc := New(arv)
+	err := kc.loadDefaultClasses()
+	if err != nil {
+		DebugPrintf("DEBUG: Unable to load the default storage classes cluster config")
+	}
 	return kc, kc.discoverServices()
 }
 
diff --git a/sdk/go/keepclient/keepclient_test.go b/sdk/go/keepclient/keepclient_test.go
index 62268fa46..ed2cee645 100644
--- a/sdk/go/keepclient/keepclient_test.go
+++ b/sdk/go/keepclient/keepclient_test.go
@@ -24,7 +24,6 @@ import (
 	"git.arvados.org/arvados.git/sdk/go/arvadosclient"
 	"git.arvados.org/arvados.git/sdk/go/arvadostest"
 	. "gopkg.in/check.v1"
-	check "gopkg.in/check.v1"
 )
 
 // Gocheck boilerplate
@@ -52,13 +51,11 @@ func pythonDir() string {
 }
 
 func (s *ServerRequiredSuite) SetUpSuite(c *C) {
-	arvadostest.StartAPI()
 	arvadostest.StartKeep(2, false)
 }
 
 func (s *ServerRequiredSuite) TearDownSuite(c *C) {
 	arvadostest.StopKeep(2)
-	arvadostest.StopAPI()
 }
 
 func (s *ServerRequiredSuite) SetUpTest(c *C) {
@@ -78,9 +75,23 @@ func (s *ServerRequiredSuite) TestMakeKeepClient(c *C) {
 	}
 }
 
+func (s *ServerRequiredSuite) TestDefaultStorageClasses(c *C) {
+	arv, err := arvadosclient.MakeArvadosClient()
+	c.Assert(err, IsNil)
+
+	cc, err := arv.ClusterConfig("StorageClasses")
+	c.Assert(err, IsNil)
+	c.Assert(cc, NotNil)
+	c.Assert(cc.(map[string]interface{})["default"], NotNil)
+
+	kc, err := MakeKeepClient(arv)
+	c.Assert(err, IsNil)
+	c.Assert(kc.DefaultStorageClasses, DeepEquals, []string{"default"})
+}
+
 func (s *ServerRequiredSuite) TestDefaultReplications(c *C) {
 	arv, err := arvadosclient.MakeArvadosClient()
-	c.Assert(err, Equals, nil)
+	c.Assert(err, IsNil)
 
 	kc, err := MakeKeepClient(arv)
 	c.Check(err, IsNil)
@@ -135,7 +146,7 @@ func RunFakeKeepServer(st http.Handler) (ks KeepServer) {
 	// bind to 0.0.0.0 or [::] which is not a valid address for Dial()
 	ks.listener, err = net.ListenTCP("tcp", &net.TCPAddr{IP: []byte{127, 0, 0, 1}, Port: 0})
 	if err != nil {
-		panic(fmt.Sprintf("Could not listen on any port"))
+		panic("Could not listen on any port")
 	}
 	ks.url = fmt.Sprintf("http://%s", ks.listener.Addr().String())
 	go http.Serve(ks.listener, st)
@@ -245,25 +256,46 @@ func (s *StandaloneSuite) TestUploadWithStorageClasses(c *C) {
 func (s *StandaloneSuite) TestPutWithStorageClasses(c *C) {
 	nServers := 5
 	for _, trial := range []struct {
-		replicas      int
-		clientClasses []string
-		putClasses    []string // putClasses takes precedence over clientClasses
-		minRequests   int
-		maxRequests   int
-		success       bool
+		replicas       int
+		defaultClasses []string
+		clientClasses  []string // clientClasses takes precedence over defaultClasses
+		putClasses     []string // putClasses takes precedence over clientClasses
+		minRequests    int
+		maxRequests    int
+		success        bool
 	}{
-		{1, []string{"class1"}, nil, 1, 1, true},
-		{2, []string{"class1"}, nil, 1, 2, true},
-		{3, []string{"class1"}, nil, 2, 3, true},
-		{1, []string{"class1", "class2"}, nil, 1, 1, true},
-		{3, nil, []string{"class1"}, 2, 3, true},
-		{1, nil, []string{"class1", "class2"}, 1, 1, true},
-		{1, []string{"class404"}, []string{"class1", "class2"}, 1, 1, true},
-		{1, []string{"class1"}, []string{"class404", "class2"}, nServers, nServers, false},
-		{nServers*2 + 1, []string{"class1"}, nil, nServers, nServers, false},
-		{1, []string{"class404"}, nil, nServers, nServers, false},
-		{1, []string{"class1", "class404"}, nil, nServers, nServers, false},
-		{1, nil, []string{"class1", "class404"}, nServers, nServers, false},
+		{1, []string{"class1"}, nil, nil, 1, 1, true},
+		{2, []string{"class1"}, nil, nil, 1, 2, true},
+		{3, []string{"class1"}, nil, nil, 2, 3, true},
+		{1, []string{"class1", "class2"}, nil, nil, 1, 1, true},
+
+		// defaultClasses doesn't matter when any of the others is specified.
+		{1, []string{"class1"}, []string{"class1"}, nil, 1, 1, true},
+		{2, []string{"class1"}, []string{"class1"}, nil, 1, 2, true},
+		{3, []string{"class1"}, []string{"class1"}, nil, 2, 3, true},
+		{1, []string{"class1"}, []string{"class1", "class2"}, nil, 1, 1, true},
+		{3, []string{"class1"}, nil, []string{"class1"}, 2, 3, true},
+		{1, []string{"class1"}, nil, []string{"class1", "class2"}, 1, 1, true},
+		{1, []string{"class1"}, []string{"class404"}, []string{"class1", "class2"}, 1, 1, true},
+		{1, []string{"class1"}, []string{"class1"}, []string{"class404", "class2"}, nServers, nServers, false},
+		{nServers*2 + 1, []string{}, []string{"class1"}, nil, nServers, nServers, false},
+		{1, []string{"class1"}, []string{"class404"}, nil, nServers, nServers, false},
+		{1, []string{"class1"}, []string{"class1", "class404"}, nil, nServers, nServers, false},
+		{1, []string{"class1"}, nil, []string{"class1", "class404"}, nServers, nServers, false},
+
+		// Talking to an older cluster (with no default storage classes advertising)
+		{1, nil, []string{"class1"}, nil, 1, 1, true},
+		{2, nil, []string{"class1"}, nil, 1, 2, true},
+		{3, nil, []string{"class1"}, nil, 2, 3, true},
+		{1, nil, []string{"class1", "class2"}, nil, 1, 1, true},
+		{3, nil, nil, []string{"class1"}, 2, 3, true},
+		{1, nil, nil, []string{"class1", "class2"}, 1, 1, true},
+		{1, nil, []string{"class404"}, []string{"class1", "class2"}, 1, 1, true},
+		{1, nil, []string{"class1"}, []string{"class404", "class2"}, nServers, nServers, false},
+		{nServers*2 + 1, []string{}, []string{"class1"}, nil, nServers, nServers, false},
+		{1, nil, []string{"class404"}, nil, nServers, nServers, false},
+		{1, nil, []string{"class1", "class404"}, nil, nServers, nServers, false},
+		{1, nil, nil, []string{"class1", "class404"}, nServers, nServers, false},
 	} {
 		c.Logf("%+v", trial)
 		st := &StubPutHandler{
@@ -280,6 +312,7 @@ func (s *StandaloneSuite) TestPutWithStorageClasses(c *C) {
 		kc, _ := MakeKeepClient(arv)
 		kc.Want_replicas = trial.replicas
 		kc.StorageClasses = trial.clientClasses
+		kc.DefaultStorageClasses = trial.defaultClasses
 		arv.ApiToken = "abc123"
 		localRoots := make(map[string]string)
 		writableLocalRoots := make(map[string]string)
@@ -295,17 +328,17 @@ func (s *StandaloneSuite) TestPutWithStorageClasses(c *C) {
 			StorageClasses: trial.putClasses,
 		})
 		if trial.success {
-			c.Check(err, check.IsNil)
+			c.Check(err, IsNil)
 		} else {
-			c.Check(err, check.NotNil)
+			c.Check(err, NotNil)
 		}
-		c.Check(len(st.handled) >= trial.minRequests, check.Equals, true, check.Commentf("len(st.handled)==%d, trial.minRequests==%d", len(st.handled), trial.minRequests))
-		c.Check(len(st.handled) <= trial.maxRequests, check.Equals, true, check.Commentf("len(st.handled)==%d, trial.maxRequests==%d", len(st.handled), trial.maxRequests))
-		if !trial.success && trial.replicas == 1 && c.Check(len(st.requests) >= 2, check.Equals, true) {
+		c.Check(len(st.handled) >= trial.minRequests, Equals, true, Commentf("len(st.handled)==%d, trial.minRequests==%d", len(st.handled), trial.minRequests))
+		c.Check(len(st.handled) <= trial.maxRequests, Equals, true, Commentf("len(st.handled)==%d, trial.maxRequests==%d", len(st.handled), trial.maxRequests))
+		if !trial.success && trial.replicas == 1 && c.Check(len(st.requests) >= 2, Equals, true) {
 			// Max concurrency should be 1. First request
 			// should have succeeded for class1. Second
 			// request should only ask for class404.
-			c.Check(st.requests[1].Header.Get("X-Keep-Storage-Classes"), check.Equals, "class404")
+			c.Check(st.requests[1].Header.Get("X-Keep-Storage-Classes"), Equals, "class404")
 		}
 	}
 }
@@ -392,7 +425,7 @@ func (s *StandaloneSuite) TestPutB(c *C) {
 		expectPath:           hash,
 		expectAPIToken:       "abc123",
 		expectBody:           "foo",
-		expectStorageClass:   "",
+		expectStorageClass:   "default",
 		returnStorageClasses: "",
 		handled:              make(chan string, 5),
 	}
@@ -436,7 +469,7 @@ func (s *StandaloneSuite) TestPutHR(c *C) {
 		expectPath:           hash,
 		expectAPIToken:       "abc123",
 		expectBody:           "foo",
-		expectStorageClass:   "",
+		expectStorageClass:   "default",
 		returnStorageClasses: "",
 		handled:              make(chan string, 5),
 	}
@@ -487,7 +520,7 @@ func (s *StandaloneSuite) TestPutWithFail(c *C) {
 		expectPath:           hash,
 		expectAPIToken:       "abc123",
 		expectBody:           "foo",
-		expectStorageClass:   "",
+		expectStorageClass:   "default",
 		returnStorageClasses: "",
 		handled:              make(chan string, 4),
 	}
@@ -549,7 +582,7 @@ func (s *StandaloneSuite) TestPutWithTooManyFail(c *C) {
 		expectPath:           hash,
 		expectAPIToken:       "abc123",
 		expectBody:           "foo",
-		expectStorageClass:   "",
+		expectStorageClass:   "default",
 		returnStorageClasses: "",
 		handled:              make(chan string, 1),
 	}
@@ -1159,7 +1192,7 @@ func (s *StandaloneSuite) TestPutBWant2ReplicasWithOnlyOneWritableLocalRoot(c *C
 		expectPath:           hash,
 		expectAPIToken:       "abc123",
 		expectBody:           "foo",
-		expectStorageClass:   "",
+		expectStorageClass:   "default",
 		returnStorageClasses: "",
 		handled:              make(chan string, 5),
 	}
@@ -1378,7 +1411,7 @@ func (s *StandaloneSuite) TestPutBRetry(c *C) {
 			expectPath:           Md5String("foo"),
 			expectAPIToken:       "abc123",
 			expectBody:           "foo",
-			expectStorageClass:   "",
+			expectStorageClass:   "default",
 			returnStorageClasses: "",
 			handled:              make(chan string, 5),
 		},
diff --git a/sdk/go/keepclient/support.go b/sdk/go/keepclient/support.go
index 633ec1896..8d299815b 100644
--- a/sdk/go/keepclient/support.go
+++ b/sdk/go/keepclient/support.go
@@ -164,7 +164,11 @@ func (kc *KeepClient) BlockWrite(ctx context.Context, req arvados.BlockWriteOpti
 		req.Hash = fmt.Sprintf("%x", m.Sum(nil))
 	}
 	if req.StorageClasses == nil {
-		req.StorageClasses = kc.StorageClasses
+		if len(kc.StorageClasses) > 0 {
+			req.StorageClasses = kc.StorageClasses
+		} else {
+			req.StorageClasses = kc.DefaultStorageClasses
+		}
 	}
 	if req.Replicas == 0 {
 		req.Replicas = kc.Want_replicas

commit dd07dbbb6e83137a949634091a536061ede4a9c5
Author: Lucas Di Pentima <lucas.dipentima at curii.com>
Date:   Fri Aug 20 13:09:39 2021 -0300

    17696: Adds ArvadosClient.ClusterConfig() to get the exported cluster's config.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima at curii.com>

diff --git a/sdk/go/arvadosclient/arvadosclient.go b/sdk/go/arvadosclient/arvadosclient.go
index 4c594625e..3081f6d82 100644
--- a/sdk/go/arvadosclient/arvadosclient.go
+++ b/sdk/go/arvadosclient/arvadosclient.go
@@ -426,6 +426,24 @@ func (c *ArvadosClient) Discovery(parameter string) (value interface{}, err erro
 	return value, ErrInvalidArgument
 }
 
+// ClusterConfig returns the value of the given key in the current cluster's
+// exported config. If key is an empty string, it'll return the entire config.
+func (c *ArvadosClient) ClusterConfig(key string) (config interface{}, err error) {
+	var clusterConfig interface{}
+	err = c.Call("GET", "config", "", "", nil, &clusterConfig)
+	if err != nil {
+		return nil, err
+	}
+	if key == "" {
+		return clusterConfig, nil
+	}
+	configData, ok := clusterConfig.(map[string]interface{})[key]
+	if !ok {
+		return nil, ErrInvalidArgument
+	}
+	return configData, nil
+}
+
 func (c *ArvadosClient) httpClient() *http.Client {
 	if c.Client != nil {
 		return c.Client
diff --git a/sdk/go/arvadosclient/arvadosclient_test.go b/sdk/go/arvadosclient/arvadosclient_test.go
index 9d6e4fe7e..27e23c1ae 100644
--- a/sdk/go/arvadosclient/arvadosclient_test.go
+++ b/sdk/go/arvadosclient/arvadosclient_test.go
@@ -158,6 +158,36 @@ func (s *ServerRequiredSuite) TestAPIDiscovery_Get_noSuchParameter(c *C) {
 	c.Assert(value, IsNil)
 }
 
+func (s *ServerRequiredSuite) TestAPIClusterConfig_Get_StorageClasses(c *C) {
+	arv, err := MakeArvadosClient()
+	c.Assert(err, IsNil)
+	data, err := arv.ClusterConfig("StorageClasses")
+	c.Assert(err, IsNil)
+	c.Assert(data, NotNil)
+	clusterConfig := data.(map[string]interface{})
+	_, ok := clusterConfig["default"]
+	c.Assert(ok, Equals, true)
+}
+
+func (s *ServerRequiredSuite) TestAPIClusterConfig_Get_All(c *C) {
+	arv, err := MakeArvadosClient()
+	c.Assert(err, IsNil)
+	data, err := arv.ClusterConfig("")
+	c.Assert(err, IsNil)
+	c.Assert(data, NotNil)
+	clusterConfig := data.(map[string]interface{})
+	_, ok := clusterConfig["StorageClasses"]
+	c.Assert(ok, Equals, true)
+}
+
+func (s *ServerRequiredSuite) TestAPIClusterConfig_Get_noSuchSection(c *C) {
+	arv, err := MakeArvadosClient()
+	c.Assert(err, IsNil)
+	data, err := arv.ClusterConfig("noSuchSection")
+	c.Assert(err, NotNil)
+	c.Assert(data, IsNil)
+}
+
 func (s *ServerRequiredSuite) TestCreateLarge(c *C) {
 	arv, err := MakeArvadosClient()
 	c.Assert(err, IsNil)

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list