[ARVADOS] updated: 0f535af4fbcb7a9f6ca1e25675b31cc9656d88f3
git at public.curoverse.com
git at public.curoverse.com
Mon Oct 19 11:04:47 EDT 2015
Summary of changes:
.../test/integration/collection_upload_test.rb | 14 +-
sdk/python/tests/run_test_server.py | 6 +
services/keepproxy/keepproxy.go | 73 ++++-----
services/keepproxy/keepproxy_test.go | 166 ++++-----------------
4 files changed, 81 insertions(+), 178 deletions(-)
via 0f535af4fbcb7a9f6ca1e25675b31cc9656d88f3 (commit)
via 813d35123538b00ab70719e247b6bb0881269460 (commit)
via de1aef9bd3cd982604b1f9f1ad8433d0b28112ee (commit)
from 0795b298257f0441f94993999e1dc718a1f64bbd (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 0f535af4fbcb7a9f6ca1e25675b31cc9656d88f3
Author: Tom Clegg <tom at curoverse.com>
Date: Sat Oct 17 04:30:19 2015 -0400
5824: Log actual client IP address (along with X-Forwarded-For header, if any).
diff --git a/services/keepproxy/keepproxy.go b/services/keepproxy/keepproxy.go
index 24750ba..8cfaa90 100644
--- a/services/keepproxy/keepproxy.go
+++ b/services/keepproxy/keepproxy.go
@@ -198,12 +198,8 @@ func (this *ApiTokenCache) RecallToken(token string) bool {
}
func GetRemoteAddress(req *http.Request) string {
- if realip := req.Header.Get("X-Real-IP"); realip != "" {
- if forwarded := req.Header.Get("X-Forwarded-For"); forwarded != realip {
- return fmt.Sprintf("%s (X-Forwarded-For %s)", realip, forwarded)
- } else {
- return realip
- }
+ if xff := req.Header.Get("X-Forwarded-For"); xff != "" {
+ return xff + "," + req.RemoteAddr
}
return req.RemoteAddr
}
commit 813d35123538b00ab70719e247b6bb0881269460
Author: Tom Clegg <tom at curoverse.com>
Date: Sat Oct 17 04:23:27 2015 -0400
5824: Simplify keepproxy test setup.
For testing, configure KeepClient to use the test proxy server,
instead of telling the API about the test proxy server and using env
vars to trick the client into discovering it.
Let the OS choose an available port for each test server instead of
pre-assigning an arbitrary port number to each test case.
Use a properly formatted fake UUID.
diff --git a/services/keepproxy/keepproxy.go b/services/keepproxy/keepproxy.go
index 8c16467..24750ba 100644
--- a/services/keepproxy/keepproxy.go
+++ b/services/keepproxy/keepproxy.go
@@ -22,8 +22,8 @@ import (
)
// Default TCP address on which to listen for requests.
-// Initialized by the -listen flag.
-const DEFAULT_ADDR = ":25107"
+// Override with -listen.
+const DefaultAddr = ":25107"
var listener net.Listener
@@ -42,7 +42,7 @@ func main() {
flagset.StringVar(
&listen,
"listen",
- DEFAULT_ADDR,
+ DefaultAddr,
"Interface on which to listen for requests, in the format "+
"ipaddr:port. e.g. -listen=10.0.1.24:8000. Use -listen=:port "+
"to listen on all network interfaces.")
@@ -79,16 +79,6 @@ func main() {
flagset.Parse(os.Args[1:])
- arv, err := arvadosclient.MakeArvadosClient()
- if err != nil {
- log.Fatalf("Error setting up arvados client %s", err.Error())
- }
-
- kc, err := keepclient.MakeKeepClient(&arv)
- if err != nil {
- log.Fatalf("Error setting up keep client %s", err.Error())
- }
-
if pidfile != "" {
f, err := os.Create(pidfile)
if err != nil {
@@ -99,16 +89,23 @@ func main() {
defer os.Remove(pidfile)
}
+ arv, err := arvadosclient.MakeArvadosClient()
+ if err != nil {
+ log.Fatalf("setting up arvados client: %v", err)
+ }
+ kc, err := keepclient.MakeKeepClient(&arv)
+ if err != nil {
+ log.Fatalf("setting up keep client: %v", err)
+ }
kc.Want_replicas = default_replicas
-
kc.Client.Timeout = time.Duration(timeout) * time.Second
+ go RefreshServicesList(kc, 5*time.Minute, 3*time.Second)
listener, err = net.Listen("tcp", listen)
if err != nil {
log.Fatalf("Could not listen on %v", listen)
}
-
- go RefreshServicesList(kc, 5*time.Minute, 3*time.Second)
+ log.Printf("Arvados Keep proxy started listening on %v", listener.Addr())
// Shut down the server gracefully (by closing the listener)
// if SIGTERM is received.
@@ -121,9 +118,7 @@ func main() {
signal.Notify(term, syscall.SIGTERM)
signal.Notify(term, syscall.SIGINT)
- log.Printf("Arvados Keep proxy started listening on %v", listener.Addr())
-
- // Start listening for requests.
+ // Start serving requests.
http.Serve(listener, MakeRESTRouter(!no_get, !no_put, kc))
log.Println("shutting down")
diff --git a/services/keepproxy/keepproxy_test.go b/services/keepproxy/keepproxy_test.go
index 6fe8fe7..f350e0b 100644
--- a/services/keepproxy/keepproxy_test.go
+++ b/services/keepproxy/keepproxy_test.go
@@ -2,21 +2,19 @@ package main
import (
"crypto/md5"
- "crypto/tls"
"fmt"
"git.curoverse.com/arvados.git/sdk/go/arvadosclient"
"git.curoverse.com/arvados.git/sdk/go/arvadostest"
"git.curoverse.com/arvados.git/sdk/go/keepclient"
- . "gopkg.in/check.v1"
- "io"
"io/ioutil"
"log"
"net/http"
- "net/url"
"os"
"strings"
"testing"
"time"
+
+ . "gopkg.in/check.v1"
)
// Gocheck boilerplate
@@ -30,6 +28,8 @@ var _ = Suite(&ServerRequiredSuite{})
// Tests that require the Keep server running
type ServerRequiredSuite struct{}
+var TestProxyUUID = "zzzzz-bi6l4-lrixqc4fxofbmzz"
+
// Wait (up to 1 second) for keepproxy to listen on a port. This
// avoids a race condition where we hit a "connection refused" error
// because we start testing the proxy too soon.
@@ -65,106 +65,31 @@ func (s *ServerRequiredSuite) TearDownSuite(c *C) {
arvadostest.StopAPI()
}
-func setupProxyService() {
-
- client := &http.Client{Transport: &http.Transport{
- TLSClientConfig: &tls.Config{InsecureSkipVerify: true}}}
-
- var req *http.Request
- var err error
- if req, err = http.NewRequest("POST", fmt.Sprintf("https://%s/arvados/v1/keep_services", os.Getenv("ARVADOS_API_HOST")), nil); err != nil {
- panic(err.Error())
- }
- req.Header.Add("Authorization", fmt.Sprintf("OAuth2 %s", os.Getenv("ARVADOS_API_TOKEN")))
-
- reader, writer := io.Pipe()
-
- req.Body = reader
-
- go func() {
- data := url.Values{}
- data.Set("keep_service", `{
- "service_host": "localhost",
- "service_port": 29950,
- "service_ssl_flag": false,
- "service_type": "proxy"
-}`)
-
- writer.Write([]byte(data.Encode()))
- writer.Close()
- }()
-
- var resp *http.Response
- if resp, err = client.Do(req); err != nil {
- panic(err.Error())
- }
- if resp.StatusCode != 200 {
- panic(resp.Status)
- }
-}
+func runProxy(c *C, args []string, bogusClientToken bool) *keepclient.KeepClient {
+ args = append([]string{"keepproxy"}, args...)
+ os.Args = append(args, "-listen=:0")
+ listener = nil
+ go main()
+ waitForListener()
-func runProxy(c *C, args []string, port int, bogusClientToken bool) keepclient.KeepClient {
- if bogusClientToken {
- os.Setenv("ARVADOS_API_TOKEN", "bogus-token")
- }
arv, err := arvadosclient.MakeArvadosClient()
c.Assert(err, Equals, nil)
- kc := keepclient.KeepClient{
- Arvados: &arv,
- Want_replicas: 2,
- Using_proxy: true,
- Client: &http.Client{},
- }
- locals := map[string]string{
- "proxy": fmt.Sprintf("http://localhost:%v", port),
- }
- writableLocals := map[string]string{
- "proxy": fmt.Sprintf("http://localhost:%v", port),
- }
- kc.SetServiceRoots(locals, writableLocals, nil)
- c.Check(kc.Using_proxy, Equals, true)
- c.Check(len(kc.LocalRoots()), Equals, 1)
- for _, root := range kc.LocalRoots() {
- c.Check(root, Equals, fmt.Sprintf("http://localhost:%v", port))
- }
- log.Print("keepclient created")
if bogusClientToken {
- arvadostest.ResetEnv()
+ arv.ApiToken = "bogus-token"
}
-
- {
- os.Args = append(args, fmt.Sprintf("-listen=:%v", port))
- listener = nil
- go main()
+ kc := keepclient.New(&arv)
+ sr := map[string]string{
+ TestProxyUUID: "http://" + listener.Addr().String(),
}
+ kc.SetServiceRoots(sr, sr, sr)
+ kc.Arvados.External = true
+ kc.Using_proxy = true
return kc
}
func (s *ServerRequiredSuite) TestPutAskGet(c *C) {
- log.Print("TestPutAndGet start")
-
- os.Args = []string{"keepproxy", "-listen=:29950"}
- listener = nil
- go main()
- time.Sleep(100 * time.Millisecond)
-
- setupProxyService()
-
- os.Setenv("ARVADOS_EXTERNAL_CLIENT", "true")
- arv, err := arvadosclient.MakeArvadosClient()
- c.Assert(err, Equals, nil)
- kc, err := keepclient.MakeKeepClient(&arv)
- c.Assert(err, Equals, nil)
- c.Check(kc.Arvados.External, Equals, true)
- c.Check(kc.Using_proxy, Equals, true)
- c.Check(len(kc.LocalRoots()), Equals, 1)
- for _, root := range kc.LocalRoots() {
- c.Check(root, Equals, "http://localhost:29950")
- }
- os.Setenv("ARVADOS_EXTERNAL_CLIENT", "")
-
- waitForListener()
+ kc := runProxy(c, nil, false)
defer closeListener()
hash := fmt.Sprintf("%x", md5.Sum([]byte("foo")))
@@ -236,15 +161,10 @@ func (s *ServerRequiredSuite) TestPutAskGet(c *C) {
c.Check(blocklen, Equals, int64(0))
log.Print("Finished Get zero block")
}
-
- log.Print("TestPutAndGet done")
}
func (s *ServerRequiredSuite) TestPutAskGetForbidden(c *C) {
- log.Print("TestPutAskGetForbidden start")
-
- kc := runProxy(c, []string{"keepproxy"}, 29951, true)
- waitForListener()
+ kc := runProxy(c, nil, true)
defer closeListener()
hash := fmt.Sprintf("%x", md5.Sum([]byte("bar")))
@@ -276,15 +196,10 @@ func (s *ServerRequiredSuite) TestPutAskGetForbidden(c *C) {
c.Check(blocklen, Equals, int64(0))
log.Print("Get")
}
-
- log.Print("TestPutAskGetForbidden done")
}
func (s *ServerRequiredSuite) TestGetDisabled(c *C) {
- log.Print("TestGetDisabled start")
-
- kc := runProxy(c, []string{"keepproxy", "-no-get"}, 29952, false)
- waitForListener()
+ kc := runProxy(c, []string{"-no-get"}, false)
defer closeListener()
hash := fmt.Sprintf("%x", md5.Sum([]byte("baz")))
@@ -316,38 +231,26 @@ func (s *ServerRequiredSuite) TestGetDisabled(c *C) {
c.Check(blocklen, Equals, int64(0))
log.Print("Get")
}
-
- log.Print("TestGetDisabled done")
}
func (s *ServerRequiredSuite) TestPutDisabled(c *C) {
- log.Print("TestPutDisabled start")
-
- kc := runProxy(c, []string{"keepproxy", "-no-put"}, 29953, false)
- waitForListener()
+ kc := runProxy(c, []string{"-no-put"}, false)
defer closeListener()
- {
- hash2, rep, err := kc.PutB([]byte("quux"))
- c.Check(hash2, Equals, "")
- c.Check(rep, Equals, 0)
- c.Check(err, Equals, keepclient.InsufficientReplicasError)
- log.Print("PutB")
- }
-
- log.Print("TestPutDisabled done")
+ hash2, rep, err := kc.PutB([]byte("quux"))
+ c.Check(hash2, Equals, "")
+ c.Check(rep, Equals, 0)
+ c.Check(err, Equals, keepclient.InsufficientReplicasError)
}
func (s *ServerRequiredSuite) TestCorsHeaders(c *C) {
- runProxy(c, []string{"keepproxy"}, 29954, false)
- waitForListener()
+ runProxy(c, nil, false)
defer closeListener()
{
client := http.Client{}
req, err := http.NewRequest("OPTIONS",
- fmt.Sprintf("http://localhost:29954/%x+3",
- md5.Sum([]byte("foo"))),
+ fmt.Sprintf("http://%s/%x+3", listener.Addr().String(), md5.Sum([]byte("foo"))),
nil)
req.Header.Add("Access-Control-Request-Method", "PUT")
req.Header.Add("Access-Control-Request-Headers", "Authorization, X-Keep-Desired-Replicas")
@@ -362,8 +265,7 @@ func (s *ServerRequiredSuite) TestCorsHeaders(c *C) {
{
resp, err := http.Get(
- fmt.Sprintf("http://localhost:29954/%x+3",
- md5.Sum([]byte("foo"))))
+ fmt.Sprintf("http://%s/%x+3", listener.Addr().String(), md5.Sum([]byte("foo"))))
c.Check(err, Equals, nil)
c.Check(resp.Header.Get("Access-Control-Allow-Headers"), Equals, "Authorization, Content-Length, Content-Type, X-Keep-Desired-Replicas")
c.Check(resp.Header.Get("Access-Control-Allow-Origin"), Equals, "*")
@@ -371,14 +273,13 @@ func (s *ServerRequiredSuite) TestCorsHeaders(c *C) {
}
func (s *ServerRequiredSuite) TestPostWithoutHash(c *C) {
- runProxy(c, []string{"keepproxy"}, 29955, false)
- waitForListener()
+ runProxy(c, nil, false)
defer closeListener()
{
client := http.Client{}
req, err := http.NewRequest("POST",
- "http://localhost:29955/",
+ "http://"+listener.Addr().String()+"/",
strings.NewReader("qux"))
req.Header.Add("Authorization", "OAuth2 4axaw8zxe0qm22wa6urpp5nskcne8z88cvbupv653y1njyi05h")
req.Header.Add("Content-Type", "application/octet-stream")
@@ -414,8 +315,7 @@ func (s *ServerRequiredSuite) TestStripHint(c *C) {
// With a valid but non-existing prefix (expect "\n")
// With an invalid prefix (expect error)
func (s *ServerRequiredSuite) TestGetIndex(c *C) {
- kc := runProxy(c, []string{"keepproxy"}, 28852, false)
- waitForListener()
+ kc := runProxy(c, nil, false)
defer closeListener()
// Put "index-data" blocks
@@ -447,7 +347,7 @@ func (s *ServerRequiredSuite) TestGetIndex(c *C) {
{hash[:3], true, false}, // with matching prefix
{"abcdef", false, false}, // with no such prefix
} {
- indexReader, err := kc.GetIndex("proxy", spec.prefix)
+ indexReader, err := kc.GetIndex(TestProxyUUID, spec.prefix)
c.Assert(err, Equals, nil)
indexResp, err := ioutil.ReadAll(indexReader)
c.Assert(err, Equals, nil)
@@ -470,6 +370,6 @@ func (s *ServerRequiredSuite) TestGetIndex(c *C) {
}
// GetIndex with invalid prefix
- _, err = kc.GetIndex("proxy", "xyz")
+ _, err = kc.GetIndex(TestProxyUUID, "xyz")
c.Assert((err != nil), Equals, true)
}
commit de1aef9bd3cd982604b1f9f1ad8433d0b28112ee
Author: Tom Clegg <tom at curoverse.com>
Date: Sat Oct 17 02:56:08 2015 -0400
5824: Refresh keepproxy services list on SIGHUP. Update Workbench upload test to expect success.
diff --git a/apps/workbench/test/integration/collection_upload_test.rb b/apps/workbench/test/integration/collection_upload_test.rb
index 5e407ce..903df90 100644
--- a/apps/workbench/test/integration/collection_upload_test.rb
+++ b/apps/workbench/test/integration/collection_upload_test.rb
@@ -52,7 +52,7 @@ class CollectionUploadTest < ActionDispatch::IntegrationTest
assert_match /_text":"\. d41d8\S+ 0:0:empty.txt\\n\. d41d8\S+ 0:0:empty\\\\040\(1\).txt\\n"/, body
end
- test "Upload non-empty files, report errors" do
+ test "Upload non-empty files" do
need_selenium "to make file uploads work"
visit page_with_token 'active', sandbox_path
find('.nav-tabs a', text: 'Upload').click
@@ -60,15 +60,9 @@ class CollectionUploadTest < ActionDispatch::IntegrationTest
attach_file 'file_selector', testfile_path('foo.txt')
assert_selector 'button:not([disabled])', text: 'Start'
click_button 'Start'
- if "test environment does not have a keepproxy yet, see #4534" != "fixed"
- using_wait_time 20 do
- assert_text :visible, 'error'
- end
- else
- assert_text :visible, 'Done!'
- visit sandbox_path+'.json'
- assert_match /_text":"\. 0cc1\S+ 0:1:a\\n\. acbd\S+ 0:3:foo.txt\\n"/, body
- end
+ assert_text :visible, 'Done!'
+ visit sandbox_path+'.json'
+ assert_match /_text":"\. 0cc1\S+ 0:1:a\\n\. acbd\S+ 0:3:foo.txt\\n"/, body
end
test "Report mixed-content error" do
diff --git a/sdk/python/tests/run_test_server.py b/sdk/python/tests/run_test_server.py
index 7c44a74..591b500 100644
--- a/sdk/python/tests/run_test_server.py
+++ b/sdk/python/tests/run_test_server.py
@@ -363,6 +363,12 @@ def run_keep(blob_signing_key=None, enforce_permissions=False, num_servers=2):
'keep_disk': {'keep_service_uuid': svc['uuid'] }
}).execute()
+ # If keepproxy is running, send SIGHUP to make it discover the new
+ # keepstore services.
+ proxypidfile = _pidfile('keepproxy')
+ if os.path.exists(proxypidfile):
+ os.kill(int(open(proxypidfile).read()), signal.SIGHUP)
+
def _stop_keep(n):
kill_server_pid(_pidfile('keep{}'.format(n)), 0)
if os.path.exists("{}/keep{}.volume".format(TEST_TMPDIR, n)):
diff --git a/services/keepproxy/keepproxy.go b/services/keepproxy/keepproxy.go
index 8e734f7..8c16467 100644
--- a/services/keepproxy/keepproxy.go
+++ b/services/keepproxy/keepproxy.go
@@ -108,7 +108,7 @@ func main() {
log.Fatalf("Could not listen on %v", listen)
}
- go RefreshServicesList(kc)
+ go RefreshServicesList(kc, 5*time.Minute, 3*time.Second)
// Shut down the server gracefully (by closing the listener)
// if SIGTERM is received.
@@ -135,27 +135,39 @@ type ApiTokenCache struct {
expireTime int64
}
-// Refresh the keep service list every five minutes.
-func RefreshServicesList(kc *keepclient.KeepClient) {
+// Refresh the keep service list on SIGHUP; when the given interval
+// has elapsed since the last refresh; and (if the last refresh
+// failed) the given errInterval has elapsed.
+func RefreshServicesList(kc *keepclient.KeepClient, interval, errInterval time.Duration) {
var previousRoots = []map[string]string{}
- var delay time.Duration = 0
+
+ timer := time.NewTimer(interval)
+ gotHUP := make(chan os.Signal, 1)
+ signal.Notify(gotHUP, syscall.SIGHUP)
+
for {
- time.Sleep(delay * time.Second)
- delay = 300
+ select {
+ case <-gotHUP:
+ case <-timer.C:
+ }
+ timer.Reset(interval)
+
if err := kc.DiscoverKeepServers(); err != nil {
- log.Println("Error retrieving services list:", err)
- delay = 3
+ log.Println("Error retrieving services list: %v (retrying in %v)", err, errInterval)
+ timer.Reset(errInterval)
continue
}
newRoots := []map[string]string{kc.LocalRoots(), kc.GatewayRoots()}
+
if !reflect.DeepEqual(previousRoots, newRoots) {
log.Printf("Updated services list: locals %v gateways %v", newRoots[0], newRoots[1])
+ previousRoots = newRoots
}
+
if len(newRoots[0]) == 0 {
- log.Print("WARNING: No local services. Retrying in 3 seconds.")
- delay = 3
+ log.Printf("WARNING: No local services (retrying in %v)", errInterval)
+ timer.Reset(errInterval)
}
- previousRoots = newRoots
}
}
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list