Skip to content

Commit

Permalink
feat: Warehouse redesign part1 (#2864)
Browse files Browse the repository at this point in the history
First part o warehouse redesign:
- warehouse sizes fixed
- defaults removed
- additional conditional logic removed
- parameters fixed
- state upgrader added (not for all yet)
- show output an parameters output added
- additional logic for not working unsets added

Common:
- added multiple planchecks
- added import checks
- added snowflake checks
- proposed how we can deal with SF defaults with optional (non-computed)
attributes; we will decide between the two approaches soon

What's missing:
- datasource
- state upgraders for all fields
- some acceptance and integration tests
- possibility to suspend warehouse before update
  • Loading branch information
sfc-gh-asawicki authored Jun 12, 2024
1 parent 269df6b commit 6664457
Show file tree
Hide file tree
Showing 37 changed files with 2,866 additions and 668 deletions.
53 changes: 42 additions & 11 deletions MIGRATION_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,7 @@ This document is meant to help you migrate your Terraform config to the new newe
describe deprecations or breaking changes and help you to change your configuration to keep the same (or similar) behavior
across different versions.

## v0.91.0 ➞ v0.92.0
### snowflake_database new alternatives
As part of the [preparation for v1](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/ROADMAP.md#preparing-essential-ga-objects-for-the-provider-v1), we split up the database resource into multiple ones:
- Standard database (in progress)
- Shared database - can be used as `snowflake_shared_database` (used to create databases from externally defined shares)
- Secondary database - can be used as `snowflake_secondary_database` (used to create replicas of databases from external sources)
From now on, please migrate and use the new database resources for their unique use cases. For more information, see the documentation for those resources on the [Terraform Registry](https://registry.terraform.io/providers/Snowflake-Labs/snowflake/latest/docs).

The split was done (and will be done for several objects during the refactor) to simplify the resource on maintainability and usage level.
Its purpose was also to divide the resources by their specific purpose rather than cramping every use case of an object into one resource.

## v0.92.0 ➞ v0.93.0
### snowflake_scim_integration resource changes
#### *(behavior change)* Renamed fields

Expand All @@ -29,6 +19,47 @@ Force new was added for the following attributes (because no usable SQL alter st
- `scim_client`
- `run_as_role`

### snowflake_warehouse resource changes
#### *(potential behavior change)* Default values removed
As part of the [redesign](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/ROADMAP.md#preparing-essential-ga-objects-for-the-provider-v1) we are removing the default values for attributes having their defaults on Snowflake side to reduce coupling with the provider. Because of that the following defaults were removed:
- `comment`
- `statement_timeout_in_seconds`
- `statement_queued_timeout_in_seconds`
- `max_concurrency_level`
- `enable_query_acceleration`
- `query_acceleration_max_scale_factor`
- `warehouse_type`

All previous defaults were aligned with the current Snowflake ones, however:

[//]: # (TODO [SNOW-1348102 - next PR]: state migrator?)
- if the given parameter was changed on the account level, terraform will try to update it

[//]: # (- TODO [SNOW-1348102 - next PR]: describe the new state approach if decided)

#### *(behavior change)* Validation changes
As part of the [redesign](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/ROADMAP.md#preparing-essential-ga-objects-for-the-provider-v1) we are adjusting validations or removing them to reduce coupling between Snowflake and the provider. Because of that the following validations were removed/adjusted/added:
- `max_cluster_count` - adjusted: added higher bound (10) according to Snowflake docs
- `min_cluster_count` - adjusted: added higher bound (10) according to Snowflake docs
- `auto_suspend` - adjusted: added `0` as valid value
- `warehouse_size` - adjusted: removed incorrect `2XLARGE`, `3XLARGE`, `4XLARGE`, `5XLARGE`, `6XLARGE` values
- `resource_monitor` - added: validation for a valid identifier (still subject to change during [identifiers rework](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/ROADMAP.md#identifiers-rework))
- `max_concurrency_level` - added: validation according to MAX_CONCURRENCY_LEVEL parameter docs
- `statement_queued_timeout_in_seconds` - added: validation according to STATEMENT_QUEUED_TIMEOUT_IN_SECONDS parameter docs
- `statement_timeout_in_seconds` - added: validation according to STATEMENT_TIMEOUT_IN_SECONDS parameter docs

#### *(behavior change)* Deprecated `wait_for_provisioning` field removed
`wait_for_provisioning` field was deprecated a long time ago. It's high time it was removed from the schema.

#### *(behavior change)* `query_acceleration_max_scale_factor` conditional logic removed
Previously, the `query_acceleration_max_scale_factor` was depending on `enable_query_acceleration` parameter, but it is not required on Snowflake side. After migration, `terraform plan` should suggest changes if `enable_query_acceleration` was earlier set to false (manually or from default) and if `query_acceleration_max_scale_factor` was set in config.

#### *(behavior change)* Boolean type changes
To easily handle three-value logic (true, false, unknown) in provider's configs, type of `auto_resume` and `enable_query_acceleration` was changed from boolean to string. This should not require updating existing configs (boolean/int value should be accepted and state will be migrated to string automatically), however we recommend changing config values to strings. Terraform should perform an action for configs lacking `auto_resume` or `enable_query_acceleration` (`ALTER WAREHOUSE UNSET AUTO_RESUME` and/or `ALTER WAREHOUSE UNSET ENABLE_QUERY_ACCELERATION` will be run underneath which should not affect the Snowflake object, because `auto_resume` and `enable_query_acceleration` are false by default).

#### *(note)* `resource_monitor` validation and diff suppression
`resource_monitor` is an identifier and handling logic may be still slightly changed as part of https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/ROADMAP.md#identifiers-rework. It should be handled automatically (without needed manual actions on user side), though, but it is not guaranteed.

## v0.89.0 ➞ v0.90.0
### snowflake_table resource changes
#### *(behavior change)* Validation to column type added
Expand Down
98 changes: 89 additions & 9 deletions docs/resources/warehouse.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@
page_title: "snowflake_warehouse Resource - terraform-provider-snowflake"
subcategory: ""
description: |-
Resource used to manage warehouse objects. For more information, check warehouse documentation https://docs.snowflake.com/en/sql-reference/commands-warehouse.
---

# snowflake_warehouse (Resource)


Resource used to manage warehouse objects. For more information, check [warehouse documentation](https://docs.snowflake.com/en/sql-reference/commands-warehouse).

## Example Usage

Expand All @@ -28,26 +28,106 @@ resource "snowflake_warehouse" "warehouse" {

### Optional

- `auto_resume` (Boolean) Specifies whether to automatically resume a warehouse when a SQL statement (e.g. query) is submitted to it.
- `auto_resume` (String) Specifies whether to automatically resume a warehouse when a SQL statement (e.g. query) is submitted to it.
- `auto_suspend` (Number) Specifies the number of seconds of inactivity after which a warehouse is automatically suspended.
- `comment` (String)
- `enable_query_acceleration` (Boolean) Specifies whether to enable the query acceleration service for queries that rely on this warehouse for compute resources.
- `comment` (String) Specifies a comment for the warehouse.
- `enable_query_acceleration` (String) Specifies whether to enable the query acceleration service for queries that rely on this warehouse for compute resources.
- `initially_suspended` (Boolean) Specifies whether the warehouse is created initially in the ‘Suspended’ state.
- `max_cluster_count` (Number) Specifies the maximum number of server clusters for the warehouse.
- `max_concurrency_level` (Number) Object parameter that specifies the concurrency level for SQL statements (i.e. queries and DML) executed by a warehouse.
- `min_cluster_count` (Number) Specifies the minimum number of server clusters for the warehouse (only applies to multi-cluster warehouses).
- `query_acceleration_max_scale_factor` (Number) Specifies the maximum scale factor for leasing compute resources for query acceleration. The scale factor is used as a multiplier based on warehouse size.
- `resource_monitor` (String) Specifies the name of a resource monitor that is explicitly assigned to the warehouse.
- `scaling_policy` (String) Specifies the policy for automatically starting and shutting down clusters in a multi-cluster warehouse running in Auto-scale mode.
- `scaling_policy` (String) Specifies the policy for automatically starting and shutting down clusters in a multi-cluster warehouse running in Auto-scale mode. Valid values are (case-insensitive): `STANDARD` | `ECONOMY`.
- `statement_queued_timeout_in_seconds` (Number) Object parameter that specifies the time, in seconds, a SQL statement (query, DDL, DML, etc.) can be queued on a warehouse before it is canceled by the system.
- `statement_timeout_in_seconds` (Number) Specifies the time, in seconds, after which a running SQL statement (query, DDL, DML, etc.) is canceled by the system
- `wait_for_provisioning` (Boolean, Deprecated) Specifies whether the warehouse, after being resized, waits for all the servers to provision before executing any queued or new queries.
- `warehouse_size` (String) Specifies the size of the virtual warehouse. Larger warehouse sizes 5X-Large and 6X-Large are currently in preview and only available on Amazon Web Services (AWS).
- `warehouse_type` (String) Specifies a STANDARD or SNOWPARK-OPTIMIZED warehouse
- `warehouse_size` (String) Specifies the size of the virtual warehouse. Valid values are (case-insensitive): `XSMALL` | `X-SMALL` | `SMALL` | `MEDIUM` | `LARGE` | `XLARGE` | `X-LARGE` | `XXLARGE` | `X2LARGE` | `2X-LARGE` | `XXXLARGE` | `X3LARGE` | `3X-LARGE` | `X4LARGE` | `4X-LARGE` | `X5LARGE` | `5X-LARGE` | `X6LARGE` | `6X-LARGE`. Consult [warehouse documentation](https://docs.snowflake.com/en/sql-reference/sql/create-warehouse#optional-properties-objectproperties) for the details.
- `warehouse_type` (String) Specifies warehouse type. Valid values are (case-insensitive): `STANDARD` | `SNOWPARK-OPTIMIZED`.

### Read-Only

- `id` (String) The ID of this resource.
- `parameters` (List of Object) Outputs the result of `SHOW PARAMETERS IN WAREHOUSE` for the given warehouse. (see [below for nested schema](#nestedatt--parameters))
- `show_output` (List of Object) Outputs the result of `SHOW WAREHOUSE` for the given warehouse. (see [below for nested schema](#nestedatt--show_output))

<a id="nestedatt--parameters"></a>
### Nested Schema for `parameters`

Read-Only:

- `max_concurrency_level` (List of Object) (see [below for nested schema](#nestedobjatt--parameters--max_concurrency_level))
- `statement_queued_timeout_in_seconds` (List of Object) (see [below for nested schema](#nestedobjatt--parameters--statement_queued_timeout_in_seconds))
- `statement_timeout_in_seconds` (List of Object) (see [below for nested schema](#nestedobjatt--parameters--statement_timeout_in_seconds))

<a id="nestedobjatt--parameters--max_concurrency_level"></a>
### Nested Schema for `parameters.max_concurrency_level`

Read-Only:

- `default` (String)
- `description` (String)
- `key` (String)
- `level` (String)
- `value` (String)


<a id="nestedobjatt--parameters--statement_queued_timeout_in_seconds"></a>
### Nested Schema for `parameters.statement_queued_timeout_in_seconds`

Read-Only:

- `default` (String)
- `description` (String)
- `key` (String)
- `level` (String)
- `value` (String)


<a id="nestedobjatt--parameters--statement_timeout_in_seconds"></a>
### Nested Schema for `parameters.statement_timeout_in_seconds`

Read-Only:

- `default` (String)
- `description` (String)
- `key` (String)
- `level` (String)
- `value` (String)



<a id="nestedatt--show_output"></a>
### Nested Schema for `show_output`

Read-Only:

- `auto_resume` (Boolean)
- `auto_suspend` (Number)
- `available` (Number)
- `comment` (String)
- `created_on` (String)
- `enable_query_acceleration` (Boolean)
- `is_current` (Boolean)
- `is_default` (Boolean)
- `max_cluster_count` (Number)
- `min_cluster_count` (Number)
- `name` (String)
- `other` (Number)
- `owner` (String)
- `owner_role_type` (String)
- `provisioning` (Number)
- `query_acceleration_max_scale_factor` (Number)
- `queued` (Number)
- `quiescing` (Number)
- `resource_monitor` (String)
- `resumed_on` (String)
- `running` (Number)
- `scaling_policy` (String)
- `size` (String)
- `started_clusters` (Number)
- `state` (String)
- `type` (String)
- `updated_on` (String)

## Import

Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ require (
github.com/gookit/color v1.5.4
github.com/hashicorp/go-cty v1.4.1-0.20200414143053-d3edf31b6320
github.com/hashicorp/go-uuid v1.0.3
github.com/hashicorp/terraform-json v0.21.0
github.com/hashicorp/terraform-plugin-framework v1.8.0
github.com/hashicorp/terraform-plugin-framework-validators v0.12.0
github.com/hashicorp/terraform-plugin-go v0.22.2
Expand Down Expand Up @@ -84,7 +85,6 @@ require (
github.com/hashicorp/hcl/v2 v2.19.1 // indirect
github.com/hashicorp/logutils v1.0.0 // indirect
github.com/hashicorp/terraform-exec v0.20.0 // indirect
github.com/hashicorp/terraform-json v0.21.0 // indirect
github.com/hashicorp/terraform-registry-address v0.2.3 // indirect
github.com/hashicorp/terraform-svchost v0.1.1 // indirect
github.com/hashicorp/yamux v0.1.1 // indirect
Expand Down
34 changes: 29 additions & 5 deletions pkg/acceptance/helpers/parameter_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package helpers

import (
"context"
"fmt"
"testing"

"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk"
Expand All @@ -26,15 +27,38 @@ func (c *ParameterClient) UpdateAccountParameterTemporarily(t *testing.T, parame
t.Helper()
ctx := context.Background()

param, err := c.client().ShowAccountParameter(ctx, parameter)
require.NoError(t, err)
param := c.ShowAccountParameter(t, parameter)
oldValue := param.Value
oldLevel := param.Level

err = c.client().SetAccountParameter(ctx, parameter, newValue)
err := c.client().SetAccountParameter(ctx, parameter, newValue)
require.NoError(t, err)

return func() {
err = c.client().SetAccountParameter(ctx, parameter, oldValue)
require.NoError(t, err)
if oldLevel == "" {
c.UnsetAccountParameter(t, parameter)
} else {
err := c.client().SetAccountParameter(ctx, parameter, oldValue)
require.NoError(t, err)
}
}
}

func (c *ParameterClient) ShowAccountParameter(t *testing.T, parameter sdk.AccountParameter) *sdk.Parameter {
t.Helper()
ctx := context.Background()

param, err := c.client().ShowAccountParameter(ctx, parameter)
require.NoError(t, err)

return param
}

// TODO [SNOW-1473408]: add unset account parameter to sdk.Parameters
func (c *ParameterClient) UnsetAccountParameter(t *testing.T, parameter sdk.AccountParameter) {
t.Helper()
ctx := context.Background()

_, err := c.context.client.ExecForTests(ctx, fmt.Sprintf("ALTER ACCOUNT UNSET %s", parameter))
require.NoError(t, err)
}
45 changes: 45 additions & 0 deletions pkg/acceptance/helpers/warehouse_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,51 @@ func (c *WarehouseClient) UpdateMaxConcurrencyLevel(t *testing.T, id sdk.Account
require.NoError(t, err)
}

func (c *WarehouseClient) UpdateWarehouseSize(t *testing.T, id sdk.AccountObjectIdentifier, newSize sdk.WarehouseSize) {
t.Helper()

ctx := context.Background()

err := c.client().Alter(ctx, id, &sdk.AlterWarehouseOptions{Set: &sdk.WarehouseSet{WarehouseSize: sdk.Pointer(newSize)}})
require.NoError(t, err)
}

func (c *WarehouseClient) UpdateWarehouseType(t *testing.T, id sdk.AccountObjectIdentifier, newType sdk.WarehouseType) {
t.Helper()

ctx := context.Background()

err := c.client().Alter(ctx, id, &sdk.AlterWarehouseOptions{Set: &sdk.WarehouseSet{WarehouseType: sdk.Pointer(newType)}})
require.NoError(t, err)
}

func (c *WarehouseClient) UpdateStatementTimeoutInSeconds(t *testing.T, id sdk.AccountObjectIdentifier, newValue int) {
t.Helper()

ctx := context.Background()

err := c.client().Alter(ctx, id, &sdk.AlterWarehouseOptions{Set: &sdk.WarehouseSet{StatementTimeoutInSeconds: sdk.Int(newValue)}})
require.NoError(t, err)
}

func (c *WarehouseClient) UpdateAutoResume(t *testing.T, id sdk.AccountObjectIdentifier, newAutoResume bool) {
t.Helper()

ctx := context.Background()

err := c.client().Alter(ctx, id, &sdk.AlterWarehouseOptions{Set: &sdk.WarehouseSet{AutoResume: sdk.Pointer(newAutoResume)}})
require.NoError(t, err)
}

func (c *WarehouseClient) Suspend(t *testing.T, id sdk.AccountObjectIdentifier) {
t.Helper()

ctx := context.Background()

err := c.client().Alter(ctx, id, &sdk.AlterWarehouseOptions{Suspend: sdk.Bool(true)})
require.NoError(t, err)
}

func (c *WarehouseClient) Show(t *testing.T, id sdk.AccountObjectIdentifier) (*sdk.Warehouse, error) {
t.Helper()
ctx := context.Background()
Expand Down
41 changes: 41 additions & 0 deletions pkg/acceptance/importchecks/import_checks.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package importchecks

import (
"fmt"

"github.com/hashicorp/terraform-plugin-testing/helper/resource"
"github.com/hashicorp/terraform-plugin-testing/terraform"
)

// ComposeImportStateCheck is based on unexported composeImportStateCheck from teststep_providers_test.go
func ComposeImportStateCheck(fs ...resource.ImportStateCheckFunc) resource.ImportStateCheckFunc {
return func(s []*terraform.InstanceState) error {
for i, f := range fs {
if err := f(s); err != nil {
return fmt.Errorf("check %d/%d error: %w", i+1, len(fs), err)
}
}
return nil
}
}

// TestCheckResourceAttrInstanceState is based on unexported testCheckResourceAttrInstanceState from teststep_providers_test.go
func TestCheckResourceAttrInstanceState(id string, attributeName, attributeValue string) resource.ImportStateCheckFunc {
return func(is []*terraform.InstanceState) error {
for _, v := range is {
if v.ID != id {
continue
}

if attrVal, ok := v.Attributes[attributeName]; ok {
if attrVal != attributeValue {
return fmt.Errorf("expected: %s got: %s", attributeValue, attrVal)
}

return nil
}
}

return fmt.Errorf("attribute %s not found in instance state", attributeName)
}
}
Loading

0 comments on commit 6664457

Please sign in to comment.