From 03a4f59a059f8ac768dc8ad31a30727f9a9f9c9e Mon Sep 17 00:00:00 2001 From: Drew Bailey <2614075+drewbailey@users.noreply.github.com> Date: Thu, 7 Nov 2019 15:39:41 -0500 Subject: [PATCH 1/3] Remove response body from websocket error If a websocket connection errors we currently return the error with a copy of the response body. The response body from the websocket can often times be completely illegible so remove it from the error string. make alloc id empty for more reliable failure un-gzip if content encoding header present --- api/allocations_test.go | 54 +++++++++++++++++++++++++++++++++++++++++ api/api.go | 11 ++++++++- 2 files changed, 64 insertions(+), 1 deletion(-) diff --git a/api/allocations_test.go b/api/allocations_test.go index a18b7e1477a7..3bb8595cd475 100644 --- a/api/allocations_test.go +++ b/api/allocations_test.go @@ -1,6 +1,8 @@ package api import ( + "context" + "os" "reflect" "sort" "testing" @@ -247,6 +249,58 @@ func TestAllocations_RescheduleInfo(t *testing.T) { } +// TestAllocations_ExecErrors ensures errors are properly formatted +func TestAllocations_ExecErrors(t *testing.T) { + c, s := makeClient(t, nil, nil) + defer s.Stop() + a := c.Allocations() + + job := &Job{ + Name: stringToPtr("foo"), + Namespace: stringToPtr(DefaultNamespace), + ID: stringToPtr("bar"), + ParentID: stringToPtr("lol"), + TaskGroups: []*TaskGroup{ + { + Name: stringToPtr("bar"), + Tasks: []*Task{ + { + Name: "task1", + }, + }, + }, + }, + } + job.Canonicalize() + + uuidGen := func() string { + ret, err := uuid.GenerateUUID() + if err != nil { + t.Fatal(err) + } + return ret + } + + alloc := &Allocation{ + ID: "", + Namespace: DefaultNamespace, + EvalID: uuidGen(), + Name: "foo-bar[1]", + NodeID: uuidGen(), + TaskGroup: *job.TaskGroups[0].Name, + JobID: *job.ID, + Job: job, + } + // Querying when no allocs exist returns nothing + sizeCh := make(chan TerminalSize, 1) + + // make a request that will result in an error + // ensure the error is what we expect + _, err := a.Exec(context.Background(), alloc, "bar", false, []string{"command"}, os.Stdin, os.Stdout, os.Stderr, sizeCh, nil) + require.Contains(t, err.Error(), "Unexpected response code: 301") + require.Contains(t, err.Error(), "Moved Permanently") +} + func TestAllocations_ShouldMigrate(t *testing.T) { t.Parallel() require.True(t, DesiredTransition{Migrate: boolToPtr(true)}.ShouldMigrate()) diff --git a/api/api.go b/api/api.go index 9f4c8ebc589e..a3f2a9c49e91 100644 --- a/api/api.go +++ b/api/api.go @@ -742,7 +742,16 @@ func (c *Client) websocket(endpoint string, q *QueryOptions) (*websocket.Conn, * // check resp status code, as it's more informative than handshake error we get from ws library if resp != nil && resp.StatusCode != 101 { var buf bytes.Buffer - io.Copy(&buf, resp.Body) + + if resp.Header.Get("Content-Encoding") == "gzip" { + greader, err := gzip.NewReader(resp.Body) + if err != nil { + return nil, nil, fmt.Errorf("Unexpected response code: %d", resp.StatusCode) + } + io.Copy(&buf, greader) + } else { + io.Copy(&buf, resp.Body) + } resp.Body.Close() return nil, nil, fmt.Errorf("Unexpected response code: %d (%s)", resp.StatusCode, buf.Bytes()) From 8c891fcb94ceb1b4ea949ae970813f4e27344952 Mon Sep 17 00:00:00 2001 From: Drew Bailey <2614075+drewbailey@users.noreply.github.com> Date: Fri, 8 Nov 2019 09:27:36 -0500 Subject: [PATCH 2/3] switch to uuid helper package --- api/allocations_test.go | 28 ++++++---------------------- 1 file changed, 6 insertions(+), 22 deletions(-) diff --git a/api/allocations_test.go b/api/allocations_test.go index 3bb8595cd475..b942ee0e0f53 100644 --- a/api/allocations_test.go +++ b/api/allocations_test.go @@ -9,7 +9,7 @@ import ( "time" - "github.com/hashicorp/go-uuid" + "github.com/hashicorp/nomad/helper/uuid" "github.com/stretchr/testify/require" ) @@ -148,20 +148,12 @@ func TestAllocations_RescheduleInfo(t *testing.T) { } job.Canonicalize() - uuidGen := func() string { - ret, err := uuid.GenerateUUID() - if err != nil { - t.Fatal(err) - } - return ret - } - alloc := &Allocation{ - ID: uuidGen(), + ID: uuid.Generate(), Namespace: DefaultNamespace, - EvalID: uuidGen(), + EvalID: uuid.Generate(), Name: "foo-bar[1]", - NodeID: uuidGen(), + NodeID: uuid.Generate(), TaskGroup: *job.TaskGroups[0].Name, JobID: *job.ID, Job: job, @@ -273,20 +265,12 @@ func TestAllocations_ExecErrors(t *testing.T) { } job.Canonicalize() - uuidGen := func() string { - ret, err := uuid.GenerateUUID() - if err != nil { - t.Fatal(err) - } - return ret - } - alloc := &Allocation{ ID: "", Namespace: DefaultNamespace, - EvalID: uuidGen(), + EvalID: uuid.Generate(), Name: "foo-bar[1]", - NodeID: uuidGen(), + NodeID: uuid.Generate(), TaskGroup: *job.TaskGroups[0].Name, JobID: *job.ID, Job: job, From 69751a787f8b4066554b92be4811e8e080de65e6 Mon Sep 17 00:00:00 2001 From: Drew Bailey <2614075+drewbailey@users.noreply.github.com> Date: Fri, 8 Nov 2019 09:30:56 -0500 Subject: [PATCH 3/3] update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e6fd3700a749..e198cfd08d8f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ BUG FIXES: * nomad: Multiple connect enabled services in the same taskgroup failed to register [[GH-6646](https://github.com/hashicorp/nomad/issues/6646)] * scheduler: Changes to devices in resource stanza should cause rescheduling [[GH-6644](https://github.com/hashicorp/nomad/issues/6644)] + * api: Decompress web socket response body if gzipped on error responses [[GH-6650](https://github.com/hashicorp/nomad/issues/6650)] ## 0.10.1 (November 4, 2019)