[ARVADOS] updated: 1.2.0-260-gb11eec52a

Git user git at public.curoverse.com
Mon Oct 29 17:09:53 EDT 2018


Summary of changes:
 lib/controller/fed_collections.go                | 186 +++++++++++------------
 lib/controller/fed_containers.go                 |   5 +-
 lib/controller/fed_generic.go                    |  14 +-
 lib/controller/federation.go                     |   9 +-
 lib/controller/federation_test.go                |   2 +-
 lib/controller/handler.go                        |  12 +-
 lib/controller/proxy.go                          |  14 +-
 sdk/go/httpserver/id_generator.go                |  12 +-
 services/api/app/models/collection.rb            |   9 +-
 services/api/app/models/container.rb             |   9 --
 services/api/test/unit/container_request_test.rb |   9 +-
 11 files changed, 152 insertions(+), 129 deletions(-)

       via  b11eec52a05753cb587621393d5c4649940149fe (commit)
       via  ddf321e922922506df1e06c854d4c77e2f942158 (commit)
       via  2da1851c3fe7b85900f18ec680d6eb77c906dda4 (commit)
      from  e92125d70c4b9b5fcaaee887942d415d4b197753 (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 b11eec52a05753cb587621393d5c4649940149fe
Author: Peter Amstutz <pamstutz at veritasgenetics.com>
Date:   Mon Oct 29 17:09:08 2018 -0400

    14262: Rewrite collectionFederatedRequestHandler PDH search to use channels
    
    Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <pamstutz at veritasgenetics.com>

diff --git a/lib/controller/fed_collections.go b/lib/controller/fed_collections.go
index 8a97c25c9..88b0f95a0 100644
--- a/lib/controller/fed_collections.go
+++ b/lib/controller/fed_collections.go
@@ -159,63 +159,6 @@ type searchRemoteClusterForPDH struct {
 	statusCode    *int
 }
 
-func (s *searchRemoteClusterForPDH) filterRemoteClusterResponse(resp *http.Response, requestError error) (newResponse *http.Response, err error) {
-	s.mtx.Lock()
-	defer s.mtx.Unlock()
-
-	if *s.sentResponse {
-		// Another request already returned a response
-		return nil, nil
-	}
-
-	if requestError != nil {
-		*s.errors = append(*s.errors, fmt.Sprintf("Request error contacting %q: %v", s.remoteID, requestError))
-		// Record the error and suppress response
-		return nil, nil
-	}
-
-	if resp.StatusCode != http.StatusOK {
-		// Suppress returning unsuccessful result.  Maybe
-		// another request will find it.
-		*s.errors = append(*s.errors, fmt.Sprintf("Response to %q from %q: %v", resp.Header.Get(httpserver.HeaderRequestID), s.remoteID, resp.Status))
-		if resp.StatusCode != http.StatusNotFound {
-			// Got a non-404 error response, convert into BadGateway
-			*s.statusCode = http.StatusBadGateway
-		}
-		return nil, nil
-	}
-
-	s.mtx.Unlock()
-
-	// This reads the response body.  We don't want to hold the
-	// lock while doing this because other remote requests could
-	// also have made it to this point, and we don't want a
-	// slow response holding the lock to block a faster response
-	// that is waiting on the lock.
-	newResponse, err = rewriteSignatures(s.remoteID, s.pdh, resp, nil)
-
-	s.mtx.Lock()
-
-	if *s.sentResponse {
-		// Another request already returned a response
-		return nil, nil
-	}
-
-	if err != nil {
-		// Suppress returning unsuccessful result.  Maybe
-		// another request will be successful.
-		*s.errors = append(*s.errors, fmt.Sprintf("Error parsing response from %q: %v", s.remoteID, err))
-		return nil, nil
-	}
-
-	// We have a successful response.  Suppress/cancel all the
-	// other requests/responses.
-	*s.sentResponse = true
-	s.cancelFunc()
-
-	return newResponse, nil
-}
-
 func (h *collectionFederatedRequestHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 	if req.Method != "GET" {
 		// Only handle GET requests right now
@@ -263,58 +206,107 @@ func (h *collectionFederatedRequestHandler) ServeHTTP(w http.ResponseWriter, req
 		return
 	}
 
-	sharedContext, cancelFunc := context.WithCancel(req.Context())
-	defer cancelFunc()
-	req = req.WithContext(sharedContext)
-
 	// Create a goroutine for each cluster in the
 	// RemoteClusters map.  The first valid result gets
 	// returned to the client.  When that happens, all
-	// other outstanding requests are cancelled or
-	// suppressed.
-	sentResponse := false
-	mtx := sync.Mutex{}
+	// other outstanding requests are cancelled
+	sharedContext, cancelFunc := context.WithCancel(req.Context())
+	req = req.WithContext(sharedContext)
 	wg := sync.WaitGroup{}
-	var errors []string
-	var errorCode int = http.StatusNotFound
+	pdh := m[1]
+	success := make(chan *http.Response)
+	errorChan := make(chan error)
 
 	// use channel as a semaphore to limit the number of concurrent
 	// requests at a time
 	sem := make(chan bool, h.handler.Cluster.RequestLimits.GetMultiClusterRequestConcurrency())
+
+	defer close(errorChan)
+	defer close(success)
 	defer close(sem)
+	defer cancelFunc()
+
 	for remoteID := range h.handler.Cluster.RemoteClusters {
 		if remoteID == h.handler.Cluster.ClusterID {
 			// No need to query local cluster again
 			continue
 		}
-		// blocks until it can put a value into the
-		// channel (which has a max queue capacity)
-		sem <- true
-		if sentResponse {
-			break
-		}
-		search := &searchRemoteClusterForPDH{m[1], remoteID, &mtx, &sentResponse,
-			&sharedContext, cancelFunc, &errors, &errorCode}
+
 		wg.Add(1)
-		go func() {
-			resp, cancel, err := h.handler.remoteClusterRequest(search.remoteID, req)
-			if cancel != nil {
-				defer cancel()
+		go func(remote string) {
+			defer wg.Done()
+			// blocks until it can put a value into the
+			// channel (which has a max queue capacity)
+			sem <- true
+			select {
+			case <-sharedContext.Done():
+				return
+			default:
+			}
+
+			resp, _, err := h.handler.remoteClusterRequest(remote, req)
+			wasSuccess := false
+			defer func() {
+				if resp != nil && !wasSuccess {
+					resp.Body.Close()
+				}
+			}()
+			// Don't need to do anything with the cancel
+			// function returned by remoteClusterRequest
+			// because the context inherits from
+			// sharedContext, so when sharedContext is
+			// cancelled it should cancel that one as
+			// well.
+			if err != nil {
+				errorChan <- err
+				return
 			}
-			newResp, err := search.filterRemoteClusterResponse(resp, err)
-			if newResp != nil || err != nil {
-				h.handler.proxy.ForwardResponse(w, newResp, err)
+			if resp.StatusCode != http.StatusOK {
+				errorChan <- HTTPError{resp.Status, resp.StatusCode}
+				return
+			}
+			select {
+			case <-sharedContext.Done():
+				return
+			default:
+			}
+
+			newResponse, err := rewriteSignatures(remote, pdh, resp, nil)
+			if err != nil {
+				errorChan <- err
+				return
+			}
+			select {
+			case <-sharedContext.Done():
+			case success <- newResponse:
+				wasSuccess = true
 			}
-			wg.Done()
 			<-sem
-		}()
+		}(remoteID)
 	}
-	wg.Wait()
+	go func() {
+		wg.Wait()
+		cancelFunc()
+	}()
 
-	if sentResponse {
-		return
-	}
+	var errors []string
+	errorCode := http.StatusNotFound
 
-	// No successful responses, so return the error
-	httpserver.Errors(w, errors, errorCode)
+	for {
+		select {
+		case newResp = <-success:
+			h.handler.proxy.ForwardResponse(w, newResp, nil)
+			return
+		case err := <-errorChan:
+			if httperr, ok := err.(HTTPError); ok {
+				if httperr.Code != http.StatusNotFound {
+					errorCode = http.StatusBadGateway
+				}
+			}
+			errors = append(errors, err.Error())
+		case <-sharedContext.Done():
+			httpserver.Errors(w, errors, errorCode)
+			return
+		}
+	}
 }

commit ddf321e922922506df1e06c854d4c77e2f942158
Author: Peter Amstutz <pamstutz at veritasgenetics.com>
Date:   Mon Oct 29 15:36:45 2018 -0400

    14262: Make sure cancel() from proxy.Do() gets called
    
    Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <pamstutz at veritasgenetics.com>

diff --git a/lib/controller/fed_collections.go b/lib/controller/fed_collections.go
index 70dbdc3f5..8a97c25c9 100644
--- a/lib/controller/fed_collections.go
+++ b/lib/controller/fed_collections.go
@@ -34,7 +34,7 @@ func rewriteSignatures(clusterID string, expectHash string,
 		return resp, requestError
 	}
 
-	if resp.StatusCode != 200 {
+	if resp.StatusCode != http.StatusOK {
 		return resp, nil
 	}
 
@@ -140,7 +140,7 @@ func filterLocalClusterResponse(resp *http.Response, requestError error) (newRes
 		return resp, requestError
 	}
 
-	if resp.StatusCode == 404 {
+	if resp.StatusCode == http.StatusNotFound {
 		// Suppress returning this result, because we want to
 		// search the federation.
 		return nil, nil
@@ -174,12 +174,11 @@ func (s *searchRemoteClusterForPDH) filterRemoteClusterResponse(resp *http.Respo
 		return nil, nil
 	}
 
-	if resp.StatusCode != 200 {
+	if resp.StatusCode != http.StatusOK {
 		// Suppress returning unsuccessful result.  Maybe
 		// another request will find it.
-		// TODO collect and return error responses.
-		*s.errors = append(*s.errors, fmt.Sprintf("Response to %q from %q: %v", httpserver.GetRequestID(resp.Header), s.remoteID, resp.Status))
-		if resp.StatusCode != 404 {
+		*s.errors = append(*s.errors, fmt.Sprintf("Response to %q from %q: %v", resp.Header.Get(httpserver.HeaderRequestID), s.remoteID, resp.Status))
+		if resp.StatusCode != http.StatusNotFound {
 			// Got a non-404 error response, convert into BadGateway
 			*s.statusCode = http.StatusBadGateway
 		}
@@ -236,7 +235,10 @@ func (h *collectionFederatedRequestHandler) ServeHTTP(w http.ResponseWriter, req
 
 		if clusterId != "" && clusterId != h.handler.Cluster.ClusterID {
 			// request for remote collection by uuid
-			resp, err := h.handler.remoteClusterRequest(clusterId, req)
+			resp, cancel, err := h.handler.remoteClusterRequest(clusterId, req)
+			if cancel != nil {
+				defer cancel()
+			}
 			newResponse, err := rewriteSignatures(clusterId, "", resp, err)
 			h.handler.proxy.ForwardResponse(w, newResponse, err)
 			return
@@ -251,7 +253,10 @@ func (h *collectionFederatedRequestHandler) ServeHTTP(w http.ResponseWriter, req
 	// Request for collection by PDH.  Search the federation.
 
 	// First, query the local cluster.
-	resp, err := h.handler.localClusterRequest(req)
+	resp, localClusterRequestCancel, err := h.handler.localClusterRequest(req)
+	if localClusterRequestCancel != nil {
+		defer localClusterRequestCancel()
+	}
 	newResp, err := filterLocalClusterResponse(resp, err)
 	if newResp != nil || err != nil {
 		h.handler.proxy.ForwardResponse(w, newResp, err)
@@ -271,7 +276,7 @@ func (h *collectionFederatedRequestHandler) ServeHTTP(w http.ResponseWriter, req
 	mtx := sync.Mutex{}
 	wg := sync.WaitGroup{}
 	var errors []string
-	var errorCode int = 404
+	var errorCode int = http.StatusNotFound
 
 	// use channel as a semaphore to limit the number of concurrent
 	// requests at a time
@@ -292,7 +297,10 @@ func (h *collectionFederatedRequestHandler) ServeHTTP(w http.ResponseWriter, req
 			&sharedContext, cancelFunc, &errors, &errorCode}
 		wg.Add(1)
 		go func() {
-			resp, err := h.handler.remoteClusterRequest(search.remoteID, req)
+			resp, cancel, err := h.handler.remoteClusterRequest(search.remoteID, req)
+			if cancel != nil {
+				defer cancel()
+			}
 			newResp, err := search.filterRemoteClusterResponse(resp, err)
 			if newResp != nil || err != nil {
 				h.handler.proxy.ForwardResponse(w, newResp, err)
diff --git a/lib/controller/fed_containers.go b/lib/controller/fed_containers.go
index ccb2401bb..a3c292583 100644
--- a/lib/controller/fed_containers.go
+++ b/lib/controller/fed_containers.go
@@ -95,7 +95,10 @@ func remoteContainerRequestCreate(
 	req.ContentLength = int64(buf.Len())
 	req.Header.Set("Content-Length", fmt.Sprintf("%v", buf.Len()))
 
-	resp, err := h.handler.remoteClusterRequest(*clusterId, req)
+	resp, cancel, err := h.handler.remoteClusterRequest(*clusterId, req)
+	if cancel != nil {
+		defer cancel()
+	}
 	h.handler.proxy.ForwardResponse(w, resp, err)
 	return true
 }
diff --git a/lib/controller/fed_generic.go b/lib/controller/fed_generic.go
index 63e61e690..7d5b63d31 100644
--- a/lib/controller/fed_generic.go
+++ b/lib/controller/fed_generic.go
@@ -6,6 +6,7 @@ package controller
 
 import (
 	"bytes"
+	"context"
 	"encoding/json"
 	"fmt"
 	"io/ioutil"
@@ -65,12 +66,16 @@ func (h *genericFederatedRequestHandler) remoteQueryUUIDs(w http.ResponseWriter,
 		rc := multiClusterQueryResponseCollector{clusterID: clusterID}
 
 		var resp *http.Response
+		var cancel context.CancelFunc
 		if clusterID == h.handler.Cluster.ClusterID {
-			resp, err = h.handler.localClusterRequest(&remoteReq)
+			resp, cancel, err = h.handler.localClusterRequest(&remoteReq)
 		} else {
-			resp, err = h.handler.remoteClusterRequest(clusterID, &remoteReq)
+			resp, cancel, err = h.handler.remoteClusterRequest(clusterID, &remoteReq)
 		}
 		rc.collectResponse(resp, err)
+		if cancel != nil {
+			cancel()
+		}
 
 		if rc.error != nil {
 			return nil, "", rc.error
@@ -304,7 +309,10 @@ func (h *genericFederatedRequestHandler) ServeHTTP(w http.ResponseWriter, req *h
 	if clusterId == "" || clusterId == h.handler.Cluster.ClusterID {
 		h.next.ServeHTTP(w, req)
 	} else {
-		resp, err := h.handler.remoteClusterRequest(clusterId, req)
+		resp, cancel, err := h.handler.remoteClusterRequest(clusterId, req)
+		if cancel != nil {
+			defer cancel()
+		}
 		h.handler.proxy.ForwardResponse(w, resp, err)
 	}
 }
diff --git a/lib/controller/federation.go b/lib/controller/federation.go
index dc0aa908c..0e016f301 100644
--- a/lib/controller/federation.go
+++ b/lib/controller/federation.go
@@ -6,6 +6,7 @@ package controller
 
 import (
 	"bytes"
+	"context"
 	"database/sql"
 	"encoding/json"
 	"fmt"
@@ -28,10 +29,10 @@ var containerRequestsRe = regexp.MustCompile(fmt.Sprintf(pathPattern, "container
 var collectionRe = regexp.MustCompile(fmt.Sprintf(pathPattern, "collections", "4zz18"))
 var collectionByPDHRe = regexp.MustCompile(`^/arvados/v1/collections/([0-9a-fA-F]{32}\+[0-9]+)+$`)
 
-func (h *Handler) remoteClusterRequest(remoteID string, req *http.Request) (*http.Response, error) {
+func (h *Handler) remoteClusterRequest(remoteID string, req *http.Request) (*http.Response, context.CancelFunc, error) {
 	remote, ok := h.Cluster.RemoteClusters[remoteID]
 	if !ok {
-		return nil, HTTPError{fmt.Sprintf("no proxy available for cluster %v", remoteID), http.StatusNotFound}
+		return nil, nil, HTTPError{fmt.Sprintf("no proxy available for cluster %v", remoteID), http.StatusNotFound}
 	}
 	scheme := remote.Scheme
 	if scheme == "" {
@@ -39,7 +40,7 @@ func (h *Handler) remoteClusterRequest(remoteID string, req *http.Request) (*htt
 	}
 	saltedReq, err := h.saltAuthToken(req, remoteID)
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 	urlOut := &url.URL{
 		Scheme:   scheme,
@@ -52,7 +53,7 @@ func (h *Handler) remoteClusterRequest(remoteID string, req *http.Request) (*htt
 	if remote.Insecure {
 		client = h.insecureClient
 	}
-	return h.proxy.ForwardRequest(saltedReq, urlOut, client)
+	return h.proxy.Do(saltedReq, urlOut, client)
 }
 
 // Buffer request body, parse form parameters in request, and then
diff --git a/lib/controller/federation_test.go b/lib/controller/federation_test.go
index 7842ad05d..f6bfca302 100644
--- a/lib/controller/federation_test.go
+++ b/lib/controller/federation_test.go
@@ -94,8 +94,8 @@ func (s *FederationSuite) SetUpTest(c *check.C) {
 func (s *FederationSuite) remoteMockHandler(w http.ResponseWriter, req *http.Request) {
 	b := &bytes.Buffer{}
 	io.Copy(b, req.Body)
-	req.Body = ioutil.NopCloser(b)
 	req.Body.Close()
+	req.Body = ioutil.NopCloser(b)
 	s.remoteMockRequests = append(s.remoteMockRequests, *req)
 }
 
diff --git a/lib/controller/handler.go b/lib/controller/handler.go
index 5e9012949..cbfaaddab 100644
--- a/lib/controller/handler.go
+++ b/lib/controller/handler.go
@@ -5,6 +5,7 @@
 package controller
 
 import (
+	"context"
 	"database/sql"
 	"errors"
 	"net"
@@ -121,10 +122,10 @@ func prepend(next http.Handler, middleware middlewareFunc) http.Handler {
 	})
 }
 
-func (h *Handler) localClusterRequest(req *http.Request) (*http.Response, error) {
+func (h *Handler) localClusterRequest(req *http.Request) (*http.Response, context.CancelFunc, error) {
 	urlOut, insecure, err := findRailsAPI(h.Cluster, h.NodeProfile)
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 	urlOut = &url.URL{
 		Scheme:   urlOut.Scheme,
@@ -137,11 +138,14 @@ func (h *Handler) localClusterRequest(req *http.Request) (*http.Response, error)
 	if insecure {
 		client = h.insecureClient
 	}
-	return h.proxy.ForwardRequest(req, urlOut, client)
+	return h.proxy.Do(req, urlOut, client)
 }
 
 func (h *Handler) proxyRailsAPI(w http.ResponseWriter, req *http.Request, next http.Handler) {
-	resp, err := h.localClusterRequest(req)
+	resp, cancel, err := h.localClusterRequest(req)
+	if cancel != nil {
+		defer cancel()
+	}
 	n, err := h.proxy.ForwardResponse(w, resp, err)
 	if err != nil {
 		httpserver.Logger(req).WithError(err).WithField("bytesCopied", n).Error("error copying response body")
diff --git a/lib/controller/proxy.go b/lib/controller/proxy.go
index b7f3c4f72..c89b9b36a 100644
--- a/lib/controller/proxy.go
+++ b/lib/controller/proxy.go
@@ -45,11 +45,11 @@ var dropHeaders = map[string]bool{
 
 type ResponseFilter func(*http.Response, error) (*http.Response, error)
 
-// Forward a request to downstream service, and return response or error.
-func (p *proxy) ForwardRequest(
+// Forward a request to upstream service, and return response or error.
+func (p *proxy) Do(
 	reqIn *http.Request,
 	urlOut *url.URL,
-	client *http.Client) (*http.Response, error) {
+	client *http.Client) (*http.Response, context.CancelFunc, error) {
 
 	// Copy headers from incoming request, then add/replace proxy
 	// headers like Via and X-Forwarded-For.
@@ -70,8 +70,9 @@ func (p *proxy) ForwardRequest(
 	hdrOut.Add("Via", reqIn.Proto+" arvados-controller")
 
 	ctx := reqIn.Context()
+	var cancel context.CancelFunc
 	if p.RequestTimeout > 0 {
-		ctx, _ = context.WithDeadline(ctx, time.Now().Add(time.Duration(p.RequestTimeout)))
+		ctx, cancel = context.WithDeadline(ctx, time.Now().Add(time.Duration(p.RequestTimeout)))
 	}
 
 	reqOut := (&http.Request{
@@ -82,10 +83,11 @@ func (p *proxy) ForwardRequest(
 		Body:   reqIn.Body,
 	}).WithContext(ctx)
 
-	return client.Do(reqOut)
+	resp, err := client.Do(reqOut)
+	return resp, cancel, err
 }
 
-// Copy a response (or error) to the upstream client
+// Copy a response (or error) to the downstream client
 func (p *proxy) ForwardResponse(w http.ResponseWriter, resp *http.Response, err error) (int64, error) {
 	if err != nil {
 		if he, ok := err.(HTTPError); ok {
diff --git a/sdk/go/httpserver/id_generator.go b/sdk/go/httpserver/id_generator.go
index 6093a8a7b..14d89873b 100644
--- a/sdk/go/httpserver/id_generator.go
+++ b/sdk/go/httpserver/id_generator.go
@@ -12,6 +12,10 @@ import (
 	"time"
 )
 
+const (
+	HeaderRequestID = "X-Request-Id"
+)
+
 // IDGenerator generates alphanumeric strings suitable for use as
 // unique IDs (a given IDGenerator will never return the same ID
 // twice).
@@ -44,16 +48,12 @@ func (g *IDGenerator) Next() string {
 func AddRequestIDs(h http.Handler) http.Handler {
 	gen := &IDGenerator{Prefix: "req-"}
 	return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
-		if req.Header.Get("X-Request-Id") == "" {
+		if req.Header.Get(HeaderRequestID) == "" {
 			if req.Header == nil {
 				req.Header = http.Header{}
 			}
-			req.Header.Set("X-Request-Id", gen.Next())
+			req.Header.Set(HeaderRequestID, gen.Next())
 		}
 		h.ServeHTTP(w, req)
 	})
 }
-
-func GetRequestID(h http.Header) string {
-	return h.Get("X-Request-Id")
-}

commit 2da1851c3fe7b85900f18ec680d6eb77c906dda4
Author: Peter Amstutz <pamstutz at veritasgenetics.com>
Date:   Mon Oct 29 14:56:26 2018 -0400

    14262: Only allow unknown PDH for images when there are remote_hosts
    
    Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <pamstutz at veritasgenetics.com>

diff --git a/services/api/app/models/collection.rb b/services/api/app/models/collection.rb
index 718ffc0d0..487043ee3 100644
--- a/services/api/app/models/collection.rb
+++ b/services/api/app/models/collection.rb
@@ -496,7 +496,14 @@ class Collection < ArvadosModel
     if loc = Keep::Locator.parse(search_term)
       loc.strip_hints!
       coll_match = readable_by(*readers).where(portable_data_hash: loc.to_s).limit(1)
-      return get_compatible_images(readers, pattern, coll_match)
+      if coll_match.any? or Rails.configuration.remote_hosts.length == 0
+        return get_compatible_images(readers, pattern, coll_match)
+      else
+        # Allow bare pdh that doesn't exist in the local database so
+        # that federated container requests which refer to remotely
+        # stored containers will validate.
+        return [Collection.new(portable_data_hash: loc.to_s)]
+      end
     end
 
     if search_tag.nil? and (n = search_term.index(":"))
diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb
index e469a49be..0d8453174 100644
--- a/services/api/app/models/container.rb
+++ b/services/api/app/models/container.rb
@@ -248,15 +248,6 @@ class Container < ArvadosModel
   def self.resolve_container_image(container_image)
     coll = Collection.for_latest_docker_image(container_image)
     if !coll
-      if loc = Keep::Locator.parse(container_image)
-        loc.strip_hints!
-        if !Collection.readable_by(current_user).where(portable_data_hash: loc.to_s).any?
-          # Allow bare pdh that doesn't exist in the local database so
-          # that federated container requests which refer to remotely
-          # stored containers will validate.
-          return loc.to_s
-        end
-      end
       raise ArvadosModel::UnresolvableContainerError.new "docker image #{container_image.inspect} not found"
     end
     coll.portable_data_hash
diff --git a/services/api/test/unit/container_request_test.rb b/services/api/test/unit/container_request_test.rb
index e4c3399c4..0fafb9903 100644
--- a/services/api/test/unit/container_request_test.rb
+++ b/services/api/test/unit/container_request_test.rb
@@ -500,7 +500,8 @@ class ContainerRequestTest < ActiveSupport::TestCase
     end
   end
 
-  ['ENOEXIST',
+  ['acbd18db4cc2f85cedef654fccc4a4d8+3',
+   'ENOEXIST',
    'arvados/apitestfixture:ENOEXIST',
   ].each do |img|
     test "container_image_for_container(#{img.inspect}) => 422" do
@@ -511,6 +512,12 @@ class ContainerRequestTest < ActiveSupport::TestCase
     end
   end
 
+  test "allow unrecognized container when there are remote_hosts" do
+    set_user_from_auth :active
+    Rails.configuration.remote_hosts = {"foooo" => "bar.com"}
+    Container.resolve_container_image('acbd18db4cc2f85cedef654fccc4a4d8+3')
+  end
+
   test "migrated docker image" do
     Rails.configuration.docker_image_formats = ['v2']
     add_docker19_migration_link

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list