Skip to content

Commit

Permalink
redis version on subscription and active_active subscription (#448)
Browse files Browse the repository at this point in the history 
* Added a test and some documentation

* Added some provider-side validation

* Spelling error
  • Loading branch information
@JohnSharpe
JohnSharpe authored Nov 20, 2023
1 parent e3e07c9 commit 3df50ae
Show file tree
Hide file tree
Showing 6 changed files with 156 additions and 130 deletions.
6 changes: 6 additions & 0 deletions docs/resources/rediscloud_active_active_subscription.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ resource "rediscloud_active_active_subscription" "subscription-resource" {
name = "subscription-name"
payment_method_id = data.rediscloud_payment_method.card.id
cloud_provider = "AWS"
redis_version = "latest"
creation_plan {
memory_limit_in_gb = 1
Expand Down Expand Up @@ -56,6 +57,7 @@ The following arguments are supported:
* `payment_method` (Optional) The payment method for the requested subscription, (either `credit-card` or `marketplace`). If `credit-card` is specified, `payment_method_id` must be defined. Default: 'credit-card'. **Modifying this attribute will force creation of a new resource.**
* `payment_method_id` - (Optional) A valid payment method pre-defined in the current account. This value is __Optional__ for AWS/GCP Marketplace accounts, but __Required__ for all other account types
* `cloud_provider` - (Optional) The cloud provider to use with the subscription, (either `AWS` or `GCP`). Default: ‘AWS’. **Modifying this attribute will force creation of a new resource.**
* `redis_version` - (Optional) Either 'default' or 'latest'. If specified, the Redis Version defines the cluster version. Default: 'default'. **Modifying this attribute will force creation of a new resource.**
* `creation_plan` - (Required) A creation plan object, documented below

The `creation_plan` block supports:
Expand Down Expand Up @@ -90,3 +92,7 @@ $ terraform import rediscloud_active_active_subscription.subscription-resource 1
```

~> **Note:** the creation_plan block will be ignored during imports.

~> **Note:** when importing an existing Subscription, upon providing a `redis_version`, Terraform will always try to
recreatethe resource. The API doesn't return this value, so we can't detect changes between states.

6 changes: 5 additions & 1 deletion docs/resources/rediscloud_subscription.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ resource "rediscloud_subscription" "subscription-resource" {
payment_method = "credit-card"
payment_method_id = data.rediscloud_payment_method.card.id
memory_storage = "ram"
redis_version = "latest"
cloud_provider {
provider = data.rediscloud_cloud_account.account.provider_type
Expand Down Expand Up @@ -62,6 +63,7 @@ The following arguments are supported:
* `payment_method` (Optional) The payment method for the requested subscription, (either `credit-card` or `marketplace`). If `credit-card` is specified, `payment_method_id` must be defined. Default: 'credit-card'. **Modifying this attribute will force creation of a new resource.**
* `payment_method_id` - (Optional) A valid payment method pre-defined in the current account. This value is __Optional__ for AWS/GCP Marketplace accounts, but __Required__ for all other account types
* `memory_storage` - (Optional) Memory storage preference: either ‘ram’ or a combination of ‘ram-and-flash’. Default: ‘ram’. **Modifying this attribute will force creation of a new resource.**
* `redis_version` - (Optional) Either 'default' or 'latest'. If specified, the Redis Version defines the cluster version. Default: 'default'. **Modifying this attribute will force creation of a new resource.**
* `allowlist` - (Optional) An allowlist object, documented below
* `cloud_provider` - (Required) A cloud provider object, documented below. **Modifying this attribute will force creation of a new resource.**
* `creation_plan` - (Required) A creation plan object, documented below
Expand Down Expand Up @@ -139,5 +141,7 @@ The `networks` block has these attributes:
```
$ terraform import rediscloud_subscription.subscription-resource 12345678
```

~> **Note:** the creation_plan block will be ignored during imports.

~> **Note:** when importing an existing Subscription, upon providing a `redis_version`, Terraform will always try to
recreate the resource. The API doesn't return this value, so we can't detect changes between states.
128 changes: 0 additions & 128 deletions provider/resource_rediscloud_acl_role_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,90 +96,6 @@ func TestAccCreateReadUpdateImportDeleteAclRole_Flexible(t *testing.T) {
})
}

// Adds a considerable time overhead without giving us any useful information
//func TestAccCreateReadUpdateImportDeleteAclRole_ActiveActive(t *testing.T) {
//
// prefix := acctest.RandomWithPrefix(testResourcePrefix)
// exampleSubscriptionName := prefix + "-subscription"
// exampleDatabaseName := prefix + "-database"
// exampleDatabasePassword := prefix + "aA.1"
//
// testRoleName := prefix + "-test-role"
//
// testCreateTerraform := fmt.Sprintf(testAccResourceRedisCloudActiveActiveSubscriptionDatabase, exampleSubscriptionName, exampleDatabaseName, exampleDatabasePassword) +
// fmt.Sprintf(testAADatabaseRole, testRoleName)
//
// testUpdateTerraform := fmt.Sprintf(testAccResourceRedisCloudActiveActiveSubscriptionDatabase, exampleSubscriptionName, exampleDatabaseName, exampleDatabasePassword) +
// fmt.Sprintf(testAADatabaseRole, testRoleName+"-updated")
//
// resource.ParallelTest(t, resource.TestCase{
// PreCheck: func() { testAccPreCheck(t) },
// ProviderFactories: providerFactories,
// CheckDestroy: testAccCheckAclRoleDestroy,
// Steps: []resource.TestStep{
// // Test role creation including association with AA database
// {
// Config: testCreateTerraform,
// Check: resource.ComposeTestCheckFunc(
// resource.TestCheckResourceAttr("rediscloud_acl_role.test", "name", testRoleName),
// resource.TestCheckResourceAttr("rediscloud_acl_role.test", "rule.#", "1"),
// resource.TestCheckResourceAttr("rediscloud_acl_role.test", "rule.0.name", "Read-Only"),
// resource.TestCheckResourceAttr("rediscloud_acl_role.test", "rule.0.database.#", "1"),
// resource.TestMatchResourceAttr("rediscloud_acl_role.test", "rule.0.database.0.subscription", regexp.MustCompile("^\\d*$")),
// resource.TestMatchResourceAttr("rediscloud_acl_role.test", "rule.0.database.0.database", regexp.MustCompile("^\\d*$")),
// resource.TestCheckResourceAttr("rediscloud_acl_role.test", "rule.0.database.0.regions.#", "2"),
// resource.TestCheckTypeSetElemAttr("rediscloud_acl_role.test", "rule.0.database.0.regions.*", "us-east-1"),
// resource.TestCheckTypeSetElemAttr("rediscloud_acl_role.test", "rule.0.database.0.regions.*", "us-east-2"),
//
// // Test role exist
// func(s *terraform.State) error {
// r := s.RootModule().Resources["rediscloud_acl_role.test"]
//
// id, err := strconv.Atoi(r.Primary.ID)
// if err != nil {
// return fmt.Errorf("couldn't parse the role ID: %s", redis.StringValue(&r.Primary.ID))
// }
//
// client := testProvider.Meta().(*apiClient)
// role, err := client.client.Roles.Get(context.TODO(), id)
// if err != nil {
// return err
// }
//
// if redis.StringValue(role.Name) != testRoleName {
// return fmt.Errorf("unexpected name value: %s", redis.StringValue(role.Name))
// }
//
// return nil
// },
// ),
// },
// // Test role is updated successfully
// {
// Config: testUpdateTerraform,
// Check: resource.ComposeTestCheckFunc(
// resource.TestCheckResourceAttr("rediscloud_acl_role.test", "name", testRoleName+"-updated"),
// resource.TestCheckResourceAttr("rediscloud_acl_role.test", "rule.#", "1"),
// resource.TestCheckResourceAttr("rediscloud_acl_role.test", "rule.0.name", "Read-Only"),
// resource.TestCheckResourceAttr("rediscloud_acl_role.test", "rule.0.database.#", "1"),
// resource.TestMatchResourceAttr("rediscloud_acl_role.test", "rule.0.database.0.subscription", regexp.MustCompile("^\\d*$")),
// resource.TestMatchResourceAttr("rediscloud_acl_role.test", "rule.0.database.0.database", regexp.MustCompile("^\\d*$")),
// resource.TestCheckResourceAttr("rediscloud_acl_role.test", "rule.0.database.0.regions.#", "2"),
// resource.TestCheckTypeSetElemAttr("rediscloud_acl_role.test", "rule.0.database.0.regions.*", "us-east-1"),
// resource.TestCheckTypeSetElemAttr("rediscloud_acl_role.test", "rule.0.database.0.regions.*", "us-east-2"),
// ),
// },
// // Test that the role is imported successfully
// {
// Config: fmt.Sprintf(testRole, testRoleName+"_updated"),
// ResourceName: "rediscloud_acl_role.test",
// ImportState: true,
// ImportStateVerify: true,
// },
// },
// })
//}

const referencableRule = `
resource "rediscloud_acl_rule" "example" {
name = "%s"
Expand All @@ -199,47 +115,3 @@ resource "rediscloud_acl_role" "test" {
}
}
`

//const testAADatabaseRole = `
//resource "rediscloud_acl_role" "test" {
// name = "%s"
// rule {
// name = "Read-Only"
// database {
// subscription = rediscloud_active_active_subscription.example.id
// database = rediscloud_active_active_subscription_database.example.db_id
// regions = [
// for r in rediscloud_active_active_subscription_database.example.override_region : r.name
// ]
// }
// }
//}
//`

//func testAccCheckAclRoleDestroy(s *terraform.State) error {
// client := testProvider.Meta().(*apiClient)
//
// for _, r := range s.RootModule().Resources {
// if r.Type != "rediscloud_acl_role" {
// continue
// }
//
// id, err := strconv.Atoi(r.Primary.ID)
// if err != nil {
// return err
// }
//
// roles, err := client.client.Roles.List(context.TODO())
// if err != nil {
// return err
// }
//
// for _, role := range roles {
// if redis.IntValue(role.ID) == id {
// return fmt.Errorf("role %d still exists", id)
// }
// }
// }
//
// return nil
//}
26 changes: 26 additions & 0 deletions provider/resource_rediscloud_active_active_subscription.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,27 @@ func resourceRedisCloudActiveActiveSubscription() *schema.Resource {
},
},
},
"redis_version": {
Description: "Version of Redis to create, either 'default' or 'latest'. Defaults to 'default'",
Type: schema.TypeString,
Optional: true,
ForceNew: true,
DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool {
if d.Id() == "" {
// Consider the property if the resource is about to be created.
return false
}

if old != new {
// The user is requesting a change
return false
}

return true
},
ValidateDiagFunc: validation.ToDiagFunc(
validation.StringMatch(regexp.MustCompile("^(default|latest)$"), "must be 'default' or 'latest'")),
},
},
}
}
Expand Down Expand Up @@ -186,6 +207,11 @@ func resourceRedisCloudActiveActiveSubscriptionCreate(ctx context.Context, d *sc
Databases: dbs,
}

