[ARVADOS] updated: a72b3d08c70f615c5a67ffd4cf816e5502edd629

git at public.curoverse.com git at public.curoverse.com
Tue Sep 29 20:34:55 EDT 2015


Summary of changes:
 doc/_includes/_install_ruby_and_bundler.liquid     |   3 +-
 doc/install/install-api-server.html.textile.liquid |   2 +-
 doc/install/install-sso.html.textile.liquid        | 255 +++++++++++----------
 .../install-workbench-app.html.textile.liquid      |   4 +-
 sdk/go/keepclient/keepclient.go                    |  38 ++-
 sdk/go/keepclient/keepclient_test.go               |  22 +-
 services/api/script/salvage_collection.rb          |   1 +
 services/keepproxy/keepproxy.go                    |  69 +++---
 services/keepproxy/keepproxy_test.go               |  34 ++-
 9 files changed, 228 insertions(+), 200 deletions(-)

       via  a72b3d08c70f615c5a67ffd4cf816e5502edd629 (commit)
       via  34227825eec21d3a393d4467b7ed768b52ac32b1 (commit)
       via  4e8c321b344d8ec23de7ee7c68b2553d87defb67 (commit)
       via  3a4b7ec9daecbff438c83380c6beccaf88d9e47c (commit)
       via  f3f86fcf67775df91937392dd74a527dcbcf1886 (commit)
       via  f235c17c253d9ece933c972039e4872ef7157533 (commit)
       via  d084c7b404fa0cb597fbba4dcb007c087da49ba5 (commit)
       via  f9620be04d0197fe88e3b4d1a1ee54231632cae6 (commit)
       via  8752f4d9f578b8898eefd739464c35c1f34fe1f9 (commit)
       via  9446aefa68d0304c368e60359a2376fc72a5b1b4 (commit)
       via  c6d10ad822ac94453d74371af0bf57072c43f018 (commit)
       via  88b4f320eb101cdac88d2b7ee15135dd67703d20 (commit)
       via  3677f1aea53f1a04811270f98a681dbcd6002e67 (commit)
      from  eaf51fcce5551ab66b6e7453938db063871845a7 (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 a72b3d08c70f615c5a67ffd4cf816e5502edd629
Author: radhika <radhika at curoverse.com>
Date:   Tue Sep 29 20:31:00 2015 -0400

    7200: Use if statement instead of switch to check http method in keepclient; strip terminating empty line from response in keepclient.
    Update tests accordingly. Also, improve keepproxy test to verify that "other" locators are included when no prefix is used in GetIndex requests.

diff --git a/sdk/go/keepclient/keepclient.go b/sdk/go/keepclient/keepclient.go
index c7a543c..314ebd5 100644
--- a/sdk/go/keepclient/keepclient.go
+++ b/sdk/go/keepclient/keepclient.go
@@ -2,6 +2,7 @@
 package keepclient
 
 import (
+	"bytes"
 	"crypto/md5"
 	"crypto/tls"
 	"errors"
@@ -29,14 +30,11 @@ var MissingArvadosApiHost = errors.New("Missing required environment variable AR
 var MissingArvadosApiToken = errors.New("Missing required environment variable ARVADOS_API_TOKEN")
 var InvalidLocatorError = errors.New("Invalid locator")
 
-// ErrorGetIndex is the generic GetIndex error
-var ErrorGetIndex = errors.New("Error getting index")
+// ErrNoSuchKeepServer is returned when GetIndex is invoked with a UUID with no matching keep server
+var ErrNoSuchKeepServer = errors.New("No keep server matching the given UUID is found")
 
-// ErrorNoSuchKeepServer is returned when GetIndex is invoked with a UUID with no matching keep server
-var ErrorNoSuchKeepServer = errors.New("No keep server matching the given UUID is found")
-
-// ErrorIncompleteIndex is returned when the Index response does not end with a new empty line
-var ErrorIncompleteIndex = errors.New("Got incomplete index")
+// ErrIncompleteIndex is returned when the Index response does not end with a new empty line
+var ErrIncompleteIndex = errors.New("Got incomplete index")
 
 const X_Keep_Desired_Replicas = "X-Keep-Desired-Replicas"
 const X_Keep_Replicas_Stored = "X-Keep-Replicas-Stored"
@@ -199,8 +197,7 @@ func (kc *KeepClient) Ask(locator string) (int64, string, error) {
 func (kc *KeepClient) GetIndex(keepServiceUUID, prefix string) (io.Reader, error) {
 	url := kc.LocalRoots()[keepServiceUUID]
 	if url == "" {
-		log.Printf("No such keep server found: %v", keepServiceUUID)
-		return nil, ErrorNoSuchKeepServer
+		return nil, ErrNoSuchKeepServer
 	}
 
 	url += "/index"
@@ -210,36 +207,35 @@ func (kc *KeepClient) GetIndex(keepServiceUUID, prefix string) (io.Reader, error
 
 	req, err := http.NewRequest("GET", url, nil)
 	if err != nil {
-		log.Printf("GET index error: %v", err)
-		return nil, ErrorGetIndex
+		return nil, err
 	}
 
 	req.Header.Add("Authorization", fmt.Sprintf("OAuth2 %s", kc.Arvados.ApiToken))
 	resp, err := kc.Client.Do(req)
-	if err != nil || resp.StatusCode != http.StatusOK {
-		log.Printf("GET index error: %v; status code: %v", err, resp.StatusCode)
-		return nil, ErrorGetIndex
+	if err != nil {
+		return nil, err
+	}
+	if resp.StatusCode != http.StatusOK {
+		return nil, fmt.Errorf("Got http status code: %d", resp.StatusCode)
 	}
 
 	var respBody []byte
 	if resp.Body != nil {
 		respBody, err = ioutil.ReadAll(resp.Body)
 		if err != nil {
-			log.Printf("GET index error: %v", err)
-			return nil, ErrorGetIndex
+			return nil, err
 		}
 
 		// Got index; verify that it is complete
 		// The response should be "\n" if no locators matched the prefix
 		// Else, it should be a list of locators followed by a blank line
-		if (!strings.HasSuffix(string(respBody), "\n\n")) && (string(respBody) != "\n") {
-			log.Printf("Got incomplete index for %v", url)
-			return nil, ErrorIncompleteIndex
+		if !bytes.Equal(respBody, []byte("\n")) && !bytes.HasSuffix(respBody, []byte("\n\n")) {
+			return nil, ErrIncompleteIndex
 		}
 	}
 
-	// Got complete index
-	return strings.NewReader(string(respBody)), nil
+	// Got complete index; strip the trailing newline and send
+	return bytes.NewReader(respBody[0 : len(respBody)-1]), nil
 }
 
 // LocalRoots() returns the map of local (i.e., disk and proxy) Keep
diff --git a/sdk/go/keepclient/keepclient_test.go b/sdk/go/keepclient/keepclient_test.go
index 3724cdf..ee60d28 100644
--- a/sdk/go/keepclient/keepclient_test.go
+++ b/sdk/go/keepclient/keepclient_test.go
@@ -952,14 +952,14 @@ func (s *StandaloneSuite) TestPutBWithNoWritableLocalRoots(c *C) {
 type StubGetIndexHandler struct {
 	c              *C
 	expectPath     string
-	expectApiToken string
+	expectAPIToken string
 	httpStatus     int
 	body           []byte
 }
 
 func (h StubGetIndexHandler) ServeHTTP(resp http.ResponseWriter, req *http.Request) {
 	h.c.Check(req.URL.Path, Equals, h.expectPath)
-	h.c.Check(req.Header.Get("Authorization"), Equals, fmt.Sprintf("OAuth2 %s", h.expectApiToken))
+	h.c.Check(req.Header.Get("Authorization"), Equals, fmt.Sprintf("OAuth2 %s", h.expectAPIToken))
 	resp.WriteHeader(h.httpStatus)
 	resp.Header().Set("Content-Length", fmt.Sprintf("%d", len(h.body)))
 	resp.Write(h.body)
@@ -973,7 +973,7 @@ func (s *StandaloneSuite) TestGetIndexWithNoPrefix(c *C) {
 		"/index",
 		"abc123",
 		http.StatusOK,
-		[]byte(string(hash) + "\n\n")}
+		[]byte(hash + "+3 1443559274\n\n")}
 
 	ks := RunFakeKeepServer(st)
 	defer ks.listener.Close()
@@ -988,7 +988,7 @@ func (s *StandaloneSuite) TestGetIndexWithNoPrefix(c *C) {
 
 	content, err2 := ioutil.ReadAll(r)
 	c.Check(err2, Equals, nil)
-	c.Check(content, DeepEquals, st.body)
+	c.Check(content, DeepEquals, st.body[0:len(st.body)-1])
 }
 
 func (s *StandaloneSuite) TestGetIndexWithPrefix(c *C) {
@@ -999,7 +999,7 @@ func (s *StandaloneSuite) TestGetIndexWithPrefix(c *C) {
 		"/index/" + hash[0:3],
 		"abc123",
 		http.StatusOK,
-		[]byte(string(hash) + "\n\n")}
+		[]byte(hash + "+3 1443559274\n\n")}
 
 	ks := RunFakeKeepServer(st)
 	defer ks.listener.Close()
@@ -1014,7 +1014,7 @@ func (s *StandaloneSuite) TestGetIndexWithPrefix(c *C) {
 
 	content, err2 := ioutil.ReadAll(r)
 	c.Check(err2, Equals, nil)
-	c.Check(content, DeepEquals, st.body)
+	c.Check(content, DeepEquals, st.body[0:len(st.body)-1])
 }
 
 func (s *StandaloneSuite) TestGetIndexIncomplete(c *C) {
@@ -1025,7 +1025,7 @@ func (s *StandaloneSuite) TestGetIndexIncomplete(c *C) {
 		"/index/" + hash[0:3],
 		"abc123",
 		http.StatusOK,
-		[]byte(string(hash))}
+		[]byte(hash)}
 
 	ks := RunFakeKeepServer(st)
 	defer ks.listener.Close()
@@ -1036,7 +1036,7 @@ func (s *StandaloneSuite) TestGetIndexIncomplete(c *C) {
 	kc.SetServiceRoots(map[string]string{"x": ks.url}, map[string]string{ks.url: ""}, nil)
 
 	_, err = kc.GetIndex("x", hash[0:3])
-	c.Check(err, Equals, ErrorIncompleteIndex)
+	c.Check(err, Equals, ErrIncompleteIndex)
 }
 
 func (s *StandaloneSuite) TestGetIndexWithNoSuchServer(c *C) {
@@ -1047,7 +1047,7 @@ func (s *StandaloneSuite) TestGetIndexWithNoSuchServer(c *C) {
 		"/index/" + hash[0:3],
 		"abc123",
 		http.StatusOK,
-		[]byte(string(hash))}
+		[]byte(hash)}
 
 	ks := RunFakeKeepServer(st)
 	defer ks.listener.Close()
@@ -1058,7 +1058,7 @@ func (s *StandaloneSuite) TestGetIndexWithNoSuchServer(c *C) {
 	kc.SetServiceRoots(map[string]string{"x": ks.url}, map[string]string{ks.url: ""}, nil)
 
 	_, err = kc.GetIndex("y", hash[0:3])
-	c.Check(err, Equals, ErrorNoSuchKeepServer)
+	c.Check(err, Equals, ErrNoSuchKeepServer)
 }
 
 func (s *StandaloneSuite) TestGetIndexWithNoSuchPrefix(c *C) {
@@ -1082,5 +1082,5 @@ func (s *StandaloneSuite) TestGetIndexWithNoSuchPrefix(c *C) {
 
 	content, err2 := ioutil.ReadAll(r)
 	c.Check(err2, Equals, nil)
-	c.Check(content, DeepEquals, st.body)
+	c.Check(content, DeepEquals, st.body[0:len(st.body)-1])
 }
diff --git a/services/keepproxy/keepproxy.go b/services/keepproxy/keepproxy.go
index 865212d..461e99d 100644
--- a/services/keepproxy/keepproxy.go
+++ b/services/keepproxy/keepproxy.go
@@ -16,7 +16,6 @@ import (
 	"os/signal"
 	"reflect"
 	"regexp"
-	"strings"
 	"sync"
 	"syscall"
 	"time"
@@ -494,7 +493,7 @@ func (this PutBlockHandler) ServeHTTP(resp http.ResponseWriter, req *http.Reques
 	}
 }
 
-// ServeHTTP implemenation for IndexHandler
+// ServeHTTP implementation for IndexHandler
 // Supports only GET requests for /index/{prefix:[0-9a-f]{0,32}}
 // For each keep server found in LocalRoots:
 //   Invokes GetIndex using keepclient
@@ -528,47 +527,39 @@ func (handler IndexHandler) ServeHTTP(resp http.ResponseWriter, req *http.Reques
 	arvclient.ApiToken = tok
 	kc.Arvados = &arvclient
 
-	var indexResp []byte
-	var reader io.Reader
-
-	switch req.Method {
-	case "GET":
-		for uuid := range kc.LocalRoots() {
-			reader, err = kc.GetIndex(uuid, prefix)
-			if err != nil {
-				break
-			}
-
-			var readBytes []byte
-			readBytes, err = ioutil.ReadAll(reader)
-			if err != nil {
-				break
-			}
+	// Only GET method is supported
+	if req.Method != "GET" {
+		status, err = http.StatusNotImplemented, MethodNotSupported
+		return
+	}
 
-			// Got index; verify that it is complete
-			// The response should be "\n" if no locators matched the prefix
-			// Else, it should be a list of locators followed by a blank line
-			if (!strings.HasSuffix(string(readBytes), "\n\n")) && (string(readBytes) != "\n") {
-				err = errors.New("Got incomplete index")
-			}
+	contentLen := 0
+	var reader io.Reader
+	for uuid := range kc.LocalRoots() {
+		reader, err = kc.GetIndex(uuid, prefix)
+		if err != nil {
+			status = http.StatusBadGateway
+			return
+		}
 
-			// Trim the extra empty new line found in response from each server
-			indexResp = append(indexResp, (readBytes[0 : len(readBytes)-1])...)
+		var readBytes []byte
+		readBytes, err = ioutil.ReadAll(reader)
+		if err != nil {
+			status = http.StatusBadGateway
+			return
 		}
 
-		// Append empty line at the end of concatenation of all server responses
-		indexResp = append(indexResp, ([]byte("\n"))...)
-	default:
-		status, err = http.StatusNotImplemented, MethodNotSupported
-		return
+		// Got index for this server; write to resp
+		n, err := resp.Write(readBytes)
+		if err != nil {
+			status = http.StatusBadGateway
+			return
+		}
+		contentLen += n
 	}
 
-	switch err {
-	case nil:
-		status = http.StatusOK
-		resp.Header().Set("Content-Length", fmt.Sprint(len(indexResp)))
-		_, err = resp.Write(indexResp)
-	default:
-		status = http.StatusBadGateway
-	}
+	// Got index from all the keep servers and wrote to resp
+	status = http.StatusOK
+	resp.Header().Set("Content-Length", fmt.Sprint(contentLen+1))
+	resp.Write([]byte("\n"))
 }
diff --git a/services/keepproxy/keepproxy_test.go b/services/keepproxy/keepproxy_test.go
index 1afe373..c623454 100644
--- a/services/keepproxy/keepproxy_test.go
+++ b/services/keepproxy/keepproxy_test.go
@@ -418,6 +418,7 @@ func (s *ServerRequiredSuite) TestGetIndex(c *C) {
 	waitForListener()
 	defer closeListener()
 
+	// Write "index-data" blocks
 	data := []byte("index-data")
 	hash := fmt.Sprintf("%x", md5.Sum(data))
 
@@ -432,37 +433,52 @@ func (s *ServerRequiredSuite) TestGetIndex(c *C) {
 	all, err := ioutil.ReadAll(reader)
 	c.Check(all, DeepEquals, data)
 
-	// GetIndex with no prefix
+	// Write some more blocks
+	otherData := []byte("some-more-index-data")
+	otherHash := fmt.Sprintf("%x", md5.Sum(otherData))
+	_, rep, err = kc.PutB(otherData)
+	c.Check(err, Equals, nil)
+
+	// GetIndex with no prefix; expect both data and otherData blocks
 	indexReader, err := kc.GetIndex("proxy", "")
 	c.Assert(err, Equals, nil)
 	indexResp, err := ioutil.ReadAll(indexReader)
 	locators := strings.Split(string(indexResp), "\n")
-	count := 0
+	matchingLocators := 0
+	otherLocators := 0
 	for _, locator := range locators {
 		if strings.HasPrefix(locator, hash) {
-			count++
+			matchingLocators++
+		} else if strings.HasPrefix(locator, otherHash) {
+			otherLocators++
 		}
 	}
-	c.Assert(2, Equals, count)
+	c.Assert(2, Equals, matchingLocators)
+	c.Assert(2, Equals, otherLocators)
 
 	// GetIndex with prefix
 	indexReader, err = kc.GetIndex("proxy", hash[0:3])
 	c.Assert(err, Equals, nil)
 	indexResp, err = ioutil.ReadAll(indexReader)
 	locators = strings.Split(string(indexResp), "\n")
-	count = 0
+	totalLocators := 0
+	matchingLocators = 0
 	for _, locator := range locators {
-		if strings.HasPrefix(locator, hash) {
-			count++
+		if locator != "" {
+			totalLocators++
+		}
+		if strings.HasPrefix(locator, hash[0:3]) {
+			matchingLocators++
 		}
 	}
-	c.Assert(2, Equals, count)
+	c.Assert(2, Equals, matchingLocators)
+	c.Assert(totalLocators, Equals, matchingLocators)
 
 	// GetIndex with valid but no such prefix
 	indexReader, err = kc.GetIndex("proxy", "abcd")
 	c.Assert(err, Equals, nil)
 	indexResp, err = ioutil.ReadAll(indexReader)
-	c.Assert(string(indexResp), Equals, "\n")
+	c.Assert(string(indexResp), Equals, "")
 
 	// GetIndex with invalid prefix
 	indexReader, err = kc.GetIndex("proxy", "xyz")

commit 34227825eec21d3a393d4467b7ed768b52ac32b1
Merge: eaf51fc 4e8c321
Author: radhika <radhika at curoverse.com>
Date:   Tue Sep 29 17:44:26 2015 -0400

    Merge branch 'master' into 7200-keepproxy-index-api


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


hooks/post-receive
-- 




More information about the arvados-commits mailing list