From 2140a3960c5478272a131b67dd24b1f5b909b0d3 Mon Sep 17 00:00:00 2001 From: Agnes Tevesz Date: Thu, 31 Oct 2024 15:08:37 -0500 Subject: [PATCH] DWX-18764 Move polling utilities to utils The timeout and threshold calculation is duplicated in every dw module, it is moved to utils to reduce duplication. --- .../dw/virtualwarehouse/hive/model_hive_vw.go | 17 +-- .../virtualwarehouse/hive/resource_hive_vw.go | 10 +- .../hive/resource_hive_vw_acc_test.go | 2 + .../hive/resource_hive_vw_test.go | 6 +- utils/utils.go | 37 ++++- utils/utils_test.go | 138 +++++++++++++++++- 6 files changed, 184 insertions(+), 26 deletions(-) diff --git a/resources/dw/virtualwarehouse/hive/model_hive_vw.go b/resources/dw/virtualwarehouse/hive/model_hive_vw.go index 67106d09..2b6d1485 100644 --- a/resources/dw/virtualwarehouse/hive/model_hive_vw.go +++ b/resources/dw/virtualwarehouse/hive/model_hive_vw.go @@ -11,8 +11,6 @@ package hive import ( - "time" - "github.com/hashicorp/terraform-plugin-framework/types" "github.com/cloudera/terraform-provider-cdp/utils" @@ -28,17 +26,6 @@ type resourceModel struct { PollingOptions *utils.PollingOptions `tfsdk:"polling_options"` } -// TODO these are the same everywhere, abstract this -func (p *resourceModel) getPollingTimeout() time.Duration { - if p.PollingOptions != nil { - return time.Duration(p.PollingOptions.PollingTimeout.ValueInt64()) * time.Minute - } - return 40 * time.Minute -} - -func (p *resourceModel) getCallFailureThreshold() int { - if p.PollingOptions != nil { - return int(p.PollingOptions.CallFailureThreshold.ValueInt64()) - } - return 3 +func (p *resourceModel) GetPollingOptions() *utils.PollingOptions { + return p.PollingOptions } diff --git a/resources/dw/virtualwarehouse/hive/resource_hive_vw.go b/resources/dw/virtualwarehouse/hive/resource_hive_vw.go index 92f2afc8..479a2078 100644 --- a/resources/dw/virtualwarehouse/hive/resource_hive_vw.go +++ b/resources/dw/virtualwarehouse/hive/resource_hive_vw.go @@ -13,13 +13,13 @@ package hive import ( "context" "fmt" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry" "strings" "time" "github.com/hashicorp/terraform-plugin-framework/resource" "github.com/hashicorp/terraform-plugin-framework/types" "github.com/hashicorp/terraform-plugin-log/tflog" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry" "github.com/cloudera/terraform-provider-cdp/cdp-sdk-go/cdp" "github.com/cloudera/terraform-provider-cdp/cdp-sdk-go/gen/dw/client/operations" @@ -90,9 +90,9 @@ func (r *hiveResource) Create(ctx context.Context, req resource.CreateRequest, r Pending: []string{"Accepted", "Creating", "Created", "Starting"}, Target: []string{"Running"}, Delay: 30 * time.Second, - Timeout: plan.getPollingTimeout(), + Timeout: utils.GetPollingTimeout(&plan, 20*time.Minute), PollInterval: 30 * time.Second, - Refresh: r.stateRefresh(ctx, clusterID, vwID, &callFailedCount, plan.getCallFailureThreshold()), + Refresh: r.stateRefresh(ctx, clusterID, vwID, &callFailedCount, utils.GetCallFailureThreshold(&plan, 3)), } if _, err = stateConf.WaitForStateContext(ctx); err != nil { resp.Diagnostics.AddError( @@ -164,9 +164,9 @@ func (r *hiveResource) Delete(ctx context.Context, req resource.DeleteRequest, r Pending: []string{"Deleting", "Running", "Stopping", "Stopped", "Creating", "Created", "Starting", "Updating"}, Target: []string{"Deleted"}, // This is not an actual state, we added it to fake the state change Delay: 30 * time.Second, - Timeout: state.getPollingTimeout(), + Timeout: utils.GetPollingTimeout(&state, 20*time.Minute), PollInterval: 30 * time.Second, - Refresh: r.stateRefresh(ctx, clusterID, vwID, &callFailedCount, state.getCallFailureThreshold()), + Refresh: r.stateRefresh(ctx, clusterID, vwID, &callFailedCount, utils.GetCallFailureThreshold(&state, 3)), } if _, err := stateConf.WaitForStateContext(ctx); err != nil { resp.Diagnostics.AddError( diff --git a/resources/dw/virtualwarehouse/hive/resource_hive_vw_acc_test.go b/resources/dw/virtualwarehouse/hive/resource_hive_vw_acc_test.go index 3b99853d..6cd4eef4 100644 --- a/resources/dw/virtualwarehouse/hive/resource_hive_vw_acc_test.go +++ b/resources/dw/virtualwarehouse/hive/resource_hive_vw_acc_test.go @@ -8,6 +8,8 @@ // OF ANY KIND, either express or implied. Refer to the License for the specific // permissions and limitations governing your use of the file. +//go:build hive + package hive_test import ( diff --git a/resources/dw/virtualwarehouse/hive/resource_hive_vw_test.go b/resources/dw/virtualwarehouse/hive/resource_hive_vw_test.go index 09d9b788..a6f7d4fe 100644 --- a/resources/dw/virtualwarehouse/hive/resource_hive_vw_test.go +++ b/resources/dw/virtualwarehouse/hive/resource_hive_vw_test.go @@ -13,14 +13,14 @@ package hive import ( "context" "fmt" - "github.com/hashicorp/terraform-plugin-framework/resource/schema/booldefault" - "github.com/hashicorp/terraform-plugin-framework/resource/schema/boolplanmodifier" - "github.com/hashicorp/terraform-plugin-framework/resource/schema/int64default" "testing" "github.com/go-openapi/runtime" "github.com/hashicorp/terraform-plugin-framework/resource" "github.com/hashicorp/terraform-plugin-framework/resource/schema" + "github.com/hashicorp/terraform-plugin-framework/resource/schema/booldefault" + "github.com/hashicorp/terraform-plugin-framework/resource/schema/boolplanmodifier" + "github.com/hashicorp/terraform-plugin-framework/resource/schema/int64default" "github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" "github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier" "github.com/hashicorp/terraform-plugin-framework/tfsdk" diff --git a/utils/utils.go b/utils/utils.go index 167f74e1..3bc88c57 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -15,11 +15,10 @@ import ( "fmt" "time" - "github.com/hashicorp/terraform-plugin-log/tflog" - "github.com/hashicorp/terraform-plugin-framework/datasource" "github.com/hashicorp/terraform-plugin-framework/resource" "github.com/hashicorp/terraform-plugin-framework/types" + "github.com/hashicorp/terraform-plugin-log/tflog" "github.com/cloudera/terraform-provider-cdp/cdp-sdk-go/cdp" ) @@ -130,3 +129,37 @@ func Int64To32Pointer(in types.Int64) *int32 { var n2 = int32(n64) return &n2 } + +type HasPollingOptions interface { + GetPollingOptions() *PollingOptions +} + +// GetPollingTimeout returns the polling timeout from the given polling options or the fallback value. In case a negative +// value is given, it will be treated as one minute. +func GetPollingTimeout[T HasPollingOptions](p T, fallback time.Duration) time.Duration { + var timeout time.Duration + if opts := p.GetPollingOptions(); opts != nil && !opts.PollingTimeout.IsNull() { + timeout = time.Duration(p.GetPollingOptions().PollingTimeout.ValueInt64()) * time.Minute + } else { + timeout = fallback + } + if timeout.Seconds() <= 0 { + return time.Minute + } + return timeout +} + +// GetCallFailureThreshold returns the call failure threshold from the given polling options or the fallback value. +// In case a negative value is given, it will be treated as zero. +func GetCallFailureThreshold[T HasPollingOptions](p T, fallback int) int { + var threshold int + if opts := p.GetPollingOptions(); opts != nil && !opts.CallFailureThreshold.IsNull() { + threshold = int(p.GetPollingOptions().CallFailureThreshold.ValueInt64()) + } else { + threshold = fallback + } + if threshold <= 0 { + return 0 + } + return threshold +} diff --git a/utils/utils_test.go b/utils/utils_test.go index 9c130703..b8060f56 100644 --- a/utils/utils_test.go +++ b/utils/utils_test.go @@ -12,9 +12,11 @@ package utils import ( "context" - "github.com/hashicorp/terraform-plugin-framework/types" "testing" "time" + + "github.com/hashicorp/terraform-plugin-framework/types" + "github.com/stretchr/testify/assert" ) const ( @@ -194,3 +196,137 @@ func TestCalculateCallFailureThresholdOrDefault(t *testing.T) { }) } } + +type TestPollingOptions struct { + PollingOptions *PollingOptions +} + +func (p *TestPollingOptions) GetPollingOptions() *PollingOptions { + return p.PollingOptions +} + +func TestGetPollingTimeout(t *testing.T) { + type args struct { + p HasPollingOptions + fallback time.Duration + } + tests := []struct { + description string + args args + want time.Duration + }{ + { + description: "Test when PollingOptions is nil and fallback is given then fallback should come back.", + args: args{ + p: &TestPollingOptions{}, + fallback: 90 * time.Minute, + }, + want: 90 * time.Minute, + }, + { + description: "Test when PollingOptions is nil and fallback is not given then 1 minute should come back.", + args: args{ + p: &TestPollingOptions{}, + fallback: 0, + }, + want: time.Minute, + }, + { + description: "Test when PollingOptions is given but PollingTimeout is nil and fallback is given then fallback should come back.", + args: args{ + p: &TestPollingOptions{PollingOptions: &PollingOptions{}}, + fallback: 90 * time.Minute, + }, + want: 90 * time.Minute, + }, + { + description: "Test when PollingOptions is given but PollingTimeout is nil and fallback is not given then 1 minute should come back.", + args: args{ + p: &TestPollingOptions{PollingOptions: &PollingOptions{}}, + fallback: 0, + }, + want: time.Minute, + }, + { + description: "Test when PollingOptions is given and PollingTimeout is given then its value should come back.", + args: args{ + p: &TestPollingOptions{PollingOptions: &PollingOptions{PollingTimeout: types.Int64Value(60)}}, + fallback: 90 * time.Minute, + }, + want: 60 * time.Minute, + }, + } + for _, tt := range tests { + t.Run(tt.description, func(t *testing.T) { + got := GetPollingTimeout(tt.args.p, tt.args.fallback) + assert.Equal(t, got, tt.want) + }) + } +} + +func TestGetCallFailureThreshold(t *testing.T) { + type args struct { + p HasPollingOptions + fallback int + } + tests := []struct { + description string + args args + want int + }{ + { + description: "Test when PollingOptions is nil and fallback is given then fallback should come back.", + args: args{ + p: &TestPollingOptions{}, + fallback: 3, + }, + want: 3, + }, + { + description: "Test when PollingOptions is nil and fallback is not given then 0 should come back.", + args: args{ + p: &TestPollingOptions{}, + fallback: 0, + }, + want: 0, + }, + { + description: "Test when PollingOptions is given but CallFailureThreshold is nil and fallback is given then fallback should come back.", + args: args{ + p: &TestPollingOptions{PollingOptions: &PollingOptions{}}, + fallback: 3, + }, + want: 3, + }, + { + description: "Test when PollingOptions is given but CallFailureThreshold is nil and fallback is not given then 0 should come back.", + args: args{ + p: &TestPollingOptions{PollingOptions: &PollingOptions{}}, + fallback: 0, + }, + want: 0, + }, + { + description: "Test when PollingOptions is given and CallFailureThreshold is given then its value should come back.", + args: args{ + p: &TestPollingOptions{PollingOptions: &PollingOptions{CallFailureThreshold: types.Int64Value(5)}}, + fallback: 3, + }, + want: 5, + }, + { + description: "Test when PollingOptions is nil and fallback is a negative int then 0 should come back.", + args: args{ + p: &TestPollingOptions{}, + fallback: -2, + }, + want: 0, + }, + } + for _, tt := range tests { + t.Run(tt.description, func(t *testing.T) { + got := GetCallFailureThreshold(tt.args.p, tt.args.fallback) + assert.Equal(t, got, tt.want) + }) + } +}