17696: Adds default storage classes config loading to KeepClient, with tests.

Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima@curii.com>

diff --git a/sdk/go/keepclient/keepclient.go b/sdk/go/keepclient/keepclient.go
index 3bc6f4afcddf7cea0e5b77b140ac1b7c6b3a7a62..4baebbd8a1913ae41f7a153fd4e87bb99ecce8d2 100644 (file)
--- 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 cddf03bc37de5ba4661674e62029585dee1e61b6..ed2cee6451dac81a29ae38dbb175d98df195e0d1 100644 (file)
--- 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
@@ -76,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)
@@ -133,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)
@@ -243,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{
@@ -278,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)
@@ -293,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")
                }
        }
 }
@@ -390,7 +425,7 @@ func (s *StandaloneSuite) TestPutB(c *C) {
                expectPath:           hash,
                expectAPIToken:       "abc123",
                expectBody:           "foo",
-               expectStorageClass:   "",
+               expectStorageClass:   "default",
                returnStorageClasses: "",
                handled:              make(chan string, 5),
        }
@@ -434,7 +469,7 @@ func (s *StandaloneSuite) TestPutHR(c *C) {
                expectPath:           hash,
                expectAPIToken:       "abc123",
                expectBody:           "foo",
-               expectStorageClass:   "",
+               expectStorageClass:   "default",
                returnStorageClasses: "",
                handled:              make(chan string, 5),
        }
@@ -485,7 +520,7 @@ func (s *StandaloneSuite) TestPutWithFail(c *C) {
                expectPath:           hash,
                expectAPIToken:       "abc123",
                expectBody:           "foo",
-               expectStorageClass:   "",
+               expectStorageClass:   "default",
                returnStorageClasses: "",
                handled:              make(chan string, 4),
        }
@@ -547,7 +582,7 @@ func (s *StandaloneSuite) TestPutWithTooManyFail(c *C) {
                expectPath:           hash,
                expectAPIToken:       "abc123",
                expectBody:           "foo",
-               expectStorageClass:   "",
+               expectStorageClass:   "default",
                returnStorageClasses: "",
                handled:              make(chan string, 1),
        }
@@ -1157,7 +1192,7 @@ func (s *StandaloneSuite) TestPutBWant2ReplicasWithOnlyOneWritableLocalRoot(c *C
                expectPath:           hash,
                expectAPIToken:       "abc123",
                expectBody:           "foo",
-               expectStorageClass:   "",
+               expectStorageClass:   "default",
                returnStorageClasses: "",
                handled:              make(chan string, 5),
        }
@@ -1376,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 633ec1896858bd484d6740b8e9dea074c12d82c9..8d299815b2dbd1d0bd52d60b9f5936904811a0b9 100644 (file)
--- 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