[ARVADOS] updated: 1.2.0-299-g6a7a7920e
Git user
git at public.curoverse.com
Thu Nov 1 11:04:22 EDT 2018
Summary of changes:
lib/controller/fed_collections.go | 18 +++----------
lib/controller/fed_containers.go | 8 +-----
lib/controller/fed_generic.go | 14 +++-------
lib/controller/federation.go | 7 +++--
lib/controller/federation_test.go | 9 +++++++
lib/controller/handler.go | 18 +++++++------
lib/controller/proxy.go | 17 +++---------
sdk/go/arvados/client.go | 57 +++++++++++----------------------------
8 files changed, 49 insertions(+), 99 deletions(-)
via 6a7a7920e8ce4b6f6743d0a644afb87e6bae63c1 (commit)
via b8d46edce637ed32a55f0f46adb4af67d690e4dc (commit)
from b5b9be4f0de954052c91ab8dbfbfe0c101f004c4 (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 6a7a7920e8ce4b6f6743d0a644afb87e6bae63c1
Author: Peter Amstutz <pamstutz at veritasgenetics.com>
Date: Thu Nov 1 11:04:01 2018 -0400
14262: Revert changes to client.go
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <pamstutz at veritasgenetics.com>
diff --git a/sdk/go/arvados/client.go b/sdk/go/arvados/client.go
index 254a0fa7d..cca9f9bf1 100644
--- a/sdk/go/arvados/client.go
+++ b/sdk/go/arvados/client.go
@@ -103,7 +103,7 @@ var reqIDGen = httpserver.IDGenerator{Prefix: "req-"}
// (*http.Client)Do().
func (c *Client) Do(req *http.Request) (*http.Response, error) {
if c.AuthToken != "" {
- req.Header.Set("Authorization", "OAuth2 "+c.AuthToken)
+ req.Header.Add("Authorization", "OAuth2 "+c.AuthToken)
}
if req.Header.Get("X-Request-Id") == "" {
@@ -193,62 +193,37 @@ func anythingToValues(params interface{}) (url.Values, error) {
return urlValues, nil
}
-func (c *Client) MakeRequest(method, path string, body io.Reader, params interface{}) (*http.Request, error) {
+// RequestAndDecode performs an API request and unmarshals the
+// response (which must be JSON) into dst. Method and body arguments
+// are the same as for http.NewRequest(). The given path is added to
+// the server's scheme/host/port to form the request URL. The given
+// params are passed via POST form or query string.
+//
+// path must not contain a query string.
+func (c *Client) RequestAndDecode(dst interface{}, method, path string, body io.Reader, params interface{}) error {
+ if body, ok := body.(io.Closer); ok {
+ // Ensure body is closed even if we error out early
+ defer body.Close()
+ }
urlString := c.apiURL(path)
urlValues, err := anythingToValues(params)
if err != nil {
- return nil, err
+ return err
}
if (method == "GET" || body != nil) && urlValues != nil {
// FIXME: what if params don't fit in URL
u, err := url.Parse(urlString)
if err != nil {
- return nil, err
+ return err
}
u.RawQuery = urlValues.Encode()
urlString = u.String()
}
req, err := http.NewRequest(method, urlString, body)
if err != nil {
- return nil, err
- }
- req.Header.Set("Content-type", "application/x-www-form-urlencoded")
-
- if c.AuthToken != "" {
- req.Header.Set("Authorization", "OAuth2 "+c.AuthToken)
- }
-
- if req.Header.Get("X-Request-Id") == "" {
- reqid, _ := c.context().Value(contextKeyRequestID).(string)
- if reqid == "" {
- reqid = reqIDGen.Next()
- }
- if req.Header == nil {
- req.Header = http.Header{"X-Request-Id": {reqid}}
- } else {
- req.Header.Set("X-Request-Id", reqid)
- }
- }
-
- return req, nil
-}
-
-// RequestAndDecode performs an API request and unmarshals the
-// response (which must be JSON) into dst. Method and body arguments
-// are the same as for http.NewRequest(). The given path is added to
-// the server's scheme/host/port to form the request URL. The given
-// params are passed via POST form or query string.
-//
-// path must not contain a query string.
-func (c *Client) RequestAndDecode(dst interface{}, method, path string, body io.Reader, params interface{}) error {
- if body, ok := body.(io.Closer); ok {
- // Ensure body is closed even if we error out early
- defer body.Close()
- }
- req, err := c.MakeRequest(method, path, body, params)
- if err != nil {
return err
}
+ req.Header.Set("Content-type", "application/x-www-form-urlencoded")
return c.DoAndDecode(dst, req)
}
commit b8d46edce637ed32a55f0f46adb4af67d690e4dc
Author: Peter Amstutz <pamstutz at veritasgenetics.com>
Date: Thu Nov 1 10:19:18 2018 -0400
14262: Move the context deadline to the top of the handler stack
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 88b0f95a0..b9cd20582 100644
--- a/lib/controller/fed_collections.go
+++ b/lib/controller/fed_collections.go
@@ -178,10 +178,7 @@ func (h *collectionFederatedRequestHandler) ServeHTTP(w http.ResponseWriter, req
if clusterId != "" && clusterId != h.handler.Cluster.ClusterID {
// request for remote collection by uuid
- resp, cancel, err := h.handler.remoteClusterRequest(clusterId, req)
- if cancel != nil {
- defer cancel()
- }
+ resp, err := h.handler.remoteClusterRequest(clusterId, req)
newResponse, err := rewriteSignatures(clusterId, "", resp, err)
h.handler.proxy.ForwardResponse(w, newResponse, err)
return
@@ -196,10 +193,7 @@ func (h *collectionFederatedRequestHandler) ServeHTTP(w http.ResponseWriter, req
// Request for collection by PDH. Search the federation.
// First, query the local cluster.
- resp, localClusterRequestCancel, err := h.handler.localClusterRequest(req)
- if localClusterRequestCancel != nil {
- defer localClusterRequestCancel()
- }
+ resp, err := h.handler.localClusterRequest(req)
newResp, err := filterLocalClusterResponse(resp, err)
if newResp != nil || err != nil {
h.handler.proxy.ForwardResponse(w, newResp, err)
@@ -244,19 +238,13 @@ func (h *collectionFederatedRequestHandler) ServeHTTP(w http.ResponseWriter, req
default:
}
- resp, _, err := h.handler.remoteClusterRequest(remote, req)
+ 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
diff --git a/lib/controller/fed_containers.go b/lib/controller/fed_containers.go
index fc627d3fa..e4c80a32c 100644
--- a/lib/controller/fed_containers.go
+++ b/lib/controller/fed_containers.go
@@ -9,7 +9,6 @@ import (
"encoding/json"
"fmt"
"io/ioutil"
- "log"
"net/http"
"git.curoverse.com/arvados.git/sdk/go/auth"
@@ -64,8 +63,6 @@ func remoteContainerRequestCreate(
// If runtime_token is not set, create a new token
if _, ok := containerRequest["runtime_token"]; !ok {
- log.Printf("ok %v", ok)
-
// First make sure supplied token is valid.
creds := auth.NewCredentials()
creds.LoadTokensFromHTTPRequest(req)
@@ -98,10 +95,7 @@ func remoteContainerRequestCreate(
req.ContentLength = int64(buf.Len())
req.Header.Set("Content-Length", fmt.Sprintf("%v", buf.Len()))
- resp, cancel, err := h.handler.remoteClusterRequest(*clusterId, req)
- if cancel != nil {
- defer cancel()
- }
+ resp, err := h.handler.remoteClusterRequest(*clusterId, req)
h.handler.proxy.ForwardResponse(w, resp, err)
return true
}
diff --git a/lib/controller/fed_generic.go b/lib/controller/fed_generic.go
index 7d5b63d31..63e61e690 100644
--- a/lib/controller/fed_generic.go
+++ b/lib/controller/fed_generic.go
@@ -6,7 +6,6 @@ package controller
import (
"bytes"
- "context"
"encoding/json"
"fmt"
"io/ioutil"
@@ -66,16 +65,12 @@ 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, cancel, err = h.handler.localClusterRequest(&remoteReq)
+ resp, err = h.handler.localClusterRequest(&remoteReq)
} else {
- resp, cancel, err = h.handler.remoteClusterRequest(clusterID, &remoteReq)
+ resp, err = h.handler.remoteClusterRequest(clusterID, &remoteReq)
}
rc.collectResponse(resp, err)
- if cancel != nil {
- cancel()
- }
if rc.error != nil {
return nil, "", rc.error
@@ -309,10 +304,7 @@ func (h *genericFederatedRequestHandler) ServeHTTP(w http.ResponseWriter, req *h
if clusterId == "" || clusterId == h.handler.Cluster.ClusterID {
h.next.ServeHTTP(w, req)
} else {
- resp, cancel, err := h.handler.remoteClusterRequest(clusterId, req)
- if cancel != nil {
- defer cancel()
- }
+ resp, err := h.handler.remoteClusterRequest(clusterId, req)
h.handler.proxy.ForwardResponse(w, resp, err)
}
}
diff --git a/lib/controller/federation.go b/lib/controller/federation.go
index 0e016f301..e08a1c167 100644
--- a/lib/controller/federation.go
+++ b/lib/controller/federation.go
@@ -6,7 +6,6 @@ package controller
import (
"bytes"
- "context"
"database/sql"
"encoding/json"
"fmt"
@@ -29,10 +28,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, context.CancelFunc, error) {
+func (h *Handler) remoteClusterRequest(remoteID string, req *http.Request) (*http.Response, error) {
remote, ok := h.Cluster.RemoteClusters[remoteID]
if !ok {
- return nil, nil, HTTPError{fmt.Sprintf("no proxy available for cluster %v", remoteID), http.StatusNotFound}
+ return nil, HTTPError{fmt.Sprintf("no proxy available for cluster %v", remoteID), http.StatusNotFound}
}
scheme := remote.Scheme
if scheme == "" {
@@ -40,7 +39,7 @@ func (h *Handler) remoteClusterRequest(remoteID string, req *http.Request) (*htt
}
saltedReq, err := h.saltAuthToken(req, remoteID)
if err != nil {
- return nil, nil, err
+ return nil, err
}
urlOut := &url.URL{
Scheme: scheme,
diff --git a/lib/controller/federation_test.go b/lib/controller/federation_test.go
index f6bfca302..da640071c 100644
--- a/lib/controller/federation_test.go
+++ b/lib/controller/federation_test.go
@@ -594,6 +594,15 @@ func (s *FederationSuite) TestCreateRemoteContainerRequestCheckRuntimeToken(c *c
`))
req.Header.Set("Authorization", "Bearer "+arvadostest.ActiveToken)
req.Header.Set("Content-type", "application/json")
+
+ np := arvados.NodeProfile{
+ Controller: arvados.SystemServiceInstance{Listen: ":"},
+ RailsAPI: arvados.SystemServiceInstance{Listen: os.Getenv("ARVADOS_TEST_API_HOST"),
+ TLS: true, Insecure: true}}
+ s.testHandler.Cluster.ClusterID = "zzzzz"
+ s.testHandler.Cluster.NodeProfiles["*"] = np
+ s.testHandler.NodeProfile = &np
+
resp := s.testRequest(req)
c.Check(resp.StatusCode, check.Equals, http.StatusOK)
var cr struct {
diff --git a/lib/controller/handler.go b/lib/controller/handler.go
index cbfaaddab..295dde7ca 100644
--- a/lib/controller/handler.go
+++ b/lib/controller/handler.go
@@ -50,6 +50,12 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
req.URL.Path = strings.Replace(req.URL.Path, "//", "/", -1)
}
}
+ if h.Cluster.HTTPRequestTimeout > 0 {
+ ctx, cancel := context.WithDeadline(req.Context(), time.Now().Add(time.Duration(h.Cluster.HTTPRequestTimeout)))
+ req = req.WithContext(ctx)
+ defer cancel()
+ }
+
h.handlerStack.ServeHTTP(w, req)
}
@@ -84,8 +90,7 @@ func (h *Handler) setup() {
h.insecureClient = &ic
h.proxy = &proxy{
- Name: "arvados-controller",
- RequestTimeout: time.Duration(h.Cluster.HTTPRequestTimeout),
+ Name: "arvados-controller",
}
}
@@ -122,10 +127,10 @@ func prepend(next http.Handler, middleware middlewareFunc) http.Handler {
})
}
-func (h *Handler) localClusterRequest(req *http.Request) (*http.Response, context.CancelFunc, error) {
+func (h *Handler) localClusterRequest(req *http.Request) (*http.Response, error) {
urlOut, insecure, err := findRailsAPI(h.Cluster, h.NodeProfile)
if err != nil {
- return nil, nil, err
+ return nil, err
}
urlOut = &url.URL{
Scheme: urlOut.Scheme,
@@ -142,10 +147,7 @@ func (h *Handler) localClusterRequest(req *http.Request) (*http.Response, contex
}
func (h *Handler) proxyRailsAPI(w http.ResponseWriter, req *http.Request, next http.Handler) {
- resp, cancel, err := h.localClusterRequest(req)
- if cancel != nil {
- defer cancel()
- }
+ resp, err := h.localClusterRequest(req)
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 c89b9b36a..c01c15235 100644
--- a/lib/controller/proxy.go
+++ b/lib/controller/proxy.go
@@ -5,18 +5,15 @@
package controller
import (
- "context"
"io"
"net/http"
"net/url"
- "time"
"git.curoverse.com/arvados.git/sdk/go/httpserver"
)
type proxy struct {
- Name string // to use in Via header
- RequestTimeout time.Duration
+ Name string // to use in Via header
}
type HTTPError struct {
@@ -49,7 +46,7 @@ type ResponseFilter func(*http.Response, error) (*http.Response, error)
func (p *proxy) Do(
reqIn *http.Request,
urlOut *url.URL,
- client *http.Client) (*http.Response, context.CancelFunc, error) {
+ client *http.Client) (*http.Response, error) {
// Copy headers from incoming request, then add/replace proxy
// headers like Via and X-Forwarded-For.
@@ -69,22 +66,16 @@ func (p *proxy) Do(
}
hdrOut.Add("Via", reqIn.Proto+" arvados-controller")
- ctx := reqIn.Context()
- var cancel context.CancelFunc
- if p.RequestTimeout > 0 {
- ctx, cancel = context.WithDeadline(ctx, time.Now().Add(time.Duration(p.RequestTimeout)))
- }
-
reqOut := (&http.Request{
Method: reqIn.Method,
URL: urlOut,
Host: reqIn.Host,
Header: hdrOut,
Body: reqIn.Body,
- }).WithContext(ctx)
+ }).WithContext(reqIn.Context())
resp, err := client.Do(reqOut)
- return resp, cancel, err
+ return resp, err
}
// Copy a response (or error) to the downstream client
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list