From 228754a99ca5641bf2e43158c4b93f64b0b91d2b Mon Sep 17 00:00:00 2001 From: Ben Johnson Date: Tue, 19 Nov 2013 16:12:58 -0700 Subject: [PATCH 01/14] mod_lock --- mod/lock/handler.go | 44 ++++++++++++++++++++++++++++++++++++++++ mod/lock/handler_test.go | 35 ++++++++++++++++++++++++++++++++ mod/mod.go | 9 +++++--- 3 files changed, 85 insertions(+), 3 deletions(-) create mode 100644 mod/lock/handler.go create mode 100644 mod/lock/handler_test.go diff --git a/mod/lock/handler.go b/mod/lock/handler.go new file mode 100644 index 000000000..ccb2a3c5d --- /dev/null +++ b/mod/lock/handler.go @@ -0,0 +1,44 @@ +package lock + +import ( + "bytes" + "net/http" + "os" + "path" + "time" + + "github.com/coreos/go-etcd/etcd" +) + +// handler manages the lock HTTP request. +type handler struct { + *mux.Router + client string +} + +// NewHandler creates an HTTP handler that can be registered on a router. +func NewHandler(addr string) (http.Handler) { + h := &handler{ + Router: mux.NewRouter(), + client: etcd.NewClient([]string{addr}), + } + h.HandleFunc("/{key:.+}", h.getLockHandler).Methods("GET") + h.HandleFunc("/{key:.+}", h.acquireLockHandler).Methods("PUT") + h.HandleFunc("/{key:.+}", h.releaseLockHandler).Methods("DELETE") +} + +// getLockHandler retrieves whether a lock has been obtained for a given key. +func (h *handler) getLockHandler(w http.ResponseWriter, req *http.Request) { + // TODO +} + +// acquireLockHandler attempts to acquire a lock on the given key. +// The lock is released when the connection is disconnected. +func (h *handler) acquireLockHandler(w http.ResponseWriter, req *http.Request) { + // TODO +} + +// releaseLockHandler forces the release of a lock on the given key. +func (h *handler) releaseLockHandler(w http.ResponseWriter, req *http.Request) { + // TODO +} diff --git a/mod/lock/handler_test.go b/mod/lock/handler_test.go new file mode 100644 index 000000000..91ecc310d --- /dev/null +++ b/mod/lock/handler_test.go @@ -0,0 +1,35 @@ +package lock + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +// Ensure that a lock can be acquired and released. +func TestModLockAcquire(t *testing.T) { + // TODO: Acquire lock. + // TODO: Check that it has been acquired. + // TODO: Release lock. + // TODO: Check that it has been released. +} + +// Ensure that a lock can be acquired and another process is blocked until released. +func TestModLockAcquireBlocked(t *testing.T) { + // TODO: Acquire lock with process #1. + // TODO: Acquire lock with process #2. + // TODO: Check that process #2 has not obtained lock. + // TODO: Release lock from process #1. + // TODO: Check that process #2 obtains the lock. + // TODO: Release lock from process #2. + // TODO: Check that no lock exists. +} + +// Ensure that an unowned lock can be released by force. +func TestModLockForceRelease(t *testing.T) { + // TODO: Acquire lock. + // TODO: Check that it has been acquired. + // TODO: Force release lock. + // TODO: Check that it has been released. + // TODO: Check that acquiring goroutine is notified that their lock has been released. +} diff --git a/mod/mod.go b/mod/mod.go index 741b19002..7964c683f 100644 --- a/mod/mod.go +++ b/mod/mod.go @@ -5,14 +5,17 @@ import ( "net/http" "github.com/coreos/etcd/mod/dashboard" + "github.com/coreos/etcd/mod/lock" "github.com/gorilla/mux" ) var ServeMux *http.Handler func HttpHandler() (handler http.Handler) { - modMux := mux.NewRouter() - modMux.PathPrefix("/dashboard/"). + r := mux.NewRouter() + r.PathPrefix("/dashboard/"). Handler(http.StripPrefix("/dashboard/", dashboard.HttpHandler())) - return modMux + r.PathPrefix("/lock/"). + Handler(http.StripPrefix("/lock", lock.NewHandler())) + return r } From 32861246b974f2c1678c1f10b2b10f7f580a5df7 Mon Sep 17 00:00:00 2001 From: Ben Johnson Date: Wed, 27 Nov 2013 14:36:14 -0700 Subject: [PATCH 02/14] mod/lock --- mod/lock/acquire_handler.go | 49 ++++++++++++++++++++++++++++ mod/lock/handler.go | 49 +++++++++++++++++----------- mod/lock/release_handler.go | 11 +++++++ mod/lock/renew_handler.go | 16 +++++++++ mod/lock/{ => tests}/handler_test.go | 23 ++++++++++--- mod/mod.go | 8 ++--- 6 files changed, 129 insertions(+), 27 deletions(-) create mode 100644 mod/lock/acquire_handler.go create mode 100644 mod/lock/release_handler.go create mode 100644 mod/lock/renew_handler.go rename mod/lock/{ => tests}/handler_test.go (62%) diff --git a/mod/lock/acquire_handler.go b/mod/lock/acquire_handler.go new file mode 100644 index 000000000..d142a3f2e --- /dev/null +++ b/mod/lock/acquire_handler.go @@ -0,0 +1,49 @@ +package lock + +import ( + "net/http" + "path" + "strconv" + + "github.com/gorilla/mux" +) + +// acquireHandler attempts to acquire a lock on the given key. +func (h *handler) acquireHandler(w http.ResponseWriter, req *http.Request) { + vars := mux.Vars(req) + keypath := path.Join(prefix, vars["key"]) + ttl, err := strconv.Atoi(vars["ttl"]) + if err != nil { + http.Error(w, "invalid ttl: " + err.Error(), http.StatusInternalServerError) + return + } + + // Create an incrementing id for the lock. + resp, err := h.client.AddChild(keypath, "X", ttl) + if err != nil { + http.Error(w, "add lock index error: " + err.Error(), http.StatusInternalServerError) + return + } + + // Extract the lock index. + index, _ := strconv.Atoi(path.Base(resp.Key)) + + // Read all indices. + resp, err = h.client.GetAll(key) + if err != nil { + http.Error(w, "lock children lookup error: " + err.Error(), http.StatusInternalServerError) + return + } + indices := extractResponseIndices(resp) + + // TODO: child_keys := parse_and_sort_child_keys + // TODO: if index == min(child_keys) then return 200 + // TODO: else: + // TODO: h.client.WatchAll(key) + // TODO: if next_lowest_key is deleted + // TODO: get_all_keys + // TODO: if index == min(child_keys) then return 200 + // TODO: rinse_and_repeat until we're the lowest. + + // TODO: +} diff --git a/mod/lock/handler.go b/mod/lock/handler.go index ccb2a3c5d..66a62be4f 100644 --- a/mod/lock/handler.go +++ b/mod/lock/handler.go @@ -1,19 +1,20 @@ package lock import ( - "bytes" + "fmt" "net/http" - "os" "path" - "time" + "github.com/gorilla/mux" "github.com/coreos/go-etcd/etcd" ) +const prefix = "/_etcd/locks" + // handler manages the lock HTTP request. type handler struct { *mux.Router - client string + client *etcd.Client } // NewHandler creates an HTTP handler that can be registered on a router. @@ -22,23 +23,33 @@ func NewHandler(addr string) (http.Handler) { Router: mux.NewRouter(), client: etcd.NewClient([]string{addr}), } - h.HandleFunc("/{key:.+}", h.getLockHandler).Methods("GET") - h.HandleFunc("/{key:.+}", h.acquireLockHandler).Methods("PUT") - h.HandleFunc("/{key:.+}", h.releaseLockHandler).Methods("DELETE") + h.StrictSlash(false) + h.HandleFunc("/{key:.*}", h.acquireHandler).Methods("POST") + h.HandleFunc("/{key_with_index:.*}", h.renewLockHandler).Methods("PUT") + h.HandleFunc("/{key_with_index:.*}", h.releaseLockHandler).Methods("DELETE") + return h } -// getLockHandler retrieves whether a lock has been obtained for a given key. -func (h *handler) getLockHandler(w http.ResponseWriter, req *http.Request) { - // TODO + +// extractResponseIndices extracts a sorted list of indicies from a response. +func extractResponseIndices(resp *etcd.Response) []int { + var indices []int + for _, kv := range resp.Kvs { + if index, _ := strconv.Atoi(path.Base(kv.Key)); index > 0 { + indicies = append(indices, index) + } + } + return indices } -// acquireLockHandler attempts to acquire a lock on the given key. -// The lock is released when the connection is disconnected. -func (h *handler) acquireLockHandler(w http.ResponseWriter, req *http.Request) { - // TODO -} - -// releaseLockHandler forces the release of a lock on the given key. -func (h *handler) releaseLockHandler(w http.ResponseWriter, req *http.Request) { - // TODO +// findPrevIndex retrieves the previous index before the given index. +func findPrevIndex(indices []int, idx int) int { + var prevIndex int + for _, index := range indices { + if index == idx { + break + } + prevIndex = index + } + return prevIndex } diff --git a/mod/lock/release_handler.go b/mod/lock/release_handler.go new file mode 100644 index 000000000..09b875183 --- /dev/null +++ b/mod/lock/release_handler.go @@ -0,0 +1,11 @@ +package lock + +import ( + "net/http" +) + +// releaseLockHandler deletes the lock. +func (h *handler) releaseLockHandler(w http.ResponseWriter, req *http.Request) { + // TODO: h.client.Delete(key_with_index) +} + diff --git a/mod/lock/renew_handler.go b/mod/lock/renew_handler.go new file mode 100644 index 000000000..da9c0b8c2 --- /dev/null +++ b/mod/lock/renew_handler.go @@ -0,0 +1,16 @@ +package lock + +import ( + "net/http" +) + +// renewLockHandler attempts to update the TTL on an existing lock. +// Returns a 200 OK if successful. Otherwie +func (h *handler) renewLockHandler(w http.ResponseWriter, req *http.Request) { + vars := mux.Vars(req) + key := path.Join(prefix, vars["key"]) + ttl := vars["ttl"] + w.Write([]byte(fmt.Sprintf("%s-%s", key, ttl))) + + // TODO: +} diff --git a/mod/lock/handler_test.go b/mod/lock/tests/handler_test.go similarity index 62% rename from mod/lock/handler_test.go rename to mod/lock/tests/handler_test.go index 91ecc310d..fbc36ea00 100644 --- a/mod/lock/handler_test.go +++ b/mod/lock/tests/handler_test.go @@ -1,17 +1,32 @@ package lock import ( + "fmt" + "net/url" "testing" + "time" + "github.com/coreos/etcd/server" + "github.com/coreos/etcd/tests" "github.com/stretchr/testify/assert" ) // Ensure that a lock can be acquired and released. func TestModLockAcquire(t *testing.T) { - // TODO: Acquire lock. - // TODO: Check that it has been acquired. - // TODO: Release lock. - // TODO: Check that it has been released. + v := url.Values{} + tests.RunServer(func(s *server.Server) { + // Acquire lock. + resp, err := tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/mod/lock"), v) + assert.NoError(t, err) + ret := tests.ReadBody(resp) + assert.Equal(t, string(ret), "XXX") + + fmt.Println("URL:", fmt.Sprintf("http://%s%s", s.URL(), "/mod/lock")) + time.Sleep(60 * time.Second) + // TODO: Check that it has been acquired. + // TODO: Release lock. + // TODO: Check that it has been released. + }) } // Ensure that a lock can be acquired and another process is blocked until released. diff --git a/mod/mod.go b/mod/mod.go index 7964c683f..9d3b54d69 100644 --- a/mod/mod.go +++ b/mod/mod.go @@ -13,9 +13,9 @@ var ServeMux *http.Handler func HttpHandler() (handler http.Handler) { r := mux.NewRouter() - r.PathPrefix("/dashboard/"). - Handler(http.StripPrefix("/dashboard/", dashboard.HttpHandler())) - r.PathPrefix("/lock/"). - Handler(http.StripPrefix("/lock", lock.NewHandler())) + r.PathPrefix("/dashboard/").Handler(http.StripPrefix("/dashboard/", dashboard.HttpHandler())) + + // TODO: Use correct addr. + r.PathPrefix("/lock").Handler(http.StripPrefix("/lock", lock.NewHandler("127.0.0.1:4001"))) return r } From 22c2935ddb3ab7ce5e642a43abb114f3778cc866 Mon Sep 17 00:00:00 2001 From: Ben Johnson Date: Wed, 27 Nov 2013 16:59:05 -0700 Subject: [PATCH 03/14] Initial mod_lock acquire. --- mod/lock/acquire_handler.go | 64 +++++++++++++------ mod/lock/handler.go | 7 +- mod/lock/renew_handler.go | 8 ++- mod/lock/tests/handler_test.go | 4 +- mod/mod.go | 5 +- server/registry.go | 1 + server/server.go | 4 +- test.sh | 16 ++--- tests/server_utils.go | 5 +- .../coreos/go-etcd/etcd/requests.go | 2 +- 10 files changed, 75 insertions(+), 41 deletions(-) diff --git a/mod/lock/acquire_handler.go b/mod/lock/acquire_handler.go index d142a3f2e..3e7f2e973 100644 --- a/mod/lock/acquire_handler.go +++ b/mod/lock/acquire_handler.go @@ -4,46 +4,72 @@ import ( "net/http" "path" "strconv" + "time" "github.com/gorilla/mux" ) // acquireHandler attempts to acquire a lock on the given key. func (h *handler) acquireHandler(w http.ResponseWriter, req *http.Request) { + h.client.SyncCluster() + vars := mux.Vars(req) keypath := path.Join(prefix, vars["key"]) - ttl, err := strconv.Atoi(vars["ttl"]) + ttl, err := strconv.Atoi(req.FormValue("ttl")) if err != nil { http.Error(w, "invalid ttl: " + err.Error(), http.StatusInternalServerError) return } // Create an incrementing id for the lock. - resp, err := h.client.AddChild(keypath, "X", ttl) + resp, err := h.client.AddChild(keypath, "-", uint64(ttl)) if err != nil { http.Error(w, "add lock index error: " + err.Error(), http.StatusInternalServerError) return } + // Keep updating TTL to make sure lock request is not expired before acquisition. + stopChan := make(chan bool) + defer close(stopChan) + go func(k string) { + stopped := false + for { + select { + case <-time.After(time.Duration(ttl / 2) * time.Second): + case <-stopChan: + stopped = true + } + h.client.Update(k, "-", uint64(ttl)) + if stopped { + break + } + } + }(resp.Key) + // Extract the lock index. index, _ := strconv.Atoi(path.Base(resp.Key)) - // Read all indices. - resp, err = h.client.GetAll(key) - if err != nil { - http.Error(w, "lock children lookup error: " + err.Error(), http.StatusInternalServerError) - return + for { + // Read all indices. + resp, err = h.client.GetAll(keypath, true) + if err != nil { + http.Error(w, "lock children lookup error: " + err.Error(), http.StatusInternalServerError) + return + } + indices := extractResponseIndices(resp) + waitIndex := resp.ModifiedIndex + prevIndex := findPrevIndex(indices, index) + + // If there is no previous index then we have the lock. + if prevIndex == 0 { + break + } + + // Otherwise watch previous index until it's gone. + _, err = h.client.Watch(path.Join(keypath, strconv.Itoa(prevIndex)), waitIndex, nil, nil) + if err != nil { + http.Error(w, "lock watch error: " + err.Error(), http.StatusInternalServerError) + return + } } - indices := extractResponseIndices(resp) - - // TODO: child_keys := parse_and_sort_child_keys - // TODO: if index == min(child_keys) then return 200 - // TODO: else: - // TODO: h.client.WatchAll(key) - // TODO: if next_lowest_key is deleted - // TODO: get_all_keys - // TODO: if index == min(child_keys) then return 200 - // TODO: rinse_and_repeat until we're the lowest. - - // TODO: } diff --git a/mod/lock/handler.go b/mod/lock/handler.go index 66a62be4f..355a6339f 100644 --- a/mod/lock/handler.go +++ b/mod/lock/handler.go @@ -1,9 +1,10 @@ package lock import ( - "fmt" "net/http" "path" + "strconv" + "sort" "github.com/gorilla/mux" "github.com/coreos/go-etcd/etcd" @@ -19,6 +20,7 @@ type handler struct { // NewHandler creates an HTTP handler that can be registered on a router. func NewHandler(addr string) (http.Handler) { + etcd.OpenDebug() h := &handler{ Router: mux.NewRouter(), client: etcd.NewClient([]string{addr}), @@ -36,9 +38,10 @@ func extractResponseIndices(resp *etcd.Response) []int { var indices []int for _, kv := range resp.Kvs { if index, _ := strconv.Atoi(path.Base(kv.Key)); index > 0 { - indicies = append(indices, index) + indices = append(indices, index) } } + sort.Ints(indices) return indices } diff --git a/mod/lock/renew_handler.go b/mod/lock/renew_handler.go index da9c0b8c2..ba9fe31d2 100644 --- a/mod/lock/renew_handler.go +++ b/mod/lock/renew_handler.go @@ -2,15 +2,17 @@ package lock import ( "net/http" + _ "path" + + _ "github.com/gorilla/mux" ) // renewLockHandler attempts to update the TTL on an existing lock. // Returns a 200 OK if successful. Otherwie func (h *handler) renewLockHandler(w http.ResponseWriter, req *http.Request) { + /* vars := mux.Vars(req) key := path.Join(prefix, vars["key"]) ttl := vars["ttl"] - w.Write([]byte(fmt.Sprintf("%s-%s", key, ttl))) - - // TODO: + */ } diff --git a/mod/lock/tests/handler_test.go b/mod/lock/tests/handler_test.go index fbc36ea00..e3caafe25 100644 --- a/mod/lock/tests/handler_test.go +++ b/mod/lock/tests/handler_test.go @@ -16,12 +16,12 @@ func TestModLockAcquire(t *testing.T) { v := url.Values{} tests.RunServer(func(s *server.Server) { // Acquire lock. - resp, err := tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/mod/lock"), v) + url := fmt.Sprintf("http://%s%s", s.URL(), "/mod/lock/foo?ttl=2") + resp, err := tests.PutForm(url, v) assert.NoError(t, err) ret := tests.ReadBody(resp) assert.Equal(t, string(ret), "XXX") - fmt.Println("URL:", fmt.Sprintf("http://%s%s", s.URL(), "/mod/lock")) time.Sleep(60 * time.Second) // TODO: Check that it has been acquired. // TODO: Release lock. diff --git a/mod/mod.go b/mod/mod.go index d9b0ee01b..7c0194f56 100644 --- a/mod/mod.go +++ b/mod/mod.go @@ -17,13 +17,12 @@ func addSlash(w http.ResponseWriter, req *http.Request) { return } -func HttpHandler() (handler http.Handler) { +func HttpHandler(addr string) http.Handler { r := mux.NewRouter() r.HandleFunc("/dashboard", addSlash) r.PathPrefix("/dashboard/").Handler(http.StripPrefix("/dashboard/", dashboard.HttpHandler())) // TODO: Use correct addr. - r.HandleFunc("/lock", addSlash) - r.PathPrefix("/lock").Handler(http.StripPrefix("/lock", lock.NewHandler("127.0.0.1:4001"))) + r.PathPrefix("/lock").Handler(http.StripPrefix("/lock", lock.NewHandler(addr))) return r } diff --git a/server/registry.go b/server/registry.go index d1d98d9ed..27b0ce414 100644 --- a/server/registry.go +++ b/server/registry.go @@ -46,6 +46,7 @@ func (r *Registry) Register(name string, peerURL string, url string) error { key := path.Join(RegistryKey, name) value := fmt.Sprintf("raft=%s&etcd=%s", peerURL, url) _, err := r.store.Create(key, value, false, store.Permanent) + fmt.Println("register.1:", key, value, err) log.Debugf("Register: %s", name) return err } diff --git a/server/server.go b/server/server.go index 4f75df2e0..f0de64a67 100644 --- a/server/server.go +++ b/server/server.go @@ -130,7 +130,7 @@ func (s *Server) installV2() { func (s *Server) installMod() { r := s.router - r.PathPrefix("/mod").Handler(http.StripPrefix("/mod", mod.HttpHandler())) + r.PathPrefix("/mod").Handler(http.StripPrefix("/mod", mod.HttpHandler(s.url))) } // Adds a v1 server handler to the router. @@ -320,12 +320,14 @@ func (s *Server) GetVersionHandler(w http.ResponseWriter, req *http.Request) err // Handler to return the current leader's raft address func (s *Server) GetLeaderHandler(w http.ResponseWriter, req *http.Request) error { leader := s.peerServer.RaftServer().Leader() + fmt.Println("/leader.1?", leader) if leader == "" { return etcdErr.NewError(etcdErr.EcodeLeaderElect, "", s.Store().Index()) } w.WriteHeader(http.StatusOK) url, _ := s.registry.PeerURL(leader) w.Write([]byte(url)) + fmt.Println("/leader.2?", leader, url) return nil } diff --git a/test.sh b/test.sh index 5cc633975..690f3a932 100755 --- a/test.sh +++ b/test.sh @@ -1,6 +1,9 @@ #!/bin/sh set -e +PKGS="./mod/lock/tests" +# PKGS="./store ./server ./server/v2/tests" + # Get GOPATH, etc from build . ./build @@ -8,14 +11,11 @@ set -e export GOPATH="${PWD}" # Unit tests -go test -i ./server -go test -v ./server - -go test -i ./server/v2/tests -go test -v ./server/v2/tests - -go test -i ./store -go test -v ./store +for PKG in $PKGS +do + go test -i $PKG + go test -v $PKG +done # Functional tests go test -i ./tests/functional diff --git a/tests/server_utils.go b/tests/server_utils.go index e3e7d5323..b02eb6371 100644 --- a/tests/server_utils.go +++ b/tests/server_utils.go @@ -23,8 +23,9 @@ func RunServer(f func(*server.Server)) { store := store.New() registry := server.NewRegistry(store) - ps := server.NewPeerServer(testName, path, testRaftURL, testRaftURL, &server.TLSConfig{Scheme: "http"}, &server.TLSInfo{}, registry, store, testSnapshotCount) - s := server.New(testName, testClientURL, testClientURL, &server.TLSConfig{Scheme: "http"}, &server.TLSInfo{}, ps, registry, store) + ps := server.NewPeerServer(testName, path, "http://" + testRaftURL, testRaftURL, &server.TLSConfig{Scheme: "http"}, &server.TLSInfo{}, registry, store, testSnapshotCount) + ps.MaxClusterSize = 9 + s := server.New(testName, "http://" + testClientURL, testClientURL, &server.TLSConfig{Scheme: "http"}, &server.TLSInfo{}, ps, registry, store) ps.SetServer(s) // Start up peer server. diff --git a/third_party/github.com/coreos/go-etcd/etcd/requests.go b/third_party/github.com/coreos/go-etcd/etcd/requests.go index 83e3b519e..4db818f97 100644 --- a/third_party/github.com/coreos/go-etcd/etcd/requests.go +++ b/third_party/github.com/coreos/go-etcd/etcd/requests.go @@ -207,7 +207,7 @@ func (c *Client) sendRequest(method string, _path string, values url.Values) (*R if err != nil { retry++ if retry > 2*len(c.cluster.Machines) { - return nil, errors.New("Cannot reach servers") + return nil, errors.New("Cannot reach servers" + err.Error()) } num := retry % len(c.cluster.Machines) logger.Debug("update.leader[", c.cluster.Leader, ",", c.cluster.Machines[num], "]") From d8e9838c382cf826b24471077b12a15934bdf8b0 Mon Sep 17 00:00:00 2001 From: Ben Johnson Date: Fri, 29 Nov 2013 16:33:49 -0700 Subject: [PATCH 04/14] Lock testing. --- mod/lock/acquire_handler.go | 6 +- mod/lock/get_index_handler.go | 30 ++++++ mod/lock/handler.go | 4 +- mod/lock/release_handler.go | 15 ++- mod/lock/renew_handler.go | 26 +++-- mod/lock/tests/handler_test.go | 190 ++++++++++++++++++++++++++++----- 6 files changed, 234 insertions(+), 37 deletions(-) create mode 100644 mod/lock/get_index_handler.go diff --git a/mod/lock/acquire_handler.go b/mod/lock/acquire_handler.go index 3e7f2e973..8ad9e528a 100644 --- a/mod/lock/acquire_handler.go +++ b/mod/lock/acquire_handler.go @@ -27,6 +27,7 @@ func (h *handler) acquireHandler(w http.ResponseWriter, req *http.Request) { http.Error(w, "add lock index error: " + err.Error(), http.StatusInternalServerError) return } + indexpath := resp.Key // Keep updating TTL to make sure lock request is not expired before acquisition. stopChan := make(chan bool) @@ -44,7 +45,7 @@ func (h *handler) acquireHandler(w http.ResponseWriter, req *http.Request) { break } } - }(resp.Key) + }(indexpath) // Extract the lock index. index, _ := strconv.Atoi(path.Base(resp.Key)) @@ -72,4 +73,7 @@ func (h *handler) acquireHandler(w http.ResponseWriter, req *http.Request) { return } } + + // Write lock index to response body. + w.Write([]byte(strconv.Itoa(index))) } diff --git a/mod/lock/get_index_handler.go b/mod/lock/get_index_handler.go new file mode 100644 index 000000000..2bb97a83c --- /dev/null +++ b/mod/lock/get_index_handler.go @@ -0,0 +1,30 @@ +package lock + +import ( + "net/http" + "path" + "strconv" + + "github.com/gorilla/mux" +) + +// getIndexHandler retrieves the current lock index. +func (h *handler) getIndexHandler(w http.ResponseWriter, req *http.Request) { + h.client.SyncCluster() + + vars := mux.Vars(req) + keypath := path.Join(prefix, vars["key"]) + + // Read all indices. + resp, err := h.client.GetAll(keypath, true) + if err != nil { + http.Error(w, "lock children lookup error: " + err.Error(), http.StatusInternalServerError) + return + } + + // Write out the index of the last one to the response body. + indices := extractResponseIndices(resp) + if len(indices) > 0 { + w.Write([]byte(strconv.Itoa(indices[0]))) + } +} diff --git a/mod/lock/handler.go b/mod/lock/handler.go index 355a6339f..43e149145 100644 --- a/mod/lock/handler.go +++ b/mod/lock/handler.go @@ -10,7 +10,7 @@ import ( "github.com/coreos/go-etcd/etcd" ) -const prefix = "/_etcd/locks" +const prefix = "/_etcd/mod/lock" // handler manages the lock HTTP request. type handler struct { @@ -20,12 +20,12 @@ type handler struct { // NewHandler creates an HTTP handler that can be registered on a router. func NewHandler(addr string) (http.Handler) { - etcd.OpenDebug() h := &handler{ Router: mux.NewRouter(), client: etcd.NewClient([]string{addr}), } h.StrictSlash(false) + h.HandleFunc("/{key:.*}", h.getIndexHandler).Methods("GET") h.HandleFunc("/{key:.*}", h.acquireHandler).Methods("POST") h.HandleFunc("/{key_with_index:.*}", h.renewLockHandler).Methods("PUT") h.HandleFunc("/{key_with_index:.*}", h.releaseLockHandler).Methods("DELETE") diff --git a/mod/lock/release_handler.go b/mod/lock/release_handler.go index 09b875183..09251f259 100644 --- a/mod/lock/release_handler.go +++ b/mod/lock/release_handler.go @@ -1,11 +1,24 @@ package lock import ( + "path" "net/http" + + "github.com/gorilla/mux" ) // releaseLockHandler deletes the lock. func (h *handler) releaseLockHandler(w http.ResponseWriter, req *http.Request) { - // TODO: h.client.Delete(key_with_index) + h.client.SyncCluster() + + vars := mux.Vars(req) + keypath := path.Join(prefix, vars["key_with_index"]) + + // Delete the lock. + _, err := h.client.Delete(keypath) + if err != nil { + http.Error(w, "delete lock index error: " + err.Error(), http.StatusInternalServerError) + return + } } diff --git a/mod/lock/renew_handler.go b/mod/lock/renew_handler.go index ba9fe31d2..7933931e4 100644 --- a/mod/lock/renew_handler.go +++ b/mod/lock/renew_handler.go @@ -1,18 +1,30 @@ package lock import ( + "path" "net/http" - _ "path" + "strconv" - _ "github.com/gorilla/mux" + "github.com/gorilla/mux" ) // renewLockHandler attempts to update the TTL on an existing lock. -// Returns a 200 OK if successful. Otherwie +// Returns a 200 OK if successful. Returns non-200 on error. func (h *handler) renewLockHandler(w http.ResponseWriter, req *http.Request) { - /* + h.client.SyncCluster() + vars := mux.Vars(req) - key := path.Join(prefix, vars["key"]) - ttl := vars["ttl"] - */ + keypath := path.Join(prefix, vars["key_with_index"]) + ttl, err := strconv.Atoi(req.FormValue("ttl")) + if err != nil { + http.Error(w, "invalid ttl: " + err.Error(), http.StatusInternalServerError) + return + } + + // Renew the lock, if it exists. + _, err = h.client.Update(keypath, "-", uint64(ttl)) + if err != nil { + http.Error(w, "renew lock index error: " + err.Error(), http.StatusInternalServerError) + return + } } diff --git a/mod/lock/tests/handler_test.go b/mod/lock/tests/handler_test.go index e3caafe25..7e9091a0f 100644 --- a/mod/lock/tests/handler_test.go +++ b/mod/lock/tests/handler_test.go @@ -2,7 +2,6 @@ package lock import ( "fmt" - "net/url" "testing" "time" @@ -12,39 +11,178 @@ import ( ) // Ensure that a lock can be acquired and released. -func TestModLockAcquire(t *testing.T) { - v := url.Values{} +func TestModLockAcquireAndRelease(t *testing.T) { tests.RunServer(func(s *server.Server) { // Acquire lock. - url := fmt.Sprintf("http://%s%s", s.URL(), "/mod/lock/foo?ttl=2") - resp, err := tests.PutForm(url, v) + body, err := testAcquireLock(s, "foo", 10) assert.NoError(t, err) - ret := tests.ReadBody(resp) - assert.Equal(t, string(ret), "XXX") + assert.Equal(t, body, "2") - time.Sleep(60 * time.Second) - // TODO: Check that it has been acquired. - // TODO: Release lock. - // TODO: Check that it has been released. + // Check that we have the lock. + body, err = testGetLockIndex(s, "foo") + assert.NoError(t, err) + assert.Equal(t, body, "2") + + // Release lock. + body, err = testReleaseLock(s, "foo", 2) + assert.NoError(t, err) + assert.Equal(t, body, "") + + // Check that we have the lock. + body, err = testGetLockIndex(s, "foo") + assert.NoError(t, err) + assert.Equal(t, body, "") }) } // Ensure that a lock can be acquired and another process is blocked until released. -func TestModLockAcquireBlocked(t *testing.T) { - // TODO: Acquire lock with process #1. - // TODO: Acquire lock with process #2. - // TODO: Check that process #2 has not obtained lock. - // TODO: Release lock from process #1. - // TODO: Check that process #2 obtains the lock. - // TODO: Release lock from process #2. - // TODO: Check that no lock exists. +func TestModLockBlockUntilAcquire(t *testing.T) { + tests.RunServer(func(s *server.Server) { + c := make(chan bool) + + // Acquire lock #1. + go func() { + body, err := testAcquireLock(s, "foo", 10) + assert.NoError(t, err) + assert.Equal(t, body, "2") + c <- true + }() + <- c + + // Acquire lock #2. + go func() { + c <- true + body, err := testAcquireLock(s, "foo", 10) + assert.NoError(t, err) + assert.Equal(t, body, "4") + }() + <- c + + time.Sleep(1 * time.Second) + + // Check that we have the lock #1. + body, err := testGetLockIndex(s, "foo") + assert.NoError(t, err) + assert.Equal(t, body, "2") + + // Release lock #1. + body, err = testReleaseLock(s, "foo", 2) + assert.NoError(t, err) + + // Check that we have lock #2. + body, err = testGetLockIndex(s, "foo") + assert.NoError(t, err) + assert.Equal(t, body, "4") + + // Release lock #2. + body, err = testReleaseLock(s, "foo", 4) + assert.NoError(t, err) + + // Check that we have no lock. + body, err = testGetLockIndex(s, "foo") + assert.NoError(t, err) + assert.Equal(t, body, "") + }) } -// Ensure that an unowned lock can be released by force. -func TestModLockForceRelease(t *testing.T) { - // TODO: Acquire lock. - // TODO: Check that it has been acquired. - // TODO: Force release lock. - // TODO: Check that it has been released. - // TODO: Check that acquiring goroutine is notified that their lock has been released. +// Ensure that a lock will be released after the TTL. +func TestModLockExpireAndRelease(t *testing.T) { + tests.RunServer(func(s *server.Server) { + c := make(chan bool) + + // Acquire lock #1. + go func() { + body, err := testAcquireLock(s, "foo", 2) + assert.NoError(t, err) + assert.Equal(t, body, "2") + c <- true + }() + <- c + + // Acquire lock #2. + go func() { + c <- true + body, err := testAcquireLock(s, "foo", 10) + assert.NoError(t, err) + assert.Equal(t, body, "4") + }() + <- c + + time.Sleep(1 * time.Second) + + // Check that we have the lock #1. + body, err := testGetLockIndex(s, "foo") + assert.NoError(t, err) + assert.Equal(t, body, "2") + + // Wait for lock #1 TTL. + time.Sleep(2 * time.Second) + + // Check that we have lock #2. + body, err = testGetLockIndex(s, "foo") + assert.NoError(t, err) + assert.Equal(t, body, "4") + }) +} + +// Ensure that a lock can be renewed. +func TestModLockRenew(t *testing.T) { + tests.RunServer(func(s *server.Server) { + // Acquire lock. + body, err := testAcquireLock(s, "foo", 3) + assert.NoError(t, err) + assert.Equal(t, body, "2") + + time.Sleep(2 * time.Second) + + // Check that we have the lock. + body, err = testGetLockIndex(s, "foo") + assert.NoError(t, err) + assert.Equal(t, body, "2") + + // Renew lock. + body, err = testRenewLock(s, "foo", 2, 3) + assert.NoError(t, err) + assert.Equal(t, body, "") + + time.Sleep(2 * time.Second) + + // Check that we still have the lock. + body, err = testGetLockIndex(s, "foo") + assert.NoError(t, err) + assert.Equal(t, body, "2") + + time.Sleep(2 * time.Second) + + // Check that lock was released. + body, err = testGetLockIndex(s, "foo") + assert.NoError(t, err) + assert.Equal(t, body, "") + }) +} + + + +func testAcquireLock(s *server.Server, key string, ttl int) (string, error) { + resp, err := tests.PostForm(fmt.Sprintf("%s/mod/lock/%s?ttl=%d", s.URL(), key, ttl), nil) + ret := tests.ReadBody(resp) + return string(ret), err +} + +func testGetLockIndex(s *server.Server, key string) (string, error) { + resp, err := tests.Get(fmt.Sprintf("%s/mod/lock/%s", s.URL(), key)) + ret := tests.ReadBody(resp) + return string(ret), err +} + +func testReleaseLock(s *server.Server, key string, index int) (string, error) { + resp, err := tests.DeleteForm(fmt.Sprintf("%s/mod/lock/%s/%d", s.URL(), key, index), nil) + ret := tests.ReadBody(resp) + return string(ret), err +} + +func testRenewLock(s *server.Server, key string, index int, ttl int) (string, error) { + resp, err := tests.PutForm(fmt.Sprintf("%s/mod/lock/%s/%d?ttl=%d", s.URL(), key, index, ttl), nil) + ret := tests.ReadBody(resp) + return string(ret), err } From df20be775cd182abeecf6628754281577691655d Mon Sep 17 00:00:00 2001 From: Ben Johnson Date: Fri, 29 Nov 2013 16:35:06 -0700 Subject: [PATCH 05/14] Fix test harness. --- server/registry.go | 1 - server/server.go | 2 - server/v2/tests/delete_handler_test.go | 6 +-- server/v2/tests/get_handler_test.go | 28 ++++++------- server/v2/tests/post_handler_test.go | 14 +++---- server/v2/tests/put_handler_test.go | 58 +++++++++++++------------- test.sh | 3 +- 7 files changed, 54 insertions(+), 58 deletions(-) diff --git a/server/registry.go b/server/registry.go index 27b0ce414..d1d98d9ed 100644 --- a/server/registry.go +++ b/server/registry.go @@ -46,7 +46,6 @@ func (r *Registry) Register(name string, peerURL string, url string) error { key := path.Join(RegistryKey, name) value := fmt.Sprintf("raft=%s&etcd=%s", peerURL, url) _, err := r.store.Create(key, value, false, store.Permanent) - fmt.Println("register.1:", key, value, err) log.Debugf("Register: %s", name) return err } diff --git a/server/server.go b/server/server.go index f0de64a67..3bce222f0 100644 --- a/server/server.go +++ b/server/server.go @@ -320,14 +320,12 @@ func (s *Server) GetVersionHandler(w http.ResponseWriter, req *http.Request) err // Handler to return the current leader's raft address func (s *Server) GetLeaderHandler(w http.ResponseWriter, req *http.Request) error { leader := s.peerServer.RaftServer().Leader() - fmt.Println("/leader.1?", leader) if leader == "" { return etcdErr.NewError(etcdErr.EcodeLeaderElect, "", s.Store().Index()) } w.WriteHeader(http.StatusOK) url, _ := s.registry.PeerURL(leader) w.Write([]byte(url)) - fmt.Println("/leader.2?", leader, url) return nil } diff --git a/server/v2/tests/delete_handler_test.go b/server/v2/tests/delete_handler_test.go index 997127a9e..7ae472090 100644 --- a/server/v2/tests/delete_handler_test.go +++ b/server/v2/tests/delete_handler_test.go @@ -19,11 +19,11 @@ func TestV2DeleteKey(t *testing.T) { tests.RunServer(func(s *server.Server) { v := url.Values{} v.Set("value", "XXX") - resp, err := tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), v) + resp, err := tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), v) tests.ReadBody(resp) - resp, err = tests.DeleteForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), url.Values{}) + resp, err = tests.DeleteForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), url.Values{}) body := tests.ReadBody(resp) assert.Nil(t, err, "") - assert.Equal(t, string(body), `{"action":"delete","key":"/foo/bar","prevValue":"XXX","modifiedIndex":2}`, "") + assert.Equal(t, string(body), `{"action":"delete","key":"/foo/bar","prevValue":"XXX","modifiedIndex":3}`, "") }) } diff --git a/server/v2/tests/get_handler_test.go b/server/v2/tests/get_handler_test.go index b15195873..417d7edae 100644 --- a/server/v2/tests/get_handler_test.go +++ b/server/v2/tests/get_handler_test.go @@ -20,14 +20,14 @@ func TestV2GetKey(t *testing.T) { tests.RunServer(func(s *server.Server) { v := url.Values{} v.Set("value", "XXX") - resp, _ := tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), v) + resp, _ := tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), v) tests.ReadBody(resp) - resp, _ = tests.Get(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar")) + resp, _ = tests.Get(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar")) body := tests.ReadBodyJSON(resp) assert.Equal(t, body["action"], "get", "") assert.Equal(t, body["key"], "/foo/bar", "") assert.Equal(t, body["value"], "XXX", "") - assert.Equal(t, body["modifiedIndex"], 1, "") + assert.Equal(t, body["modifiedIndex"], 2, "") }) } @@ -42,19 +42,19 @@ func TestV2GetKeyRecursively(t *testing.T) { v := url.Values{} v.Set("value", "XXX") v.Set("ttl", "10") - resp, _ := tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/x"), v) + resp, _ := tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/x"), v) tests.ReadBody(resp) v.Set("value", "YYY") - resp, _ = tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/y/z"), v) + resp, _ = tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/y/z"), v) tests.ReadBody(resp) - resp, _ = tests.Get(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo?recursive=true")) + resp, _ = tests.Get(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo?recursive=true")) body := tests.ReadBodyJSON(resp) assert.Equal(t, body["action"], "get", "") assert.Equal(t, body["key"], "/foo", "") assert.Equal(t, body["dir"], true, "") - assert.Equal(t, body["modifiedIndex"], 1, "") + assert.Equal(t, body["modifiedIndex"], 2, "") assert.Equal(t, len(body["kvs"].([]interface{})), 2, "") kv0 := body["kvs"].([]interface{})[0].(map[string]interface{}) @@ -82,7 +82,7 @@ func TestV2WatchKey(t *testing.T) { var body map[string]interface{} c := make(chan bool) go func() { - resp, _ := tests.Get(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar?wait=true")) + resp, _ := tests.Get(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar?wait=true")) body = tests.ReadBodyJSON(resp) c <- true }() @@ -94,7 +94,7 @@ func TestV2WatchKey(t *testing.T) { // Set a value. v := url.Values{} v.Set("value", "XXX") - resp, _ := tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), v) + resp, _ := tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), v) tests.ReadBody(resp) // A response should follow from the GET above. @@ -111,7 +111,7 @@ func TestV2WatchKey(t *testing.T) { assert.Equal(t, body["action"], "set", "") assert.Equal(t, body["key"], "/foo/bar", "") assert.Equal(t, body["value"], "XXX", "") - assert.Equal(t, body["modifiedIndex"], 1, "") + assert.Equal(t, body["modifiedIndex"], 2, "") }) } @@ -126,7 +126,7 @@ func TestV2WatchKeyWithIndex(t *testing.T) { var body map[string]interface{} c := make(chan bool) go func() { - resp, _ := tests.Get(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar?wait=true&waitIndex=2")) + resp, _ := tests.Get(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar?wait=true&waitIndex=3")) body = tests.ReadBodyJSON(resp) c <- true }() @@ -138,7 +138,7 @@ func TestV2WatchKeyWithIndex(t *testing.T) { // Set a value (before given index). v := url.Values{} v.Set("value", "XXX") - resp, _ := tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), v) + resp, _ := tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), v) tests.ReadBody(resp) // Make sure response didn't fire early. @@ -147,7 +147,7 @@ func TestV2WatchKeyWithIndex(t *testing.T) { // Set a value (before given index). v.Set("value", "YYY") - resp, _ = tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), v) + resp, _ = tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), v) tests.ReadBody(resp) // A response should follow from the GET above. @@ -164,6 +164,6 @@ func TestV2WatchKeyWithIndex(t *testing.T) { assert.Equal(t, body["action"], "set", "") assert.Equal(t, body["key"], "/foo/bar", "") assert.Equal(t, body["value"], "YYY", "") - assert.Equal(t, body["modifiedIndex"], 2, "") + assert.Equal(t, body["modifiedIndex"], 3, "") }) } diff --git a/server/v2/tests/post_handler_test.go b/server/v2/tests/post_handler_test.go index 856633ef0..ada6db101 100644 --- a/server/v2/tests/post_handler_test.go +++ b/server/v2/tests/post_handler_test.go @@ -18,21 +18,21 @@ import ( func TestV2CreateUnique(t *testing.T) { tests.RunServer(func(s *server.Server) { // POST should add index to list. - resp, _ := tests.PostForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), nil) + resp, _ := tests.PostForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), nil) body := tests.ReadBodyJSON(resp) assert.Equal(t, body["action"], "create", "") - assert.Equal(t, body["key"], "/foo/bar/1", "") + assert.Equal(t, body["key"], "/foo/bar/2", "") assert.Equal(t, body["dir"], true, "") - assert.Equal(t, body["modifiedIndex"], 1, "") + assert.Equal(t, body["modifiedIndex"], 2, "") // Second POST should add next index to list. - resp, _ = tests.PostForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), nil) + resp, _ = tests.PostForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), nil) body = tests.ReadBodyJSON(resp) - assert.Equal(t, body["key"], "/foo/bar/2", "") + assert.Equal(t, body["key"], "/foo/bar/3", "") // POST to a different key should add index to that list. - resp, _ = tests.PostForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/baz"), nil) + resp, _ = tests.PostForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/baz"), nil) body = tests.ReadBodyJSON(resp) - assert.Equal(t, body["key"], "/foo/baz/3", "") + assert.Equal(t, body["key"], "/foo/baz/4", "") }) } diff --git a/server/v2/tests/put_handler_test.go b/server/v2/tests/put_handler_test.go index 3ee642604..c72995c81 100644 --- a/server/v2/tests/put_handler_test.go +++ b/server/v2/tests/put_handler_test.go @@ -19,10 +19,10 @@ func TestV2SetKey(t *testing.T) { tests.RunServer(func(s *server.Server) { v := url.Values{} v.Set("value", "XXX") - resp, err := tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), v) + resp, err := tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), v) body := tests.ReadBody(resp) assert.Nil(t, err, "") - assert.Equal(t, string(body), `{"action":"set","key":"/foo/bar","value":"XXX","modifiedIndex":1}`, "") + assert.Equal(t, string(body), `{"action":"set","key":"/foo/bar","value":"XXX","modifiedIndex":2}`, "") }) } @@ -36,7 +36,7 @@ func TestV2SetKeyWithTTL(t *testing.T) { v := url.Values{} v.Set("value", "XXX") v.Set("ttl", "20") - resp, _ := tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), v) + resp, _ := tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), v) body := tests.ReadBodyJSON(resp) assert.Equal(t, body["ttl"], 20, "") @@ -55,7 +55,7 @@ func TestV2SetKeyWithBadTTL(t *testing.T) { v := url.Values{} v.Set("value", "XXX") v.Set("ttl", "bad_ttl") - resp, _ := tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), v) + resp, _ := tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), v) body := tests.ReadBodyJSON(resp) assert.Equal(t, body["errorCode"], 202, "") assert.Equal(t, body["message"], "The given TTL in POST form is not a number", "") @@ -72,7 +72,7 @@ func TestV2CreateKeySuccess(t *testing.T) { v := url.Values{} v.Set("value", "XXX") v.Set("prevExist", "false") - resp, _ := tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), v) + resp, _ := tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), v) body := tests.ReadBodyJSON(resp) assert.Equal(t, body["value"], "XXX", "") }) @@ -88,9 +88,9 @@ func TestV2CreateKeyFail(t *testing.T) { v := url.Values{} v.Set("value", "XXX") v.Set("prevExist", "false") - resp, _ := tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), v) + resp, _ := tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), v) tests.ReadBody(resp) - resp, _ = tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), v) + resp, _ = tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), v) body := tests.ReadBodyJSON(resp) assert.Equal(t, body["errorCode"], 105, "") assert.Equal(t, body["message"], "Already exists", "") @@ -108,12 +108,12 @@ func TestV2UpdateKeySuccess(t *testing.T) { v := url.Values{} v.Set("value", "XXX") - resp, _ := tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), v) + resp, _ := tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), v) tests.ReadBody(resp) v.Set("value", "YYY") v.Set("prevExist", "true") - resp, _ = tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), v) + resp, _ = tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), v) body := tests.ReadBodyJSON(resp) assert.Equal(t, body["action"], "update", "") assert.Equal(t, body["prevValue"], "XXX", "") @@ -127,11 +127,11 @@ func TestV2UpdateKeySuccess(t *testing.T) { func TestV2UpdateKeyFailOnValue(t *testing.T) { tests.RunServer(func(s *server.Server) { v := url.Values{} - resp, _ := tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo"), v) + resp, _ := tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo"), v) v.Set("value", "YYY") v.Set("prevExist", "true") - resp, _ = tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), v) + resp, _ = tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), v) body := tests.ReadBodyJSON(resp) assert.Equal(t, body["errorCode"], 100, "") assert.Equal(t, body["message"], "Key Not Found", "") @@ -149,7 +149,7 @@ func TestV2UpdateKeyFailOnMissingDirectory(t *testing.T) { v := url.Values{} v.Set("value", "YYY") v.Set("prevExist", "true") - resp, _ := tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), v) + resp, _ := tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), v) body := tests.ReadBodyJSON(resp) assert.Equal(t, body["errorCode"], 100, "") assert.Equal(t, body["message"], "Key Not Found", "") @@ -166,16 +166,16 @@ func TestV2SetKeyCASOnIndexSuccess(t *testing.T) { tests.RunServer(func(s *server.Server) { v := url.Values{} v.Set("value", "XXX") - resp, _ := tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), v) + resp, _ := tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), v) tests.ReadBody(resp) v.Set("value", "YYY") - v.Set("prevIndex", "1") - resp, _ = tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), v) + v.Set("prevIndex", "2") + resp, _ = tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), v) body := tests.ReadBodyJSON(resp) assert.Equal(t, body["action"], "compareAndSwap", "") assert.Equal(t, body["prevValue"], "XXX", "") assert.Equal(t, body["value"], "YYY", "") - assert.Equal(t, body["modifiedIndex"], 2, "") + assert.Equal(t, body["modifiedIndex"], 3, "") }) } @@ -188,16 +188,16 @@ func TestV2SetKeyCASOnIndexFail(t *testing.T) { tests.RunServer(func(s *server.Server) { v := url.Values{} v.Set("value", "XXX") - resp, _ := tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), v) + resp, _ := tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), v) tests.ReadBody(resp) v.Set("value", "YYY") v.Set("prevIndex", "10") - resp, _ = tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), v) + resp, _ = tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), v) body := tests.ReadBodyJSON(resp) assert.Equal(t, body["errorCode"], 101, "") assert.Equal(t, body["message"], "Test Failed", "") - assert.Equal(t, body["cause"], "[ != XXX] [10 != 1]", "") - assert.Equal(t, body["index"], 1, "") + assert.Equal(t, body["cause"], "[ != XXX] [10 != 2]", "") + assert.Equal(t, body["index"], 2, "") }) } @@ -210,7 +210,7 @@ func TestV2SetKeyCASWithInvalidIndex(t *testing.T) { v := url.Values{} v.Set("value", "YYY") v.Set("prevIndex", "bad_index") - resp, _ := tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), v) + resp, _ := tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), v) body := tests.ReadBodyJSON(resp) assert.Equal(t, body["errorCode"], 203, "") assert.Equal(t, body["message"], "The given index in POST form is not a number", "") @@ -227,16 +227,16 @@ func TestV2SetKeyCASOnValueSuccess(t *testing.T) { tests.RunServer(func(s *server.Server) { v := url.Values{} v.Set("value", "XXX") - resp, _ := tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), v) + resp, _ := tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), v) tests.ReadBody(resp) v.Set("value", "YYY") v.Set("prevValue", "XXX") - resp, _ = tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), v) + resp, _ = tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), v) body := tests.ReadBodyJSON(resp) assert.Equal(t, body["action"], "compareAndSwap", "") assert.Equal(t, body["prevValue"], "XXX", "") assert.Equal(t, body["value"], "YYY", "") - assert.Equal(t, body["modifiedIndex"], 2, "") + assert.Equal(t, body["modifiedIndex"], 3, "") }) } @@ -249,16 +249,16 @@ func TestV2SetKeyCASOnValueFail(t *testing.T) { tests.RunServer(func(s *server.Server) { v := url.Values{} v.Set("value", "XXX") - resp, _ := tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), v) + resp, _ := tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), v) tests.ReadBody(resp) v.Set("value", "YYY") v.Set("prevValue", "AAA") - resp, _ = tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), v) + resp, _ = tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), v) body := tests.ReadBodyJSON(resp) assert.Equal(t, body["errorCode"], 101, "") assert.Equal(t, body["message"], "Test Failed", "") - assert.Equal(t, body["cause"], "[AAA != XXX] [0 != 1]", "") - assert.Equal(t, body["index"], 1, "") + assert.Equal(t, body["cause"], "[AAA != XXX] [0 != 2]", "") + assert.Equal(t, body["index"], 2, "") }) } @@ -271,7 +271,7 @@ func TestV2SetKeyCASWithMissingValueFails(t *testing.T) { v := url.Values{} v.Set("value", "XXX") v.Set("prevValue", "") - resp, _ := tests.PutForm(fmt.Sprintf("http://%s%s", s.URL(), "/v2/keys/foo/bar"), v) + resp, _ := tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), v) body := tests.ReadBodyJSON(resp) assert.Equal(t, body["errorCode"], 201, "") assert.Equal(t, body["message"], "PrevValue is Required in POST form", "") diff --git a/test.sh b/test.sh index 690f3a932..246b03ad4 100755 --- a/test.sh +++ b/test.sh @@ -1,8 +1,7 @@ #!/bin/sh set -e -PKGS="./mod/lock/tests" -# PKGS="./store ./server ./server/v2/tests" +PKGS="./store ./server ./server/v2/tests ./mod/lock/tests" # Get GOPATH, etc from build . ./build From ded3cc24c0fb47d60695a2d2ffe731664576a074 Mon Sep 17 00:00:00 2001 From: Xiang Li Date: Mon, 2 Dec 2013 22:53:36 -0500 Subject: [PATCH 06/14] fix redirect url should include rawquery --- server/v2/get_handler.go | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/server/v2/get_handler.go b/server/v2/get_handler.go index 2f48fc32a..9a67ea2ae 100644 --- a/server/v2/get_handler.go +++ b/server/v2/get_handler.go @@ -4,6 +4,7 @@ import ( "encoding/json" "fmt" "net/http" + "net/url" "strconv" etcdErr "github.com/coreos/etcd/error" @@ -24,9 +25,17 @@ func GetHandler(w http.ResponseWriter, req *http.Request, s Server) error { if req.FormValue("consistent") == "true" && s.State() != raft.Leader { leader := s.Leader() hostname, _ := s.ClientURL(leader) - url := hostname + req.URL.Path - log.Debugf("Redirect consistent get to %s", url) - http.Redirect(w, req, url, http.StatusTemporaryRedirect) + + url, err := url.Parse(hostname) + if err != nil { + log.Warn("Redirect cannot parse hostName ", hostname) + return err + } + url.RawQuery = req.URL.RawQuery + url.Path = req.URL.Path + + log.Debugf("Redirect consistent get to %s", url.String()) + http.Redirect(w, req, url.String(), http.StatusTemporaryRedirect) return nil } From 0dc428b5d68355c24c71f789d09b4c3856238c09 Mon Sep 17 00:00:00 2001 From: Brandon Philips Date: Wed, 4 Dec 2013 12:06:41 -0800 Subject: [PATCH 07/14] fix(README): use the new response format update all of the examples to use the new response format. --- README.md | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/README.md b/README.md index a74363eb4..207a4e7f3 100644 --- a/README.md +++ b/README.md @@ -83,7 +83,7 @@ curl -L http://127.0.0.1:4001/v2/keys/message -X PUT -d value="Hello world" ``` ```json -{"action":"set","key":"/message","value":"Hello world","modifiedIndex":2} +{"action":"set","node":{"key":"/message","value":"Hello world","modifiedIndex":2,"createdIndex":2}} ``` This response contains four fields. @@ -112,7 +112,7 @@ curl -L http://127.0.0.1:4001/v2/keys/message ``` ```json -{"action":"get","key":"/message","value":"Hello world","modifiedIndex":2} +{"action":"get","node":{"key":"/message","value":"Hello world","modifiedIndex":2,"createdIndex":2}} ``` @@ -125,10 +125,10 @@ curl -L http://127.0.0.1:4001/v2/keys/message -XPUT -d value="Hello etcd" ``` ```json -{"action":"set","key":"/message","prevValue":"Hello world","value":"Hello etcd","index":3} +{"action":"set","node":{"key":"/message","prevValue":"Hello world","value":"Hello etcd","modifiedIndex":3,"createdIndex":3}} ``` -Notice that the `prevValue` is set to the previous value of the key - `Hello world`. +Notice that `node.prevValue` is set to the previous value of the key - `Hello world`. It is useful when you want to atomically set a value to a key and get its old value. @@ -141,7 +141,7 @@ curl -L http://127.0.0.1:4001/v2/keys/message -XDELETE ``` ```json -{"action":"delete","key":"/message","prevValue":"Hello etcd","modifiedIndex":4} +{"action":"delete","node":{"key":"/message","prevValue":"Hello etcd","modifiedIndex":4,"createdIndex":3}} ``` @@ -155,7 +155,7 @@ curl -L http://127.0.0.1:4001/v2/keys/foo -XPUT -d value=bar -d ttl=5 ``` ```json -{"action":"set","key":"/foo","value":"bar","expiration":"2013-11-12T20:21:22.629352334-05:00","ttl":5,"modifiedIndex":5} +{"action":"set","node":{"key":"/foo","value":"bar","expiration":"2013-12-04T12:01:21.874888581-08:00","ttl":5,"modifiedIndex":5,"createdIndex":5}} ``` Note the two new fields in response: @@ -201,7 +201,7 @@ curl -L http://127.0.0.1:4001/v2/keys/foo -XPUT -d value=bar The first terminal should get the notification and return with the same response as the set request. ```json -{"action":"set","key":"/foo","value":"bar","modifiedIndex":7} +{"action":"set","node":{"key":"/foo","value":"bar","modifiedIndex":7,"createdIndex":7}} ``` However, the watch command can do more than this. @@ -274,10 +274,10 @@ curl -L http://127.0.0.1:4001/v2/keys/foo?prevValue=one -XPUT -d value=two The response should be ```json -{"action":"compareAndSwap","key":"/foo","prevValue":"one","value":"two","modifiedIndex":9} +{"action":"compareAndSwap","node":{"key":"/foo","prevValue":"one","value":"two","modifiedIndex":9,"createdIndex":8}} ``` -We successfully changed the value from “one” to “two” since we gave the correct previous value. +We successfully changed the value from "one" to "two" since we gave the correct previous value. ### Listing a directory @@ -295,7 +295,7 @@ curl -L http://127.0.0.1:4001/v2/keys/foo_dir/foo -XPUT -d value=bar ``` ```json -{"action":"set","key":"/foo_dir/foo","value":"bar","modifiedIndex":10} +{"action":"set","node":{"key":"/foo_dir/foo","value":"bar","modifiedIndex":2,"createdIndex":2}} ``` Now we can list the keys under root `/`: @@ -307,7 +307,7 @@ curl -L http://127.0.0.1:4001/v2/keys/ We should see the response as an array of items: ```json -{"action":"get","key":"/","dir":true,"kvs":[{"key":"/foo","value":"two","modifiedIndex":9},{"key":"/foo_dir","dir":true,"modifiedIndex":10}],"modifiedIndex":0} +{"action":"get","node":{"key":"/","dir":true,"nodes":[{"key":"/foo_dir","dir":true,"modifiedIndex":2,"createdIndex":2}]}} ``` Here we can see `/foo` is a key-value pair under `/` and `/foo_dir` is a directory. @@ -318,7 +318,7 @@ curl -L http://127.0.0.1:4001/v2/keys/?recursive=true ``` ```json -{"action":"get","key":"/","dir":true,"kvs":[{"key":"/foo","value":"two","modifiedIndex":9},{"key":"/foo_dir","dir":true,"kvs":[{"key":"/foo_dir/foo","value":"bar","modifiedIndex":10}],"modifiedIndex":10}],"modifiedIndex":0} +{"action":"get","node":{"key":"/","dir":true,"nodes":[{"key":"/foo_dir","dir":true,"nodes":[{"key":"/foo_dir/foo","value":"bar","modifiedIndex":2,"createdIndex":2}],"modifiedIndex":2,"createdIndex":2}]}} ``` @@ -333,7 +333,7 @@ curl -L http://127.0.0.1:4001/v2/keys/foo_dir?recursive=true -XDELETE ``` ```json -{"action":"delete","key":"/foo_dir","dir":true,"modifiedIndex":11} +{"action":"delete","node":{"key":"/foo_dir","dir":true,"modifiedIndex":11,"createdIndex":10}} ``` @@ -349,7 +349,7 @@ curl -L http://127.0.0.1:4001/v2/keys/_message -XPUT -d value="Hello hidden worl ``` ```json -{"action":"set","key":"/_message","value":"Hello hidden world","modifiedIndex":12} +{"action":"set","node":{"key":"/_message","value":"Hello hidden world","modifiedIndex":3,"createdIndex":3}} ``` @@ -360,7 +360,7 @@ curl -L http://127.0.0.1:4001/v2/keys/message -XPUT -d value="Hello world" ``` ```json -{"action":"set","key":"/message","value":"Hello world","modifiedIndex":13} +{"action":"set","node":{"key":"/message","value":"Hello world","modifiedIndex":4,"createdIndex":4}} ``` Now let's try to get a listing of keys under the root directory, `/`: @@ -370,7 +370,7 @@ curl -L http://127.0.0.1:4001/v2/keys/ ``` ```json -{"action":"get","key":"/","dir":true,"kvs":[{"key":"/foo","value":"two","modifiedIndex":9},{"key":"/message","value":"Hello world","modifiedIndex":13}],"modifiedIndex":0} +{"action":"get","node":{"key":"/","dir":true,"nodes":[{"key":"/foo_dir","dir":true,"modifiedIndex":2,"createdIndex":2},{"key":"/message","value":"Hello world","modifiedIndex":4,"createdIndex":4}]}} ``` Here we see the `/message` key but our hidden `/_message` key is not returned. @@ -468,7 +468,7 @@ TLS handshake, Finished (20) And also the response from the server: ```json -{"action":"set","key":"/foo","prevValue":"bar","value":"bar","modifiedIndex":3} +{"action":"set","node":{"key":"/foo","prevValue":"two","value":"bar","modifiedIndex":12,"createdIndex":12}} ``` @@ -516,7 +516,7 @@ curl -L http://127.0.0.1:4001/v2/keys/_etcd/machines ``` ```json -[{"action":"get","key":"/_etcd/machines/machine1","value":"raft=http://127.0.0.1:7001\u0026etcd=http://127.0.0.1:4001","index":1},{"action":"get","key":"/_etcd/machines/machine2","value":"raft=http://127.0.0.1:7002\u0026etcd=http://127.0.0.1:4002","index":1},{"action":"get","key":"/_etcd/machines/machine3","value":"raft=http://127.0.0.1:7003\u0026etcd=http://127.0.0.1:4003","index":1}] +{"action":"get","node":{"key":"/_etcd/machines","dir":true,"nodes":[{"key":"/_etcd/machines/machine1","value":"raft=http://127.0.0.1:7001\u0026etcd=http://127.0.0.1:4001","modifiedIndex":1,"createdIndex":1},{"key":"/_etcd/machines/machine2","value":"raft=http://127.0.0.1:7002\u0026etcd=http://127.0.0.1:4002","modifiedIndex":2,"createdIndex":2},{"key":"/_etcd/machines/machine3","value":"raft=http://127.0.0.1:7003\u0026etcd=http://127.0.0.1:4003","modifiedIndex":3,"createdIndex":3}],"modifiedIndex":1,"createdIndex":1}} ``` We can also get the current leader in the cluster: @@ -538,7 +538,7 @@ curl -L http://127.0.0.1:4001/v2/keys/foo -XPUT -d value=bar ``` ```json -{"action":"set","key":"/foo","value":"bar","modifiedIndex":4} +{"action":"set","node":{"key":"/foo","value":"bar","modifiedIndex":4,"createdIndex":4}} ``` @@ -579,7 +579,7 @@ curl -L http://127.0.0.1:4002/v2/keys/foo ``` ```json -{"action":"get","key":"/foo","value":"bar","index":4} +{"action":"get","node":{"key":"/foo","value":"bar","modifiedIndex":4,"createdIndex":4}} ``` From 7ec3f861e4aa6d40c5b06ffe6fed0f9b2d16b677 Mon Sep 17 00:00:00 2001 From: Brandon Philips Date: Wed, 4 Dec 2013 13:48:38 -0800 Subject: [PATCH 08/14] fix(README): prettify json fixed using vim and python -m json.tool --- README.md | 266 +++++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 244 insertions(+), 22 deletions(-) diff --git a/README.md b/README.md index 207a4e7f3..a95da5eec 100644 --- a/README.md +++ b/README.md @@ -83,7 +83,15 @@ curl -L http://127.0.0.1:4001/v2/keys/message -X PUT -d value="Hello world" ``` ```json -{"action":"set","node":{"key":"/message","value":"Hello world","modifiedIndex":2,"createdIndex":2}} +{ + "action": "set", + "node": { + "createdIndex": 2, + "key": "/message", + "modifiedIndex": 2, + "value": "Hello world" + } +} ``` This response contains four fields. @@ -112,7 +120,15 @@ curl -L http://127.0.0.1:4001/v2/keys/message ``` ```json -{"action":"get","node":{"key":"/message","value":"Hello world","modifiedIndex":2,"createdIndex":2}} +{ + "action": "get", + "node": { + "createdIndex": 2, + "key": "/message", + "modifiedIndex": 2, + "value": "Hello world" + } +} ``` @@ -125,7 +141,16 @@ curl -L http://127.0.0.1:4001/v2/keys/message -XPUT -d value="Hello etcd" ``` ```json -{"action":"set","node":{"key":"/message","prevValue":"Hello world","value":"Hello etcd","modifiedIndex":3,"createdIndex":3}} +{ + "action": "set", + "node": { + "createdIndex": 3, + "key": "/message", + "modifiedIndex": 3, + "prevValue": "Hello world", + "value": "Hello etcd" + } +} ``` Notice that `node.prevValue` is set to the previous value of the key - `Hello world`. @@ -141,7 +166,15 @@ curl -L http://127.0.0.1:4001/v2/keys/message -XDELETE ``` ```json -{"action":"delete","node":{"key":"/message","prevValue":"Hello etcd","modifiedIndex":4,"createdIndex":3}} +{ + "action": "delete", + "node": { + "createdIndex": 3, + "key": "/message", + "modifiedIndex": 4, + "prevValue": "Hello etcd" + } +} ``` @@ -155,7 +188,17 @@ curl -L http://127.0.0.1:4001/v2/keys/foo -XPUT -d value=bar -d ttl=5 ``` ```json -{"action":"set","node":{"key":"/foo","value":"bar","expiration":"2013-12-04T12:01:21.874888581-08:00","ttl":5,"modifiedIndex":5,"createdIndex":5}} +{ + "action": "set", + "node": { + "createdIndex": 5, + "expiration": "2013-12-04T12:01:21.874888581-08:00", + "key": "/foo", + "modifiedIndex": 5, + "ttl": 5, + "value": "bar" + } +} ``` Note the two new fields in response: @@ -175,7 +218,12 @@ curl -L http://127.0.0.1:4001/v2/keys/foo If the TTL has expired, the key will be deleted, and you will be returned a 100. ```json -{"errorCode":100,"message":"Key Not Found","cause":"/foo","index":6} +{ + "cause": "/foo", + "errorCode": 100, + "index": 6, + "message": "Key Not Found" +} ``` @@ -201,7 +249,15 @@ curl -L http://127.0.0.1:4001/v2/keys/foo -XPUT -d value=bar The first terminal should get the notification and return with the same response as the set request. ```json -{"action":"set","node":{"key":"/foo","value":"bar","modifiedIndex":7,"createdIndex":7}} +{ + "action": "set", + "node": { + "createdIndex": 7, + "key": "/foo", + "modifiedIndex": 7, + "value": "bar" + } +} ``` However, the watch command can do more than this. @@ -248,7 +304,12 @@ curl -L http://127.0.0.1:4001/v2/keys/foo?prevExist=false -XPUT -d value=three The error code explains the problem: ```json -{"errorCode":105,"message":"Already exists","cause":"/foo","index":39776} +{ + "cause": "/foo", + "errorCode": 105, + "index": 39776, + "message": "Already exists" +} ``` Now lets provide a `prevValue` parameter: @@ -260,7 +321,12 @@ curl -L http://127.0.0.1:4001/v2/keys/foo?prevValue=two -XPUT -d value=three This will try to compare the previous value of the key and the previous value we provided. If they are equal, the value of the key will change to three. ```json -{"errorCode":101,"message":"Test Failed","cause":"[two != one] [0 != 8]","index":8} +{ + "cause": "[two != one] [0 != 8]", + "errorCode": 101, + "index": 8, + "message": "Test Failed" +} ``` which means `CompareAndSwap` failed. @@ -274,7 +340,16 @@ curl -L http://127.0.0.1:4001/v2/keys/foo?prevValue=one -XPUT -d value=two The response should be ```json -{"action":"compareAndSwap","node":{"key":"/foo","prevValue":"one","value":"two","modifiedIndex":9,"createdIndex":8}} +{ + "action": "compareAndSwap", + "node": { + "createdIndex": 8, + "key": "/foo", + "modifiedIndex": 9, + "prevValue": "one", + "value": "two" + } +} ``` We successfully changed the value from "one" to "two" since we gave the correct previous value. @@ -295,7 +370,15 @@ curl -L http://127.0.0.1:4001/v2/keys/foo_dir/foo -XPUT -d value=bar ``` ```json -{"action":"set","node":{"key":"/foo_dir/foo","value":"bar","modifiedIndex":2,"createdIndex":2}} +{ + "action": "set", + "node": { + "createdIndex": 2, + "key": "/foo_dir/foo", + "modifiedIndex": 2, + "value": "bar" + } +} ``` Now we can list the keys under root `/`: @@ -307,7 +390,21 @@ curl -L http://127.0.0.1:4001/v2/keys/ We should see the response as an array of items: ```json -{"action":"get","node":{"key":"/","dir":true,"nodes":[{"key":"/foo_dir","dir":true,"modifiedIndex":2,"createdIndex":2}]}} +{ + "action": "get", + "node": { + "dir": true, + "key": "/", + "nodes": [ + { + "createdIndex": 2, + "dir": true, + "key": "/foo_dir", + "modifiedIndex": 2 + } + ] + } +} ``` Here we can see `/foo` is a key-value pair under `/` and `/foo_dir` is a directory. @@ -318,7 +415,29 @@ curl -L http://127.0.0.1:4001/v2/keys/?recursive=true ``` ```json -{"action":"get","node":{"key":"/","dir":true,"nodes":[{"key":"/foo_dir","dir":true,"nodes":[{"key":"/foo_dir/foo","value":"bar","modifiedIndex":2,"createdIndex":2}],"modifiedIndex":2,"createdIndex":2}]}} +{ + "action": "get", + "node": { + "dir": true, + "key": "/", + "nodes": [ + { + "createdIndex": 2, + "dir": true, + "key": "/foo_dir", + "modifiedIndex": 2, + "nodes": [ + { + "createdIndex": 2, + "key": "/foo_dir/foo", + "modifiedIndex": 2, + "value": "bar" + } + ] + } + ] + } +} ``` @@ -333,7 +452,15 @@ curl -L http://127.0.0.1:4001/v2/keys/foo_dir?recursive=true -XDELETE ``` ```json -{"action":"delete","node":{"key":"/foo_dir","dir":true,"modifiedIndex":11,"createdIndex":10}} +{ + "action": "delete", + "node": { + "createdIndex": 10, + "dir": true, + "key": "/foo_dir", + "modifiedIndex": 11 + } +} ``` @@ -349,7 +476,15 @@ curl -L http://127.0.0.1:4001/v2/keys/_message -XPUT -d value="Hello hidden worl ``` ```json -{"action":"set","node":{"key":"/_message","value":"Hello hidden world","modifiedIndex":3,"createdIndex":3}} +{ + "action": "set", + "node": { + "createdIndex": 3, + "key": "/_message", + "modifiedIndex": 3, + "value": "Hello hidden world" + } +} ``` @@ -360,7 +495,15 @@ curl -L http://127.0.0.1:4001/v2/keys/message -XPUT -d value="Hello world" ``` ```json -{"action":"set","node":{"key":"/message","value":"Hello world","modifiedIndex":4,"createdIndex":4}} +{ + "action": "set", + "node": { + "createdIndex": 4, + "key": "/message", + "modifiedIndex": 4, + "value": "Hello world" + } +} ``` Now let's try to get a listing of keys under the root directory, `/`: @@ -370,7 +513,27 @@ curl -L http://127.0.0.1:4001/v2/keys/ ``` ```json -{"action":"get","node":{"key":"/","dir":true,"nodes":[{"key":"/foo_dir","dir":true,"modifiedIndex":2,"createdIndex":2},{"key":"/message","value":"Hello world","modifiedIndex":4,"createdIndex":4}]}} +{ + "action": "get", + "node": { + "dir": true, + "key": "/", + "nodes": [ + { + "createdIndex": 2, + "dir": true, + "key": "/foo_dir", + "modifiedIndex": 2 + }, + { + "createdIndex": 4, + "key": "/message", + "modifiedIndex": 4, + "value": "Hello world" + } + ] + } +} ``` Here we see the `/message` key but our hidden `/_message` key is not returned. @@ -421,7 +584,13 @@ SSLv3, TLS handshake, Finished (20): And also the response from the etcd server: ```json -{"action":"set","key":"/foo","prevValue":"bar","value":"bar","modifiedIndex":3} +{ + "action": "set", + "key": "/foo", + "modifiedIndex": 3, + "prevValue": "bar", + "value": "bar" +} ``` @@ -468,7 +637,16 @@ TLS handshake, Finished (20) And also the response from the server: ```json -{"action":"set","node":{"key":"/foo","prevValue":"two","value":"bar","modifiedIndex":12,"createdIndex":12}} +{ + "action": "set", + "node": { + "createdIndex": 12, + "key": "/foo", + "modifiedIndex": 12, + "prevValue": "two", + "value": "bar" + } +} ``` @@ -516,7 +694,35 @@ curl -L http://127.0.0.1:4001/v2/keys/_etcd/machines ``` ```json -{"action":"get","node":{"key":"/_etcd/machines","dir":true,"nodes":[{"key":"/_etcd/machines/machine1","value":"raft=http://127.0.0.1:7001\u0026etcd=http://127.0.0.1:4001","modifiedIndex":1,"createdIndex":1},{"key":"/_etcd/machines/machine2","value":"raft=http://127.0.0.1:7002\u0026etcd=http://127.0.0.1:4002","modifiedIndex":2,"createdIndex":2},{"key":"/_etcd/machines/machine3","value":"raft=http://127.0.0.1:7003\u0026etcd=http://127.0.0.1:4003","modifiedIndex":3,"createdIndex":3}],"modifiedIndex":1,"createdIndex":1}} +{ + "action": "get", + "node": { + "createdIndex": 1, + "dir": true, + "key": "/_etcd/machines", + "modifiedIndex": 1, + "nodes": [ + { + "createdIndex": 1, + "key": "/_etcd/machines/machine1", + "modifiedIndex": 1, + "value": "raft=http://127.0.0.1:7001&etcd=http://127.0.0.1:4001" + }, + { + "createdIndex": 2, + "key": "/_etcd/machines/machine2", + "modifiedIndex": 2, + "value": "raft=http://127.0.0.1:7002&etcd=http://127.0.0.1:4002" + }, + { + "createdIndex": 3, + "key": "/_etcd/machines/machine3", + "modifiedIndex": 3, + "value": "raft=http://127.0.0.1:7003&etcd=http://127.0.0.1:4003" + } + ] + } +} ``` We can also get the current leader in the cluster: @@ -538,7 +744,15 @@ curl -L http://127.0.0.1:4001/v2/keys/foo -XPUT -d value=bar ``` ```json -{"action":"set","node":{"key":"/foo","value":"bar","modifiedIndex":4,"createdIndex":4}} +{ + "action": "set", + "node": { + "createdIndex": 4, + "key": "/foo", + "modifiedIndex": 4, + "value": "bar" + } +} ``` @@ -579,7 +793,15 @@ curl -L http://127.0.0.1:4002/v2/keys/foo ``` ```json -{"action":"get","node":{"key":"/foo","value":"bar","modifiedIndex":4,"createdIndex":4}} +{ + "action": "get", + "node": { + "createdIndex": 4, + "key": "/foo", + "modifiedIndex": 4, + "value": "bar" + } +} ``` From f3d438a93fb10443fb303361ff5ea37181357425 Mon Sep 17 00:00:00 2001 From: Ben Johnson Date: Wed, 4 Dec 2013 16:23:27 -0700 Subject: [PATCH 09/14] Add mod/lock connection monitoring. --- mod/lock/acquire_handler.go | 91 ++++++++++++++++++++++++++++--------- test.sh | 10 ++-- 2 files changed, 76 insertions(+), 25 deletions(-) diff --git a/mod/lock/acquire_handler.go b/mod/lock/acquire_handler.go index 8ad9e528a..db5cbba8b 100644 --- a/mod/lock/acquire_handler.go +++ b/mod/lock/acquire_handler.go @@ -6,13 +6,22 @@ import ( "strconv" "time" + "github.com/coreos/go-etcd/etcd" "github.com/gorilla/mux" ) // acquireHandler attempts to acquire a lock on the given key. +// The "key" parameter specifies the resource to lock. +// The "ttl" parameter specifies how long the lock will persist for. +// The "timeout" parameter specifies how long the request should wait for the lock. func (h *handler) acquireHandler(w http.ResponseWriter, req *http.Request) { h.client.SyncCluster() + // Setup connection watcher. + closeNotifier, _ := w.(http.CloseNotifier) + closeChan := closeNotifier.CloseNotify() + + // Parse "key" and "ttl" query parameters. vars := mux.Vars(req) keypath := path.Join(prefix, vars["key"]) ttl, err := strconv.Atoi(req.FormValue("ttl")) @@ -20,6 +29,16 @@ func (h *handler) acquireHandler(w http.ResponseWriter, req *http.Request) { http.Error(w, "invalid ttl: " + err.Error(), http.StatusInternalServerError) return } + + // Parse "timeout" parameter. + var timeout int + if len(req.FormValue("timeout")) == 0 { + timeout = -1 + } else if timeout, err = strconv.Atoi(req.FormValue("timeout")); err != nil { + http.Error(w, "invalid timeout: " + err.Error(), http.StatusInternalServerError) + return + } + timeout = timeout + 1 // Create an incrementing id for the lock. resp, err := h.client.AddChild(keypath, "-", uint64(ttl)) @@ -30,32 +49,31 @@ func (h *handler) acquireHandler(w http.ResponseWriter, req *http.Request) { indexpath := resp.Key // Keep updating TTL to make sure lock request is not expired before acquisition. - stopChan := make(chan bool) - defer close(stopChan) - go func(k string) { - stopped := false - for { - select { - case <-time.After(time.Duration(ttl / 2) * time.Second): - case <-stopChan: - stopped = true - } - h.client.Update(k, "-", uint64(ttl)) - if stopped { - break - } + stop := make(chan bool) + go h.ttlKeepAlive(indexpath, ttl, stop) + + // Monitor for broken connection. + stopWatchChan := make(chan bool) + go func() { + select { + case <-closeChan: + stopWatchChan <- true + case <-stop: + // Stop watching for connection disconnect. } - }(indexpath) + }() // Extract the lock index. index, _ := strconv.Atoi(path.Base(resp.Key)) + // Wait until we successfully get a lock or we get a failure. + var success bool for { // Read all indices. resp, err = h.client.GetAll(keypath, true) if err != nil { http.Error(w, "lock children lookup error: " + err.Error(), http.StatusInternalServerError) - return + break } indices := extractResponseIndices(resp) waitIndex := resp.ModifiedIndex @@ -63,17 +81,48 @@ func (h *handler) acquireHandler(w http.ResponseWriter, req *http.Request) { // If there is no previous index then we have the lock. if prevIndex == 0 { + success = true break } // Otherwise watch previous index until it's gone. - _, err = h.client.Watch(path.Join(keypath, strconv.Itoa(prevIndex)), waitIndex, nil, nil) - if err != nil { + _, err = h.client.Watch(path.Join(keypath, strconv.Itoa(prevIndex)), waitIndex, nil, stopWatchChan) + if err == etcd.ErrWatchStoppedByUser { + break + } else if err != nil { http.Error(w, "lock watch error: " + err.Error(), http.StatusInternalServerError) - return + break } } - // Write lock index to response body. - w.Write([]byte(strconv.Itoa(index))) + // Check for connection disconnect before we write the lock index. + select { + case <-stopWatchChan: + success = false + default: + } + + // Stop the ttl keep-alive. + close(stop) + + if success { + // Write lock index to response body if we acquire the lock. + h.client.Update(indexpath, "-", uint64(ttl)) + w.Write([]byte(strconv.Itoa(index))) + } else { + // Make sure key is deleted if we couldn't acquire. + h.client.Delete(indexpath) + } +} + +// ttlKeepAlive continues to update a key's TTL until the stop channel is closed. +func (h *handler) ttlKeepAlive(k string, ttl int, stop chan bool) { + for { + select { + case <-time.After(time.Duration(ttl / 2) * time.Second): + h.client.Update(k, "-", uint64(ttl)) + case <-stop: + return + } + } } diff --git a/test.sh b/test.sh index 246b03ad4..cb4c51fa9 100755 --- a/test.sh +++ b/test.sh @@ -1,7 +1,9 @@ #!/bin/sh set -e -PKGS="./store ./server ./server/v2/tests ./mod/lock/tests" +if [ -z "$PKG" ]; then + PKG="./store ./server ./server/v2/tests ./mod/lock/tests" +fi # Get GOPATH, etc from build . ./build @@ -10,10 +12,10 @@ PKGS="./store ./server ./server/v2/tests ./mod/lock/tests" export GOPATH="${PWD}" # Unit tests -for PKG in $PKGS +for i in $PKG do - go test -i $PKG - go test -v $PKG + go test -i $i + go test -v $i done # Functional tests From e76b7d1e8b1a2bde0d63e84fe2bd63f9480bb745 Mon Sep 17 00:00:00 2001 From: Ben Johnson Date: Wed, 4 Dec 2013 22:24:04 -0700 Subject: [PATCH 10/14] Add mod/lock version. --- mod/lock/{ => v2}/acquire_handler.go | 2 +- mod/lock/{ => v2}/get_index_handler.go | 2 +- mod/lock/{ => v2}/handler.go | 2 +- mod/lock/{ => v2}/release_handler.go | 2 +- mod/lock/{ => v2}/renew_handler.go | 2 +- mod/lock/{ => v2}/tests/handler_test.go | 8 ++++---- mod/mod.go | 4 ++-- test.sh | 2 +- 8 files changed, 12 insertions(+), 12 deletions(-) rename mod/lock/{ => v2}/acquire_handler.go (99%) rename mod/lock/{ => v2}/get_index_handler.go (98%) rename mod/lock/{ => v2}/handler.go (99%) rename mod/lock/{ => v2}/release_handler.go (97%) rename mod/lock/{ => v2}/renew_handler.go (98%) rename mod/lock/{ => v2}/tests/handler_test.go (92%) diff --git a/mod/lock/acquire_handler.go b/mod/lock/v2/acquire_handler.go similarity index 99% rename from mod/lock/acquire_handler.go rename to mod/lock/v2/acquire_handler.go index db5cbba8b..de82cdd16 100644 --- a/mod/lock/acquire_handler.go +++ b/mod/lock/v2/acquire_handler.go @@ -1,4 +1,4 @@ -package lock +package v2 import ( "net/http" diff --git a/mod/lock/get_index_handler.go b/mod/lock/v2/get_index_handler.go similarity index 98% rename from mod/lock/get_index_handler.go rename to mod/lock/v2/get_index_handler.go index 2bb97a83c..2393da76c 100644 --- a/mod/lock/get_index_handler.go +++ b/mod/lock/v2/get_index_handler.go @@ -1,4 +1,4 @@ -package lock +package v2 import ( "net/http" diff --git a/mod/lock/handler.go b/mod/lock/v2/handler.go similarity index 99% rename from mod/lock/handler.go rename to mod/lock/v2/handler.go index 43e149145..2713758b3 100644 --- a/mod/lock/handler.go +++ b/mod/lock/v2/handler.go @@ -1,4 +1,4 @@ -package lock +package v2 import ( "net/http" diff --git a/mod/lock/release_handler.go b/mod/lock/v2/release_handler.go similarity index 97% rename from mod/lock/release_handler.go rename to mod/lock/v2/release_handler.go index 09251f259..b41157ef2 100644 --- a/mod/lock/release_handler.go +++ b/mod/lock/v2/release_handler.go @@ -1,4 +1,4 @@ -package lock +package v2 import ( "path" diff --git a/mod/lock/renew_handler.go b/mod/lock/v2/renew_handler.go similarity index 98% rename from mod/lock/renew_handler.go rename to mod/lock/v2/renew_handler.go index 7933931e4..cdd65b3aa 100644 --- a/mod/lock/renew_handler.go +++ b/mod/lock/v2/renew_handler.go @@ -1,4 +1,4 @@ -package lock +package v2 import ( "path" diff --git a/mod/lock/tests/handler_test.go b/mod/lock/v2/tests/handler_test.go similarity index 92% rename from mod/lock/tests/handler_test.go rename to mod/lock/v2/tests/handler_test.go index 7e9091a0f..b589865bf 100644 --- a/mod/lock/tests/handler_test.go +++ b/mod/lock/v2/tests/handler_test.go @@ -164,25 +164,25 @@ func TestModLockRenew(t *testing.T) { func testAcquireLock(s *server.Server, key string, ttl int) (string, error) { - resp, err := tests.PostForm(fmt.Sprintf("%s/mod/lock/%s?ttl=%d", s.URL(), key, ttl), nil) + resp, err := tests.PostForm(fmt.Sprintf("%s/mod/lock/v2/%s?ttl=%d", s.URL(), key, ttl), nil) ret := tests.ReadBody(resp) return string(ret), err } func testGetLockIndex(s *server.Server, key string) (string, error) { - resp, err := tests.Get(fmt.Sprintf("%s/mod/lock/%s", s.URL(), key)) + resp, err := tests.Get(fmt.Sprintf("%s/mod/lock/v2/%s", s.URL(), key)) ret := tests.ReadBody(resp) return string(ret), err } func testReleaseLock(s *server.Server, key string, index int) (string, error) { - resp, err := tests.DeleteForm(fmt.Sprintf("%s/mod/lock/%s/%d", s.URL(), key, index), nil) + resp, err := tests.DeleteForm(fmt.Sprintf("%s/mod/lock/v2/%s/%d", s.URL(), key, index), nil) ret := tests.ReadBody(resp) return string(ret), err } func testRenewLock(s *server.Server, key string, index int, ttl int) (string, error) { - resp, err := tests.PutForm(fmt.Sprintf("%s/mod/lock/%s/%d?ttl=%d", s.URL(), key, index, ttl), nil) + resp, err := tests.PutForm(fmt.Sprintf("%s/mod/lock/v2/%s/%d?ttl=%d", s.URL(), key, index, ttl), nil) ret := tests.ReadBody(resp) return string(ret), err } diff --git a/mod/mod.go b/mod/mod.go index 7c0194f56..b5625db3f 100644 --- a/mod/mod.go +++ b/mod/mod.go @@ -6,7 +6,7 @@ import ( "path" "github.com/coreos/etcd/mod/dashboard" - "github.com/coreos/etcd/mod/lock" + lock2 "github.com/coreos/etcd/mod/lock/v2" "github.com/gorilla/mux" ) @@ -23,6 +23,6 @@ func HttpHandler(addr string) http.Handler { r.PathPrefix("/dashboard/").Handler(http.StripPrefix("/dashboard/", dashboard.HttpHandler())) // TODO: Use correct addr. - r.PathPrefix("/lock").Handler(http.StripPrefix("/lock", lock.NewHandler(addr))) + r.PathPrefix("/lock/v2").Handler(http.StripPrefix("/lock/v2", lock2.NewHandler(addr))) return r } diff --git a/test.sh b/test.sh index cb4c51fa9..ae40d8200 100755 --- a/test.sh +++ b/test.sh @@ -2,7 +2,7 @@ set -e if [ -z "$PKG" ]; then - PKG="./store ./server ./server/v2/tests ./mod/lock/tests" + PKG="./store ./server ./server/v2/tests ./mod/lock/v2/tests" fi # Get GOPATH, etc from build From b784ced5175d77d2ce17657f71ebb3858f4dafda Mon Sep 17 00:00:00 2001 From: Ben Johnson Date: Wed, 4 Dec 2013 22:39:59 -0700 Subject: [PATCH 11/14] Update mod/lock versioning. --- mod/lock/v2/tests/handler_test.go | 8 ++++---- mod/mod.go | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/mod/lock/v2/tests/handler_test.go b/mod/lock/v2/tests/handler_test.go index b589865bf..b07572bbe 100644 --- a/mod/lock/v2/tests/handler_test.go +++ b/mod/lock/v2/tests/handler_test.go @@ -164,25 +164,25 @@ func TestModLockRenew(t *testing.T) { func testAcquireLock(s *server.Server, key string, ttl int) (string, error) { - resp, err := tests.PostForm(fmt.Sprintf("%s/mod/lock/v2/%s?ttl=%d", s.URL(), key, ttl), nil) + resp, err := tests.PostForm(fmt.Sprintf("%s/mod/v2/lock/%s?ttl=%d", s.URL(), key, ttl), nil) ret := tests.ReadBody(resp) return string(ret), err } func testGetLockIndex(s *server.Server, key string) (string, error) { - resp, err := tests.Get(fmt.Sprintf("%s/mod/lock/v2/%s", s.URL(), key)) + resp, err := tests.Get(fmt.Sprintf("%s/mod/v2/lock/%s", s.URL(), key)) ret := tests.ReadBody(resp) return string(ret), err } func testReleaseLock(s *server.Server, key string, index int) (string, error) { - resp, err := tests.DeleteForm(fmt.Sprintf("%s/mod/lock/v2/%s/%d", s.URL(), key, index), nil) + resp, err := tests.DeleteForm(fmt.Sprintf("%s/mod/v2/lock/%s/%d", s.URL(), key, index), nil) ret := tests.ReadBody(resp) return string(ret), err } func testRenewLock(s *server.Server, key string, index int, ttl int) (string, error) { - resp, err := tests.PutForm(fmt.Sprintf("%s/mod/lock/v2/%s/%d?ttl=%d", s.URL(), key, index, ttl), nil) + resp, err := tests.PutForm(fmt.Sprintf("%s/mod/v2/lock/%s/%d?ttl=%d", s.URL(), key, index, ttl), nil) ret := tests.ReadBody(resp) return string(ret), err } diff --git a/mod/mod.go b/mod/mod.go index b5625db3f..34a380689 100644 --- a/mod/mod.go +++ b/mod/mod.go @@ -23,6 +23,6 @@ func HttpHandler(addr string) http.Handler { r.PathPrefix("/dashboard/").Handler(http.StripPrefix("/dashboard/", dashboard.HttpHandler())) // TODO: Use correct addr. - r.PathPrefix("/lock/v2").Handler(http.StripPrefix("/lock/v2", lock2.NewHandler(addr))) + r.PathPrefix("/v2/lock").Handler(http.StripPrefix("/v2/lock", lock2.NewHandler(addr))) return r } From b22c6fed4db9e4011dda1d8fe3bca01debc11357 Mon Sep 17 00:00:00 2001 From: Alex Polvi Date: Thu, 5 Dec 2013 18:07:51 +0000 Subject: [PATCH 12/14] fix(Dockerfile): remove bogus -bind-addr, and use default -addr --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index e025979db..fd624c5fa 100644 --- a/Dockerfile +++ b/Dockerfile @@ -7,4 +7,4 @@ RUN apt-get install -y golang ADD . /opt/etcd RUN cd /opt/etcd && ./build EXPOSE 4001 7001 -ENTRYPOINT ["/opt/etcd/etcd", "-addr", "0.0.0.0:4001", "-bind-addr", "0.0.0.0:7001"] +ENTRYPOINT ["/opt/etcd/etcd"] From e7839e8c574d05b05a510f18aef0c1f3a46b82c7 Mon Sep 17 00:00:00 2001 From: Brian Waldon Date: Tue, 3 Dec 2013 08:40:18 -0800 Subject: [PATCH 13/14] fix(etcd): Fix forced config reset When a server name or a data directory were not provided, the reset functionality would fail to clear out config files from the appropriate place. This calcualtes the default server name and data directory before reset is called. --- etcd.go | 10 ---------- server/config.go | 20 +++++++++++++++----- server/config_test.go | 18 ++++++++++++++++++ 3 files changed, 33 insertions(+), 15 deletions(-) diff --git a/etcd.go b/etcd.go index b72880d0f..0567c65df 100644 --- a/etcd.go +++ b/etcd.go @@ -52,16 +52,6 @@ func main() { profile(config.CPUProfileFile) } - // Only guess the machine name if there is no data dir specified - // because the info file will should have our name - if config.Name == "" && config.DataDir == "" { - config.NameFromHostname() - } - - if config.DataDir == "" && config.Name != "" { - config.DataDirFromName() - } - if config.DataDir == "" { log.Fatal("The data dir was not set and could not be guessed from machine name") } diff --git a/server/config.go b/server/config.go index 939a2580d..d09893820 100644 --- a/server/config.go +++ b/server/config.go @@ -131,6 +131,11 @@ func (c *Config) Load(arguments []string) error { return fmt.Errorf("sanitize: %v", err) } + // Force remove server configuration if specified. + if c.Force { + c.Reset() + } + return nil } @@ -278,11 +283,6 @@ func (c *Config) LoadFlags(arguments []string) error { c.CorsOrigins = trimsplit(cors, ",") } - // Force remove server configuration if specified. - if c.Force { - c.Reset() - } - return nil } @@ -404,6 +404,16 @@ func (c *Config) Sanitize() error { return fmt.Errorf("Peer Listen Host: %s", err) } + // Only guess the machine name if there is no data dir specified + // because the info file should have our name + if c.Name == "" && c.DataDir == "" { + c.NameFromHostname() + } + + if c.DataDir == "" && c.Name != "" { + c.DataDirFromName() + } + return nil } diff --git a/server/config_test.go b/server/config_test.go index f991c9765..5571773cc 100644 --- a/server/config_test.go +++ b/server/config_test.go @@ -313,6 +313,24 @@ func TestConfigNameFlag(t *testing.T) { assert.Equal(t, c.Name, "test-name", "") } +// Ensures that a Name gets guessed if not specified +func TestConfigNameGuess(t *testing.T) { + c := NewConfig() + assert.Nil(t, c.LoadFlags([]string{}), "") + assert.Nil(t, c.Sanitize()) + name, _ := os.Hostname() + assert.Equal(t, c.Name, name, "") +} + +// Ensures that a DataDir gets guessed if not specified +func TestConfigDataDirGuess(t *testing.T) { + c := NewConfig() + assert.Nil(t, c.LoadFlags([]string{}), "") + assert.Nil(t, c.Sanitize()) + name, _ := os.Hostname() + assert.Equal(t, c.DataDir, name+".etcd", "") +} + // Ensures that Snapshot can be parsed from the environment. func TestConfigSnapshotEnv(t *testing.T) { withEnv("ETCD_SNAPSHOT", "1", func(c *Config) { From 9db521ca048633488a9adca92ef6b37708630a03 Mon Sep 17 00:00:00 2001 From: Brandon Philips Date: Thu, 5 Dec 2013 11:45:16 -0800 Subject: [PATCH 14/14] fix(server): override port of bind Allow people to specify ports on the `-bind-addr` arguments so that they can use randomly assigned port numbers in containers. --- server/config.go | 22 +++++++++++++++------- server/config_test.go | 23 +++++++++++++++++++++++ server/usage.go | 20 ++++++++++---------- 3 files changed, 48 insertions(+), 17 deletions(-) diff --git a/server/config.go b/server/config.go index 939a2580d..d1b67fb2b 100644 --- a/server/config.go +++ b/server/config.go @@ -435,7 +435,7 @@ func (c *Config) PeerTLSConfig() (TLSConfig, error) { return c.PeerTLSInfo().Config() } -// sanitizeURL will cleanup a host string in the format hostname:port and +// sanitizeURL will cleanup a host string in the format hostname[:port] and // attach a schema. func sanitizeURL(host string, defaultScheme string) (string, error) { // Blank URLs are fine input, just return it @@ -466,15 +466,23 @@ func sanitizeBindAddr(bindAddr string, addr string) (string, error) { return "", err } - ahost, aport, err := net.SplitHostPort(aurl.Host) + // If it is a valid host:port simply return with no further checks. + bhost, bport, err := net.SplitHostPort(bindAddr) + if err == nil && bhost != "" { + return bindAddr, nil + } + + // SplitHostPort makes the host optional, but we don't want that. + if bhost == "" && bport != "" { + return "", fmt.Errorf("IP required can't use a port only") + } + + // bindAddr doesn't have a port if we reach here so take the port from the + // advertised URL. + _, aport, err := net.SplitHostPort(aurl.Host) if err != nil { return "", err } - // If the listen host isn't set use the advertised host - if bindAddr == "" { - bindAddr = ahost - } - return net.JoinHostPort(bindAddr, aport), nil } diff --git a/server/config_test.go b/server/config_test.go index f991c9765..d4dede140 100644 --- a/server/config_test.go +++ b/server/config_test.go @@ -223,6 +223,29 @@ func TestConfigBindAddrFlag(t *testing.T) { assert.Equal(t, c.BindAddr, "127.0.0.1:4003", "") } +// Ensures that a the Listen Host port overrides the advertised port +func TestConfigBindAddrOverride(t *testing.T) { + c := NewConfig() + assert.Nil(t, c.LoadFlags([]string{"-addr", "127.0.0.1:4009", "-bind-addr", "127.0.0.1:4010"}), "") + assert.Nil(t, c.Sanitize()) + assert.Equal(t, c.BindAddr, "127.0.0.1:4010", "") +} + +// Ensures that a the Listen Host inherits its port from the advertised addr +func TestConfigBindAddrInheritPort(t *testing.T) { + c := NewConfig() + assert.Nil(t, c.LoadFlags([]string{"-addr", "127.0.0.1:4009", "-bind-addr", "127.0.0.1"}), "") + assert.Nil(t, c.Sanitize()) + assert.Equal(t, c.BindAddr, "127.0.0.1:4009", "") +} + +// Ensures that a port only argument errors out +func TestConfigBindAddrErrorOnNoHost(t *testing.T) { + c := NewConfig() + assert.Nil(t, c.LoadFlags([]string{"-addr", "127.0.0.1:4009", "-bind-addr", ":4010"}), "") + assert.Error(t, c.Sanitize()) +} + // Ensures that the peers can be parsed from the environment. func TestConfigPeersEnv(t *testing.T) { withEnv("ETCD_PEERS", "coreos.com:4001,coreos.com:4002", func(c *Config) { diff --git a/server/usage.go b/server/usage.go index 3809fb04b..e8969c344 100644 --- a/server/usage.go +++ b/server/usage.go @@ -31,18 +31,18 @@ Cluster Configuration Options: should match the peer's '-peer-addr' flag. Client Communication Options: - -addr= The public host:port used for client communication. - -bind-addr= The listening hostname used for client communication. - -ca-file= Path to the client CA file. - -cert-file= Path to the client cert file. - -key-file= Path to the client key file. + -addr= The public host:port used for client communication. + -bind-addr= The listening host:port used for client communication. + -ca-file= Path to the client CA file. + -cert-file= Path to the client cert file. + -key-file= Path to the client key file. Peer Communication Options: - -peer-addr= The public host:port used for peer communication. - -peer-bind-addr= The listening hostname used for peer communication. - -peer-ca-file= Path to the peer CA file. - -peer-cert-file= Path to the peer cert file. - -peer-key-file= Path to the peer key file. + -peer-addr= The public host:port used for peer communication. + -peer-bind-addr= The listening host:port used for peer communication. + -peer-ca-file= Path to the peer CA file. + -peer-cert-file= Path to the peer cert file. + -peer-key-file= Path to the peer key file. Other Options: -max-result-buffer Max size of the result buffer.