[arvados] updated: 2.7.0-5896-g4a68dab517

git repository hosting git at public.arvados.org
Sat Feb 10 02:14:45 UTC 2024


Summary of changes:
 sdk/go/keepclient/keepclient.go         |  2 ++
 services/keepstore/keepstore.go         |  9 ++++--
 services/keepstore/keepstore_test.go    | 49 ++++++++++++++++++++++++++++++---
 services/keepstore/proxy_remote_test.go |  2 +-
 services/keepstore/router.go            |  2 ++
 services/keepstore/router_test.go       | 42 ++++++++++++++++++++++++++++
 6 files changed, 98 insertions(+), 8 deletions(-)

       via  4a68dab51715a65ce88c4e2d44070a82ffcfcd83 (commit)
       via  c1b930267f4a270bb408dcd07bfaf5f02bfd820a (commit)
      from  328d837ec0ec33b04a1298d2224c804ceb4fe91e (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 4a68dab51715a65ce88c4e2d44070a82ffcfcd83
Author: Tom Clegg <tom at curii.com>
Date:   Fri Feb 9 21:14:32 2024 -0500

    2960: Update tests, continued.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/sdk/go/keepclient/keepclient.go b/sdk/go/keepclient/keepclient.go
index 2bd7996b59..64f7e47b7e 100644
--- a/sdk/go/keepclient/keepclient.go
+++ b/sdk/go/keepclient/keepclient.go
@@ -75,6 +75,8 @@ type ErrNotFound struct {
 	multipleResponseError
 }
 
+func (*ErrNotFound) HTTPStatus() int { return http.StatusNotFound }
+
 type InsufficientReplicasError struct{ error }
 
 type OversizeBlockError struct{ error }
diff --git a/services/keepstore/keepstore.go b/services/keepstore/keepstore.go
index 66da881b75..11958f1545 100644
--- a/services/keepstore/keepstore.go
+++ b/services/keepstore/keepstore.go
@@ -246,6 +246,7 @@ func (ks *keepstore) BlockRead(ctx context.Context, opts arvados.BlockReadOption
 }
 
 func (ks *keepstore) blockReadRemote(ctx context.Context, opts arvados.BlockReadOptions) (int, error) {
+	ks.logger.Infof("blockReadRemote(%s)", opts.Locator)
 	token := ctxToken(ctx)
 	if token == "" {
 		return 0, errNoTokenProvided
@@ -299,6 +300,7 @@ func (ks *keepstore) blockReadRemote(ctx context.Context, opts arvados.BlockRead
 	}
 	defer ks.bufferPool.Put(buf)
 	writebuf := bytes.NewBuffer(buf[:0])
+	ks.logger.Infof("blockReadRemote(%s): remote read(%s)", opts.Locator, locator)
 	_, err = remoteClient.BlockRead(ctx, arvados.BlockReadOptions{
 		Locator: locator,
 		WriteTo: writebuf,
@@ -654,7 +656,7 @@ func parseLocator(loc string) (locatorInfo, error) {
 			if part[0] == 'A' {
 				li.signed = true
 			}
-			if part[1] == 'R' {
+			if part[0] == 'R' {
 				li.remote = true
 			}
 		}
diff --git a/services/keepstore/keepstore_test.go b/services/keepstore/keepstore_test.go
index 385289a411..4dfbe0bc72 100644
--- a/services/keepstore/keepstore_test.go
+++ b/services/keepstore/keepstore_test.go
@@ -599,6 +599,51 @@ func (s *keepstoreSuite) TestBlockWrite_SkipReadOnly(c *C) {
 	c.Check(ks.mounts["zzzzz-nyw5e-222222222222222"].volume.(*stubVolume).stubLog.String(), HasLen, 0)
 }
 
+func (s *keepstoreSuite) TestParseLocator(c *C) {
+	for _, trial := range []struct {
+		locator string
+		ok      bool
+		expect  locatorInfo
+	}{
+		{locator: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
+			ok: true},
+		{locator: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa+1234",
+			ok: true, expect: locatorInfo{size: 1234}},
+		{locator: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa+1234+Abcdef at abcdef",
+			ok: true, expect: locatorInfo{size: 1234, signed: true}},
+		{locator: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa+1234+Rzzzzz-abcdef",
+			ok: true, expect: locatorInfo{size: 1234, remote: true}},
+		{locator: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa+12345+Zexample+Rzzzzz-abcdef",
+			ok: true, expect: locatorInfo{size: 12345, remote: true}},
+		// invalid: hash length != 32
+		{locator: "",
+			ok: false},
+		{locator: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
+			ok: false},
+		{locator: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa+1234",
+			ok: false},
+		{locator: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaabb",
+			ok: false},
+		{locator: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaabb+1234",
+			ok: false},
+		// invalid: first hint is not size
+		{locator: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa+Abcdef+1234",
+			ok: false},
+	} {
+		c.Logf("=== %s", trial.locator)
+		li, err := parseLocator(trial.locator)
+		if !trial.ok {
+			c.Check(err, NotNil)
+			continue
+		}
+		c.Check(err, IsNil)
+		c.Check(li.hash, Equals, trial.locator[:32])
+		c.Check(li.size, Equals, trial.expect.size)
+		c.Check(li.signed, Equals, trial.expect.signed)
+		c.Check(li.remote, Equals, trial.expect.remote)
+	}
+}
+
 func init() {
 	driver["stub"] = func(params newVolumeParams) (volume, error) {
 		v := &stubVolume{
diff --git a/services/keepstore/proxy_remote_test.go b/services/keepstore/proxy_remote_test.go
index 8479b5a4b4..9787b3232c 100644
--- a/services/keepstore/proxy_remote_test.go
+++ b/services/keepstore/proxy_remote_test.go
@@ -175,7 +175,7 @@ func (s *proxyRemoteSuite) TestProxyRemote(c *check.C) {
 			expectSignature:  true,
 		},
 	} {
-		c.Logf("trial: %s", trial.label)
+		c.Logf("=== trial: %s", trial.label)
 
 		s.remoteKeepRequests = 0
 
diff --git a/services/keepstore/router.go b/services/keepstore/router.go
index d0043c978b..d5b7571409 100644
--- a/services/keepstore/router.go
+++ b/services/keepstore/router.go
@@ -18,6 +18,7 @@ import (
 	"git.arvados.org/arvados.git/lib/service"
 	"git.arvados.org/arvados.git/sdk/go/arvados"
 	"git.arvados.org/arvados.git/sdk/go/auth"
+	"git.arvados.org/arvados.git/sdk/go/ctxlog"
 	"github.com/gorilla/mux"
 )
 
@@ -92,6 +93,7 @@ func (rtr *router) handleBlockRead(w http.ResponseWriter, req *http.Request) {
 		LocalLocator: localLocator,
 	})
 	if n == 0 && err != nil {
+		ctxlog.FromContext(req.Context()).WithError(err).Infof("err type %T", err)
 		rtr.handleError(w, req, err)
 		return
 	}

commit c1b930267f4a270bb408dcd07bfaf5f02bfd820a
Author: Tom Clegg <tom at curii.com>
Date:   Fri Feb 9 13:45:55 2024 -0500

    2960: Update tests, continued.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/services/keepstore/keepstore.go b/services/keepstore/keepstore.go
index d90d7a5b75..66da881b75 100644
--- a/services/keepstore/keepstore.go
+++ b/services/keepstore/keepstore.go
@@ -559,8 +559,9 @@ func (ks *keepstore) BlockTouch(ctx context.Context, locator string) error {
 		}
 		err := mnt.BlockTouch(li.hash)
 		if err == nil {
-			errToCaller = nil
-		} else if !os.IsNotExist(err) && errToCaller != nil {
+			return nil
+		}
+		if !os.IsNotExist(err) {
 			errToCaller = err
 		}
 	}
diff --git a/services/keepstore/keepstore_test.go b/services/keepstore/keepstore_test.go
index 885e9937db..385289a411 100644
--- a/services/keepstore/keepstore_test.go
+++ b/services/keepstore/keepstore_test.go
@@ -325,10 +325,6 @@ func (s *keepstoreSuite) TestBlockWrite_MultipleStorageClasses(c *C) {
 	}
 }
 
-func (s *keepstoreSuite) TestBlockTouch(c *C) {
-	c.Fatal("todo")
-}
-
 func (s *keepstoreSuite) TestBlockTrash(c *C) {
 	s.cluster.Volumes = map[string]arvados.Volume{
 		"zzzzz-nyw5e-000000000000000": {Replication: 1, Driver: "stub"},
diff --git a/services/keepstore/router_test.go b/services/keepstore/router_test.go
index d06e5088ac..04ff665178 100644
--- a/services/keepstore/router_test.go
+++ b/services/keepstore/router_test.go
@@ -149,6 +149,48 @@ func sortCommaSeparated(s string) string {
 	return strings.Join(slice, ", ")
 }
 
+func (s *routerSuite) TestBlockTouch(c *C) {
+	router, cancel := testRouter(c, s.cluster, nil)
+	defer cancel()
+
+	resp := call(router, "TOUCH", "http://example/"+fooHash+"+3", s.cluster.SystemRootToken, nil, nil)
+	c.Check(resp.Code, Equals, http.StatusNotFound)
+
+	vol0 := router.keepstore.mountsW[0].volume.(*stubVolume)
+	err := vol0.BlockWrite(context.Background(), fooHash, []byte("foo"))
+	c.Assert(err, IsNil)
+	vol1 := router.keepstore.mountsW[1].volume.(*stubVolume)
+	err = vol1.BlockWrite(context.Background(), fooHash, []byte("foo"))
+	c.Assert(err, IsNil)
+
+	t1 := time.Now()
+	resp = call(router, "TOUCH", "http://example/"+fooHash+"+3", s.cluster.SystemRootToken, nil, nil)
+	c.Check(resp.Code, Equals, http.StatusOK)
+	t2 := time.Now()
+
+	// Unauthorized request is a no-op
+	resp = call(router, "TOUCH", "http://example/"+fooHash+"+3", arvadostest.ActiveTokenV2, nil, nil)
+	c.Check(resp.Code, Equals, http.StatusForbidden)
+
+	// Volume 0 mtime should be updated
+	t, err := vol0.Mtime(fooHash)
+	c.Check(err, IsNil)
+	c.Check(t.After(t1), Equals, true)
+	c.Check(t.Before(t2), Equals, true)
+
+	// Volume 1 mtime should not be updated
+	t, err = vol1.Mtime(fooHash)
+	c.Check(err, IsNil)
+	c.Check(t.Before(t1), Equals, true)
+
+	err = vol0.BlockTrash(fooHash)
+	c.Assert(err, IsNil)
+	err = vol1.BlockTrash(fooHash)
+	c.Assert(err, IsNil)
+	resp = call(router, "TOUCH", "http://example/"+fooHash+"+3", s.cluster.SystemRootToken, nil, nil)
+	c.Check(resp.Code, Equals, http.StatusNotFound)
+}
+
 func (s *routerSuite) TestBlockTrash(c *C) {
 	router, cancel := testRouter(c, s.cluster, nil)
 	defer cancel()

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list