From 4e25e8e2350579581b88bf96e67c18627008e65e Mon Sep 17 00:00:00 2001 From: Chris Baker <1675087+cgbaker@users.noreply.github.com> Date: Mon, 5 Apr 2021 21:21:41 +0000 Subject: [PATCH 1/6] documenting test for #10301 enable -race detector for testing api --- .circleci/config.yml | 5 +++++ api/api_test.go | 28 +++++++++++++++++++++++++++- 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 29faee5b748..37413e94c46 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -293,11 +293,15 @@ jobs: goarch: type: string default: "amd64" + enable_race_testing: + type: string + default: "" environment: GOTEST_PKGS_EXCLUDE: "<< parameters.exclude_packages >>" GOTEST_PKGS: "<< parameters.test_packages >>" GOTEST_MOD: "<< parameters.test_module >>" GOTESTARCH: "<< parameters.goarch >>" + ENABLE_RACE: "<< parameters.enable_race_testing >>" steps: - checkout - install-golang @@ -616,6 +620,7 @@ workflows: name: "test-api" test_module: "api" filters: *backend_test_branches_filter + enable_race_testing: "1" - test-container: name: "test-devices" test_packages: "./devices/..." diff --git a/api/api_test.go b/api/api_test.go index 0048ecdfe67..dda4a571a13 100644 --- a/api/api_test.go +++ b/api/api_test.go @@ -13,9 +13,10 @@ import ( "testing" "time" - "github.com/hashicorp/nomad/api/internal/testutil" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/hashicorp/nomad/api/internal/testutil" ) type configCallback func(c *Config) @@ -538,3 +539,28 @@ func TestCloneHttpClient(t *testing.T) { }) } + +func TestClient_HeaderRaceCondition(t *testing.T) { + require := require.New(t) + + conf := DefaultConfig() + conf.Headers = map[string][]string{ + "test-header": {"a"}, + } + client, err := NewClient(conf) + require.NoError(err) + + c := make(chan int) + + go func() { + req, _ := client.newRequest("GET", "/any/path/will/do") + r, _ := req.toHTTP() + c <- len(r.Header) + }() + req, _ := client.newRequest("GET", "/any/path/will/do") + r, _ := req.toHTTP() + + require.Len(r.Header, 2, "local request should have two headers") + require.Equal(2, <-c, "goroutine request should have two headers") + require.Len(conf.Headers, 1, "config headers should not mutate") +} From 60648672518b642ea3b4dcab431ca521b3e6597b Mon Sep 17 00:00:00 2001 From: Chris Baker <1675087+cgbaker@users.noreply.github.com> Date: Tue, 6 Apr 2021 18:04:56 +0000 Subject: [PATCH 2/6] sdk: header map copy to avoid race condition in #10301 --- CHANGELOG.md | 1 + api/api.go | 4 ++-- vendor/github.com/hashicorp/nomad/api/api.go | 4 ++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b819742925f..21ecee3dcdc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ IMPROVEMENTS: BUG FIXES: * agent: Only allow querying Prometheus formatted metrics if Prometheus is enabled within the config [[GH-10140](https://github.com/hashicorp/nomad/pull/10140)] + * api: Fixed race condition around headers in API client [[GH-10302](https://github.com/hashicorp/nomad/issues/10302)] * api: Added missing devices block to AllocatedTaskResources [[GH-10064](https://github.com/hashicorp/nomad/pull/10064)] * cli: Fixed a bug where non-int proxy port would panic CLI [[GH-10072](https://github.com/hashicorp/nomad/issues/10072)] * cli: Fixed a bug where `nomad operator debug` incorrectly parsed https Consul API URLs. [[GH-10082](https://github.com/hashicorp/nomad/pull/10082)] diff --git a/api/api.go b/api/api.go index d2deff27478..7ce7d9a1368 100644 --- a/api/api.go +++ b/api/api.go @@ -685,8 +685,8 @@ func (c *Client) newRequest(method, path string) (*request, error) { } } - if c.config.Headers != nil { - r.header = c.config.Headers + for key, values := range c.config.Headers { + r.header[key] = values } return r, nil diff --git a/vendor/github.com/hashicorp/nomad/api/api.go b/vendor/github.com/hashicorp/nomad/api/api.go index d2deff27478..7ce7d9a1368 100644 --- a/vendor/github.com/hashicorp/nomad/api/api.go +++ b/vendor/github.com/hashicorp/nomad/api/api.go @@ -685,8 +685,8 @@ func (c *Client) newRequest(method, path string) (*request, error) { } } - if c.config.Headers != nil { - r.header = c.config.Headers + for key, values := range c.config.Headers { + r.header[key] = values } return r, nil From c249b8d4445ab70643e21e0ceadfbe0e6cd0af80 Mon Sep 17 00:00:00 2001 From: Chris Baker <1675087+cgbaker@users.noreply.github.com> Date: Tue, 6 Apr 2021 20:27:12 +0000 Subject: [PATCH 3/6] updated changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 21ecee3dcdc..88a129dff02 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,7 +17,7 @@ IMPROVEMENTS: BUG FIXES: * agent: Only allow querying Prometheus formatted metrics if Prometheus is enabled within the config [[GH-10140](https://github.com/hashicorp/nomad/pull/10140)] - * api: Fixed race condition around headers in API client [[GH-10302](https://github.com/hashicorp/nomad/issues/10302)] + * api: Fixed a panic that may occur on concurrent access to an SDK client [[GH-10302](https://github.com/hashicorp/nomad/issues/10302)] * api: Added missing devices block to AllocatedTaskResources [[GH-10064](https://github.com/hashicorp/nomad/pull/10064)] * cli: Fixed a bug where non-int proxy port would panic CLI [[GH-10072](https://github.com/hashicorp/nomad/issues/10072)] * cli: Fixed a bug where `nomad operator debug` incorrectly parsed https Consul API URLs. [[GH-10082](https://github.com/hashicorp/nomad/pull/10082)] From 1da78a43dc837e26b768f6e83779c4b2d4043feb Mon Sep 17 00:00:00 2001 From: Chris Baker <1675087+cgbaker@users.noreply.github.com> Date: Tue, 6 Apr 2021 20:38:52 +0000 Subject: [PATCH 4/6] refactor enable_race circleci parameter --- .circleci/config.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 37413e94c46..b710bfd6b8e 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -294,14 +294,14 @@ jobs: type: string default: "amd64" enable_race_testing: - type: string - default: "" + type: boolean + default: false environment: GOTEST_PKGS_EXCLUDE: "<< parameters.exclude_packages >>" GOTEST_PKGS: "<< parameters.test_packages >>" GOTEST_MOD: "<< parameters.test_module >>" GOTESTARCH: "<< parameters.goarch >>" - ENABLE_RACE: "<< parameters.enable_race_testing >>" + ENABLE_RACE: << parameters.enable_race_testing >> && "TRUE" || "" steps: - checkout - install-golang @@ -620,7 +620,7 @@ workflows: name: "test-api" test_module: "api" filters: *backend_test_branches_filter - enable_race_testing: "1" + enable_race_testing: true - test-container: name: "test-devices" test_packages: "./devices/..." From 04166d6c80fad25a8feb77062ba8c3db0db9117e Mon Sep 17 00:00:00 2001 From: Chris Baker <1675087+cgbaker@users.noreply.github.com> Date: Tue, 6 Apr 2021 21:00:45 +0000 Subject: [PATCH 5/6] second attempt at fixing circle --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index b710bfd6b8e..1c722a09172 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -301,7 +301,7 @@ jobs: GOTEST_PKGS: "<< parameters.test_packages >>" GOTEST_MOD: "<< parameters.test_module >>" GOTESTARCH: "<< parameters.goarch >>" - ENABLE_RACE: << parameters.enable_race_testing >> && "TRUE" || "" + ENABLE_RACE: <<# parameters.enable_race_testing >>TRUE<> steps: - checkout - install-golang From aef0e4a67632c1c4ff66bd10705cce3347008544 Mon Sep 17 00:00:00 2001 From: Chris Baker <1675087+cgbaker@users.noreply.github.com> Date: Tue, 6 Apr 2021 21:29:55 +0000 Subject: [PATCH 6/6] third attempt at fixing circle --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 1c722a09172..864863f316a 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -301,7 +301,7 @@ jobs: GOTEST_PKGS: "<< parameters.test_packages >>" GOTEST_MOD: "<< parameters.test_module >>" GOTESTARCH: "<< parameters.goarch >>" - ENABLE_RACE: <<# parameters.enable_race_testing >>TRUE<> + ENABLE_RACE: "<<# parameters.enable_race_testing >>TRUE<>" steps: - checkout - install-golang