1885: Added logging of invalid requests. Added logging when the server list is

update.  Improved KeepClient error reporting.

diff --git a/sdk/go/src/arvados.org/keepclient/keepclient.go b/sdk/go/src/arvados.org/keepclient/keepclient.go
index 5ba6257859f5437f5b2109a016d172e26e49a0d1..8d26b32790dc4f5f164d526332163bce80162b38 100644 (file)
--- a/sdk/go/src/arvados.org/keepclient/keepclient.go
+++ b/sdk/go/src/arvados.org/keepclient/keepclient.go
@@ -23,6 +23,8 @@ const BLOCKSIZE = 64 * 1024 * 1024
 var BlockNotFound = errors.New("Block not found")
 var InsufficientReplicasError = errors.New("Could not write sufficient replicas")
 var OversizeBlockError = errors.New("Block too big")
+var MissingArvadosApiHost = errors.New("Missing required environment variable ARVADOS_API_HOST")
+var MissingArvadosApiToken = errors.New("Missing required environment variable ARVADOS_API_TOKEN")
 
 const X_Keep_Desired_Replicas = "X-Keep-Desired-Replicas"
 const X_Keep_Replicas_Stored = "X-Keep-Replicas-Stored"
@@ -58,6 +60,13 @@ func MakeKeepClient() (kc KeepClient, err error) {
                Using_proxy: false,
                External:    external}
 
+       if os.Getenv("ARVADOS_API_HOST") == "" {
+               return kc, MissingArvadosApiHost
+       }
+       if os.Getenv("ARVADOS_API_TOKEN") == "" {
+               return kc, MissingArvadosApiToken
+       }
+
        err = (&kc).DiscoverKeepServers()
 
        return kc, err

diff --git a/sdk/go/src/arvados.org/keepclient/support.go b/sdk/go/src/arvados.org/keepclient/support.go
index f924137567dcba59ebca806bdefbdbd14a406eb6..913f7c78ffbeb2b9d37e446f0e6aca8ba3a3d8ed 100644 (file)
--- a/sdk/go/src/arvados.org/keepclient/support.go
+++ b/sdk/go/src/arvados.org/keepclient/support.go
@@ -47,7 +47,7 @@ func (this *KeepClient) DiscoverKeepServers() error {
                return err
        }
 
-       if resp.StatusCode != 200 {
+       if resp.StatusCode != http.StatusOK {
                // fall back on keep disks
                if req, err = http.NewRequest("GET", fmt.Sprintf("https://%s/arvados/v1/keep_disks", this.ApiServer), nil); err != nil {
                        return err
@@ -56,6 +56,9 @@ func (this *KeepClient) DiscoverKeepServers() error {
                if resp, err = this.Client.Do(req); err != nil {
                        return err
                }
+               if resp.StatusCode != http.StatusOK {
+                       return errors.New(resp.Status)
+               }
        }
 
        type svcList struct {

diff --git a/services/keep/src/arvados.org/keepproxy/keepproxy.go b/services/keep/src/arvados.org/keepproxy/keepproxy.go
index 9e0b2ff90c29fe070ec30e3bd11f96f1038f9a87..2d7e276d8ff5a7d106c221ce5f942d58c0191acf 100644 (file)
--- a/services/keep/src/arvados.org/keepproxy/keepproxy.go
+++ b/services/keep/src/arvados.org/keepproxy/keepproxy.go
@@ -65,7 +65,7 @@ func main() {
 
        kc, err := keepclient.MakeKeepClient()
        if err != nil {
-               log.Fatal(err)
+               log.Fatalf("Error setting up keep client %s", err.Error())
        }
 
        if pidfile != "" {
@@ -87,6 +87,8 @@ func main() {
 
        go RefreshServicesList(&kc)
 
+       log.Printf("Arvados Keep proxy started listening on %v with server list %v", listener.Addr(), kc.ServiceRoots())
+
        // Start listening for requests.
        http.Serve(listener, MakeRESTRouter(!no_get, !no_put, &kc))
 }
@@ -101,7 +103,14 @@ type ApiTokenCache struct {
 func RefreshServicesList(kc *keepclient.KeepClient) {
        for {
                time.Sleep(300 * time.Second)
+               oldservices := kc.ServiceRoots()
                kc.DiscoverKeepServers()
+               newservices := kc.ServiceRoots()
+               s1 := fmt.Sprint(oldservices)
+               s2 := fmt.Sprint(newservices)
+               if s1 != s2 {
+                       log.Printf("Updated server list to %v", s2)
+               }
        }
 }
 
@@ -167,7 +176,7 @@ func CheckAuthorizationHeader(kc keepclient.KeepClient, cache *ApiTokenCache, re
 
        var usersreq *http.Request
 
-       if usersreq, err = http.NewRequest("GET", fmt.Sprintf("https://%s/arvados/v1/users/current", kc.ApiServer), nil); err != nil {
+       if usersreq, err = http.NewRequest("HEAD", fmt.Sprintf("https://%s/arvados/v1/users/current", kc.ApiServer), nil); err != nil {
                // Can't construct the request
                log.Printf("%s: CheckAuthorizationHeader error: %v", GetRemoteAddress(req), err)
                return false
@@ -206,6 +215,8 @@ type PutBlockHandler struct {
        *ApiTokenCache
 }
 
+type InvalidPathHandler struct{}
+
 // MakeRESTRouter
 //     Returns a mux.Router that passes GET and PUT requests to the
 //     appropriate handlers.
@@ -233,9 +244,16 @@ func MakeRESTRouter(
                ph.Methods("PUT")
        }
 
+       rest.NotFoundHandler = InvalidPathHandler{}
+
        return rest
 }
 
+func (this InvalidPathHandler) ServeHTTP(resp http.ResponseWriter, req *http.Request) {
+       log.Printf("%s: %s %s unroutable", GetRemoteAddress(req), req.Method, req.URL.Path)
+       http.Error(resp, "Bad request", http.StatusBadRequest)
+}
+
 func (this GetBlockHandler) ServeHTTP(resp http.ResponseWriter, req *http.Request) {
 
        kc := *this.KeepClient