From 53afb746dfc0ddbaaf392a2737beca550f619bf1 Mon Sep 17 00:00:00 2001 From: Ben Hoyt Date: Mon, 16 Aug 2021 14:19:57 +1200 Subject: [PATCH 1/4] Add wait-change API endpoint and client function --- client/changes.go | 24 +++++++++ client/changes_test.go | 44 +++++++++++++++++ internal/daemon/api.go | 4 ++ internal/daemon/api_changes.go | 46 ++++++++++++++--- internal/daemon/api_changes_test.go | 77 +++++++++++++++++++++++++++++ 5 files changed, 189 insertions(+), 6 deletions(-) diff --git a/client/changes.go b/client/changes.go index f11919665..42568c678 100644 --- a/client/changes.go +++ b/client/changes.go @@ -157,3 +157,27 @@ func (client *Client) Changes(opts *ChangesOptions) ([]*Change, error) { return chgs, err } + +// WaitChangeOptions holds the options for the WaitChange call. +type WaitChangeOptions struct { + Timeout time.Duration +} + +// WaitChange waits for a given change to be finished (whether or not it had +// an error). If a timeout is specified, wait at most that long. +func (client *Client) WaitChange(id string, opts *WaitChangeOptions) (*Change, error) { + var chgd changeAndData + + query := url.Values{} + if opts != nil && opts.Timeout != 0 { + query.Set("timeout", opts.Timeout.String()) + } + + _, err := client.doSync("GET", "/v1/changes/"+id+"/wait", query, nil, nil, &chgd) + if err != nil { + return nil, err + } + + chgd.Change.data = chgd.Data + return &chgd.Change, nil +} diff --git a/client/changes_test.go b/client/changes_test.go index 033939d98..83214b72c 100644 --- a/client/changes_test.go +++ b/client/changes_test.go @@ -56,6 +56,50 @@ func (cs *clientSuite) TestClientChange(c *check.C) { }) } +func (cs *clientSuite) TestClientWaitChange(c *check.C) { + cs.testClientWaitChange(c, "foo", nil, "http://localhost/v1/changes/foo/wait") +} + +func (cs *clientSuite) TestClientWaitChangeTimeout(c *check.C) { + opts := &client.WaitChangeOptions{ + Timeout: 30 * time.Second, + } + cs.testClientWaitChange(c, "bar", opts, "http://localhost/v1/changes/bar/wait?timeout=30s") +} + +func (cs *clientSuite) testClientWaitChange(c *check.C, changeID string, opts *client.WaitChangeOptions, expectedURL string) { + cs.rsp = `{"type": "sync", "result": { + "id": "uno", + "kind": "foo", + "summary": "...", + "status": "Do", + "ready": false, + "spawn-time": "2016-04-21T01:02:03Z", + "ready-time": "2016-04-21T01:02:04Z", + "tasks": [{"kind": "bar", "summary": "...", "status": "Do", "progress": {"done": 0, "total": 1}, "spawn-time": "2016-04-21T01:02:03Z", "ready-time": "2016-04-21T01:02:04Z"}] +}}` + + chg, err := cs.cli.WaitChange(changeID, opts) + c.Assert(err, check.IsNil) + c.Assert(cs.req.URL.String(), check.Equals, expectedURL) + c.Check(chg, check.DeepEquals, &client.Change{ + ID: "uno", + Kind: "foo", + Summary: "...", + Status: "Do", + Tasks: []*client.Task{{ + Kind: "bar", + Summary: "...", + Status: "Do", + Progress: client.TaskProgress{Done: 0, Total: 1}, + SpawnTime: time.Date(2016, 04, 21, 1, 2, 3, 0, time.UTC), + ReadyTime: time.Date(2016, 04, 21, 1, 2, 4, 0, time.UTC), + }}, + SpawnTime: time.Date(2016, 04, 21, 1, 2, 3, 0, time.UTC), + ReadyTime: time.Date(2016, 04, 21, 1, 2, 4, 0, time.UTC), + }) +} + func (cs *clientSuite) TestClientChangeData(c *check.C) { cs.rsp = `{"type": "sync", "result": { "id": "uno", diff --git a/internal/daemon/api.go b/internal/daemon/api.go index 861af0861..a3d401ac4 100644 --- a/internal/daemon/api.go +++ b/internal/daemon/api.go @@ -41,6 +41,10 @@ var api = []*Command{{ UserOK: true, GET: v1GetChange, POST: v1PostChange, +}, { + Path: "/v1/changes/{id}/wait", + UserOK: true, + GET: v1GetChangeWait, }, { Path: "/v1/services", UserOK: true, diff --git a/internal/daemon/api_changes.go b/internal/daemon/api_changes.go index c767e175b..cc7fdea3b 100644 --- a/internal/daemon/api_changes.go +++ b/internal/daemon/api_changes.go @@ -165,18 +165,52 @@ func v1GetChanges(c *Command, r *http.Request, _ *userState) Response { } func v1GetChange(c *Command, r *http.Request, _ *userState) Response { - chID := muxVars(r)["id"] - state := c.d.overlord.State() - state.Lock() - defer state.Unlock() - chg := state.Change(chID) + changeID := muxVars(r)["id"] + st := c.d.overlord.State() + st.Lock() + defer st.Unlock() + chg := st.Change(changeID) if chg == nil { - return statusNotFound("cannot find change with id %q", chID) + return statusNotFound("cannot find change with id %q", changeID) } return SyncResponse(change2changeInfo(chg)) } +func v1GetChangeWait(c *Command, r *http.Request, _ *userState) Response { + changeID := muxVars(r)["id"] + st := c.d.overlord.State() + st.Lock() + change := st.Change(changeID) + st.Unlock() + if change == nil { + return statusNotFound("cannot find change with id %q", changeID) + } + + timeoutStr := r.URL.Query().Get("timeout") + if timeoutStr != "" { + // Timeout specified, wait till change is ready or timeout occurs, + // whichever is first. + timeout, err := time.ParseDuration(timeoutStr) + if err != nil { + return statusBadRequest("invalid timeout %q: %v", timeoutStr, err) + } + timer := time.NewTimer(timeout) + select { + case <-change.Ready(): + timer.Stop() // change ready, release timer resources + case <-timer.C: + } + } else { + // No timeout, wait indefinitely for change to be ready. + <-change.Ready() + } + + st.Lock() + defer st.Unlock() + return SyncResponse(change2changeInfo(change)) +} + func v1PostChange(c *Command, r *http.Request, _ *userState) Response { chID := muxVars(r)["id"] state := c.d.overlord.State() diff --git a/internal/daemon/api_changes_test.go b/internal/daemon/api_changes_test.go index 2f1743d2f..17c936093 100644 --- a/internal/daemon/api_changes_test.go +++ b/internal/daemon/api_changes_test.go @@ -364,3 +364,80 @@ func (s *apiSuite) TestStateChangeAbortIsReady(c *check.C) { "message": fmt.Sprintf("cannot abort change %s with nothing pending", ids[0]), }) } + +func (s *apiSuite) TestStateChangeNotFound(c *check.C) { + s.daemon(c) + req, err := http.NewRequest("GET", "/v1/changes/x/wait", nil) + c.Assert(err, check.IsNil) + rsp := v1GetChangeWait(apiCmd("/v1/changes/{id}/wait"), req, nil).(*resp) + c.Check(rsp.Status, check.Equals, 404) +} + +func (s *apiSuite) TestStateChangeInvalidTimeout(c *check.C) { + // Setup + d := s.daemon(c) + st := d.overlord.State() + st.Lock() + change := st.NewChange("exec", "Exec") + task := st.NewTask("exec", "Exec") + change.AddAll(state.NewTaskSet(task)) + st.Unlock() + + // Execute + s.vars = map[string]string{"id": change.ID()} + req, err := http.NewRequest("GET", "/v1/changes/"+change.ID()+"/wait?timeout=BAD", nil) + c.Assert(err, check.IsNil) + rsp := v1GetChangeWait(apiCmd("/v1/changes/{id}/wait"), req, nil).(*resp) + + // Verify + c.Check(rsp.Status, check.Equals, 400) +} + +func (s *apiSuite) TestStateChangeWait(c *check.C) { + ready := s.testStateChangeWait(c, "", func(st *state.State, change *state.Change) { + // Mark change ready after a short interval + time.Sleep(10 * time.Millisecond) + st.Lock() + change.SetStatus(state.DoneStatus) + st.Unlock() + }) + c.Check(ready, check.Equals, true) +} + +func (s *apiSuite) TestStateChangeWaitTimeout(c *check.C) { + ready := s.testStateChangeWait(c, "?timeout=10ms", func(st *state.State, change *state.Change) {}) + c.Check(ready, check.Equals, false) +} + +func (s *apiSuite) testStateChangeWait(c *check.C, query string, markReady func(st *state.State, change *state.Change)) bool { + // Setup + d := s.daemon(c) + st := d.overlord.State() + st.Lock() + change := st.NewChange("exec", "Exec") + task := st.NewTask("exec", "Exec") + change.AddAll(state.NewTaskSet(task)) + st.Unlock() + go markReady(st, change) + + // Execute + s.vars = map[string]string{"id": change.ID()} + req, err := http.NewRequest("GET", "/v1/changes/"+change.ID()+"/wait"+query, nil) + c.Assert(err, check.IsNil) + rsp := v1GetChangeWait(apiCmd("/v1/changes/{id}/wait"), req, nil).(*resp) + rec := httptest.NewRecorder() + rsp.ServeHTTP(rec, req) + + // Verify + c.Check(rec.Code, check.Equals, 200) + c.Check(rsp.Status, check.Equals, 200) + c.Check(rsp.Result, check.NotNil) + + var body map[string]interface{} + err = json.Unmarshal(rec.Body.Bytes(), &body) + c.Check(err, check.IsNil) + result := body["result"].(map[string]interface{}) + c.Check(result["id"].(string), check.Equals, change.ID()) + c.Check(result["kind"].(string), check.Equals, "exec") + return result["ready"].(bool) +} From 87bea499bd827d7eb54ecbc768617ab1b26d0256 Mon Sep 17 00:00:00 2001 From: Ben Hoyt Date: Mon, 16 Aug 2021 15:24:47 +1200 Subject: [PATCH 2/4] Handle request.Done() in wait-change API --- internal/daemon/api_changes.go | 6 +++++- internal/daemon/api_changes_test.go | 23 +++++++++++++++++++---- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/internal/daemon/api_changes.go b/internal/daemon/api_changes.go index cc7fdea3b..908592e12 100644 --- a/internal/daemon/api_changes.go +++ b/internal/daemon/api_changes.go @@ -200,10 +200,14 @@ func v1GetChangeWait(c *Command, r *http.Request, _ *userState) Response { case <-change.Ready(): timer.Stop() // change ready, release timer resources case <-timer.C: + case <-r.Context().Done(): } } else { // No timeout, wait indefinitely for change to be ready. - <-change.Ready() + select { + case <-change.Ready(): + case <-r.Context().Done(): + } } st.Lock() diff --git a/internal/daemon/api_changes_test.go b/internal/daemon/api_changes_test.go index 17c936093..6afd41828 100644 --- a/internal/daemon/api_changes_test.go +++ b/internal/daemon/api_changes_test.go @@ -16,6 +16,7 @@ package daemon import ( "bytes" + "context" "encoding/json" "fmt" "net/http" @@ -394,7 +395,7 @@ func (s *apiSuite) TestStateChangeInvalidTimeout(c *check.C) { } func (s *apiSuite) TestStateChangeWait(c *check.C) { - ready := s.testStateChangeWait(c, "", func(st *state.State, change *state.Change) { + ready := s.testStateChangeWait(context.Background(), c, "", func(st *state.State, change *state.Change) { // Mark change ready after a short interval time.Sleep(10 * time.Millisecond) st.Lock() @@ -404,12 +405,26 @@ func (s *apiSuite) TestStateChangeWait(c *check.C) { c.Check(ready, check.Equals, true) } +func (s *apiSuite) TestStateChangeWaitCancel(c *check.C) { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Millisecond) + defer cancel() + ready := s.testStateChangeWait(ctx, c, "", func(st *state.State, change *state.Change) {}) + c.Check(ready, check.Equals, false) +} + func (s *apiSuite) TestStateChangeWaitTimeout(c *check.C) { - ready := s.testStateChangeWait(c, "?timeout=10ms", func(st *state.State, change *state.Change) {}) + ready := s.testStateChangeWait(context.Background(), c, "?timeout=10ms", func(st *state.State, change *state.Change) {}) + c.Check(ready, check.Equals, false) +} + +func (s *apiSuite) TestStateChangeWaitTimeoutCancel(c *check.C) { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Millisecond) + defer cancel() + ready := s.testStateChangeWait(ctx, c, "?timeout=20ms", func(st *state.State, change *state.Change) {}) c.Check(ready, check.Equals, false) } -func (s *apiSuite) testStateChangeWait(c *check.C, query string, markReady func(st *state.State, change *state.Change)) bool { +func (s *apiSuite) testStateChangeWait(ctx context.Context, c *check.C, query string, markReady func(st *state.State, change *state.Change)) bool { // Setup d := s.daemon(c) st := d.overlord.State() @@ -422,7 +437,7 @@ func (s *apiSuite) testStateChangeWait(c *check.C, query string, markReady func( // Execute s.vars = map[string]string{"id": change.ID()} - req, err := http.NewRequest("GET", "/v1/changes/"+change.ID()+"/wait"+query, nil) + req, err := http.NewRequestWithContext(ctx, "GET", "/v1/changes/"+change.ID()+"/wait"+query, nil) c.Assert(err, check.IsNil) rsp := v1GetChangeWait(apiCmd("/v1/changes/{id}/wait"), req, nil).(*resp) rec := httptest.NewRecorder() From b1010149fcac5095f26d7f4af1ac1ad0fa45a0f0 Mon Sep 17 00:00:00 2001 From: Ben Hoyt Date: Tue, 17 Aug 2021 16:52:17 +1200 Subject: [PATCH 3/4] Clarify timeout/error behaviour in client docs --- client/changes.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/client/changes.go b/client/changes.go index 42568c678..537c8237f 100644 --- a/client/changes.go +++ b/client/changes.go @@ -160,11 +160,15 @@ func (client *Client) Changes(opts *ChangesOptions) ([]*Change, error) { // WaitChangeOptions holds the options for the WaitChange call. type WaitChangeOptions struct { + // If non-zero, wait at most this long before returning the current change + // data. The timeout elapsing is not considered an error, so if a timeout + // is specified, the caller should check Change.Ready to determine whether + // the change is actually finished. Timeout time.Duration } -// WaitChange waits for a given change to be finished (whether or not it had -// an error). If a timeout is specified, wait at most that long. +// WaitChange waits for a given change to be finished (whether or not there +// was an error, which is indicated by Change.Err being set). func (client *Client) WaitChange(id string, opts *WaitChangeOptions) (*Change, error) { var chgd changeAndData From 8133562ce2a79b4b41e8e77444d2da4d8ba59394 Mon Sep 17 00:00:00 2001 From: Ben Hoyt Date: Tue, 31 Aug 2021 14:27:52 +1200 Subject: [PATCH 4/4] Change WaitChange timeout to an error; clearer doc comments --- client/changes.go | 11 ++-- client/changes_test.go | 26 ++++---- internal/daemon/api_changes.go | 3 + internal/daemon/api_changes_test.go | 96 +++++++++++++++-------------- internal/daemon/response.go | 7 +-- 5 files changed, 75 insertions(+), 68 deletions(-) diff --git a/client/changes.go b/client/changes.go index 537c8237f..7cdb8f88c 100644 --- a/client/changes.go +++ b/client/changes.go @@ -160,15 +160,14 @@ func (client *Client) Changes(opts *ChangesOptions) ([]*Change, error) { // WaitChangeOptions holds the options for the WaitChange call. type WaitChangeOptions struct { - // If non-zero, wait at most this long before returning the current change - // data. The timeout elapsing is not considered an error, so if a timeout - // is specified, the caller should check Change.Ready to determine whether - // the change is actually finished. + // If nonzero, wait at most this long before returning. If a timeout + // occurs, WaitChange will return an error. Timeout time.Duration } -// WaitChange waits for a given change to be finished (whether or not there -// was an error, which is indicated by Change.Err being set). +// WaitChange waits for the change to be finished. If the wait operation +// succeeds, the returned Change.Err string will be non-empty if the change +// itself had an error. func (client *Client) WaitChange(id string, opts *WaitChangeOptions) (*Change, error) { var chgd changeAndData diff --git a/client/changes_test.go b/client/changes_test.go index 83214b72c..728e00045 100644 --- a/client/changes_test.go +++ b/client/changes_test.go @@ -15,6 +15,7 @@ package client_test import ( + "fmt" "io/ioutil" "time" @@ -57,17 +58,6 @@ func (cs *clientSuite) TestClientChange(c *check.C) { } func (cs *clientSuite) TestClientWaitChange(c *check.C) { - cs.testClientWaitChange(c, "foo", nil, "http://localhost/v1/changes/foo/wait") -} - -func (cs *clientSuite) TestClientWaitChangeTimeout(c *check.C) { - opts := &client.WaitChangeOptions{ - Timeout: 30 * time.Second, - } - cs.testClientWaitChange(c, "bar", opts, "http://localhost/v1/changes/bar/wait?timeout=30s") -} - -func (cs *clientSuite) testClientWaitChange(c *check.C, changeID string, opts *client.WaitChangeOptions, expectedURL string) { cs.rsp = `{"type": "sync", "result": { "id": "uno", "kind": "foo", @@ -79,9 +69,9 @@ func (cs *clientSuite) testClientWaitChange(c *check.C, changeID string, opts *c "tasks": [{"kind": "bar", "summary": "...", "status": "Do", "progress": {"done": 0, "total": 1}, "spawn-time": "2016-04-21T01:02:03Z", "ready-time": "2016-04-21T01:02:04Z"}] }}` - chg, err := cs.cli.WaitChange(changeID, opts) + chg, err := cs.cli.WaitChange("foo", nil) c.Assert(err, check.IsNil) - c.Assert(cs.req.URL.String(), check.Equals, expectedURL) + c.Assert(cs.req.URL.String(), check.Equals, "http://localhost/v1/changes/foo/wait") c.Check(chg, check.DeepEquals, &client.Change{ ID: "uno", Kind: "foo", @@ -100,6 +90,16 @@ func (cs *clientSuite) testClientWaitChange(c *check.C, changeID string, opts *c }) } +func (cs *clientSuite) TestClientWaitChangeTimeout(c *check.C) { + cs.err = fmt.Errorf(`timed out waiting for change`) + opts := &client.WaitChangeOptions{ + Timeout: 30 * time.Second, + } + _, err := cs.cli.WaitChange("bar", opts) + c.Assert(cs.req.URL.String(), check.Equals, "http://localhost/v1/changes/bar/wait?timeout=30s") + c.Assert(err, check.ErrorMatches, `.*timed out waiting for change.*`) +} + func (cs *clientSuite) TestClientChangeData(c *check.C) { cs.rsp = `{"type": "sync", "result": { "id": "uno", diff --git a/internal/daemon/api_changes.go b/internal/daemon/api_changes.go index 908592e12..043d5d748 100644 --- a/internal/daemon/api_changes.go +++ b/internal/daemon/api_changes.go @@ -200,13 +200,16 @@ func v1GetChangeWait(c *Command, r *http.Request, _ *userState) Response { case <-change.Ready(): timer.Stop() // change ready, release timer resources case <-timer.C: + return statusGatewayTimeout("timed out waiting for change after %s", timeout) case <-r.Context().Done(): + return statusInternalError("request cancelled") } } else { // No timeout, wait indefinitely for change to be ready. select { case <-change.Ready(): case <-r.Context().Done(): + return statusInternalError("request cancelled") } } diff --git a/internal/daemon/api_changes_test.go b/internal/daemon/api_changes_test.go index 6afd41828..47e6d66e1 100644 --- a/internal/daemon/api_changes_test.go +++ b/internal/daemon/api_changes_test.go @@ -366,7 +366,7 @@ func (s *apiSuite) TestStateChangeAbortIsReady(c *check.C) { }) } -func (s *apiSuite) TestStateChangeNotFound(c *check.C) { +func (s *apiSuite) TestWaitChangeNotFound(c *check.C) { s.daemon(c) req, err := http.NewRequest("GET", "/v1/changes/x/wait", nil) c.Assert(err, check.IsNil) @@ -374,57 +374,72 @@ func (s *apiSuite) TestStateChangeNotFound(c *check.C) { c.Check(rsp.Status, check.Equals, 404) } -func (s *apiSuite) TestStateChangeInvalidTimeout(c *check.C) { - // Setup - d := s.daemon(c) - st := d.overlord.State() - st.Lock() - change := st.NewChange("exec", "Exec") - task := st.NewTask("exec", "Exec") - change.AddAll(state.NewTaskSet(task)) - st.Unlock() - - // Execute - s.vars = map[string]string{"id": change.ID()} - req, err := http.NewRequest("GET", "/v1/changes/"+change.ID()+"/wait?timeout=BAD", nil) - c.Assert(err, check.IsNil) - rsp := v1GetChangeWait(apiCmd("/v1/changes/{id}/wait"), req, nil).(*resp) - - // Verify +func (s *apiSuite) TestWaitChangeInvalidTimeout(c *check.C) { + rec, rsp, _ := s.testWaitChange(context.Background(), c, "?timeout=BAD", nil) + c.Check(rec.Code, check.Equals, 400) c.Check(rsp.Status, check.Equals, 400) + c.Check(rsp.Type, check.Equals, ResponseTypeError) + result := rsp.Result.(*errorResult) + c.Check(result.Message, check.Matches, "invalid timeout .*") } -func (s *apiSuite) TestStateChangeWait(c *check.C) { - ready := s.testStateChangeWait(context.Background(), c, "", func(st *state.State, change *state.Change) { +func (s *apiSuite) TestWaitChangeSuccess(c *check.C) { + rec, rsp, changeID := s.testWaitChange(context.Background(), c, "", func(st *state.State, change *state.Change) { // Mark change ready after a short interval time.Sleep(10 * time.Millisecond) st.Lock() change.SetStatus(state.DoneStatus) st.Unlock() }) - c.Check(ready, check.Equals, true) + + c.Check(rec.Code, check.Equals, 200) + c.Check(rsp.Status, check.Equals, 200) + c.Check(rsp.Type, check.Equals, ResponseTypeSync) + c.Check(rsp.Result, check.NotNil) + + var body map[string]interface{} + err := json.Unmarshal(rec.Body.Bytes(), &body) + c.Check(err, check.IsNil) + result := body["result"].(map[string]interface{}) + c.Check(result["id"].(string), check.Equals, changeID) + c.Check(result["kind"].(string), check.Equals, "exec") + c.Check(result["ready"].(bool), check.Equals, true) } -func (s *apiSuite) TestStateChangeWaitCancel(c *check.C) { +func (s *apiSuite) TestWaitChangeCancel(c *check.C) { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Millisecond) defer cancel() - ready := s.testStateChangeWait(ctx, c, "", func(st *state.State, change *state.Change) {}) - c.Check(ready, check.Equals, false) + + rec, rsp, _ := s.testWaitChange(ctx, c, "", nil) + c.Check(rec.Code, check.Equals, 500) + c.Check(rsp.Status, check.Equals, 500) + c.Check(rsp.Type, check.Equals, ResponseTypeError) + result := rsp.Result.(*errorResult) + c.Check(result.Message, check.Equals, "request cancelled") } -func (s *apiSuite) TestStateChangeWaitTimeout(c *check.C) { - ready := s.testStateChangeWait(context.Background(), c, "?timeout=10ms", func(st *state.State, change *state.Change) {}) - c.Check(ready, check.Equals, false) +func (s *apiSuite) TestWaitChangeTimeout(c *check.C) { + rec, rsp, _ := s.testWaitChange(context.Background(), c, "?timeout=10ms", nil) + c.Check(rec.Code, check.Equals, 504) + c.Check(rsp.Status, check.Equals, 504) + c.Check(rsp.Type, check.Equals, ResponseTypeError) + result := rsp.Result.(*errorResult) + c.Check(result.Message, check.Matches, "timed out waiting for change .*") } -func (s *apiSuite) TestStateChangeWaitTimeoutCancel(c *check.C) { +func (s *apiSuite) TestWaitChangeTimeoutCancel(c *check.C) { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Millisecond) defer cancel() - ready := s.testStateChangeWait(ctx, c, "?timeout=20ms", func(st *state.State, change *state.Change) {}) - c.Check(ready, check.Equals, false) + + rec, rsp, _ := s.testWaitChange(ctx, c, "?timeout=20ms", nil) + c.Check(rec.Code, check.Equals, 500) + c.Check(rsp.Status, check.Equals, 500) + c.Check(rsp.Type, check.Equals, ResponseTypeError) + result := rsp.Result.(*errorResult) + c.Check(result.Message, check.Equals, "request cancelled") } -func (s *apiSuite) testStateChangeWait(ctx context.Context, c *check.C, query string, markReady func(st *state.State, change *state.Change)) bool { +func (s *apiSuite) testWaitChange(ctx context.Context, c *check.C, query string, markReady func(st *state.State, change *state.Change)) (*httptest.ResponseRecorder, *resp, string) { // Setup d := s.daemon(c) st := d.overlord.State() @@ -433,7 +448,10 @@ func (s *apiSuite) testStateChangeWait(ctx context.Context, c *check.C, query st task := st.NewTask("exec", "Exec") change.AddAll(state.NewTaskSet(task)) st.Unlock() - go markReady(st, change) + + if markReady != nil { + go markReady(st, change) + } // Execute s.vars = map[string]string{"id": change.ID()} @@ -442,17 +460,5 @@ func (s *apiSuite) testStateChangeWait(ctx context.Context, c *check.C, query st rsp := v1GetChangeWait(apiCmd("/v1/changes/{id}/wait"), req, nil).(*resp) rec := httptest.NewRecorder() rsp.ServeHTTP(rec, req) - - // Verify - c.Check(rec.Code, check.Equals, 200) - c.Check(rsp.Status, check.Equals, 200) - c.Check(rsp.Result, check.NotNil) - - var body map[string]interface{} - err = json.Unmarshal(rec.Body.Bytes(), &body) - c.Check(err, check.IsNil) - result := body["result"].(map[string]interface{}) - c.Check(result["id"].(string), check.Equals, change.ID()) - c.Check(result["kind"].(string), check.Equals, "exec") - return result["ready"].(bool) + return rec, rsp, change.ID() } diff --git a/internal/daemon/response.go b/internal/daemon/response.go index 6671d2e6a..56b06cb96 100644 --- a/internal/daemon/response.go +++ b/internal/daemon/response.go @@ -194,12 +194,11 @@ type errorResponder func(string, ...interface{}) Response // Standard error responses. var ( + statusBadRequest = makeErrorResponder(400) statusUnauthorized = makeErrorResponder(401) + statusForbidden = makeErrorResponder(403) statusNotFound = makeErrorResponder(404) - statusBadRequest = makeErrorResponder(400) statusMethodNotAllowed = makeErrorResponder(405) statusInternalError = makeErrorResponder(500) - statusNotImplemented = makeErrorResponder(501) - statusForbidden = makeErrorResponder(403) - statusConflict = makeErrorResponder(409) + statusGatewayTimeout = makeErrorResponder(504) )