redisVersion := d.Get("redis_version").(string)
if d.Get("redis_version").(string) != "" {
createSubscriptionRequest.RedisVersion = redis.String(redisVersion)
}

subId, err := api.client.Subscription.Create(ctx, createSubscriptionRequest)
if err != nil {
return diag.FromErr(err)
Expand Down
28 changes: 27 additions & 1 deletion provider/resource_rediscloud_subscription.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,27 @@ func resourceRedisCloudSubscription() *schema.Resource {
},
},
},
"redis_version": {
Description: "Version of Redis to create, either 'default' or 'latest'. Defaults to 'default'",
Type: schema.TypeString,
Optional: true,
ForceNew: true,
DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool {
if d.Id() == "" {
// Consider the property if the resource is about to be created.
return false
}

if old != new {
// The user is requesting a change
return false
}

return true
},
ValidateDiagFunc: validation.ToDiagFunc(
validation.StringMatch(regexp.MustCompile("^(default|latest)$"), "must be 'default' or 'latest'")),
},
},
}
}
Expand Down Expand Up @@ -332,6 +353,11 @@ func resourceRedisCloudSubscriptionCreate(ctx context.Context, d *schema.Resourc
Databases: dbs,
}

redisVersion := d.Get("redis_version").(string)
if d.Get("redis_version").(string) != "" {
createSubscriptionRequest.RedisVersion = redis.String(redisVersion)
}

