From 37b1be9c1af62bf7f0b4ea7b8ac14b9d414a06fe Mon Sep 17 00:00:00 2001 From: Vandy Liu <33995460+vandyliu@users.noreply.github.com> Date: Thu, 16 May 2024 19:52:35 -0700 Subject: [PATCH] Fix creating and updating hosted deployments with scaling specs (#62) --- docs/resources/deployment.md | 13 +- internal/provider/models/deployment.go | 186 +++++++++------- .../provider/resources/resource_cluster.go | 24 ++- .../resources/resource_cluster_test.go | 1 + .../provider/resources/resource_deployment.go | 104 +++++++-- .../resources/resource_deployment_test.go | 202 ++++++++++++++++-- internal/provider/schemas/scaling.go | 21 +- 7 files changed, 415 insertions(+), 136 deletions(-) diff --git a/docs/resources/deployment.md b/docs/resources/deployment.md index 6aa5a310..8d8eadff 100644 --- a/docs/resources/deployment.md +++ b/docs/resources/deployment.md @@ -171,24 +171,27 @@ Read-Only: ### Nested Schema for `scaling_spec` -Optional: +Required: -- `hibernation_spec` (Attributes) (see [below for nested schema](#nestedatt--scaling_spec--hibernation_spec)) +- `hibernation_spec` (Attributes) Hibernation configuration for the deployment. The deployment will hibernate according to the schedules defined in this configuration. To remove the hibernation configuration, set scaling_spec to null. (see [below for nested schema](#nestedatt--scaling_spec--hibernation_spec)) ### Nested Schema for `scaling_spec.hibernation_spec` Optional: -- `override` (Attributes) (see [below for nested schema](#nestedatt--scaling_spec--hibernation_spec--override)) -- `schedules` (Attributes Set) (see [below for nested schema](#nestedatt--scaling_spec--hibernation_spec--schedules)) +- `override` (Attributes) Hibernation override configuration. Set to null to remove the override. (see [below for nested schema](#nestedatt--scaling_spec--hibernation_spec--override)) +- `schedules` (Attributes Set) List of hibernation schedules. Set to null to remove all schedules. (see [below for nested schema](#nestedatt--scaling_spec--hibernation_spec--schedules)) ### Nested Schema for `scaling_spec.hibernation_spec.override` -Optional: +Required: - `is_hibernating` (Boolean) + +Optional: + - `override_until` (String) Read-Only: diff --git a/internal/provider/models/deployment.go b/internal/provider/models/deployment.go index 1ec4f07a..ed877f09 100644 --- a/internal/provider/models/deployment.go +++ b/internal/provider/models/deployment.go @@ -244,23 +244,13 @@ func (data *DeploymentResource) ReadFromResponse( data.SchedulerSize = types.StringPointerValue((*string)(deployment.SchedulerSize)) data.IsDevelopmentMode = types.BoolPointerValue(deployment.IsDevelopmentMode) data.IsHighAvailability = types.BoolPointerValue(deployment.IsHighAvailability) - - // Currently, the scaling status and spec are only available in development mode - // However, there is a bug in the API where the scaling status and spec are returned even if the deployment is not in development mode for updated deployments - // This is a workaround to handle the bug until the API is fixed - // Issue here: https://github.com/astronomer/astro/issues/21073 - if deployment.IsDevelopmentMode != nil && *deployment.IsDevelopmentMode { - data.ScalingStatus, diags = ScalingStatusTypesObject(ctx, deployment.ScalingStatus) - if diags.HasError() { - return diags - } - data.ScalingSpec, diags = ScalingSpecTypesObject(ctx, deployment.ScalingSpec) - if diags.HasError() { - return diags - } - } else { - data.ScalingStatus = types.ObjectNull(schemas.ScalingStatusAttributeTypes()) - data.ScalingSpec = types.ObjectNull(schemas.ScalingSpecAttributeTypes()) + data.ScalingStatus, diags = ScalingStatusTypesObject(ctx, deployment.ScalingStatus) + if diags.HasError() { + return diags + } + data.ScalingSpec, diags = ScalingSpecTypesObject(ctx, deployment.ScalingSpec) + if diags.HasError() { + return diags } return nil @@ -351,23 +341,13 @@ func (data *DeploymentDataSource) ReadFromResponse( data.SchedulerSize = types.StringPointerValue((*string)(deployment.SchedulerSize)) data.IsDevelopmentMode = types.BoolPointerValue(deployment.IsDevelopmentMode) data.IsHighAvailability = types.BoolPointerValue(deployment.IsHighAvailability) - - // Currently, the scaling status and spec are only available in development mode - // However, there is a bug in the API where the scaling status and spec are returned even if the deployment is not in development mode for updated deployments - // This is a workaround to handle the bug until the API is fixed - // Issue here: https://github.com/astronomer/astro/issues/21073 - if deployment.IsDevelopmentMode != nil && *deployment.IsDevelopmentMode { - data.ScalingStatus, diags = ScalingStatusTypesObject(ctx, deployment.ScalingStatus) - if diags.HasError() { - return diags - } - data.ScalingSpec, diags = ScalingSpecTypesObject(ctx, deployment.ScalingSpec) - if diags.HasError() { - return diags - } - } else { - data.ScalingStatus = types.ObjectNull(schemas.ScalingStatusAttributeTypes()) - data.ScalingSpec = types.ObjectNull(schemas.ScalingSpecAttributeTypes()) + data.ScalingStatus, diags = ScalingStatusTypesObject(ctx, deployment.ScalingStatus) + if diags.HasError() { + return diags + } + data.ScalingSpec, diags = ScalingSpecTypesObject(ctx, deployment.ScalingSpec) + if diags.HasError() { + return diags } return nil @@ -459,11 +439,11 @@ func WorkerQueueDataSourceTypesObject( } type DeploymentScalingSpec struct { - HibernationSpec HibernationSpec `tfsdk:"hibernation_spec"` + HibernationSpec types.Object `tfsdk:"hibernation_spec"` } type DeploymentStatus struct { - HibernationStatus HibernationStatus `tfsdk:"hibernation_status"` + HibernationStatus types.Object `tfsdk:"hibernation_status"` } type HibernationStatus struct { @@ -474,8 +454,8 @@ type HibernationStatus struct { } type HibernationSpec struct { - Override HibernationSpecOverride `tfsdk:"override"` - Schedules []HibernationSchedule `tfsdk:"schedules"` + Override types.Object `tfsdk:"override"` + Schedules types.Set `tfsdk:"schedules"` } type HibernationSpecOverride struct { @@ -491,54 +471,108 @@ type HibernationSchedule struct { WakeAtCron types.String `tfsdk:"wake_at_cron"` } +func HibernationStatusTypesObject( + ctx context.Context, + hibernationStatus *platform.DeploymentHibernationStatus, +) (types.Object, diag.Diagnostics) { + if hibernationStatus == nil { + return types.ObjectNull(schemas.HibernationStatusAttributeTypes()), nil + } + + obj := HibernationStatus{ + IsHibernating: types.BoolValue(hibernationStatus.IsHibernating), + NextEventType: types.StringPointerValue((*string)(hibernationStatus.NextEventType)), + NextEventAt: types.StringPointerValue(hibernationStatus.NextEventAt), + Reason: types.StringPointerValue(hibernationStatus.Reason), + } + return types.ObjectValueFrom(ctx, schemas.HibernationStatusAttributeTypes(), obj) +} + +func HibernationOverrideTypesObject( + ctx context.Context, + hibernationOverride *platform.DeploymentHibernationOverride, +) (types.Object, diag.Diagnostics) { + if hibernationOverride == nil { + return types.ObjectNull(schemas.HibernationOverrideAttributeTypes()), nil + } + obj := HibernationSpecOverride{ + IsHibernating: types.BoolPointerValue(hibernationOverride.IsHibernating), + IsActive: types.BoolPointerValue(hibernationOverride.IsActive), + } + if hibernationOverride.OverrideUntil != nil { + obj.OverrideUntil = types.StringValue(hibernationOverride.OverrideUntil.Format(time.RFC3339)) + } + return types.ObjectValueFrom(ctx, schemas.HibernationOverrideAttributeTypes(), obj) +} + +func HibernationScheduleTypesObject( + ctx context.Context, + schedule platform.DeploymentHibernationSchedule, +) (types.Object, diag.Diagnostics) { + obj := HibernationSchedule{ + Description: types.StringPointerValue(schedule.Description), + HibernateAtCron: types.StringValue(schedule.HibernateAtCron), + IsEnabled: types.BoolValue(schedule.IsEnabled), + WakeAtCron: types.StringValue(schedule.WakeAtCron), + } + return types.ObjectValueFrom(ctx, schemas.HibernationScheduleAttributeTypes(), obj) +} + +func HibernationSpecTypesObject( + ctx context.Context, + hibernationSpec *platform.DeploymentHibernationSpec, +) (types.Object, diag.Diagnostics) { + if hibernationSpec == nil || (hibernationSpec.Override == nil && hibernationSpec.Schedules == nil) { + return types.ObjectNull(schemas.HibernationSpecAttributeTypes()), nil + } + + override, diags := HibernationOverrideTypesObject(ctx, hibernationSpec.Override) + if diags.HasError() { + return types.ObjectNull(schemas.HibernationSpecAttributeTypes()), diags + } + schedules, diags := utils.ObjectSet(ctx, hibernationSpec.Schedules, schemas.HibernationScheduleAttributeTypes(), HibernationScheduleTypesObject) + if diags.HasError() { + return types.ObjectNull(schemas.HibernationSpecAttributeTypes()), diags + } + obj := HibernationSpec{ + Override: override, + Schedules: schedules, + } + return types.ObjectValueFrom(ctx, schemas.HibernationSpecAttributeTypes(), obj) +} + func ScalingStatusTypesObject( ctx context.Context, scalingStatus *platform.DeploymentScalingStatus, ) (types.Object, diag.Diagnostics) { - if scalingStatus != nil && scalingStatus.HibernationStatus != nil { - obj := DeploymentStatus{ - HibernationStatus: HibernationStatus{ - IsHibernating: types.BoolValue(scalingStatus.HibernationStatus.IsHibernating), - NextEventType: types.StringPointerValue((*string)(scalingStatus.HibernationStatus.NextEventType)), - NextEventAt: types.StringPointerValue(scalingStatus.HibernationStatus.NextEventAt), - Reason: types.StringPointerValue(scalingStatus.HibernationStatus.Reason), - }, - } - return types.ObjectValueFrom(ctx, schemas.ScalingStatusAttributeTypes(), obj) + if scalingStatus == nil { + return types.ObjectNull(schemas.ScalingStatusAttributeTypes()), nil + } + + hibernationStatus, diags := HibernationStatusTypesObject(ctx, scalingStatus.HibernationStatus) + if diags.HasError() { + return types.ObjectNull(schemas.ScalingStatusAttributeTypes()), diags } - return types.ObjectNull(schemas.ScalingStatusAttributeTypes()), nil + obj := DeploymentStatus{ + HibernationStatus: hibernationStatus, + } + return types.ObjectValueFrom(ctx, schemas.ScalingStatusAttributeTypes(), obj) } func ScalingSpecTypesObject( ctx context.Context, scalingSpec *platform.DeploymentScalingSpec, ) (types.Object, diag.Diagnostics) { - if scalingSpec != nil && scalingSpec.HibernationSpec != nil && (scalingSpec.HibernationSpec.Override != nil || scalingSpec.HibernationSpec.Schedules != nil) { - obj := DeploymentScalingSpec{ - HibernationSpec: HibernationSpec{}, - } - if scalingSpec.HibernationSpec.Override != nil { - obj.HibernationSpec.Override = HibernationSpecOverride{ - IsHibernating: types.BoolPointerValue(scalingSpec.HibernationSpec.Override.IsHibernating), - IsActive: types.BoolPointerValue(scalingSpec.HibernationSpec.Override.IsActive), - } - if scalingSpec.HibernationSpec.Override.OverrideUntil != nil { - obj.HibernationSpec.Override.OverrideUntil = types.StringValue(scalingSpec.HibernationSpec.Override.OverrideUntil.Format(time.RFC3339)) - } - } - if scalingSpec.HibernationSpec.Schedules != nil { - schedules := make([]HibernationSchedule, 0, len(*scalingSpec.HibernationSpec.Schedules)) - for _, schedule := range *scalingSpec.HibernationSpec.Schedules { - schedules = append(schedules, HibernationSchedule{ - Description: types.StringPointerValue(schedule.Description), - HibernateAtCron: types.StringValue(schedule.HibernateAtCron), - IsEnabled: types.BoolValue(schedule.IsEnabled), - WakeAtCron: types.StringValue(schedule.WakeAtCron), - }) - } - obj.HibernationSpec.Schedules = schedules - } - return types.ObjectValueFrom(ctx, schemas.ScalingSpecAttributeTypes(), obj) + if scalingSpec == nil { + return types.ObjectNull(schemas.ScalingSpecAttributeTypes()), nil + } + + hibernationSpec, diags := HibernationSpecTypesObject(ctx, scalingSpec.HibernationSpec) + if diags.HasError() { + return types.ObjectNull(schemas.ScalingSpecAttributeTypes()), diags + } + obj := DeploymentScalingSpec{ + HibernationSpec: hibernationSpec, } - return types.ObjectNull(schemas.ScalingSpecAttributeTypes()), nil + return types.ObjectValueFrom(ctx, schemas.ScalingSpecAttributeTypes(), obj) } diff --git a/internal/provider/resources/resource_cluster.go b/internal/provider/resources/resource_cluster.go index 0295b7d6..6bace7e2 100644 --- a/internal/provider/resources/resource_cluster.go +++ b/internal/provider/resources/resource_cluster.go @@ -89,7 +89,6 @@ func (r *ClusterResource) Create( return } - var diags diag.Diagnostics var createClusterRequest platform.CreateClusterRequest switch platform.ClusterCloudProvider(data.CloudProvider.ValueString()) { @@ -221,7 +220,6 @@ func (r *ClusterResource) Create( Refresh: r.resourceRefreshFunc(ctx, cluster.JSON200.Id), Timeout: 3 * time.Hour, MinTimeout: 1 * time.Minute, - Delay: 5 * time.Minute, } // readyCluster is the final state of the cluster after it has reached a target status @@ -375,7 +373,6 @@ func (r *ClusterResource) Update( Refresh: r.resourceRefreshFunc(ctx, cluster.JSON200.Id), Timeout: 3 * time.Hour, MinTimeout: 1 * time.Minute, - Delay: 5 * time.Minute, } // readyCluster is the final state of the cluster after it has reached a target status @@ -447,7 +444,6 @@ func (r *ClusterResource) Delete( Refresh: r.resourceRefreshFunc(ctx, data.Id.ValueString()), Timeout: 1 * time.Hour, MinTimeout: 30 * time.Second, - Delay: 1 * time.Minute, } _, err = stateConf.WaitForStateContext(ctx) @@ -493,7 +489,7 @@ func (r *ClusterResource) ValidateConfig( } func validateAwsConfig(ctx context.Context, data *models.ClusterResource) diag.Diagnostics { - var diags diag.Diagnostics + diags := make(diag.Diagnostics, 0) // Unallowed values if !data.TenantId.IsNull() { @@ -524,7 +520,7 @@ func validateAwsConfig(ctx context.Context, data *models.ClusterResource) diag.D } func validateAzureConfig(ctx context.Context, data *models.ClusterResource) diag.Diagnostics { - var diags diag.Diagnostics + diags := make(diag.Diagnostics, 0) // Unallowed values if !data.ServicePeeringRange.IsNull() { @@ -549,7 +545,7 @@ func validateAzureConfig(ctx context.Context, data *models.ClusterResource) diag } func validateGcpConfig(ctx context.Context, data *models.ClusterResource) diag.Diagnostics { - var diags diag.Diagnostics + diags := make(diag.Diagnostics, 0) // required values if data.ServicePeeringRange.IsNull() { @@ -600,6 +596,18 @@ func (r *ClusterResource) resourceRefreshFunc(ctx context.Context, clusterId str if diagnostic != nil { return nil, "", fmt.Errorf("error getting cluster %s", diagnostic.Detail()) } - return cluster.JSON200, string(cluster.JSON200.Status), nil + if cluster != nil && cluster.JSON200 != nil { + switch cluster.JSON200.Status { + case platform.ClusterStatusCREATED: + return cluster.JSON200, string(cluster.JSON200.Status), nil + case platform.ClusterStatusUPDATEFAILED, platform.ClusterStatusCREATEFAILED: + return cluster.JSON200, string(cluster.JSON200.Status), fmt.Errorf("cluster mutation failed for cluster '%v'", cluster.JSON200.Id) + case platform.ClusterStatusCREATING, platform.ClusterStatusUPDATING: + return cluster.JSON200, string(cluster.JSON200.Status), nil + default: + return cluster.JSON200, string(cluster.JSON200.Status), fmt.Errorf("unexpected cluster status '%v' for cluster '%v'", cluster.JSON200.Status, cluster.JSON200.Id) + } + } + return nil, "", fmt.Errorf("error getting cluster %s", clusterId) } } diff --git a/internal/provider/resources/resource_cluster_test.go b/internal/provider/resources/resource_cluster_test.go index f9e33575..17b5bd8b 100644 --- a/internal/provider/resources/resource_cluster_test.go +++ b/internal/provider/resources/resource_cluster_test.go @@ -26,6 +26,7 @@ const SKIP_CLUSTER_RESOURCE_TESTS = "SKIP_CLUSTER_RESOURCE_TESTS" const SKIP_CLUSTER_RESOURCE_TESTS_REASON = "Skipping dedicated cluster (and dedicated deployment) resource tests. To run these tests, unset the SKIP_CLUSTER_RESOURCE_TESTS environment variable." func TestAcc_ResourceClusterAwsWithDedicatedDeployments(t *testing.T) { + t.Skip("AWS cluster creation is currently not working on dev") if os.Getenv(SKIP_CLUSTER_RESOURCE_TESTS) == "True" { t.Skip(SKIP_CLUSTER_RESOURCE_TESTS_REASON) } diff --git a/internal/provider/resources/resource_deployment.go b/internal/provider/resources/resource_deployment.go index 25360b98..91e13761 100644 --- a/internal/provider/resources/resource_deployment.go +++ b/internal/provider/resources/resource_deployment.go @@ -382,7 +382,7 @@ func (r *DeploymentResource) Update( } // update request - var diags diag.Diagnostics + diags := make(diag.Diagnostics, 0) var updateDeploymentRequest platform.UpdateDeploymentRequest var envVars []platform.DeploymentEnvironmentVariableRequest @@ -674,7 +674,7 @@ func (r *DeploymentResource) ValidateConfig( } func validateHybridConfig(ctx context.Context, data *models.DeploymentResource) diag.Diagnostics { - var diags diag.Diagnostics + diags := make(diag.Diagnostics, 0) // Required hybrid values if data.SchedulerAu.IsNull() { diags.AddError( @@ -770,7 +770,7 @@ func validateHybridConfig(ctx context.Context, data *models.DeploymentResource) } func validateStandardConfig(ctx context.Context, data *models.DeploymentResource) diag.Diagnostics { - var diags diag.Diagnostics + diags := make(diag.Diagnostics, 0) // Required standard values if data.Region.IsNull() { diags.AddError( @@ -797,7 +797,7 @@ func validateStandardConfig(ctx context.Context, data *models.DeploymentResource func validateHostedConfig(ctx context.Context, data *models.DeploymentResource) diag.Diagnostics { // Required hosted values - var diags diag.Diagnostics + diags := make(diag.Diagnostics, 0) if data.SchedulerSize.IsNull() { diags.AddError( "scheduler_size is required for 'STANDARD' and 'DEDICATED' deployment", @@ -877,6 +877,37 @@ func validateHostedConfig(ctx context.Context, data *models.DeploymentResource) ) } + // Need to check that scaling_spec has either override or schedules + if !data.ScalingSpec.IsNull() { + var scalingSpec models.DeploymentScalingSpec + diags = append(diags, data.ScalingSpec.As(ctx, &scalingSpec, basetypes.ObjectAsOptions{ + UnhandledNullAsEmpty: true, + UnhandledUnknownAsEmpty: true, + })...) + if diags.HasError() { + tflog.Error(ctx, "failed to convert scaling spec", map[string]interface{}{"error": diags}) + return diags + } + + // scalingSpec.HibernationSpec is required if ScalingSpec is set via schemas/deployment.go + var hibernationSpec models.HibernationSpec + diags = scalingSpec.HibernationSpec.As(ctx, &hibernationSpec, basetypes.ObjectAsOptions{ + UnhandledNullAsEmpty: true, + UnhandledUnknownAsEmpty: true, + }) + if diags.HasError() { + tflog.Error(ctx, "failed to convert hibernation spec", map[string]interface{}{"error": diags}) + return diags + } + if hibernationSpec.Override.IsNull() && hibernationSpec.Schedules.IsNull() { + diags.AddError( + "scaling_spec (hibernation) must have either override or schedules", + "Please provide either override or schedules in 'scaling_spec.hibernation_spec'", + ) + return diags + } + } + // Need to check worker_queues for hosted deployments have `astro_machine` and do not have `node_pool_id` if len(data.WorkerQueues.Elements()) > 0 { var workerQueues []models.WorkerQueueResource @@ -901,7 +932,7 @@ func validateHostedConfig(ctx context.Context, data *models.DeploymentResource) } func validateClusterIdConfig(ctx context.Context, data *models.DeploymentResource) diag.Diagnostics { - var diags diag.Diagnostics + diags := make(diag.Diagnostics, 0) // Required clusterId value if data.ClusterId.IsNull() { diags.AddError( @@ -929,27 +960,63 @@ func validateClusterIdConfig(ctx context.Context, data *models.DeploymentResourc // RequestScalingSpec converts a Terraform object to a platform.DeploymentScalingSpecRequest to be used in create and update requests func RequestScalingSpec(ctx context.Context, scalingSpecObj types.Object) (*platform.DeploymentScalingSpecRequest, diag.Diagnostics) { if scalingSpecObj.IsNull() { + // If the scaling spec is not set, return nil for the request return nil, nil } - var scalingSpec models.DeploymentScalingSpec diags := scalingSpecObj.As(ctx, &scalingSpec, basetypes.ObjectAsOptions{ - UnhandledNullAsEmpty: false, - UnhandledUnknownAsEmpty: false, + UnhandledNullAsEmpty: true, + UnhandledUnknownAsEmpty: true, }) if diags.HasError() { + tflog.Error(ctx, "failed to convert scaling spec", map[string]interface{}{"error": diags}) return nil, diags } - platformScalingSpec := &platform.DeploymentScalingSpecRequest{ - HibernationSpec: &platform.DeploymentHibernationSpecRequest{ - Override: &platform.DeploymentHibernationOverrideRequest{ - IsHibernating: scalingSpec.HibernationSpec.Override.IsHibernating.ValueBoolPointer(), - OverrideUntil: scalingSpec.HibernationSpec.Override.OverrideUntil.ValueStringPointer(), - }, - }, + + platformScalingSpec := &platform.DeploymentScalingSpecRequest{} + if scalingSpec.HibernationSpec.IsNull() { + // If the hibernation spec is not set, return a scaling spec without hibernation spec for the request + platformScalingSpec.HibernationSpec = nil + return platformScalingSpec, nil + } + var hibernationSpec models.HibernationSpec + diags = scalingSpec.HibernationSpec.As(ctx, &hibernationSpec, basetypes.ObjectAsOptions{ + UnhandledNullAsEmpty: true, + UnhandledUnknownAsEmpty: true, + }) + if diags.HasError() { + tflog.Error(ctx, "failed to convert hibernation spec", map[string]interface{}{"error": diags}) + return nil, diags + } + platformScalingSpec.HibernationSpec = &platform.DeploymentHibernationSpecRequest{} + + if hibernationSpec.Override.IsNull() && hibernationSpec.Schedules.IsNull() { + // If the hibernation spec is set but both override and schedules are not set, return an empty hibernation spec for the request + return platformScalingSpec, nil } - if len(scalingSpec.HibernationSpec.Schedules) > 0 { - schedules := lo.Map(scalingSpec.HibernationSpec.Schedules, func(schedule models.HibernationSchedule, _ int) platform.DeploymentHibernationSchedule { + if !hibernationSpec.Override.IsNull() { + var override models.HibernationSpecOverride + diags = hibernationSpec.Override.As(ctx, &override, basetypes.ObjectAsOptions{ + UnhandledNullAsEmpty: true, + UnhandledUnknownAsEmpty: true, + }) + if diags.HasError() { + tflog.Error(ctx, "failed to convert hibernation override", map[string]interface{}{"error": diags}) + return nil, diags + } + platformScalingSpec.HibernationSpec.Override = &platform.DeploymentHibernationOverrideRequest{ + IsHibernating: override.IsHibernating.ValueBoolPointer(), + OverrideUntil: override.OverrideUntil.ValueStringPointer(), + } + } + if !hibernationSpec.Schedules.IsNull() { + var schedules []models.HibernationSchedule + diags = hibernationSpec.Schedules.ElementsAs(ctx, &schedules, false) + if diags.HasError() { + tflog.Error(ctx, "failed to convert hibernation schedules", map[string]interface{}{"error": diags}) + return nil, diags + } + requestSchedules := lo.Map(schedules, func(schedule models.HibernationSchedule, _ int) platform.DeploymentHibernationSchedule { return platform.DeploymentHibernationSchedule{ Description: schedule.Description.ValueStringPointer(), HibernateAtCron: schedule.HibernateAtCron.ValueString(), @@ -957,8 +1024,9 @@ func RequestScalingSpec(ctx context.Context, scalingSpecObj types.Object) (*plat WakeAtCron: schedule.WakeAtCron.ValueString(), } }) - platformScalingSpec.HibernationSpec.Schedules = &schedules + platformScalingSpec.HibernationSpec.Schedules = &requestSchedules } + return platformScalingSpec, nil } diff --git a/internal/provider/resources/resource_deployment_test.go b/internal/provider/resources/resource_deployment_test.go index 73b981ae..0a7de918 100644 --- a/internal/provider/resources/resource_deployment_test.go +++ b/internal/provider/resources/resource_deployment_test.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "os" + "regexp" "strings" "testing" @@ -282,7 +283,7 @@ func TestAcc_ResourceDeploymentStandard(t *testing.T) { ResourceName: azureCeleryResourceVar, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"external_ips", "oidc_issuer_url", "environment_variables.1.value"}, // environment_variables.0.value is a secret value + ImportStateVerifyIgnore: []string{"external_ips", "oidc_issuer_url", "scaling_status", "environment_variables.1.value"}, // environment_variables.0.value is a secret value }, }, }) @@ -300,8 +301,8 @@ func TestAcc_ResourceDeploymentStandard(t *testing.T) { Config: astronomerprovider.ProviderConfig(t, true) + standardDeployment(standardDeploymentInput{ Name: gcpKubernetesDeploymentName, Description: utils.TestResourceDescription, - Region: "westus2", - CloudProvider: "AZURE", + Region: "us-east4", + CloudProvider: "GCP", Executor: "KUBERNETES", SchedulerSize: "SMALL", IncludeEnvironmentVariables: true, @@ -309,8 +310,8 @@ func TestAcc_ResourceDeploymentStandard(t *testing.T) { Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr(gcpKubernetesResourceVar, "name", gcpKubernetesDeploymentName), resource.TestCheckResourceAttr(gcpKubernetesResourceVar, "description", utils.TestResourceDescription), - resource.TestCheckResourceAttr(gcpKubernetesResourceVar, "region", "westus2"), - resource.TestCheckResourceAttr(gcpKubernetesResourceVar, "cloud_provider", "AZURE"), + resource.TestCheckResourceAttr(gcpKubernetesResourceVar, "region", "us-east4"), + resource.TestCheckResourceAttr(gcpKubernetesResourceVar, "cloud_provider", "GCP"), resource.TestCheckResourceAttr(gcpKubernetesResourceVar, "executor", "KUBERNETES"), resource.TestCheckResourceAttr(gcpKubernetesResourceVar, "worker_queues.#", "0"), resource.TestCheckResourceAttr(gcpKubernetesResourceVar, "scheduler_size", "SMALL"), @@ -324,7 +325,149 @@ func TestAcc_ResourceDeploymentStandard(t *testing.T) { ResourceName: gcpKubernetesResourceVar, ImportState: true, ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"external_ips", "oidc_issuer_url", "environment_variables.1.value"}, // environment_variables.0.value is a secret value + ImportStateVerifyIgnore: []string{"external_ips", "oidc_issuer_url", "scaling_status", "environment_variables.1.value"}, // environment_variables.0.value is a secret value + }, + }, + }) +} + +func TestAcc_ResourceDeploymentStandardScalingSpec(t *testing.T) { + namePrefix := utils.GenerateTestResourceName(10) + + scalingSpecDeploymentName := fmt.Sprintf("%v_scaling_spec", namePrefix) + scalingSpecResourceVar := fmt.Sprintf("astro_deployment.%v", scalingSpecDeploymentName) + + // standard aws deployment + resource.Test(t, resource.TestCase{ + ProtoV6ProviderFactories: astronomerprovider.TestAccProtoV6ProviderFactories, + PreCheck: func() { astronomerprovider.TestAccPreCheck(t) }, + Steps: []resource.TestStep{ + { + Config: astronomerprovider.ProviderConfig(t, true) + developmentDeployment(scalingSpecDeploymentName, + `scaling_spec = {}`, + ), + ExpectError: regexp.MustCompile(`Inappropriate value for attribute "scaling_spec"`), + }, + { + Config: astronomerprovider.ProviderConfig(t, true) + developmentDeployment(scalingSpecDeploymentName, + `scaling_spec = { + hibernation_spec = {} + }`), + ExpectError: regexp.MustCompile(`scaling_spec \(hibernation\) must have either override or schedules`), + }, + { + Config: astronomerprovider.ProviderConfig(t, true) + developmentDeployment(scalingSpecDeploymentName, + ` + scaling_spec = { + hibernation_spec = { + override = {} + } + }`), + ExpectError: regexp.MustCompile(`Inappropriate value for attribute "scaling_spec"`), + }, + { + Config: astronomerprovider.ProviderConfig(t, true) + developmentDeployment(scalingSpecDeploymentName, + `scaling_spec = { + hibernation_spec = { + override = { + override_until = "2075-01-01T00:00:00Z" + } + } + }`), + ExpectError: regexp.MustCompile(`Inappropriate value for attribute "scaling_spec"`), + }, + { + Config: astronomerprovider.ProviderConfig(t, true) + developmentDeployment(scalingSpecDeploymentName, + `scaling_spec = { + hibernation_spec = { + schedules = [] + } + }`), + ExpectError: regexp.MustCompile(`Attribute scaling_spec.hibernation_spec.schedules set must contain at least 1`), // schedules must have at least one element + }, + { + Config: astronomerprovider.ProviderConfig(t, true) + developmentDeployment(scalingSpecDeploymentName, ` `), // no scaling spec should be allowed, + Check: resource.ComposeTestCheckFunc( + resource.TestCheckNoResourceAttr(scalingSpecResourceVar, "scaling_spec"), + ), + }, + { + Config: astronomerprovider.ProviderConfig(t, true) + developmentDeployment(scalingSpecDeploymentName, + `scaling_spec = { + hibernation_spec = { + schedules = [{ + hibernate_at_cron = "1 * * * *" + is_enabled = true + wake_at_cron = "59 * * * *" + }] + } + }`), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr(scalingSpecResourceVar, "scaling_spec.hibernation_spec.schedules.0.hibernate_at_cron", "1 * * * *"), + resource.TestCheckResourceAttr(scalingSpecResourceVar, "scaling_spec.hibernation_spec.schedules.0.is_enabled", "true"), + resource.TestCheckResourceAttr(scalingSpecResourceVar, "scaling_spec.hibernation_spec.schedules.0.wake_at_cron", "59 * * * *"), + resource.TestCheckNoResourceAttr(scalingSpecResourceVar, "scaling_spec.hibernation_spec.override"), + ), + }, + { + Config: astronomerprovider.ProviderConfig(t, true) + developmentDeployment(scalingSpecDeploymentName, + `scaling_spec = { + hibernation_spec = { + override = { + is_hibernating = true + } + } + }`), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr(scalingSpecResourceVar, "scaling_spec.hibernation_spec.override.is_hibernating", "true"), + resource.TestCheckNoResourceAttr(scalingSpecResourceVar, "scaling_spec.hibernation_spec.schedules"), + ), + }, + { + Config: astronomerprovider.ProviderConfig(t, true) + developmentDeployment(scalingSpecDeploymentName, + `scaling_spec = { + hibernation_spec = { + override = { + is_hibernating = true + override_until = "2075-01-01T00:00:00Z" + } + } + }`), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr(scalingSpecResourceVar, "scaling_spec.hibernation_spec.override.is_hibernating", "true"), + resource.TestCheckResourceAttr(scalingSpecResourceVar, "scaling_spec.hibernation_spec.override.override_until", "2075-01-01T00:00:00Z"), + resource.TestCheckNoResourceAttr(scalingSpecResourceVar, "scaling_spec.hibernation_spec.schedules"), + ), + }, + { + Config: astronomerprovider.ProviderConfig(t, true) + developmentDeployment(scalingSpecDeploymentName, + `scaling_spec = { + hibernation_spec = { + schedules = [{ + hibernate_at_cron = "1 * * * *" + is_enabled = true + wake_at_cron = "59 * * * *" + }], + override = { + is_hibernating = true + override_until = "2075-01-01T00:00:00Z" + } + } + }`), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr(scalingSpecResourceVar, "scaling_spec.hibernation_spec.override.is_hibernating", "true"), + resource.TestCheckResourceAttr(scalingSpecResourceVar, "scaling_spec.hibernation_spec.override.override_until", "2075-01-01T00:00:00Z"), + resource.TestCheckResourceAttr(scalingSpecResourceVar, "scaling_spec.hibernation_spec.schedules.0.hibernate_at_cron", "1 * * * *"), + resource.TestCheckResourceAttr(scalingSpecResourceVar, "scaling_spec.hibernation_spec.schedules.0.is_enabled", "true"), + resource.TestCheckResourceAttr(scalingSpecResourceVar, "scaling_spec.hibernation_spec.schedules.0.wake_at_cron", "59 * * * *"), + ), + }, + // Import existing deployment and check it is correctly imported - https://stackoverflow.com/questions/68824711/how-can-i-test-terraform-import-in-acceptance-tests + { + ResourceName: scalingSpecResourceVar, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"external_ips"}, }, }, }) @@ -460,6 +603,19 @@ resource "astro_deployment" "%v" { envVarsStr(input.IncludeEnvironmentVariables), wqStr, taskPodNodePoolIdStr) } +func developmentDeployment(scalingSpecDeploymentName, scalingSpec string) string { + return standardDeployment(standardDeploymentInput{ + Name: scalingSpecDeploymentName, + Description: utils.TestResourceDescription, + Region: "us-east4", + CloudProvider: "GCP", + Executor: "CELERY", + SchedulerSize: "SMALL", + IsDevelopmentMode: true, + ScalingSpec: scalingSpec, + }) +} + type standardDeploymentInput struct { Name string Description string @@ -469,6 +625,7 @@ type standardDeploymentInput struct { IncludeEnvironmentVariables bool SchedulerSize string IsDevelopmentMode bool + ScalingSpec string } func standardDeployment(input standardDeploymentInput) string { @@ -478,20 +635,25 @@ func standardDeployment(input standardDeploymentInput) string { } var scalingSpecStr string - if input.IsDevelopmentMode == true { - scalingSpecStr = `scaling_spec = { - hibernation_spec = { - schedules = [{ - hibernate_at_cron = "1 * * * *" - is_enabled = true - wake_at_cron = "59 * * * *" - }] - override = { - is_hibernating = true - override_until = "2030-04-25T12:58:00+05:30" - } - } - }` + if input.IsDevelopmentMode { + if input.ScalingSpec == "" { + scalingSpecStr = ` + scaling_spec = { + hibernation_spec = { + schedules = [{ + hibernate_at_cron = "1 * * * *" + is_enabled = true + wake_at_cron = "59 * * * *" + }] + override = { + is_hibernating = true + override_until = "2075-04-25T12:58:00+05:30" + } + } + }` + } else { + scalingSpecStr = input.ScalingSpec + } } return fmt.Sprintf(` resource "astro_workspace" "%v_workspace" { diff --git a/internal/provider/schemas/scaling.go b/internal/provider/schemas/scaling.go index b38fe087..7d421451 100644 --- a/internal/provider/schemas/scaling.go +++ b/internal/provider/schemas/scaling.go @@ -1,7 +1,6 @@ package schemas import ( - "github.com/hashicorp/terraform-plugin-framework-validators/boolvalidator" "github.com/hashicorp/terraform-plugin-framework-validators/setvalidator" "github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator" "github.com/hashicorp/terraform-plugin-framework/attr" @@ -99,8 +98,9 @@ func HibernationScheduleDataSourceSchemaAttributes() map[string]datasourceSchema func ScalingSpecResourceSchemaAttributes() map[string]resourceSchema.Attribute { return map[string]resourceSchema.Attribute{ "hibernation_spec": resourceSchema.SingleNestedAttribute{ - Attributes: HibernationSpecResourceSchemaAttributes(), - Optional: true, + Attributes: HibernationSpecResourceSchemaAttributes(), + Required: true, + MarkdownDescription: "Hibernation configuration for the deployment. The deployment will hibernate according to the schedules defined in this configuration. To remove the hibernation configuration, set scaling_spec to null.", }, } } @@ -108,15 +108,18 @@ func ScalingSpecResourceSchemaAttributes() map[string]resourceSchema.Attribute { func HibernationSpecResourceSchemaAttributes() map[string]resourceSchema.Attribute { return map[string]resourceSchema.Attribute{ "override": resourceSchema.SingleNestedAttribute{ - Attributes: HibernationOverrideResourceSchemaAttributes(), - Optional: true, + MarkdownDescription: "Hibernation override configuration. Set to null to remove the override.", + Attributes: HibernationOverrideResourceSchemaAttributes(), + Optional: true, }, "schedules": resourceSchema.SetNestedAttribute{ + MarkdownDescription: "List of hibernation schedules. Set to null to remove all schedules.", NestedObject: resourceSchema.NestedAttributeObject{ Attributes: HibernationScheduleResourceSchemaAttributes(), }, Validators: []validator.Set{ setvalidator.SizeAtMost(10), + setvalidator.SizeAtLeast(1), }, Optional: true, }, @@ -129,13 +132,13 @@ func HibernationOverrideResourceSchemaAttributes() map[string]resourceSchema.Att Computed: true, }, "is_hibernating": resourceSchema.BoolAttribute{ - Optional: true, - Validators: []validator.Bool{ - boolvalidator.AlsoRequires(path.MatchRelative().AtParent().AtName("override_until")), - }, + Required: true, }, "override_until": resourceSchema.StringAttribute{ Optional: true, + Validators: []validator.String{ + stringvalidator.AlsoRequires(path.MatchRelative().AtParent().AtName("is_hibernating")), + }, }, } }