Feature #17389

Updated by Nico C├ęsar 10 months ago

Goals of this ticket:

* to have all the expected behaviour for keepproxy
* to add to the documentation this behaviour (if needed)
* to add necesary tests to make sure we comply with this behaviour (if needed)

Current tetsts for storage classes and desired replicas:

<pre>
func (s *ServerRequiredSuite) TestStorageClassesHeader(c *C) {
kc := runProxy(c, false, false)
defer closeListener()

// Set up fake keepstore to record request headers
var hdr http.Header
ts := httptest.NewServer(http.HandlerFunc(
func(w http.ResponseWriter, r *http.Request) {
hdr = r.Header
http.Error(w, "Error", http.StatusInternalServerError)
}))
defer ts.Close()

// Point keepproxy router's keepclient to the fake keepstore
sr := map[string]string{
TestProxyUUID: ts.URL,
}
router.(*proxyHandler).KeepClient.SetServiceRoots(sr, sr, sr)

// Set up client to ask for storage classes to keepproxy
kc.StorageClasses = []string{"secure"}
content := []byte("Very important data")
_, _, err := kc.PutB(content)
c.Check(err, NotNil)
c.Check(hdr.Get("X-Keep-Storage-Classes"), Equals, "secure")
}

(..)

func (s *ServerRequiredSuite) TestDesiredReplicas(c *C) {
kc := runProxy(c, false, false)
defer closeListener()

content := []byte("TestDesiredReplicas")
hash := fmt.Sprintf("%x", md5.Sum(content))

for _, kc.Want_replicas = range []int{0, 1, 2} {
locator, rep, err := kc.PutB(content)
c.Check(err, Equals, nil)
c.Check(rep, Equals, kc.Want_replicas)
if rep > 0 {
c.Check(locator, Matches, fmt.Sprintf(`^%s\+%d(\+.+)?$`, hash, len(content)))
}
}
}
</pre>

Relevant struct in sdk/go/keepclient/keepclient.go

<pre>
// 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

// 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

// Disable automatic discovery of keep services
disableDiscovery bool
}
</pre>

To be discussed in groom meeting:

* Is keepproxy already "forwarding to the appropriate keepstore service." (quote from the epic #16107)
*
is there any extra case that needs to be tested? tested
* what is the relevant documentation for this cases? cases

Back