From 01a6abb07a7637179bb60c8f8ae4f91ed53176f7 Mon Sep 17 00:00:00 2001 From: Prateek Arora Date: Thu, 5 Dec 2024 18:59:14 -0800 Subject: [PATCH] Add image_version to Impala TF resource --- docs/resources/dw_vw_impala.md | 1 + resources/dw/resource_dw_acc_test.go | 13 +- .../impala/model_impala_vw.go | 1 + .../impala/resource_impala_vw.go | 122 +++++++++++------- .../impala/resource_impala_vw_acc_test.go | 44 ++++++- .../impala/resource_impala_vw_test.go | 7 + .../impala/schema_impala_vw.go | 5 + 7 files changed, 146 insertions(+), 47 deletions(-) diff --git a/docs/resources/dw_vw_impala.md b/docs/resources/dw_vw_impala.md index 58c2e2dc..a4a8bc0c 100644 --- a/docs/resources/dw_vw_impala.md +++ b/docs/resources/dw_vw_impala.md @@ -23,6 +23,7 @@ description: |- ### Optional +- `image_version` (String) Image version of the impala. - `polling_options` (Attributes) Polling related configuration options that could specify various values that will be used during CDP resource creation. (see [below for nested schema](#nestedatt--polling_options)) ### Read-Only diff --git a/resources/dw/resource_dw_acc_test.go b/resources/dw/resource_dw_acc_test.go index c70e7c72..ec542ce0 100644 --- a/resources/dw/resource_dw_acc_test.go +++ b/resources/dw/resource_dw_acc_test.go @@ -166,7 +166,8 @@ func TestAccDwCluster_Basic(t *testing.T) { testAccAwsDataLakeConfig(&dlParams), testAccAwsClusterBasicConfig(&envParams), testAccDwCatalog(), - testAccHiveVirtualWarehouse(cdpacctest.RandomShortWithPrefix("tf-hive"))), + testAccHiveVirtualWarehouse(cdpacctest.RandomShortWithPrefix("tf-hive")), + testAccImpalaVirtualWarehouse(cdpacctest.RandomShortWithPrefix("tf-impala"))), Check: resource.ComposeAggregateTestCheckFunc( resource.TestCheckResourceAttr(resourceName, "name", envParams.Name), resource.TestCheckResourceAttr(resourceName, "status", "Accepted"), @@ -310,3 +311,13 @@ func testCheckClusterDestroy(s *terraform.State) error { } return nil } + +func testAccImpalaVirtualWarehouse(name string) string { + return fmt.Sprintf(` + resource "cdp_dw_vw_impala" "test_impala" { + cluster_id = cdp_dw_aws_cluster.test_data_warehouse_aws.cluster_id + database_catalog_id = cdp_dw_database_catalog.test_catalog.id + name = %[1]q + } + `, name) +} diff --git a/resources/dw/virtualwarehouse/impala/model_impala_vw.go b/resources/dw/virtualwarehouse/impala/model_impala_vw.go index ffdcbbaf..ea579fa4 100644 --- a/resources/dw/virtualwarehouse/impala/model_impala_vw.go +++ b/resources/dw/virtualwarehouse/impala/model_impala_vw.go @@ -23,6 +23,7 @@ type resourceModel struct { Name types.String `tfsdk:"name"` LastUpdated types.String `tfsdk:"last_updated"` Status types.String `tfsdk:"status"` + ImageVersion types.String `tfsdk:"image_version"` PollingOptions *utils.PollingOptions `tfsdk:"polling_options"` } diff --git a/resources/dw/virtualwarehouse/impala/resource_impala_vw.go b/resources/dw/virtualwarehouse/impala/resource_impala_vw.go index ae727a62..a397b7bf 100644 --- a/resources/dw/virtualwarehouse/impala/resource_impala_vw.go +++ b/resources/dw/virtualwarehouse/impala/resource_impala_vw.go @@ -13,11 +13,11 @@ package impala import ( "context" "fmt" + "github.com/hashicorp/terraform-plugin-framework/types" "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" @@ -61,64 +61,42 @@ func (r *impalaResource) Create(ctx context.Context, req resource.CreateRequest, return } - // Generate API request body from plan - vw := operations.NewCreateVwParamsWithContext(ctx). - WithInput(&models.CreateVwRequest{ - Name: plan.Name.ValueStringPointer(), - ClusterID: plan.ClusterID.ValueStringPointer(), - DbcID: plan.DatabaseCatalogID.ValueStringPointer(), - VwType: models.VwTypeImpala.Pointer(), - }) + // Create the VW Request using a helper + vwhCreateRequest := createVwRequestFromPlan(&plan) + tflog.Debug(ctx, fmt.Sprintf("CreateVw request: %+v", vwhCreateRequest)) - // Create new virtual warehouse - response, err := r.client.Dw.Operations.CreateVw(vw) - if err != nil { + // Make API request to create VW + response, err := r.client.Dw.Operations.CreateVw( + operations.NewCreateVwParamsWithContext(ctx).WithInput(vwhCreateRequest), + ) + if err != nil || response.GetPayload() == nil { resp.Diagnostics.AddError( "Error creating Impala virtual warehouse", - "Could not create Impala, unexpected error: "+err.Error(), + fmt.Sprintf("Could not create Impala, unexpected error: %v", err), ) return } + tflog.Debug(ctx, fmt.Sprintf("CreateVw response: %+v", response.GetPayload())) - payload := response.GetPayload() - clusterID := plan.ClusterID.ValueStringPointer() - vwID := &payload.VwID - - if opts := plan.PollingOptions; !(opts != nil && opts.Async.ValueBool()) { - callFailedCount := 0 - stateConf := &retry.StateChangeConf{ - Pending: []string{"Accepted", "Creating", "Created", "Starting"}, - Target: []string{"Running"}, - Delay: 30 * time.Second, - Timeout: utils.GetPollingTimeout(&plan, 20*time.Minute), - PollInterval: 30 * time.Second, - Refresh: r.stateRefresh(ctx, clusterID, vwID, &callFailedCount, utils.GetCallFailureThreshold(&plan, 3)), - } - if _, err = stateConf.WaitForStateContext(ctx); err != nil { - resp.Diagnostics.AddError( - "Error waiting for Data Warehouse Impala virtual warehouse", - "Could not create Impala, unexpected error: "+err.Error(), - ) - return - } + // Wait for the VW to reach Running state + if err := r.waitForVwRunning(ctx, &plan, response.GetPayload()); err != nil { + resp.Diagnostics.AddError( + "Error waiting for Data Warehouse Impala virtual warehouse", + fmt.Sprintf("Could not create Impala, unexpected error: %v", err), + ) + return } - desc := operations.NewDescribeVwParamsWithContext(ctx). - WithInput(&models.DescribeVwRequest{VwID: vwID, ClusterID: clusterID}) - describe, err := r.client.Dw.Operations.DescribeVw(desc) - if err != nil { + + // Fetch and map the response to plan using a helper + if err := r.populatePlanFromDescribe(ctx, &plan, &response.GetPayload().VwID); err != nil { resp.Diagnostics.AddError( "Error creating Impala virtual warehouse", - "Could not describe Impala, unexpected error: "+err.Error(), + fmt.Sprintf("Could not describe Impala, unexpected error: %v", err), ) return } - impala := describe.GetPayload() - plan.ID = types.StringValue(impala.Vw.ID) - plan.DatabaseCatalogID = types.StringValue(impala.Vw.DbcID) - plan.Name = types.StringValue(impala.Vw.Name) - plan.Status = types.StringValue(impala.Vw.Status) - plan.LastUpdated = types.StringValue(time.Now().Format(time.RFC850)) + // Save the updated plan into state diags = resp.State.Set(ctx, plan) resp.Diagnostics.Append(diags...) } @@ -204,3 +182,57 @@ func (r *impalaResource) stateRefresh(ctx context.Context, clusterID *string, vw return vw, vw.Vw.Status, nil } } + +func createVwRequestFromPlan(plan *resourceModel) *models.CreateVwRequest { + req := &models.CreateVwRequest{ + Name: plan.Name.ValueStringPointer(), + ClusterID: plan.ClusterID.ValueStringPointer(), + DbcID: plan.DatabaseCatalogID.ValueStringPointer(), + VwType: models.VwTypeImpala.Pointer(), + } + if imageVersion := plan.ImageVersion.ValueString(); imageVersion != "" { + req.ImageVersion = imageVersion + } + return req +} + +func (r *impalaResource) waitForVwRunning(ctx context.Context, plan *resourceModel, payload *models.CreateVwResponse) error { + clusterID := plan.ClusterID.ValueStringPointer() + vwID := &payload.VwID + + opts := plan.PollingOptions + + if opts == nil || !opts.Async.ValueBool() { + callFailedCount := 0 + stateConf := &retry.StateChangeConf{ + Pending: []string{"Accepted", "Creating", "Created", "Starting"}, + Target: []string{"Running"}, + Delay: 30 * time.Second, + Timeout: utils.GetPollingTimeout(plan, 20*time.Minute), + PollInterval: 30 * time.Second, + Refresh: r.stateRefresh(ctx, clusterID, vwID, &callFailedCount, utils.GetCallFailureThreshold(plan, 3)), + } + _, err := stateConf.WaitForStateContext(ctx) + return err + } + return nil +} + +func (r *impalaResource) populatePlanFromDescribe(ctx context.Context, plan *resourceModel, vwID *string) error { + desc := operations.NewDescribeVwParamsWithContext(ctx). + WithInput(&models.DescribeVwRequest{VwID: vwID, ClusterID: plan.ClusterID.ValueStringPointer()}) + describe, err := r.client.Dw.Operations.DescribeVw(desc) + if err != nil { + return err + } + + impala := describe.GetPayload() + plan.ID = types.StringValue(impala.Vw.ID) + plan.DatabaseCatalogID = types.StringValue(impala.Vw.DbcID) + plan.Name = types.StringValue(impala.Vw.Name) + plan.Status = types.StringValue(impala.Vw.Status) + plan.ImageVersion = types.StringValue(impala.Vw.CdhVersion) + plan.LastUpdated = types.StringValue(time.Now().Format(time.RFC850)) + + return nil +} diff --git a/resources/dw/virtualwarehouse/impala/resource_impala_vw_acc_test.go b/resources/dw/virtualwarehouse/impala/resource_impala_vw_acc_test.go index b5fe2e87..2240af7f 100644 --- a/resources/dw/virtualwarehouse/impala/resource_impala_vw_acc_test.go +++ b/resources/dw/virtualwarehouse/impala/resource_impala_vw_acc_test.go @@ -32,6 +32,7 @@ type impalaTestParameters struct { Name string ClusterID string DatabaseCatalogID string + ImageVersion string } func ImpalaPreCheck(t *testing.T) { @@ -80,8 +81,9 @@ func testAccImpalaBasicConfig(params impalaTestParameters) string { cluster_id = %[1]q database_catalog_id = %[2]q name = %[3]q + image_version = %[4]q } - `, params.ClusterID, params.DatabaseCatalogID, params.Name) + `, params.ClusterID, params.DatabaseCatalogID, params.Name, params.ImageVersion) } func testCheckImpalaDestroy(s *terraform.State) error { @@ -108,3 +110,43 @@ func testCheckImpalaDestroy(s *terraform.State) error { } return nil } + +// Set RUN_IMAGE_VERSION_TESTS to run this test +func TestAccImpalaImageVersion(t *testing.T) { + + OptionalPreCheck(t, "RUN_IMAGE_VERSION_TESTS") + params := impalaTestParameters{ + Name: cdpacctest.RandomShortWithPrefix(cdpacctest.ResourcePrefix), + ClusterID: os.Getenv("CDW_CLUSTER_ID"), + DatabaseCatalogID: os.Getenv("CDW_DATABASE_CATALOG_ID"), + ImageVersion: "2024.0.19.0-301", + } + resource.Test(t, resource.TestCase{ + PreCheck: func() { + cdpacctest.PreCheck(t) + ImpalaPreCheck(t) + }, + ProtoV6ProviderFactories: cdpacctest.TestAccProtoV6ProviderFactories, + CheckDestroy: testCheckImpalaDestroy, + Steps: []resource.TestStep{ + // Create and Read testing + { + Config: utils.Concat( + cdpacctest.TestAccCdpProviderConfig(), + testAccImpalaBasicConfig(params)), + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr("cdp_dw_vw_impala.test_impala", "name", params.Name), + resource.TestCheckResourceAttr("cdp_dw_vw_impala.test_impala", "cluster_id", params.ClusterID), + resource.TestCheckResourceAttr("cdp_dw_vw_impala.test_impala", "database_catalog_id", params.DatabaseCatalogID), + ), + }, + // Delete testing automatically occurs in TestCase + }, + }) +} + +func OptionalPreCheck(t *testing.T, envVar string) { + if os.Getenv(envVar) == "" { + t.Skipf("Skipping test: environment variable %s is not set", envVar) + } +} diff --git a/resources/dw/virtualwarehouse/impala/resource_impala_vw_test.go b/resources/dw/virtualwarehouse/impala/resource_impala_vw_test.go index b8efadcb..3ff12246 100644 --- a/resources/dw/virtualwarehouse/impala/resource_impala_vw_test.go +++ b/resources/dw/virtualwarehouse/impala/resource_impala_vw_test.go @@ -69,6 +69,11 @@ var testImpalaSchema = schema.Schema{ Computed: true, MarkdownDescription: "The status of the database catalog.", }, + "image_version": schema.StringAttribute{ + Computed: true, + Optional: true, + MarkdownDescription: "Image version of the impala.", + }, "polling_options": schema.SingleNestedAttribute{ MarkdownDescription: "Polling related configuration options that could specify various values that will be used during CDP resource creation.", Optional: true, @@ -122,6 +127,7 @@ func createRawImpalaResource() tftypes.Value { "name": tftypes.String, "last_updated": tftypes.String, "status": tftypes.String, + "image_version": tftypes.String, "polling_options": tftypes.Object{ AttributeTypes: map[string]tftypes.Type{ "async": tftypes.Bool, @@ -136,6 +142,7 @@ func createRawImpalaResource() tftypes.Value { "database_catalog_id": tftypes.NewValue(tftypes.String, "database-catalog-id"), "name": tftypes.NewValue(tftypes.String, ""), "last_updated": tftypes.NewValue(tftypes.String, ""), + "image_version": tftypes.NewValue(tftypes.String, nil), "status": tftypes.NewValue(tftypes.String, "Running"), "polling_options": tftypes.NewValue( tftypes.Object{ diff --git a/resources/dw/virtualwarehouse/impala/schema_impala_vw.go b/resources/dw/virtualwarehouse/impala/schema_impala_vw.go index 6952edbd..65649f6e 100644 --- a/resources/dw/virtualwarehouse/impala/schema_impala_vw.go +++ b/resources/dw/virtualwarehouse/impala/schema_impala_vw.go @@ -53,6 +53,11 @@ var impalaSchema = schema.Schema{ Computed: true, MarkdownDescription: "The status of the database catalog.", }, + "image_version": schema.StringAttribute{ + Computed: true, + Optional: true, + MarkdownDescription: "Image version of the impala.", + }, "polling_options": schema.SingleNestedAttribute{ MarkdownDescription: "Polling related configuration options that could specify various values that will be used during CDP resource creation.", Optional: true,