From da4bc9e65d775659ceb944ca4492283c55a5b90a Mon Sep 17 00:00:00 2001 From: Marcin Kaciuba Date: Mon, 19 Feb 2024 13:08:20 +0100 Subject: [PATCH 1/5] fix: retry in case of credentials error --- castai/resource_eks_cluster.go | 3 +- castai/resource_gke_cluster.go | 2 +- castai/sdk/utils_response.go | 21 ++++++++++++ castai/sdk/utils_response_test.go | 54 +++++++++++++++++++++++++++++++ 4 files changed, 78 insertions(+), 2 deletions(-) create mode 100644 castai/sdk/utils_response_test.go diff --git a/castai/resource_eks_cluster.go b/castai/resource_eks_cluster.go index 76f84192..2f154a25 100644 --- a/castai/resource_eks_cluster.go +++ b/castai/resource_eks_cluster.go @@ -201,9 +201,10 @@ func updateClusterSettings(ctx context.Context, data *schema.ResourceData, clien } err = sdk.StatusOk(response) // In case of malformed user request return error to user right away. - if response.StatusCode() == 400 { + if response.StatusCode() == 400 && !sdk.IsCredentialsError(response) { return backoff.Permanent(err) } + return err }, backoff.NewExponentialBackOff()); err != nil { return fmt.Errorf("updating cluster configuration: %w", err) diff --git a/castai/resource_gke_cluster.go b/castai/resource_gke_cluster.go index dfeba45a..d9c4a6c5 100644 --- a/castai/resource_gke_cluster.go +++ b/castai/resource_gke_cluster.go @@ -209,7 +209,7 @@ func updateGKEClusterSettings(ctx context.Context, data *schema.ResourceData, cl } err = sdk.StatusOk(response) // In case of malformed user request return error to user right away. - if response.StatusCode() == 400 { + if response.StatusCode() == 400 && !sdk.IsCredentialsError(response) { return backoff.Permanent(err) } return err diff --git a/castai/sdk/utils_response.go b/castai/sdk/utils_response.go index e53313ee..16a0058f 100644 --- a/castai/sdk/utils_response.go +++ b/castai/sdk/utils_response.go @@ -1,6 +1,7 @@ package sdk import ( + "encoding/json" "fmt" "net/http" ) @@ -32,3 +33,23 @@ func checkResponse(response Response, err error, expectedStatus int) error { return nil } + +type ErrorResponse struct { + Message string `json:"message"` + FieldViolations []struct { + Field string `json:"field"` + Description string `json:"description"` + } `json:"fieldViolations"` +} + +func IsCredentialsError(response Response) bool { + buf := response.GetBody() + + var errResponse ErrorResponse + err := json.Unmarshal(buf, &errResponse) + if err != nil { + return false + } + + return errResponse.Message == "Forbidden" && len(errResponse.FieldViolations) > 0 && errResponse.FieldViolations[0].Field == "credentials" +} diff --git a/castai/sdk/utils_response_test.go b/castai/sdk/utils_response_test.go new file mode 100644 index 00000000..f8b6df12 --- /dev/null +++ b/castai/sdk/utils_response_test.go @@ -0,0 +1,54 @@ +package sdk + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func Test_IsCredentialsError(t *testing.T) { + t.Run("credentials error", func(t *testing.T) { + t.Parallel() + r := require.New(t) + + body := `{ + "message": "Forbidden", + "fieldViolations": + [{"field": "credentials", "description": ""}] +}` + resp := ExternalClusterAPIReconcileClusterResponse{ + Body: []byte(body), + } + result := IsCredentialsError(resp) + r.True(result) + }) + t.Run("not credentials error", func(t *testing.T) { + t.Parallel() + r := require.New(t) + + body := `{ + "message": "something", + "fieldViolations": + [{"field": "credentials", "description": ""}] +}` + resp := ExternalClusterAPIReconcileClusterResponse{ + Body: []byte(body), + } + result := IsCredentialsError(resp) + r.False(result) + }) + t.Run("no fields in message", func(t *testing.T) { + t.Parallel() + r := require.New(t) + + body := `{ + "message": "something", + "fieldViolations":[] +}` + resp := ExternalClusterAPIReconcileClusterResponse{ + Body: []byte(body), + } + result := IsCredentialsError(resp) + r.False(result) + }) +} From 713ab45ca9c5ed2a0f9562c09be5e7765fc59b61 Mon Sep 17 00:00:00 2001 From: Marcin Kaciuba Date: Mon, 19 Feb 2024 15:23:59 +0100 Subject: [PATCH 2/5] add log of body --- castai/resource_node_template_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/castai/resource_node_template_test.go b/castai/resource_node_template_test.go index 66871127..bed4208f 100644 --- a/castai/resource_node_template_test.go +++ b/castai/resource_node_template_test.go @@ -4,12 +4,13 @@ import ( "bytes" "context" "fmt" - "github.com/samber/lo" "io" "net/http" "testing" "time" + "github.com/samber/lo" + "github.com/golang/mock/gomock" "github.com/hashicorp/go-cty/cty" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" @@ -592,7 +593,7 @@ func testAccCheckNodeTemplateDestroy(s *terraform.State) error { return nil } - return fmt.Errorf("node template %q still exists; %v", id, response) + return fmt.Errorf("node template %q still exists; %v", id, string(response.GetBody())) } return nil From a73a2b7560cbf5d5f692833c6f02e2f4f00b7347 Mon Sep 17 00:00:00 2001 From: Marcin Kaciuba Date: Mon, 19 Feb 2024 15:44:09 +0100 Subject: [PATCH 3/5] add retry --- castai/resource_node_template_test.go | 28 ++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/castai/resource_node_template_test.go b/castai/resource_node_template_test.go index bed4208f..bc9da11a 100644 --- a/castai/resource_node_template_test.go +++ b/castai/resource_node_template_test.go @@ -11,6 +11,7 @@ import ( "github.com/samber/lo" + "github.com/cenkalti/backoff/v4" "github.com/golang/mock/gomock" "github.com/hashicorp/go-cty/cty" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" @@ -581,19 +582,24 @@ func testAccCheckNodeTemplateDestroy(s *terraform.State) error { id := rs.Primary.ID clusterID := rs.Primary.Attributes["cluster_id"] - response, err := client.NodeTemplatesAPIListNodeTemplatesWithResponse(ctx, clusterID, &sdk.NodeTemplatesAPIListNodeTemplatesParams{IncludeDefault: lo.ToPtr(false)}) - if err != nil { + + if err := backoff.Retry(func() error { + response, err := client.NodeTemplatesAPIListNodeTemplatesWithResponse(ctx, clusterID, &sdk.NodeTemplatesAPIListNodeTemplatesParams{IncludeDefault: lo.ToPtr(false)}) + if err != nil { + return err + } + if response.StatusCode() == http.StatusNotFound { + return nil + } + if len(*response.JSON200.Items) == 0 { + // Should be no templates + return nil + } + + return fmt.Errorf("node template %q still exists; %v", id, string(response.GetBody())) + }, backoff.NewExponentialBackOff()); err != nil { return err } - if response.StatusCode() == http.StatusNotFound { - return nil - } - if len(*response.JSON200.Items) == 0 { - // Should be no templates - return nil - } - - return fmt.Errorf("node template %q still exists; %v", id, string(response.GetBody())) } return nil From 97b49855021836b5710fc3382e3d17caf57ac32f Mon Sep 17 00:00:00 2001 From: Marcin Kaciuba Date: Mon, 19 Feb 2024 16:03:18 +0100 Subject: [PATCH 4/5] add retry --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 6d5a9578..f79f2cde 100644 --- a/Makefile +++ b/Makefile @@ -54,7 +54,7 @@ test: .PHONY: testacc testacc: @echo "==> Running acceptance tests" - TF_ACC=1 go test ./castai/... '-run=^TestAcc' -v -timeout 16m + TF_ACC=1 go test ./castai/... '-run=^TestAcc' -v -timeout 20m .PHONY: validate-terraform-examples validate-terraform-examples: From a80899a3df3175bb70fb48c9508b993a62178501 Mon Sep 17 00:00:00 2001 From: Marcin Kaciuba Date: Mon, 19 Feb 2024 17:02:23 +0100 Subject: [PATCH 5/5] remove retry --- castai/resource_node_template_test.go | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/castai/resource_node_template_test.go b/castai/resource_node_template_test.go index bc9da11a..99881f28 100644 --- a/castai/resource_node_template_test.go +++ b/castai/resource_node_template_test.go @@ -11,7 +11,6 @@ import ( "github.com/samber/lo" - "github.com/cenkalti/backoff/v4" "github.com/golang/mock/gomock" "github.com/hashicorp/go-cty/cty" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" @@ -583,23 +582,19 @@ func testAccCheckNodeTemplateDestroy(s *terraform.State) error { id := rs.Primary.ID clusterID := rs.Primary.Attributes["cluster_id"] - if err := backoff.Retry(func() error { - response, err := client.NodeTemplatesAPIListNodeTemplatesWithResponse(ctx, clusterID, &sdk.NodeTemplatesAPIListNodeTemplatesParams{IncludeDefault: lo.ToPtr(false)}) - if err != nil { - return err - } - if response.StatusCode() == http.StatusNotFound { - return nil - } - if len(*response.JSON200.Items) == 0 { - // Should be no templates - return nil - } - - return fmt.Errorf("node template %q still exists; %v", id, string(response.GetBody())) - }, backoff.NewExponentialBackOff()); err != nil { + response, err := client.NodeTemplatesAPIListNodeTemplatesWithResponse(ctx, clusterID, &sdk.NodeTemplatesAPIListNodeTemplatesParams{IncludeDefault: lo.ToPtr(false)}) + if err != nil { return err } + if response.StatusCode() == http.StatusNotFound { + return nil + } + if len(*response.JSON200.Items) == 0 { + // Should be no templates + return nil + } + + return fmt.Errorf("node template %q still exists; %v", id, string(response.GetBody())) } return nil