[arvados] created: 2.6.0-263-gfb5de708b

git repository hosting git at public.arvados.org
Fri Jun 9 13:32:24 UTC 2023


        at  fb5de708b8e1466d2e541a618b9eca7c92a7d6f2 (commit)


commit fb5de708b8e1466d2e541a618b9eca7c92a7d6f2
Author: Tom Clegg <tom at curii.com>
Date:   Fri Jun 9 09:31:37 2023 -0400

    20425: Fix response status for mixed-status backend errors.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/lib/controller/federation/conn.go b/lib/controller/federation/conn.go
index 679b24ea5..b4b747644 100644
--- a/lib/controller/federation/conn.go
+++ b/lib/controller/federation/conn.go
@@ -178,20 +178,29 @@ func (conn *Conn) tryLocalThenRemotes(ctx context.Context, forwardedFor string,
 			errchan <- fn(ctx, remoteID, be)
 		}()
 	}
-	all404 := true
+	returncode := http.StatusNotFound
 	var errs []error
 	for i := 0; i < cap(errchan); i++ {
 		err := <-errchan
 		if err == nil {
 			return nil
 		}
-		all404 = all404 && errStatus(err) == http.StatusNotFound
 		errs = append(errs, err)
+		if code := errStatus(err); code >= 500 || code == http.StatusTooManyRequests {
+			// If any of the remotes have a retryable
+			// error (and none succeed) we'll return 502.
+			returncode = http.StatusBadGateway
+		} else if code != http.StatusNotFound && returncode != http.StatusBadGateway {
+			// If some of the remotes have non-retryable
+			// non-404 errors (and none succeed or have
+			// retryable errors) we'll return 422.
+			returncode = http.StatusUnprocessableEntity
+		}
 	}
