From 84f5089553f001c9d3ed20bfb84e9cb7e52b19f9 Mon Sep 17 00:00:00 2001 From: gregnuttall Date: Mon, 2 Oct 2023 11:53:43 +0100 Subject: [PATCH 01/18] extend timeouts for subscriptions --- provider/resource_rediscloud_subscription.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/provider/resource_rediscloud_subscription.go b/provider/resource_rediscloud_subscription.go index ea5d5b22..3ca71b1e 100644 --- a/provider/resource_rediscloud_subscription.go +++ b/provider/resource_rediscloud_subscription.go @@ -711,7 +711,7 @@ func waitForSubscriptionToBeActive(ctx context.Context, id int, api *apiClient) Delay: 10 * time.Second, Pending: []string{subscriptions.SubscriptionStatusPending}, Target: []string{subscriptions.SubscriptionStatusActive}, - Timeout: 20 * time.Minute, + Timeout: 100 * time.Minute, Refresh: func() (result interface{}, state string, err error) { log.Printf("[DEBUG] Waiting for subscription %d to be active", id) @@ -774,7 +774,7 @@ func waitForDatabaseToBeActive(ctx context.Context, subId, id int, api *apiClien databases.StatusProxyPolicyChangeDraft, }, Target: []string{databases.StatusActive}, - Timeout: 10 * time.Minute, + Timeout: 30 * time.Minute, Refresh: func() (result interface{}, state string, err error) { log.Printf("[DEBUG] Waiting for database %d to be active", id) From f17c40d9a9740544125283be057cea3cd82d85ff Mon Sep 17 00:00:00 2001 From: gregnuttall Date: Mon, 2 Oct 2023 14:17:45 +0100 Subject: [PATCH 02/18] adds TODO comment to make the timeout configurable --- provider/resource_rediscloud_subscription.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/provider/resource_rediscloud_subscription.go b/provider/resource_rediscloud_subscription.go index 3ca71b1e..e488da72 100644 --- a/provider/resource_rediscloud_subscription.go +++ b/provider/resource_rediscloud_subscription.go @@ -711,7 +711,7 @@ func waitForSubscriptionToBeActive(ctx context.Context, id int, api *apiClient) Delay: 10 * time.Second, Pending: []string{subscriptions.SubscriptionStatusPending}, Target: []string{subscriptions.SubscriptionStatusActive}, - Timeout: 100 * time.Minute, + Timeout: 100 * time.Minute, // TODO: make this configurable Refresh: func() (result interface{}, state string, err error) { log.Printf("[DEBUG] Waiting for subscription %d to be active", id) From b8fa46242a935de8d2eab66ff6c92301ce868a48 Mon Sep 17 00:00:00 2001 From: gregnuttall Date: Mon, 2 Oct 2023 17:06:01 +0100 Subject: [PATCH 03/18] retrigger build From f2c13437f2b975023d91eb985be4fc20da8bf2f2 Mon Sep 17 00:00:00 2001 From: gregnuttall Date: Tue, 3 Oct 2023 15:46:55 +0100 Subject: [PATCH 04/18] retrigger build From 663e73d1855d78ece5a317ad7d1b7569a3bd12ce Mon Sep 17 00:00:00 2001 From: gregnuttall Date: Wed, 4 Oct 2023 09:40:42 +0100 Subject: [PATCH 05/18] retrigger build From 698679743838999da515f4f45efda35d70ef24d7 Mon Sep 17 00:00:00 2001 From: Hieu Doan Date: Wed, 4 Oct 2023 13:51:35 +0100 Subject: [PATCH 06/18] Add a logic to handle a case when roles can be changed back to the 'pending state; Increase timeout for deleting a role --- provider/resource_rediscloud_acl_role.go | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/provider/resource_rediscloud_acl_role.go b/provider/resource_rediscloud_acl_role.go index 2ee30aa1..92081230 100644 --- a/provider/resource_rediscloud_acl_role.go +++ b/provider/resource_rediscloud_acl_role.go @@ -178,6 +178,28 @@ func resourceRedisCloudAclRoleDelete(ctx context.Context, d *schema.ResourceData return diag.FromErr(err) } + // Every now and then the status of the role changes back to the 'pending' state after being fully created. + // The DELETE request will fail if the role is pending. + // This block queries the endpoint until the role is no longer in the 'pending' state. + err = retry.RetryContext(ctx, 5*time.Minute, func() *retry.RetryError { + role, err := api.client.Roles.Get(ctx, id) + if err != nil { + // This was an unexpected error + return retry.NonRetryableError(fmt.Errorf("error getting role: %s", err)) + } + status := redis.StringValue(role.Status) + if status != roles.StatusPending { + // The role is ready for deletion + return nil + } else { + return retry.RetryableError(fmt.Errorf("can't delete the role %d in state %s", id, status)) + } + }) + + if err != nil { + return diag.FromErr(err) + } + err = api.client.Roles.Delete(ctx, id) if err != nil { @@ -187,7 +209,7 @@ func resourceRedisCloudAclRoleDelete(ctx context.Context, d *schema.ResourceData d.SetId("") // Wait until it's really disappeared - err = retry.RetryContext(ctx, 5*time.Minute, func() *retry.RetryError { + err = retry.RetryContext(ctx, 10*time.Minute, func() *retry.RetryError { role, err := api.client.Roles.Get(ctx, id) if err != nil { From 3b3edf5913bfe0fc84d3ae5510927f2da3a915bf Mon Sep 17 00:00:00 2001 From: Hieu Doan Date: Thu, 5 Oct 2023 11:52:39 +0100 Subject: [PATCH 07/18] verify the status of acl rules before deleting acl roles --- provider/resource_rediscloud_acl_role.go | 30 +++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/provider/resource_rediscloud_acl_role.go b/provider/resource_rediscloud_acl_role.go index 92081230..3f289f1d 100644 --- a/provider/resource_rediscloud_acl_role.go +++ b/provider/resource_rediscloud_acl_role.go @@ -3,14 +3,15 @@ package provider import ( "context" "fmt" + "log" + "strconv" + "time" + "github.com/RedisLabs/rediscloud-go-api/redis" "github.com/RedisLabs/rediscloud-go-api/service/access_control_lists/roles" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" - "log" - "strconv" - "time" ) func resourceRedisCloudAclRole() *schema.Resource { @@ -182,11 +183,34 @@ func resourceRedisCloudAclRoleDelete(ctx context.Context, d *schema.ResourceData // The DELETE request will fail if the role is pending. // This block queries the endpoint until the role is no longer in the 'pending' state. err = retry.RetryContext(ctx, 5*time.Minute, func() *retry.RetryError { + var assignedRulesIds []int role, err := api.client.Roles.Get(ctx, id) if err != nil { // This was an unexpected error return retry.NonRetryableError(fmt.Errorf("error getting role: %s", err)) } + + // Check if the rules are not in the pending state + for _, redisRule := range role.RedisRules { + assignedRulesIds = append(assignedRulesIds, *redisRule.RuleId) + } + rules, err := api.client.RedisRules.List(ctx) + if err != nil { + // This was an unexpected error + return retry.NonRetryableError(fmt.Errorf("error getting role: %s", err)) + } + for _, rule := range rules { + for _, asassignedRuleId := range assignedRulesIds { + ruleId := *rule.ID + if ruleId == asassignedRuleId { + ruleStatus := *rule.Status + if ruleStatus != "active" { + return retry.RetryableError(fmt.Errorf("can't delete the role %d in if a rule (%d) is in %s", id, ruleId, ruleStatus)) + } + } + } + } + status := redis.StringValue(role.Status) if status != roles.StatusPending { // The role is ready for deletion From 441062f2356e71a1f239037e7dc953296d821679 Mon Sep 17 00:00:00 2001 From: Hieu Doan Date: Thu, 5 Oct 2023 12:35:00 +0100 Subject: [PATCH 08/18] change error message for a role with non-active rule --- provider/resource_rediscloud_acl_role.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/provider/resource_rediscloud_acl_role.go b/provider/resource_rediscloud_acl_role.go index 3f289f1d..640bf439 100644 --- a/provider/resource_rediscloud_acl_role.go +++ b/provider/resource_rediscloud_acl_role.go @@ -205,7 +205,7 @@ func resourceRedisCloudAclRoleDelete(ctx context.Context, d *schema.ResourceData if ruleId == asassignedRuleId { ruleStatus := *rule.Status if ruleStatus != "active" { - return retry.RetryableError(fmt.Errorf("can't delete the role %d in if a rule (%d) is in %s", id, ruleId, ruleStatus)) + return retry.RetryableError(fmt.Errorf("can't delete the role %d if the rule (%d) is in %s", id, ruleId, ruleStatus)) } } } From c977db9f68cbc13832335179a58ddf2920be9331 Mon Sep 17 00:00:00 2001 From: Hieu Doan Date: Thu, 5 Oct 2023 13:42:03 +0100 Subject: [PATCH 09/18] use struct fields for comparison --- provider/resource_rediscloud_acl_role.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/provider/resource_rediscloud_acl_role.go b/provider/resource_rediscloud_acl_role.go index 640bf439..f64f6ad9 100644 --- a/provider/resource_rediscloud_acl_role.go +++ b/provider/resource_rediscloud_acl_role.go @@ -9,6 +9,7 @@ import ( "github.com/RedisLabs/rediscloud-go-api/redis" "github.com/RedisLabs/rediscloud-go-api/service/access_control_lists/roles" + "github.com/RedisLabs/rediscloud-go-api/service/access_control_lists/redis_rules" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" @@ -204,7 +205,7 @@ func resourceRedisCloudAclRoleDelete(ctx context.Context, d *schema.ResourceData ruleId := *rule.ID if ruleId == asassignedRuleId { ruleStatus := *rule.Status - if ruleStatus != "active" { + if ruleStatus != redis_rules.StatusActive { return retry.RetryableError(fmt.Errorf("can't delete the role %d if the rule (%d) is in %s", id, ruleId, ruleStatus)) } } From a8d0eb35183ee7d5cf8be3c43df83e13f5ba5d4b Mon Sep 17 00:00:00 2001 From: Hieu Doan Date: Fri, 6 Oct 2023 11:21:21 +0100 Subject: [PATCH 10/18] revert changes --- provider/resource_rediscloud_acl_role.go | 55 ++---------------------- 1 file changed, 4 insertions(+), 51 deletions(-) diff --git a/provider/resource_rediscloud_acl_role.go b/provider/resource_rediscloud_acl_role.go index f64f6ad9..2ee30aa1 100644 --- a/provider/resource_rediscloud_acl_role.go +++ b/provider/resource_rediscloud_acl_role.go @@ -3,16 +3,14 @@ package provider import ( "context" "fmt" - "log" - "strconv" - "time" - "github.com/RedisLabs/rediscloud-go-api/redis" "github.com/RedisLabs/rediscloud-go-api/service/access_control_lists/roles" - "github.com/RedisLabs/rediscloud-go-api/service/access_control_lists/redis_rules" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "log" + "strconv" + "time" ) func resourceRedisCloudAclRole() *schema.Resource { @@ -180,51 +178,6 @@ func resourceRedisCloudAclRoleDelete(ctx context.Context, d *schema.ResourceData return diag.FromErr(err) } - // Every now and then the status of the role changes back to the 'pending' state after being fully created. - // The DELETE request will fail if the role is pending. - // This block queries the endpoint until the role is no longer in the 'pending' state. - err = retry.RetryContext(ctx, 5*time.Minute, func() *retry.RetryError { - var assignedRulesIds []int - role, err := api.client.Roles.Get(ctx, id) - if err != nil { - // This was an unexpected error - return retry.NonRetryableError(fmt.Errorf("error getting role: %s", err)) - } - - // Check if the rules are not in the pending state - for _, redisRule := range role.RedisRules { - assignedRulesIds = append(assignedRulesIds, *redisRule.RuleId) - } - rules, err := api.client.RedisRules.List(ctx) - if err != nil { - // This was an unexpected error - return retry.NonRetryableError(fmt.Errorf("error getting role: %s", err)) - } - for _, rule := range rules { - for _, asassignedRuleId := range assignedRulesIds { - ruleId := *rule.ID - if ruleId == asassignedRuleId { - ruleStatus := *rule.Status - if ruleStatus != redis_rules.StatusActive { - return retry.RetryableError(fmt.Errorf("can't delete the role %d if the rule (%d) is in %s", id, ruleId, ruleStatus)) - } - } - } - } - - status := redis.StringValue(role.Status) - if status != roles.StatusPending { - // The role is ready for deletion - return nil - } else { - return retry.RetryableError(fmt.Errorf("can't delete the role %d in state %s", id, status)) - } - }) - - if err != nil { - return diag.FromErr(err) - } - err = api.client.Roles.Delete(ctx, id) if err != nil { @@ -234,7 +187,7 @@ func resourceRedisCloudAclRoleDelete(ctx context.Context, d *schema.ResourceData d.SetId("") // Wait until it's really disappeared - err = retry.RetryContext(ctx, 10*time.Minute, func() *retry.RetryError { + err = retry.RetryContext(ctx, 5*time.Minute, func() *retry.RetryError { role, err := api.client.Roles.Get(ctx, id) if err != nil { From 856b120078f1421886f0f2c01dc4662fdca6e2e3 Mon Sep 17 00:00:00 2001 From: Hieu Doan Date: Fri, 6 Oct 2023 11:43:21 +0100 Subject: [PATCH 11/18] make the subscription timeout configurable --- provider/resource_rediscloud_subscription.go | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/provider/resource_rediscloud_subscription.go b/provider/resource_rediscloud_subscription.go index e488da72..6c5789ce 100644 --- a/provider/resource_rediscloud_subscription.go +++ b/provider/resource_rediscloud_subscription.go @@ -5,6 +5,7 @@ import ( "context" "fmt" "log" + "os" "regexp" "strconv" "time" @@ -707,11 +708,22 @@ func createDatabase(dbName string, idx *int, modules []*subscriptions.CreateModu } func waitForSubscriptionToBeActive(ctx context.Context, id int, api *apiClient) error { + // Allow configuring the subscription timeout via an environment variable. + timeout := 100 + val, _ := os.LookupEnv("REDISCLOUD_SUBSCRIPTION_TIMEOUT") + if len(val) != 0 { + envTimeout, err := strconv.Atoi(val) + if err != nil { + return err + } + timeout = envTimeout + } + wait := &retry.StateChangeConf{ Delay: 10 * time.Second, Pending: []string{subscriptions.SubscriptionStatusPending}, Target: []string{subscriptions.SubscriptionStatusActive}, - Timeout: 100 * time.Minute, // TODO: make this configurable + Timeout: time.Duration(timeout) * time.Minute, Refresh: func() (result interface{}, state string, err error) { log.Printf("[DEBUG] Waiting for subscription %d to be active", id) From 6aa319149c08cae960765005c420714d7d1f79c4 Mon Sep 17 00:00:00 2001 From: Hieu Doan Date: Fri, 6 Oct 2023 12:31:13 +0100 Subject: [PATCH 12/18] change timeout value to 30 get timeout from env as a function --- provider/resource_rediscloud_subscription.go | 37 ++++++++++++++------ 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/provider/resource_rediscloud_subscription.go b/provider/resource_rediscloud_subscription.go index 6c5789ce..975cfcc1 100644 --- a/provider/resource_rediscloud_subscription.go +++ b/provider/resource_rediscloud_subscription.go @@ -708,22 +708,17 @@ func createDatabase(dbName string, idx *int, modules []*subscriptions.CreateModu } func waitForSubscriptionToBeActive(ctx context.Context, id int, api *apiClient) error { - // Allow configuring the subscription timeout via an environment variable. - timeout := 100 - val, _ := os.LookupEnv("REDISCLOUD_SUBSCRIPTION_TIMEOUT") - if len(val) != 0 { - envTimeout, err := strconv.Atoi(val) - if err != nil { - return err - } - timeout = envTimeout + // TODO: Set timeout based on the timeouts block, instead of an env var. + timeout, err := getSubscriptionTimeout() + if err != nil { + return err } wait := &retry.StateChangeConf{ Delay: 10 * time.Second, Pending: []string{subscriptions.SubscriptionStatusPending}, Target: []string{subscriptions.SubscriptionStatusActive}, - Timeout: time.Duration(timeout) * time.Minute, + Timeout: timeout, Refresh: func() (result interface{}, state string, err error) { log.Printf("[DEBUG] Waiting for subscription %d to be active", id) @@ -744,11 +739,16 @@ func waitForSubscriptionToBeActive(ctx context.Context, id int, api *apiClient) } func waitForSubscriptionToBeDeleted(ctx context.Context, id int, api *apiClient) error { + timeout, err := getSubscriptionTimeout() + if err != nil { + return err + } + wait := &retry.StateChangeConf{ Delay: 10 * time.Second, Pending: []string{subscriptions.SubscriptionStatusDeleting}, Target: []string{"deleted"}, - Timeout: 10 * time.Minute, + Timeout: timeout, Refresh: func() (result interface{}, state string, err error) { log.Printf("[DEBUG] Waiting for subscription %d to be deleted", id) @@ -967,3 +967,18 @@ func readPaymentMethodID(d *schema.ResourceData) (*int, error) { } return nil, nil } + +func getSubscriptionTimeout() (time.Duration, error) { + // Allow configuring the subscription timeout via an environment variable. + timeout := 30 + val, _ := os.LookupEnv("REDISCLOUD_SUBSCRIPTION_TIMEOUT") + if len(val) != 0 { + envTimeout, err := strconv.Atoi(val) + if err != nil { + return time.Duration(0), err + } + timeout = envTimeout + } + timeoutDuration := time.Duration(timeout) * time.Minute + return timeoutDuration, nil +} \ No newline at end of file From dab98b52153c8fed4a7354d9de529e5867ffc70f Mon Sep 17 00:00:00 2001 From: Hieu Doan Date: Fri, 6 Oct 2023 15:10:32 +0100 Subject: [PATCH 13/18] check the status of the associated dbs before updating/deleting --- provider/resource_rediscloud_acl_role.go | 73 ++++++++++++++++++++++-- 1 file changed, 69 insertions(+), 4 deletions(-) diff --git a/provider/resource_rediscloud_acl_role.go b/provider/resource_rediscloud_acl_role.go index 2ee30aa1..6eb781a5 100644 --- a/provider/resource_rediscloud_acl_role.go +++ b/provider/resource_rediscloud_acl_role.go @@ -3,14 +3,16 @@ package provider import ( "context" "fmt" + "log" + "strconv" + "time" + "github.com/RedisLabs/rediscloud-go-api/redis" + "github.com/RedisLabs/rediscloud-go-api/service/access_control_lists/redis_rules" "github.com/RedisLabs/rediscloud-go-api/service/access_control_lists/roles" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" - "log" - "strconv" - "time" ) func resourceRedisCloudAclRole() *schema.Resource { @@ -155,6 +157,8 @@ func resourceRedisCloudAclRoleUpdate(ctx context.Context, d *schema.ResourceData rules := extractRules(d) updateRoleRequest.RedisRules = rules + waitForRulesDbToBeActive(ctx, d, api) + err = api.client.Roles.Update(ctx, id, updateRoleRequest) if err != nil { return diag.FromErr(err) @@ -178,6 +182,53 @@ func resourceRedisCloudAclRoleDelete(ctx context.Context, d *schema.ResourceData return diag.FromErr(err) } + waitForRulesDbToBeActive(ctx, d, api) + + // Every now and then the status of the role changes back to the 'pending' state after being fully created. + // The DELETE request will fail if the role is pending. + // This block queries the endpoint until the role is no longer in the 'pending' state. + err = retry.RetryContext(ctx, 5*time.Minute, func() *retry.RetryError { + var assignedRulesIds []int + role, err := api.client.Roles.Get(ctx, id) + if err != nil { + // This was an unexpected error + return retry.NonRetryableError(fmt.Errorf("error getting role: %s", err)) + } + + // Check if the rules are not in the pending state + for _, redisRule := range role.RedisRules { + assignedRulesIds = append(assignedRulesIds, *redisRule.RuleId) + } + rules, err := api.client.RedisRules.List(ctx) + if err != nil { + // This was an unexpected error + return retry.NonRetryableError(fmt.Errorf("error getting role: %s", err)) + } + for _, rule := range rules { + for _, asassignedRuleId := range assignedRulesIds { + ruleId := *rule.ID + if ruleId == asassignedRuleId { + ruleStatus := *rule.Status + if ruleStatus != redis_rules.StatusActive { + return retry.RetryableError(fmt.Errorf("can't delete the role %d if the rule (%d) is in %s", id, ruleId, ruleStatus)) + } + } + } + } + + status := redis.StringValue(role.Status) + if status != roles.StatusPending { + // The role is ready for deletion + return nil + } else { + return retry.RetryableError(fmt.Errorf("can't delete the role %d in state %s", id, status)) + } + }) + + if err != nil { + return diag.FromErr(err) + } + err = api.client.Roles.Delete(ctx, id) if err != nil { @@ -187,7 +238,7 @@ func resourceRedisCloudAclRoleDelete(ctx context.Context, d *schema.ResourceData d.SetId("") // Wait until it's really disappeared - err = retry.RetryContext(ctx, 5*time.Minute, func() *retry.RetryError { + err = retry.RetryContext(ctx, 10*time.Minute, func() *retry.RetryError { role, err := api.client.Roles.Get(ctx, id) if err != nil { @@ -306,3 +357,17 @@ func waitForAclRoleToBeActive(ctx context.Context, id int, api *apiClient) error return nil } + +func waitForRulesDbToBeActive(ctx context.Context, d *schema.ResourceData, api *apiClient) error { + // wait for the assosiacted dbs to be in the active state + extractedRules := extractRules(d) + for _, er := range extractedRules { + for _, db := range er.Databases { + err := waitForDatabaseToBeActive(ctx, *db.SubscriptionId, *db.DatabaseId, api) + if err != nil { + return err + } + } + } + return nil +} From 102a0e0ef92391331ab46e46108d6639a6019e6b Mon Sep 17 00:00:00 2001 From: Hieu Doan Date: Fri, 6 Oct 2023 17:04:07 +0100 Subject: [PATCH 14/18] use a function: wait for acl role to be active before update and delete --- provider/resource_rediscloud_acl_role.go | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/provider/resource_rediscloud_acl_role.go b/provider/resource_rediscloud_acl_role.go index 6eb781a5..35792a02 100644 --- a/provider/resource_rediscloud_acl_role.go +++ b/provider/resource_rediscloud_acl_role.go @@ -159,6 +159,11 @@ func resourceRedisCloudAclRoleUpdate(ctx context.Context, d *schema.ResourceData waitForRulesDbToBeActive(ctx, d, api) + err = waitForAclRoleToBeActive(ctx, id, api) + if err != nil { + return diag.FromErr(err) + } + err = api.client.Roles.Update(ctx, id, updateRoleRequest) if err != nil { return diag.FromErr(err) @@ -202,7 +207,7 @@ func resourceRedisCloudAclRoleDelete(ctx context.Context, d *schema.ResourceData rules, err := api.client.RedisRules.List(ctx) if err != nil { // This was an unexpected error - return retry.NonRetryableError(fmt.Errorf("error getting role: %s", err)) + return retry.NonRetryableError(fmt.Errorf("error getting rules: %s", err)) } for _, rule := range rules { for _, asassignedRuleId := range assignedRulesIds { @@ -215,20 +220,19 @@ func resourceRedisCloudAclRoleDelete(ctx context.Context, d *schema.ResourceData } } } - - status := redis.StringValue(role.Status) - if status != roles.StatusPending { - // The role is ready for deletion - return nil - } else { - return retry.RetryableError(fmt.Errorf("can't delete the role %d in state %s", id, status)) - } + // Unclear at this point what's going on! + return retry.NonRetryableError(fmt.Errorf("unexpected error getting role")) }) if err != nil { return diag.FromErr(err) } + err = waitForAclRoleToBeActive(ctx, id, api) + if err != nil { + return diag.FromErr(err) + } + err = api.client.Roles.Delete(ctx, id) if err != nil { From 27faf15cdf57acd05d7d99e6963cd37a52cfbc14 Mon Sep 17 00:00:00 2001 From: Hieu Doan Date: Mon, 9 Oct 2023 08:58:37 +0100 Subject: [PATCH 15/18] revert back the ACL changes to keep only changes related to timeouts --- provider/resource_rediscloud_acl_role.go | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/provider/resource_rediscloud_acl_role.go b/provider/resource_rediscloud_acl_role.go index 35792a02..6eb781a5 100644 --- a/provider/resource_rediscloud_acl_role.go +++ b/provider/resource_rediscloud_acl_role.go @@ -159,11 +159,6 @@ func resourceRedisCloudAclRoleUpdate(ctx context.Context, d *schema.ResourceData waitForRulesDbToBeActive(ctx, d, api) - err = waitForAclRoleToBeActive(ctx, id, api) - if err != nil { - return diag.FromErr(err) - } - err = api.client.Roles.Update(ctx, id, updateRoleRequest) if err != nil { return diag.FromErr(err) @@ -207,7 +202,7 @@ func resourceRedisCloudAclRoleDelete(ctx context.Context, d *schema.ResourceData rules, err := api.client.RedisRules.List(ctx) if err != nil { // This was an unexpected error - return retry.NonRetryableError(fmt.Errorf("error getting rules: %s", err)) + return retry.NonRetryableError(fmt.Errorf("error getting role: %s", err)) } for _, rule := range rules { for _, asassignedRuleId := range assignedRulesIds { @@ -220,15 +215,16 @@ func resourceRedisCloudAclRoleDelete(ctx context.Context, d *schema.ResourceData } } } - // Unclear at this point what's going on! - return retry.NonRetryableError(fmt.Errorf("unexpected error getting role")) - }) - if err != nil { - return diag.FromErr(err) - } + status := redis.StringValue(role.Status) + if status != roles.StatusPending { + // The role is ready for deletion + return nil + } else { + return retry.RetryableError(fmt.Errorf("can't delete the role %d in state %s", id, status)) + } + }) - err = waitForAclRoleToBeActive(ctx, id, api) if err != nil { return diag.FromErr(err) } From 1d4c8c808af9fecc86614226c5a9107310334e3a Mon Sep 17 00:00:00 2001 From: Hieu Doan Date: Mon, 9 Oct 2023 09:27:03 +0100 Subject: [PATCH 16/18] update the subscription docs for timeouts --- docs/resources/rediscloud_active_active_subscription.md | 3 +++ docs/resources/rediscloud_subscription.md | 3 +++ 2 files changed, 6 insertions(+) diff --git a/docs/resources/rediscloud_active_active_subscription.md b/docs/resources/rediscloud_active_active_subscription.md index 4ddbf651..c2c9e3e9 100644 --- a/docs/resources/rediscloud_active_active_subscription.md +++ b/docs/resources/rediscloud_active_active_subscription.md @@ -81,6 +81,9 @@ The `timeouts` block allows you to specify [timeouts](https://www.terraform.io/d * `update` - (Defaults to 30 mins) Used when updating the subscription * `delete` - (Defaults to 10 mins) Used when destroying the subscription +~> **Note:** If the subscription still times out. You may also need to set `REDISCLOUD_SUBSCRIPTION_TIMEOUT`. +For example, to increase timeouts to 60 minutes set `REDISCLOUD_SUBSCRIPTION_TIMEOUT=60`. + ## Import `rediscloud_active_active_subscription` can be imported using the ID of the subscription, e.g. diff --git a/docs/resources/rediscloud_subscription.md b/docs/resources/rediscloud_subscription.md index 91bd63b9..cc723a97 100644 --- a/docs/resources/rediscloud_subscription.md +++ b/docs/resources/rediscloud_subscription.md @@ -120,6 +120,9 @@ The `timeouts` block allows you to specify [timeouts](https://www.terraform.io/d * `update` - (Defaults to 30 mins) Used when updating the subscription * `delete` - (Defaults to 10 mins) Used when destroying the subscription +~> **Note:** If the subscription still times out. You may also need to set `REDISCLOUD_SUBSCRIPTION_TIMEOUT`. +For example, to increase timeouts to 60 minutes set `REDISCLOUD_SUBSCRIPTION_TIMEOUT=60`. + ## Attribute reference The `region` block has these attributes: From 0026de413d94f5b3abfc7a9cd1d087068958663e Mon Sep 17 00:00:00 2001 From: Hieu Doan Date: Mon, 9 Oct 2023 09:30:54 +0100 Subject: [PATCH 17/18] revert back the ACL changes to keep only changes related to timeouts --- provider/resource_rediscloud_acl_role.go | 73 ++---------------------- 1 file changed, 4 insertions(+), 69 deletions(-) diff --git a/provider/resource_rediscloud_acl_role.go b/provider/resource_rediscloud_acl_role.go index 6eb781a5..2ee30aa1 100644 --- a/provider/resource_rediscloud_acl_role.go +++ b/provider/resource_rediscloud_acl_role.go @@ -3,16 +3,14 @@ package provider import ( "context" "fmt" - "log" - "strconv" - "time" - "github.com/RedisLabs/rediscloud-go-api/redis" - "github.com/RedisLabs/rediscloud-go-api/service/access_control_lists/redis_rules" "github.com/RedisLabs/rediscloud-go-api/service/access_control_lists/roles" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "log" + "strconv" + "time" ) func resourceRedisCloudAclRole() *schema.Resource { @@ -157,8 +155,6 @@ func resourceRedisCloudAclRoleUpdate(ctx context.Context, d *schema.ResourceData rules := extractRules(d) updateRoleRequest.RedisRules = rules - waitForRulesDbToBeActive(ctx, d, api) - err = api.client.Roles.Update(ctx, id, updateRoleRequest) if err != nil { return diag.FromErr(err) @@ -182,53 +178,6 @@ func resourceRedisCloudAclRoleDelete(ctx context.Context, d *schema.ResourceData return diag.FromErr(err) } - waitForRulesDbToBeActive(ctx, d, api) - - // Every now and then the status of the role changes back to the 'pending' state after being fully created. - // The DELETE request will fail if the role is pending. - // This block queries the endpoint until the role is no longer in the 'pending' state. - err = retry.RetryContext(ctx, 5*time.Minute, func() *retry.RetryError { - var assignedRulesIds []int - role, err := api.client.Roles.Get(ctx, id) - if err != nil { - // This was an unexpected error - return retry.NonRetryableError(fmt.Errorf("error getting role: %s", err)) - } - - // Check if the rules are not in the pending state - for _, redisRule := range role.RedisRules { - assignedRulesIds = append(assignedRulesIds, *redisRule.RuleId) - } - rules, err := api.client.RedisRules.List(ctx) - if err != nil { - // This was an unexpected error - return retry.NonRetryableError(fmt.Errorf("error getting role: %s", err)) - } - for _, rule := range rules { - for _, asassignedRuleId := range assignedRulesIds { - ruleId := *rule.ID - if ruleId == asassignedRuleId { - ruleStatus := *rule.Status - if ruleStatus != redis_rules.StatusActive { - return retry.RetryableError(fmt.Errorf("can't delete the role %d if the rule (%d) is in %s", id, ruleId, ruleStatus)) - } - } - } - } - - status := redis.StringValue(role.Status) - if status != roles.StatusPending { - // The role is ready for deletion - return nil - } else { - return retry.RetryableError(fmt.Errorf("can't delete the role %d in state %s", id, status)) - } - }) - - if err != nil { - return diag.FromErr(err) - } - err = api.client.Roles.Delete(ctx, id) if err != nil { @@ -238,7 +187,7 @@ func resourceRedisCloudAclRoleDelete(ctx context.Context, d *schema.ResourceData d.SetId("") // Wait until it's really disappeared - err = retry.RetryContext(ctx, 10*time.Minute, func() *retry.RetryError { + err = retry.RetryContext(ctx, 5*time.Minute, func() *retry.RetryError { role, err := api.client.Roles.Get(ctx, id) if err != nil { @@ -357,17 +306,3 @@ func waitForAclRoleToBeActive(ctx context.Context, id int, api *apiClient) error return nil } - -func waitForRulesDbToBeActive(ctx context.Context, d *schema.ResourceData, api *apiClient) error { - // wait for the assosiacted dbs to be in the active state - extractedRules := extractRules(d) - for _, er := range extractedRules { - for _, db := range er.Databases { - err := waitForDatabaseToBeActive(ctx, *db.SubscriptionId, *db.DatabaseId, api) - if err != nil { - return err - } - } - } - return nil -} From a8e0313387716b0668c639b2b6dad4c60948b23c Mon Sep 17 00:00:00 2001 From: Hieu Doan Date: Mon, 9 Oct 2023 14:34:52 +0100 Subject: [PATCH 18/18] remove the notes from docs --- docs/resources/rediscloud_active_active_subscription.md | 3 --- docs/resources/rediscloud_subscription.md | 3 --- 2 files changed, 6 deletions(-) diff --git a/docs/resources/rediscloud_active_active_subscription.md b/docs/resources/rediscloud_active_active_subscription.md index c2c9e3e9..4ddbf651 100644 --- a/docs/resources/rediscloud_active_active_subscription.md +++ b/docs/resources/rediscloud_active_active_subscription.md @@ -81,9 +81,6 @@ The `timeouts` block allows you to specify [timeouts](https://www.terraform.io/d * `update` - (Defaults to 30 mins) Used when updating the subscription * `delete` - (Defaults to 10 mins) Used when destroying the subscription -~> **Note:** If the subscription still times out. You may also need to set `REDISCLOUD_SUBSCRIPTION_TIMEOUT`. -For example, to increase timeouts to 60 minutes set `REDISCLOUD_SUBSCRIPTION_TIMEOUT=60`. - ## Import `rediscloud_active_active_subscription` can be imported using the ID of the subscription, e.g. diff --git a/docs/resources/rediscloud_subscription.md b/docs/resources/rediscloud_subscription.md index cc723a97..91bd63b9 100644 --- a/docs/resources/rediscloud_subscription.md +++ b/docs/resources/rediscloud_subscription.md @@ -120,9 +120,6 @@ The `timeouts` block allows you to specify [timeouts](https://www.terraform.io/d * `update` - (Defaults to 30 mins) Used when updating the subscription * `delete` - (Defaults to 10 mins) Used when destroying the subscription -~> **Note:** If the subscription still times out. You may also need to set `REDISCLOUD_SUBSCRIPTION_TIMEOUT`. -For example, to increase timeouts to 60 minutes set `REDISCLOUD_SUBSCRIPTION_TIMEOUT=60`. - ## Attribute reference The `region` block has these attributes: