From 96013f38eece151a498d68dcf1280ed27042f08f Mon Sep 17 00:00:00 2001 From: Igor Kliushnikov <122813874+igorkliushnikov@users.noreply.github.com> Date: Wed, 14 Feb 2024 22:12:28 +0100 Subject: [PATCH] provider: support for the postgresql_flexible_server.restart_server_on_configuration_value_change property (#23811) * Add PostgresqlFlexibleServer to features block * Corrected Subscription feature test case name * postgres-flex-server: make restarts on config changes optional; add docs * Rewrite checks functions to reduce repetitive code * postgres-flex-configuration: corrected tf-code formatting * psql-flex-server-config: corrected formatting in test file with terrafmt --- internal/features/defaults.go | 3 + internal/features/user_flags.go | 29 +++-- internal/provider/features.go | 25 +++++ internal/provider/features_test.go | 86 +++++++++++++- ..._flexible_server_configuration_resource.go | 20 ++-- ...ible_server_configuration_resource_test.go | 106 ++++++++++++++++-- .../docs/guides/features-block.html.markdown | 10 ++ ...lexible_server_configuration.html.markdown | 2 + 8 files changed, 248 insertions(+), 33 deletions(-) diff --git a/internal/features/defaults.go b/internal/features/defaults.go index 133d6a01a8871..e2e3bc34ec901 100644 --- a/internal/features/defaults.go +++ b/internal/features/defaults.go @@ -56,5 +56,8 @@ func Default() UserFeatures { Subscription: SubscriptionFeatures{ PreventCancellationOnDestroy: false, }, + PostgresqlFlexibleServer: PostgresqlFlexibleServerFeatures{ + RestartServerOnConfigurationValueChange: true, + }, } } diff --git a/internal/features/user_flags.go b/internal/features/user_flags.go index a0b1cfe9f4481..00da953c43881 100644 --- a/internal/features/user_flags.go +++ b/internal/features/user_flags.go @@ -4,18 +4,19 @@ package features type UserFeatures struct { - ApiManagement ApiManagementFeatures - AppConfiguration AppConfigurationFeatures - ApplicationInsights ApplicationInsightFeatures - CognitiveAccount CognitiveAccountFeatures - VirtualMachine VirtualMachineFeatures - VirtualMachineScaleSet VirtualMachineScaleSetFeatures - KeyVault KeyVaultFeatures - TemplateDeployment TemplateDeploymentFeatures - LogAnalyticsWorkspace LogAnalyticsWorkspaceFeatures - ResourceGroup ResourceGroupFeatures - ManagedDisk ManagedDiskFeatures - Subscription SubscriptionFeatures + ApiManagement ApiManagementFeatures + AppConfiguration AppConfigurationFeatures + ApplicationInsights ApplicationInsightFeatures + CognitiveAccount CognitiveAccountFeatures + VirtualMachine VirtualMachineFeatures + VirtualMachineScaleSet VirtualMachineScaleSetFeatures + KeyVault KeyVaultFeatures + TemplateDeployment TemplateDeploymentFeatures + LogAnalyticsWorkspace LogAnalyticsWorkspaceFeatures + ResourceGroup ResourceGroupFeatures + ManagedDisk ManagedDiskFeatures + Subscription SubscriptionFeatures + PostgresqlFlexibleServer PostgresqlFlexibleServerFeatures } type CognitiveAccountFeatures struct { @@ -79,3 +80,7 @@ type AppConfigurationFeatures struct { type SubscriptionFeatures struct { PreventCancellationOnDestroy bool } + +type PostgresqlFlexibleServerFeatures struct { + RestartServerOnConfigurationValueChange bool +} diff --git a/internal/provider/features.go b/internal/provider/features.go index c6a416af55fb4..d2d3cc96d11cf 100644 --- a/internal/provider/features.go +++ b/internal/provider/features.go @@ -284,6 +284,21 @@ func schemaFeatures(supportLegacyTestSuite bool) *pluginsdk.Schema { }, }, }, + + "postgresql_flexible_server": { + Type: pluginsdk.TypeList, + Optional: true, + MaxItems: 1, + Elem: &pluginsdk.Resource{ + Schema: map[string]*pluginsdk.Schema{ + "restart_server_on_configuration_value_change": { + Type: pluginsdk.TypeBool, + Optional: true, + Default: true, + }, + }, + }, + }, } // this is a temporary hack to enable us to gradually add provider blocks to test configurations @@ -481,5 +496,15 @@ func expandFeatures(input []interface{}) features.UserFeatures { } } + if raw, ok := val["postgresql_flexible_server"]; ok { + items := raw.([]interface{}) + if len(items) > 0 { + subscriptionRaw := items[0].(map[string]interface{}) + if v, ok := subscriptionRaw["restart_server_on_configuration_value_change"]; ok { + featuresMap.PostgresqlFlexibleServer.RestartServerOnConfigurationValueChange = v.(bool) + } + } + } + return featuresMap } diff --git a/internal/provider/features_test.go b/internal/provider/features_test.go index 2e35abf8a3850..384da683c8d9a 100644 --- a/internal/provider/features_test.go +++ b/internal/provider/features_test.go @@ -71,6 +71,9 @@ func TestExpandFeatures(t *testing.T) { Subscription: features.SubscriptionFeatures{ PreventCancellationOnDestroy: false, }, + PostgresqlFlexibleServer: features.PostgresqlFlexibleServerFeatures{ + RestartServerOnConfigurationValueChange: true, + }, }, }, { @@ -122,6 +125,11 @@ func TestExpandFeatures(t *testing.T) { "expand_without_downtime": true, }, }, + "postgresql_flexible_server": []interface{}{ + map[string]interface{}{ + "restart_server_on_configuration_value_change": true, + }, + }, "resource_group": []interface{}{ map[string]interface{}{ "prevent_deletion_if_contains_resources": true, @@ -204,6 +212,9 @@ func TestExpandFeatures(t *testing.T) { ForceDelete: true, ScaleToZeroOnDelete: true, }, + PostgresqlFlexibleServer: features.PostgresqlFlexibleServerFeatures{ + RestartServerOnConfigurationValueChange: true, + }, }, }, { @@ -255,6 +266,11 @@ func TestExpandFeatures(t *testing.T) { "expand_without_downtime": false, }, }, + "postgresql_flexible_server": []interface{}{ + map[string]interface{}{ + "restart_server_on_configuration_value_change": false, + }, + }, "resource_group": []interface{}{ map[string]interface{}{ "prevent_deletion_if_contains_resources": false, @@ -337,6 +353,9 @@ func TestExpandFeatures(t *testing.T) { RollInstancesWhenRequired: false, ScaleToZeroOnDelete: false, }, + PostgresqlFlexibleServer: features.PostgresqlFlexibleServerFeatures{ + RestartServerOnConfigurationValueChange: false, + }, }, }, } @@ -1237,7 +1256,7 @@ func TestExpandFeaturesSubscription(t *testing.T) { }, }, { - Name: "No Downtime Resize Enabled", + Name: "Subscription cancellation on destroy is Disabled", Input: []interface{}{ map[string]interface{}{ "subscription": []interface{}{ @@ -1263,3 +1282,68 @@ func TestExpandFeaturesSubscription(t *testing.T) { } } } + +func TestExpandFeaturesPosgresqlFlexibleServer(t *testing.T) { + testData := []struct { + Name string + Input []interface{} + EnvVars map[string]interface{} + Expected features.UserFeatures + }{ + { + Name: "Empty Block", + Input: []interface{}{ + map[string]interface{}{ + "postgresql_flexible_server": []interface{}{}, + }, + }, + Expected: features.UserFeatures{ + PostgresqlFlexibleServer: features.PostgresqlFlexibleServerFeatures{ + RestartServerOnConfigurationValueChange: true, + }, + }, + }, + { + Name: "Postgresql Flexible Server restarts on configuration change is Enabled", + Input: []interface{}{ + map[string]interface{}{ + "postgresql_flexible_server": []interface{}{ + map[string]interface{}{ + "restart_server_on_configuration_value_change": true, + }, + }, + }, + }, + Expected: features.UserFeatures{ + PostgresqlFlexibleServer: features.PostgresqlFlexibleServerFeatures{ + RestartServerOnConfigurationValueChange: true, + }, + }, + }, + { + Name: "Postgresql Flexible Server restarts on configuration change is Disabled", + Input: []interface{}{ + map[string]interface{}{ + "postgresql_flexible_server": []interface{}{ + map[string]interface{}{ + "restart_server_on_configuration_value_change": false, + }, + }, + }, + }, + Expected: features.UserFeatures{ + PostgresqlFlexibleServer: features.PostgresqlFlexibleServerFeatures{ + RestartServerOnConfigurationValueChange: false, + }, + }, + }, + } + + for _, testCase := range testData { + t.Logf("[DEBUG] Test Case: %q", testCase.Name) + result := expandFeatures(testCase.Input) + if !reflect.DeepEqual(result.Subscription, testCase.Expected.Subscription) { + t.Fatalf("Expected %+v but got %+v", result.Subscription, testCase.Expected.Subscription) + } + } +} diff --git a/internal/services/postgres/postgresql_flexible_server_configuration_resource.go b/internal/services/postgres/postgresql_flexible_server_configuration_resource.go index ccd23946ce61d..799ec8f208020 100644 --- a/internal/services/postgres/postgresql_flexible_server_configuration_resource.go +++ b/internal/services/postgres/postgresql_flexible_server_configuration_resource.go @@ -100,11 +100,13 @@ func resourceFlexibleServerConfigurationUpdate(d *pluginsdk.ResourceData, meta i if isDynamicConfig := props.IsDynamicConfig; isDynamicConfig != nil && !*isDynamicConfig { if isReadOnly := props.IsReadOnly; isReadOnly != nil && !*isReadOnly { - restartClient := meta.(*clients.Client).Postgres.ServerRestartClient - restartServerId := serverrestart.NewFlexibleServerID(id.SubscriptionId, id.ResourceGroupName, id.FlexibleServerName) + if meta.(*clients.Client).Features.PostgresqlFlexibleServer.RestartServerOnConfigurationValueChange { + restartClient := meta.(*clients.Client).Postgres.ServerRestartClient + restartServerId := serverrestart.NewFlexibleServerID(id.SubscriptionId, id.ResourceGroupName, id.FlexibleServerName) - if err = restartClient.ServersRestartThenPoll(ctx, restartServerId, serverrestart.RestartParameter{}); err != nil { - return fmt.Errorf("restarting server %s: %+v", id, err) + if err = restartClient.ServersRestartThenPoll(ctx, restartServerId, serverrestart.RestartParameter{}); err != nil { + return fmt.Errorf("restarting server %s: %+v", id, err) + } } } } @@ -186,11 +188,13 @@ func resourceFlexibleServerConfigurationDelete(d *pluginsdk.ResourceData, meta i if isDynamicConfig := props.IsDynamicConfig; isDynamicConfig != nil && !*isDynamicConfig { if isReadOnly := props.IsReadOnly; isReadOnly != nil && !*isReadOnly { - restartClient := meta.(*clients.Client).Postgres.ServerRestartClient - restartServerId := serverrestart.NewFlexibleServerID(id.SubscriptionId, id.ResourceGroupName, id.FlexibleServerName) + if meta.(*clients.Client).Features.PostgresqlFlexibleServer.RestartServerOnConfigurationValueChange { + restartClient := meta.(*clients.Client).Postgres.ServerRestartClient + restartServerId := serverrestart.NewFlexibleServerID(id.SubscriptionId, id.ResourceGroupName, id.FlexibleServerName) - if err = restartClient.ServersRestartThenPoll(ctx, restartServerId, serverrestart.RestartParameter{}); err != nil { - return fmt.Errorf("restarting server %s: %+v", id, err) + if err = restartClient.ServersRestartThenPoll(ctx, restartServerId, serverrestart.RestartParameter{}); err != nil { + return fmt.Errorf("restarting server %s: %+v", id, err) + } } } } diff --git a/internal/services/postgres/postgresql_flexible_server_configuration_resource_test.go b/internal/services/postgres/postgresql_flexible_server_configuration_resource_test.go index 375b85af5fbf4..d46927729d4ab 100644 --- a/internal/services/postgres/postgresql_flexible_server_configuration_resource_test.go +++ b/internal/services/postgres/postgresql_flexible_server_configuration_resource_test.go @@ -37,7 +37,7 @@ func TestAccFlexibleServerConfiguration_backslashQuote(t *testing.T) { { Config: r.template(data), Check: acceptance.ComposeTestCheckFunc( - data.CheckWithClientForResource(r.checkReset(name), "azurerm_postgresql_flexible_server.test"), + data.CheckWithClientForResource(r.checkWith(name, resetToDefaultCheck), "azurerm_postgresql_flexible_server.test"), ), }, }) @@ -60,7 +60,7 @@ func TestAccFlexibleServerConfiguration_azureExtensions(t *testing.T) { { Config: r.template(data), Check: acceptance.ComposeTestCheckFunc( - data.CheckWithClientForResource(r.checkReset(name), "azurerm_postgresql_flexible_server.test"), + data.CheckWithClientForResource(r.checkWith(name, resetToDefaultCheck), "azurerm_postgresql_flexible_server.test"), ), }, }) @@ -83,7 +83,7 @@ func TestAccFlexibleServerConfiguration_pgbouncerEnabled(t *testing.T) { { Config: r.template(data), Check: acceptance.ComposeTestCheckFunc( - data.CheckWithClientForResource(r.checkReset(name), "azurerm_postgresql_flexible_server.test"), + data.CheckWithClientForResource(r.checkWith(name, resetToDefaultCheck), "azurerm_postgresql_flexible_server.test"), ), }, }) @@ -114,7 +114,7 @@ func TestAccFlexibleServerConfiguration_updateApplicationName(t *testing.T) { }) } -func (r PostgresqlFlexibleServerConfigurationResource) checkReset(configurationName string) acceptance.ClientCheckFunc { +func (r PostgresqlFlexibleServerConfigurationResource) checkWith(configurationName string, checker func(*configurations.ConfigurationProperties) error) acceptance.ClientCheckFunc { return func(ctx context.Context, clients *clients.Client, state *pluginsdk.InstanceState) error { id, err := configurations.ParseFlexibleServerID(state.ID) if err != nil { @@ -139,13 +139,8 @@ func (r PostgresqlFlexibleServerConfigurationResource) checkReset(configurationN if model := resp.Model; model != nil { if props := model.Properties; props != nil { - if props.Value != nil && props.DefaultValue != nil { - actualValue := *props.Value - defaultValue := *props.DefaultValue - - if defaultValue != actualValue { - return fmt.Errorf("Azure Postgresql Flexible Server configuration wasn't set to the default value. Expected '%s' - got '%s': \n%+v", defaultValue, actualValue, resp) - } + if err = checker(props); err != nil { + return fmt.Errorf("%s: \n%+v", err, resp) } } } @@ -154,6 +149,31 @@ func (r PostgresqlFlexibleServerConfigurationResource) checkReset(configurationN } } +func resetToDefaultCheck(props *configurations.ConfigurationProperties) error { + if props.Value != nil && props.DefaultValue != nil { + actualValue := *props.Value + defaultValue := *props.DefaultValue + + if defaultValue != actualValue { + return fmt.Errorf("Azure Postgresql Flexible Server configuration wasn't set to the default value. Expected '%s' - got '%s'", defaultValue, actualValue) + } + } + return nil +} + +func pendingRestartCheck(expectedValue bool) func(*configurations.ConfigurationProperties) error { + return func(props *configurations.ConfigurationProperties) error { + if props.Value != nil && props.IsConfigPendingRestart != nil { + actualValue := *props.IsConfigPendingRestart + + if actualValue != expectedValue { + return fmt.Errorf("Azure Postgresql Flexible Server configuration IsConfigPendingRestart is expected'%v', but got ='%v'", expectedValue, actualValue) + } + } + return nil + } +} + func TestAccFlexibleServerConfiguration_multiplePostgresqlFlexibleServerConfigurations(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_postgresql_flexible_server_configuration", "test") r := PostgresqlFlexibleServerConfigurationResource{} @@ -180,13 +200,40 @@ func TestAccFlexibleServerConfiguration_restartServerForStaticParameters(t *test check.That(data.ResourceName).ExistsInAzure(r), check.That(data.ResourceName).Key("name").HasValue(name), check.That(data.ResourceName).Key("value").HasValue("5"), + // by default "static" parameter change triggers server restart + data.CheckWithClientForResource(r.checkWith(name, pendingRestartCheck(false)), "azurerm_postgresql_flexible_server.test"), ), }, data.ImportStep(), { Config: r.template(data), Check: acceptance.ComposeTestCheckFunc( - data.CheckWithClientForResource(r.checkReset(name), "azurerm_postgresql_flexible_server.test"), + data.CheckWithClientForResource(r.checkWith(name, resetToDefaultCheck), "azurerm_postgresql_flexible_server.test"), + ), + }, + }) +} + +func TestAccFlexibleServerConfiguration_doesNotRestartServer_whenFeatureIsDisabled(t *testing.T) { + data := acceptance.BuildTestData(t, "azurerm_postgresql_flexible_server_configuration", "test") + r := PostgresqlFlexibleServerConfigurationResource{} + name := "cron.max_running_jobs" + data.ResourceTest(t, r, []acceptance.TestStep{ + { + Config: r.withDisabledServerRestarts(data, name, "5"), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + check.That(data.ResourceName).Key("name").HasValue(name), + check.That(data.ResourceName).Key("value").HasValue("5"), + // when the feature is disabled, "static" parameter change does not trigger server restart and config stays in "pending-restart" state + data.CheckWithClientForResource(r.checkWith(name, pendingRestartCheck(true)), "azurerm_postgresql_flexible_server.test"), + ), + }, + data.ImportStep(), + { + Config: r.template(data), + Check: acceptance.ComposeTestCheckFunc( + data.CheckWithClientForResource(r.checkWith(name, resetToDefaultCheck), "azurerm_postgresql_flexible_server.test"), ), }, }) @@ -264,3 +311,38 @@ resource "azurerm_postgresql_flexible_server_configuration" "test5" { } `, PostgresqlFlexibleServerResource{}.complete(data)) } + +func (r PostgresqlFlexibleServerConfigurationResource) withDisabledServerRestarts(data acceptance.TestData, name, value string) string { + return fmt.Sprintf(` +provider "azurerm" { + features { + postgresql_flexible_server { + restart_server_on_configuration_value_change = false + } + } +} + +resource "azurerm_resource_group" "test" { + name = "acctestRG-postgresql-%d" + location = "%s" +} + +resource "azurerm_postgresql_flexible_server" "test" { + name = "acctest-fs-%d" + resource_group_name = azurerm_resource_group.test.name + location = azurerm_resource_group.test.location + administrator_login = "adminTerraform" + administrator_password = "QAZwsx123" + storage_mb = 32768 + version = "12" + sku_name = "GP_Standard_D2s_v3" + zone = "2" +} + +resource "azurerm_postgresql_flexible_server_configuration" "test" { + name = "%s" + server_id = azurerm_postgresql_flexible_server.test.id + value = "%s" +} +`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, name, value) +} diff --git a/website/docs/guides/features-block.html.markdown b/website/docs/guides/features-block.html.markdown index 36ace635190a5..dc3a92bddb363 100644 --- a/website/docs/guides/features-block.html.markdown +++ b/website/docs/guides/features-block.html.markdown @@ -58,6 +58,10 @@ provider "azurerm" { expand_without_downtime = true } + postgresql_flexible_server { + restart_server_on_configuration_value_change = true + } + resource_group { prevent_deletion_if_contains_resources = true } @@ -183,6 +187,12 @@ The `managed_disk` block supports the following: --- +The `postgresql_flexible_server` block supports the following: + +* `restart_server_on_configuration_value_change` - (Optional) Should the `postgresql_flexible_server` restart after static server parameter change or removal? Defaults to `true`. + +--- + The `resource_group` block supports the following: * `prevent_deletion_if_contains_resources` - (Optional) Should the `azurerm_resource_group` resource check that there are no Resources within the Resource Group during deletion? This means that all Resources within the Resource Group must be deleted prior to deleting the Resource Group. Defaults to `true`. diff --git a/website/docs/r/postgresql_flexible_server_configuration.html.markdown b/website/docs/r/postgresql_flexible_server_configuration.html.markdown index 229b9aa9dd527..7a74414a0cfca 100644 --- a/website/docs/r/postgresql_flexible_server_configuration.html.markdown +++ b/website/docs/r/postgresql_flexible_server_configuration.html.markdown @@ -10,6 +10,8 @@ description: |- Sets a PostgreSQL Configuration value on a Azure PostgreSQL Flexible Server. +~> **Note:** Changes to static server parameters will automatically trigger Azure Flex Server restart. This behavior can be disabled in the provider `features` block by setting the `restart_server_on_configuration_value_change` field to `false` within the `postgresql_flexible_server` block. + ## Example Usage ```hcl