From 642ff2ed83ce7cbc6fefe50e23b6fb282d29d602 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 --- .github/workflows/acceptance-tests.yml | 25 +-------- internal/schemautil/custom_diff.go | 65 +++++++++++++++++++++- internal/schemautil/schemautil.go | 10 ++++ internal/schemautil/service.go | 33 ++--------- internal/sdkprovider/service/pg/pg_test.go | 2 +- 5 files changed, 80 insertions(+), 55 deletions(-) diff --git a/.github/workflows/acceptance-tests.yml b/.github/workflows/acceptance-tests.yml index 0ec24d82e..523293166 100644 --- a/.github/workflows/acceptance-tests.yml +++ b/.github/workflows/acceptance-tests.yml @@ -40,30 +40,7 @@ jobs: fail-fast: false matrix: pkg: [ - kafka, - kafkatopic, - kafkaschema, - account, - cassandra, - clickhouse, - connectionpool, - flink, - pg, - grafana, - influxdb, - m3db, - mysql, - opensearch, - organization, - project, - redis, - servicecomponent, - staticip, - serviceintegration, - vpc, - dragonfly, - thanos, - valkey + pg ] steps: diff --git a/internal/schemautil/custom_diff.go b/internal/schemautil/custom_diff.go index 5f149295b..2e1344899 100644 --- a/internal/schemautil/custom_diff.go +++ b/internal/schemautil/custom_diff.go @@ -6,11 +6,15 @@ import ( "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 +31,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 +261,59 @@ func checkForMultipleValues(v cty.Value) error { return nil } + +// 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 HasAttribute(diff, k) || HasAttribute(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/schemautil.go b/internal/schemautil/schemautil.go index e82e50725..29044ff83 100644 --- a/internal/schemautil/schemautil.go +++ b/internal/schemautil/schemautil.go @@ -12,6 +12,7 @@ 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/helper/schema" ) @@ -511,3 +512,12 @@ func Remarshal(in, out any) error { } return json.Unmarshal(b, out) } + +type resourceData interface { + GetRawConfig() cty.Value +} + +func HasAttribute(d resourceData, key string) bool { + c := d.GetRawConfig() + return !c.IsNull() && c.AsValueMap()[key].IsNull() +} diff --git a/internal/schemautil/service.go b/internal/schemautil/service.go index 8d043efb2..6b1b65229 100644 --- a/internal/schemautil/service.go +++ b/internal/schemautil/service.go @@ -516,17 +516,6 @@ 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 @@ -691,31 +680,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"), ), },