subId, err := api.client.Subscription.Create(ctx, createSubscriptionRequest)
if err != nil {
return append(diags, diag.FromErr(err)...)
Expand Down Expand Up @@ -627,7 +653,7 @@ func buildSubscriptionCreatePlanDatabases(memoryStorage string, planMap map[stri
errDiag := diag.Diagnostic{
Severity: diag.Error,
Summary: "subscription could not be created: throughput may not be measured by `operations-per-second` while the `RediSearch` module is active",
Detail: "throughput may not be measured by `operations-per-second` while the `RediSearch` module is active. use an alternative measurement like `number_of_shards`",
Detail: "throughput may not be measured by `operations-per-second` while the `RediSearch` module is active. use an alternative measurement like `number-of-shards`",
}
// Short-circuit here, this is an unrecoverable situation
return nil, append(diags, errDiag)
Expand Down
92 changes: 92 additions & 0 deletions provider/resource_rediscloud_subscription_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,52 @@ func TestAccResourceRedisCloudSubscription_SearchModuleIncompatibleWithOperation
})
}

func TestAccResourceRedisCloudSubscription_RedisVersion(t *testing.T) {
name := acctest.RandomWithPrefix(testResourcePrefix)
testCloudAccountName := os.Getenv("AWS_TEST_CLOUD_ACCOUNT_NAME")

identifier := ""

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t); testAccAwsPreExistingCloudAccountPreCheck(t) },
ProviderFactories: providerFactories,
CheckDestroy: testAccCheckSubscriptionDestroy,
Steps: []resource.TestStep{
{
Config: fmt.Sprintf(testAccResourceRedisCloudSubscriptionWithRedisVersion, testCloudAccountName, name, ""),
Check: resource.ComposeTestCheckFunc(
// Take a snapshot of the ID
func(s *terraform.State) error {
r := s.RootModule().Resources["rediscloud_subscription.test"]
identifier = r.Primary.ID
return nil
},
),
},
{
Config: fmt.Sprintf(testAccResourceRedisCloudSubscriptionWithRedisVersion, testCloudAccountName, name, "redis_version = \"latest\""),
Check: resource.ComposeTestCheckFunc(
// Take a snapshot of the ID
func(s *terraform.State) error {
r := s.RootModule().Resources["rediscloud_subscription.test"]
if r.Primary.ID == identifier {
return fmt.Errorf("entity should have a different identifier, but was still %s", identifier)
}
return nil
},
),
},
{
Config: fmt.Sprintf(testAccResourceRedisCloudSubscriptionWithRedisVersion, testCloudAccountName, name, ""),
ResourceName: "rediscloud_subscription.test",
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"creation_plan", "redis_version"},
},
},
})
}

