[arvados] created: 2.7.1-41-g26fcb9e9e2

[git repository hosting]

        at  26fcb9e9e22526a11badb61b6d782876914008e8 (commit)


commit 26fcb9e9e22526a11badb61b6d782876914008e8
Author: Tom Clegg <tom at curii.com>
Date:   Mon Apr 1 10:09:12 2024 -0400

    21636: Return 500 instead of 404 for unexpected volume errors.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/services/keepstore/handler_test.go b/services/keepstore/handler_test.go
index 5bdafb77c2..64b78a3f66 100644
--- a/services/keepstore/handler_test.go
+++ b/services/keepstore/handler_test.go
@@ -17,6 +17,7 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"net/http"
 	"net/http/httptest"
@@ -185,20 +186,24 @@ func (s *HandlerSuite) TestGetHandler(c *check.C) {
 
 	// Authenticated request, signed locator
 	// => 503 Server busy (transient error)
-
-	// Set up the block owning volume to respond with errors
-	vols[0].Volume.(*MockVolume).Bad = true
-	vols[0].Volume.(*MockVolume).BadVolumeError = VolumeBusyError
-	response = IssueRequest(s.handler, &RequestTester{
-		method:   "GET",
-		uri:      signedLocator,
-		apiToken: knownToken,
-	})
-	// A transient error from one volume while the other doesn't find the block
-	// should make the service return a 503 so that clients can retry.
-	ExpectStatusCode(c,
-		"Volume backend busy",
-		503, response)
+	for _, stubErr := range []error{VolumeBusyError, NotFoundError, errors.New("misc error")} {
+		c.Logf("stubErr %T %s", stubErr, stubErr)
+		// Set up the block owning volume to respond with error
+		vols[0].Volume.(*MockVolume).Bad = true
+		vols[0].Volume.(*MockVolume).BadVolumeError = stubErr
+		response = IssueRequest(s.handler, &RequestTester{
+			method:   "GET",
+			uri:      signedLocator,
+			apiToken: knownToken,
+		})
+		// A transient error from one volume while the other doesn't find the block
+		// should make the service return a 503 so that clients can retry.
+		if err, ok := stubErr.(*KeepError); ok {
+			ExpectStatusCode(c, "volume error status", err.HTTPCode, response)
+		} else {
+			ExpectStatusCode(c, "volume error status", 500, response)
+		}
+	}
 }
 
 // Test PutBlockHandler on the following situations:
diff --git a/services/keepstore/handlers.go b/services/keepstore/handlers.go
index abeb20fe86..35b18d1095 100644
--- a/services/keepstore/handlers.go
+++ b/services/keepstore/handlers.go
@@ -679,7 +679,7 @@ func GetBlock(ctx context.Context, volmgr *RRVolumeManager, hash string, buf []b
 	log := ctxlog.FromContext(ctx)
 
 	// Attempt to read the requested hash from a keep volume.
-	errorToCaller := NotFoundError
+	var errorToCaller error = NotFoundError
 
 	for _, vol := range volmgr.AllReadable() {
 		size, err := vol.Get(ctx, hash, buf)
@@ -688,20 +688,19 @@ func GetBlock(ctx context.Context, volmgr *RRVolumeManager, hash string, buf []b
 			return 0, ErrClientDisconnect
 		default:
 		}
-		if err != nil {
+		if os.IsNotExist(err) {
 			// IsNotExist is an expected error and may be
 			// ignored. All other errors are logged. In
 			// any case we continue trying to read other
 			// volumes. If all volumes report IsNotExist,
 			// we return a NotFoundError.
-			if !os.IsNotExist(err) {
-				log.WithError(err).Errorf("Get(%s) failed on %s", hash, vol)
-			}
-			// If some volume returns a transient error, return it to the caller
-			// instead of "Not found" so it can retry.
-			if err == VolumeBusyError {
-				errorToCaller = err.(*KeepError)
-			}
+			continue
+		} else if err != nil {
+			// If some volume returns a transient error,
+			// return it to the caller instead of "Not
+			// found" so it can retry.
+			log.WithError(err).Errorf("Get(%s) failed on %s", hash, vol)
+			errorToCaller = err
 			continue
 		}
 		// Check the file checksum.

commit 9777dd40e5c95d49ceb5e066d0d55c2afc8ce034
Author: Tom Clegg <tom at curii.com>
Date:   Fri Mar 29 16:58:23 2024 -0400

    21636: Set ExpiryWindow to avoid credential expiry races.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/services/keepstore/s3aws_volume.go b/services/keepstore/s3aws_volume.go
index 18b30f4638..b0520b258a 100644
--- a/services/keepstore/s3aws_volume.go
+++ b/services/keepstore/s3aws_volume.go
@@ -214,7 +214,17 @@ func (v *S3AWSVolume) check(ec2metadataHostname string) error {
 	creds := aws.NewChainProvider(
 		[]aws.CredentialsProvider{
 			aws.NewStaticCredentialsProvider(v.AccessKeyID, v.SecretAccessKey, v.AuthToken),
-			ec2rolecreds.New(ec2metadata.New(cfg)),
+			ec2rolecreds.New(ec2metadata.New(cfg), func(opts *ec2rolecreds.ProviderOptions) {
+				// (from aws-sdk-go-v2 comments)
+				// "allow the credentials to trigger
+				// refreshing prior to the credentials
+				// actually expiring. This is
+				// beneficial so race conditions with
+				// expiring credentials do not cause
+				// request to fail unexpectedly due to
+				// ExpiredTokenException exceptions."
+				opts.ExpiryWindow = time.Minute
+			}),
 		})
 
 	cfg.Credentials = creds

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


hooks/post-receive
--