-	if all404 {
+	if returncode == http.StatusNotFound {
 		return notFoundError{}
 	}
-	return httpErrorf(http.StatusBadGateway, "errors: %v", errs)
+	return httpErrorf(returncode, "errors: %v", errs)
 }
 
 func (conn *Conn) CollectionCreate(ctx context.Context, options arvados.CreateOptions) (arvados.Collection, error) {

commit dfd4060738737bad15cc5f34f44a81003661ae6e
Author: Tom Clegg <tom at curii.com>
Date:   Mon Jun 5 15:15:17 2023 -0400

    20425: Test response status for mixed-status backend errors.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/lib/controller/federation/collection_test.go b/lib/controller/federation/collection_test.go
new file mode 100644
index 000000000..8256819ef
--- /dev/null
+++ b/lib/controller/federation/collection_test.go
@@ -0,0 +1,106 @@
+// Copyright (C) The Arvados Authors. All rights reserved.
+//
+// SPDX-License-Identifier: AGPL-3.0
+
+package federation
+
+import (
+	"context"
+	"fmt"
+	"net/http"
+
+	"git.arvados.org/arvados.git/lib/ctrlctx"
+	"git.arvados.org/arvados.git/sdk/go/arvados"
+	"git.arvados.org/arvados.git/sdk/go/arvadostest"
+	"git.arvados.org/arvados.git/sdk/go/auth"
+	"git.arvados.org/arvados.git/sdk/go/ctxlog"
+	"git.arvados.org/arvados.git/sdk/go/httpserver"
+	check "gopkg.in/check.v1"
+)
+
+var _ = check.Suite(&collectionSuite{})
+
+type collectionSuite struct {
+	FederationSuite
+}
+
+func (s *collectionSuite) TestMultipleBackendFailureStatus(c *check.C) {
+	nxPDH := "a4f995dd0c08216f37cb1bdec990f0cd+1234"
+	s.cluster.ClusterID = "local"
+	for _, trial := range []struct {
+		label        string
+		token        string
+		localStatus  int
+		remoteStatus map[string]int
+		expectStatus int
+	}{
+		{
+			"all backends return 404 => 404",
+			arvadostest.SystemRootToken,
+			http.StatusNotFound,
+			map[string]int{
+				"aaaaa": http.StatusNotFound,
+				"bbbbb": http.StatusNotFound,
+			},
+			http.StatusNotFound,
+		},
+		{
+			"all backends return 401 => 401 (e.g., bad token)",
+			arvadostest.SystemRootToken,
+			http.StatusUnauthorized,
+			map[string]int{
+				"aaaaa": http.StatusUnauthorized,
+				"bbbbb": http.StatusUnauthorized,
+			},
+			http.StatusUnauthorized,
+		},
+		{
+			"local 404, remotes 403 => 422 (mix of non-retryable errors)",
+			arvadostest.SystemRootToken,
+			http.StatusNotFound,
+			map[string]int{
+				"aaaaa": http.StatusForbidden,
+				"bbbbb": http.StatusForbidden,
+			},
+			http.StatusUnprocessableEntity,
+		},
+		{
+			"local 404, remotes 401/403/404 => 422 (mix of non-retryable errors)",
+			arvadostest.SystemRootToken,
+			http.StatusNotFound,
+			map[string]int{
+				"aaaaa": http.StatusUnauthorized,
+				"bbbbb": http.StatusForbidden,
+				"ccccc": http.StatusNotFound,
+			},
+			http.StatusUnprocessableEntity,
+		},
+		{
+			"local 404, remotes 401/403/500 => 502 (at least one remote is retryable)",
+			arvadostest.SystemRootToken,
+			http.StatusNotFound,
+			map[string]int{
+				"aaaaa": http.StatusUnauthorized,
+				"bbbbb": http.StatusForbidden,
+				"ccccc": http.StatusInternalServerError,
+			},
+			http.StatusBadGateway,
+		},
+	} {
+		c.Logf("trial: %v", trial)
+		s.fed = New(s.ctx, s.cluster, nil, (&ctrlctx.DBConnector{PostgreSQL: s.cluster.PostgreSQL}).GetDB)
+		s.fed.local = &arvadostest.APIStub{Error: httpserver.ErrorWithStatus(fmt.Errorf("stub error %d", trial.localStatus), trial.localStatus)}
+		for id, status := range trial.remoteStatus {
+			s.addDirectRemote(c, id, &arvadostest.APIStub{Error: httpserver.ErrorWithStatus(fmt.Errorf("stub error %d", status), status)})
+		}
+
+		ctx := context.Background()
+		ctx = ctxlog.Context(ctx, ctxlog.TestLogger(c))
+		if trial.token != "" {
+			ctx = auth.NewContext(ctx, &auth.Credentials{Tokens: []string{trial.token}})
+		}
+
+		_, err := s.fed.CollectionGet(s.ctx, arvados.GetOptions{UUID: nxPDH})
+		c.Check(err.(httpserver.HTTPStatusError).HTTPStatus(), check.Equals, trial.expectStatus)
+	}
+}

commit 03897749e81684d7aef47e1df18c5d47e1bf8c25
Author: Tom Clegg <tom at curii.com>
Date:   Mon Jun 5 10:50:07 2023 -0400

    20425: Revert "12684/20425: Skip TestContainerInputOnDifferentCluster"
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/lib/controller/integration_test.go b/lib/controller/integration_test.go
index 0557aa3fd..a47d93edc 100644
--- a/lib/controller/integration_test.go
+++ b/lib/controller/integration_test.go
@@ -1149,15 +1149,6 @@ func (s *IntegrationSuite) TestRunTrivialContainer(c *check.C) {
 }
 
 func (s *IntegrationSuite) TestContainerInputOnDifferentCluster(c *check.C) {
-	// As of Arvados 2.6.2 (April 2023), this test was going down the
-	// `if outcoll.UUID == ""` branch, checking that FUSE reports a specific
-	// error.
-	// With increased PySDK/FUSE retries from #12684, this test now trips up
-	// on #20425. The test times out as FUSE spends a long time retrying a
-	// request that will never succeed.
-	// This early skip can be removed after #20425 is fixed.
-	c.Skip("blocked by <https://dev.arvados.org/issues/20425>")
-	return
 	conn := s.super.Conn("z1111")
 	rootctx, _, _ := s.super.RootClients("z1111")
 	userctx, ac, _, _ := s.super.UserClients("z1111", rootctx, c, conn, s.oidcprovider.AuthEmail, true)

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list