// Checks that modules are allocated correctly into each creation-plan db if there are multiple modules, including "RedisGraph" and the number of databases is one.
func TestModulesAllocationWhenGraphAndQuantityIsOne(t *testing.T) {
numDatabases := 1
Expand Down Expand Up @@ -588,6 +634,52 @@ resource "rediscloud_subscription" "example" {
}
`

const testAccResourceRedisCloudSubscriptionWithRedisVersion = `
data "rediscloud_payment_method" "card" {
card_type = "Visa"
}
data "rediscloud_cloud_account" "account" {
exclude_internal_account = true
provider_type = "AWS"
name = "%s"
}
resource "rediscloud_subscription" "test" {
name = "%s"
payment_method_id = data.rediscloud_payment_method.card.id
memory_storage = "ram"
# redis_version here
%s
allowlist {
cidrs = ["192.168.0.0/16"]
security_group_ids = []
}
cloud_provider {
provider = data.rediscloud_cloud_account.account.provider_type
cloud_account_id = data.rediscloud_cloud_account.account.id
region {
region = "eu-west-1"
networking_deployment_cidr = "10.0.0.0/24"
preferred_availability_zones = ["eu-west-1a"]
}
}
creation_plan {
memory_limit_in_gb = 1
quantity = 1
replication=false
support_oss_cluster_api=false
throughput_measurement_by = "operations-per-second"
throughput_measurement_value = 10000
modules = []
}
}
`

const testAccResourceRedisCloudSubscriptionPreferredAZsModulesOptional = `
data "rediscloud_payment_method" "card" {
card_type = "Visa"
Expand Down

0 comments on commit 3df50ae

Please sign in to comment.