From ecad51b5fbceae5df6b1d27604be343b0596f00a Mon Sep 17 00:00:00 2001 From: Murad Biashimov Date: Wed, 30 Oct 2024 09:48:32 +0100 Subject: [PATCH] feat: allow to purge additional_disk_space --- internal/schemautil/custom_diff.go | 68 ++++++++++++++++- internal/schemautil/helpers.go | 7 ++ internal/schemautil/service.go | 85 +++++++++------------- internal/sdkprovider/service/pg/pg_test.go | 2 +- 4 files changed, 109 insertions(+), 53 deletions(-) diff --git a/internal/schemautil/custom_diff.go b/internal/schemautil/custom_diff.go index 5f149295b..5f8b4055f 100644 --- a/internal/schemautil/custom_diff.go +++ b/internal/schemautil/custom_diff.go @@ -2,15 +2,20 @@ package schemautil import ( "context" + "errors" "fmt" "strings" "github.com/aiven/aiven-go-client/v2" + avngen "github.com/aiven/go-client-codegen" + "github.com/aiven/go-client-codegen/handler/service" "github.com/docker/go-units" "github.com/hashicorp/go-cty/cty" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "golang.org/x/exp/slices" + + "github.com/aiven/terraform-provider-aiven/internal/common" ) func CustomizeDiffGenericService(serviceType string) schema.CustomizeDiffFunc { @@ -27,7 +32,10 @@ func CustomizeDiffGenericService(serviceType string) schema.CustomizeDiffFunc { ), customdiff.IfValueChange("additional_disk_space", ShouldNotBeEmpty, - CustomizeDiffCheckDiskSpace, + customdiff.Sequence( + CustomizeDiffCheckDiskSpace, + CustomizeDiffAdditionalDiskSpace, + ), ), customdiff.IfValueChange("service_integrations", ShouldNotBeEmpty, @@ -254,3 +262,61 @@ func checkForMultipleValues(v cty.Value) error { return nil } + +var ErrAutoscalerConflict = errors.New("autoscaler integration is enabled, additional_disk_space cannot be set") + +// CustomizeDiffAdditionalDiskSpace +// 1. checks that additional_disk_space is not set if autoscaler is enabled +// 2. outputs a diff for a computed field, which otherwise would be suppressed when removed +func CustomizeDiffAdditionalDiskSpace(ctx context.Context, diff *schema.ResourceDiff, _ interface{}) error { + client, err := common.GenClient() + if err != nil { + return err + } + + s, err := client.ServiceGet(ctx, diff.Get("project").(string), diff.Get("service_name").(string)) + if avngen.IsNotFound(err) { + // The service does not exist, so we cannot check if autoscaler is enabled + return nil + } + + if err != nil { + return err + } + + isAutoscalerEnabled := false + for _, i := range s.ServiceIntegrations { + if i.IntegrationType == service.IntegrationTypeAutoscaler { + isAutoscalerEnabled = true + break + } + } + + k := "additional_disk_space" + + // There are three possible sources of additional_disk_space value: + // 1. It is explicitly set in config file + // 2. Computed: disk_space - plan.disk_space = additional_disk_space + // 3. Computed: autoscaler is enabled, so additional_disk_space is managed by the autoscaler + if HasConfigValue(diff, k) || HasConfigValue(diff, "disk_space") { + if isAutoscalerEnabled { + // Autoscaler is enabled, so we cannot set additional_disk_space + return ErrAutoscalerConflict + } + + // It is in the config file, lets TF handle it + return nil + } + + if isAutoscalerEnabled { + // If the autoscaler is enabled, we don't need to manage the field, + // it will change its value automatically + return nil + } + + // It is not set but has a value (ShouldNotBeEmpty proves it has). + // That means the value is being removed. + // We must output a diff for the computed field, + // which otherwise TF will suppress it + return diff.SetNew(k, "0B") +} diff --git a/internal/schemautil/helpers.go b/internal/schemautil/helpers.go index ed903351d..8bc93a794 100644 --- a/internal/schemautil/helpers.go +++ b/internal/schemautil/helpers.go @@ -8,15 +8,22 @@ import ( "github.com/aiven/aiven-go-client/v2" "github.com/aiven/go-client-codegen/handler/service" "github.com/docker/go-units" + "github.com/hashicorp/go-cty/cty" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" ) // ResourceStateOrResourceDiff either *schema.ResourceState or *schema.ResourceDiff type ResourceStateOrResourceDiff interface { + GetRawConfig() cty.Value GetOk(key string) (interface{}, bool) Get(key string) interface{} } +func HasConfigValue(d ResourceStateOrResourceDiff, key string) bool { + c := d.GetRawConfig() + return !(c.IsNull() || c.AsValueMap()[key].IsNull()) +} + // PlanParameters service plan aparameters type PlanParameters struct { DiskSizeMBDefault int diff --git a/internal/schemautil/service.go b/internal/schemautil/service.go index 8d043efb2..c528fdcc4 100644 --- a/internal/schemautil/service.go +++ b/internal/schemautil/service.go @@ -56,8 +56,6 @@ const ( ServiceTypeValkey = "valkey" ) -var ErrAutoscalerConflict = errors.New("autoscaler integration is enabled, additional_disk_space cannot be set") - var TechEmailsResourceSchema = &schema.Resource{ Schema: map[string]*schema.Schema{ "email": { @@ -474,13 +472,6 @@ func ResourceServiceUpdate(ctx context.Context, d *schema.ResourceData, m interf } } - // On service update, we send a default disc space value for a common - // if the TF user does not specify it - diskSpace, err := getDiskSpaceFromStateOrDiff(ctx, d, client) - if err != nil { - return diag.Errorf("error getting default disc space: %s", err) - } - projectName, serviceName, err := SplitResourceID2(d.Id()) if err != nil { return diag.Errorf("error splitting service id (%s): %s", d.Id(), err) @@ -516,26 +507,31 @@ func ResourceServiceUpdate(ctx context.Context, d *schema.ResourceData, m interf return diag.FromErr(err) } - // Note: Only service_integrations are needed here, but no specific API exists yet. - s, err := avnGen.ServiceGet(ctx, projectName, serviceName, service.ServiceGetIncludeSecrets(true)) - if err != nil { - return diag.Errorf("unable to GET service %s: %s", d.Id(), err) - } - - autoscalerIntegrations := getIntegrationsForTerraform(s.ServiceIntegrations, service.IntegrationTypeAutoscaler) - if _, ok := d.GetOk("additional_disk_space"); ok && len(autoscalerIntegrations) > 0 { - return diag.FromErr(ErrAutoscalerConflict) - } - cloud := d.Get("cloud_name").(string) plan := d.Get("plan").(string) powered := true terminationProtection := d.Get("termination_protection").(bool) + // Sends disk size only when there is no scaler enabled var diskSpaceMb *int - if diskSpace > 0 { - diskSpaceMb = &diskSpace + s, err := avnGen.ServiceGet(ctx, projectName, serviceName) + if err != nil { + return nil + } + + if len(getIntegrationsForTerraform(s.ServiceIntegrations, service.IntegrationTypeAutoscaler)) == 0 { + // On service update, we send a default disc space value for a common + // if the TF user does not specify it + diskSpace, err := getDiskSpaceFromStateOrDiff(ctx, d, client) + if err != nil { + return diag.Errorf("error getting default disc space: %s", err) + } + + if diskSpace > 0 { + diskSpaceMb = &diskSpace + } } + serviceUpdate := &service.ServiceUpdateIn{ Cloud: &cloud, Plan: &plan, @@ -578,11 +574,17 @@ func ResourceServiceUpdate(ctx context.Context, d *schema.ResourceData, m interf return ResourceServiceRead(ctx, d, m) } +// getDiskSpaceFromStateOrDiff three cases: +// 1. disk_space is set +// 2. plan disk space +// 3. plan disk space + additional_disk_space func getDiskSpaceFromStateOrDiff(ctx context.Context, d ResourceStateOrResourceDiff, client *aiven.Client) (int, error) { - var diskSpace int + if v, ok := d.GetOk("disk_space"); ok { + return ConvertToDiskSpaceMB(v.(string)), nil + } // Get service plan specific defaults - servicePlanParams, err := GetServicePlanParametersFromSchema(ctx, client, d) + plan, err := GetServicePlanParametersFromSchema(ctx, client, d) if err != nil { if aiven.IsNotFound(err) { return 0, nil @@ -590,15 +592,10 @@ func getDiskSpaceFromStateOrDiff(ctx context.Context, d ResourceStateOrResourceD return 0, fmt.Errorf("unable to get service plan parameters: %w", err) } - // Use `additional_disk_space` if set - if ads, ok := d.GetOk("additional_disk_space"); ok { - diskSpace = servicePlanParams.DiskSizeMBDefault + ConvertToDiskSpaceMB(ads.(string)) - } else if ds, ok := d.GetOk("disk_space"); ok { - // Use `disk_space` if set... - diskSpace = ConvertToDiskSpaceMB(ds.(string)) - } else { - // ... otherwise, use the default disk space - diskSpace = servicePlanParams.DiskSizeMBDefault + // Adds additional_disk_space only if it is in the config + diskSpace := plan.DiskSizeMBDefault + if HasConfigValue(d, "additional_disk_space") { + diskSpace += ConvertToDiskSpaceMB(d.Get("additional_disk_space").(string)) } return diskSpace, nil @@ -691,31 +688,17 @@ func copyServicePropertiesFromAPIResponseToTerraform( if s.DiskSpaceMb != nil { diskSpace = *s.DiskSpaceMb } - additionalDiskSpace := diskSpace - servicePlanParams.DiskSizeMBDefault - - _, isAdditionalDiskSpaceSet := d.GetOk("additional_disk_space") - _, isDiskSpaceSet := d.GetOk("disk_space") - // Handles two different cases: - // - // 1. During import when neither `additional_disk_space` nor `disk_space` are set - // 2. During create / update when `additional_disk_space` is set - if additionalDiskSpace > 0 && (!isDiskSpaceSet || isAdditionalDiskSpaceSet) { - if err := d.Set("additional_disk_space", HumanReadableByteSize(additionalDiskSpace*units.MiB)); err != nil { - return err - } - if err := d.Set("disk_space", nil); err != nil { - return err - } + additionalDiskSpace := diskSpace - servicePlanParams.DiskSizeMBDefault + if err := d.Set("additional_disk_space", HumanReadableByteSize(additionalDiskSpace*units.MiB)); err != nil { + return err } + _, isDiskSpaceSet := d.GetOk("disk_space") if isDiskSpaceSet && diskSpace > 0 { if err := d.Set("disk_space", HumanReadableByteSize(diskSpace*units.MiB)); err != nil { return err } - if err := d.Set("additional_disk_space", nil); err != nil { - return err - } } if err := d.Set("disk_space_used", HumanReadableByteSize(diskSpace*units.MiB)); err != nil { diff --git a/internal/sdkprovider/service/pg/pg_test.go b/internal/sdkprovider/service/pg/pg_test.go index 22eb37a56..13290a95b 100644 --- a/internal/sdkprovider/service/pg/pg_test.go +++ b/internal/sdkprovider/service/pg/pg_test.go @@ -265,7 +265,7 @@ func TestAccAivenPG_deleting_additional_disk_size(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "maintenance_window_dow", "monday"), resource.TestCheckResourceAttr(resourceName, "maintenance_window_time", "10:00:00"), resource.TestCheckResourceAttr(resourceName, "disk_space_default", "80GiB"), - resource.TestCheckResourceAttr(resourceName, "disk_space_used", "100GiB"), + resource.TestCheckResourceAttr(resourceName, "disk_space_used", "80GiB"), resource.TestCheckResourceAttr(resourceName, "termination_protection", "false"), ), },