fix: redis support maz modified (#979)

* fix: redis support maz modified

* fix: redis prepaid testcase

Author: Kagashino

tencentcloud/commom_test.go

@@ -2,6 +2,10 @@ package tencentcloud
 
 import (
 	"testing"
+
+	sdkErrors "github.com/tencentcloud/tencentcloud-sdk-go/tencentcloud/common/errors"
+
+	"github.com/stretchr/testify/assert"
 )
 
 func TestIsContains(t *testing.T) {
@@ -71,3 +75,53 @@ func TestIsContains(t *testing.T) {
 		}
 	}
 }
+
+func TestGetListIncrement(t *testing.T) {
+	var (
+		old1      = []int{1, 2, 2, 3, 5}
+		new1      = []int{1, 2, 3, 2, 4, 5, 6, 3}
+		expected1 = []int{4, 6, 3}
+	)
+	actual1, _ := GetListIncrement(old1, new1)
+	assert.Equalf(t, expected1, actual1, "incr1 should equal, got %v %v", expected1, actual1)
+
+	var (
+		old2      = []int{1, 2, 4, 5}
+		new2      = []int{1, 2, 3, 2, 4, 5, 6, 3}
+		expected2 = []int{3, 2, 6, 3}
+	)
+	actual2, _ := GetListIncrement(old2, new2)
+	assert.Equalf(t, expected2, actual2, "incr1 should equal, got %v %v", expected2, actual2)
+
+	var (
+		old3      = []int{1, 2, 4, 5, 3, 6, 3, 2}
+		new3      = []int{1, 2, 3, 2, 4, 5, 6, 3}
+		expected3 = make([]int, 0)
+	)
+	actual3, _ := GetListIncrement(old3, new3)
+	assert.Equalf(t, expected3, actual3, "incr1 should equal, got %v %v", expected3, actual3)
+
+	var (
+		old4 = []int{1}
+		new4 = []int{2}
+	)
+
+	_, err := GetListIncrement(old4, new4)
+	assert.EqualError(t, err, "elem 1 not exist")
+
+}
+
+func TestIsExpectError(t *testing.T) {
+
+	err := sdkErrors.NewTencentCloudSDKError("ClientError.NetworkError", "", "")
+
+	expectedFull := []string{"ClientError.NetworkError"}
+	expectedShort := []string{"ClientError"}
+	unExpectedMatchHead := []string{"ClientError.HttpStatusCodeError"}
+	unExpectedShort := []string{"SystemError"}
+
+	assert.Equalf(t, isExpectError(err, expectedFull), true, "")
+	assert.Equalf(t, isExpectError(err, expectedShort), true, "")
+	assert.Equalf(t, isExpectError(err, unExpectedMatchHead), false, "")
+	assert.Equalf(t, isExpectError(err, unExpectedShort), false, "")
+}

tencentcloud/common.go

@@ -147,7 +147,7 @@ func retryError(err error, additionRetryableError ...string) *resource.RetryErro
 	return resource.NonRetryableError(err)
 }
 
-// isExpectError returns whether error is expect error
+// isExpectError returns whether error is expected error
 func isExpectError(err error, expectError []string) bool {
 	e, ok := err.(*sdkErrors.TencentCloudSDKError)
 	if !ok {
@@ -276,3 +276,33 @@ func IsContains(array interface{}, value interface{}) bool {
 		return reflect.DeepEqual(array, value)
 	}
 }
+
+func FindIntListIndex(list []int, elem int) int {
+	for i, v := range list {
+		if v == elem {
+			return i
+		}
+	}
+	return -1
+}
+
+func GetListIncrement(o []int, n []int) (result []int, err error) {
+	result = append(result, n...)
+	if len(o) > len(n) {
+		err = fmt.Errorf("new list elem count %d less than old: %d", len(n), len(o))
+		return
+	}
+	for _, v := range o {
+		index := FindIntListIndex(result, v)
+		if index == -1 {
+			err = fmt.Errorf("elem %d not exist", v)
+			return
+		}
+		if index+1 >= len(result) {
+			result = result[:index]
+		} else {
+			result = append(result[:index], result[index+1:]...)
+		}
+	}
+	return
+}

tencentcloud/internal/helper/transform.go

@@ -77,6 +77,14 @@ func InterfacesStringsPoint(configured []interface{}) []*string {
 	return vs
 }
 
+func InterfacesIntegers(configured []interface{}) []int {
+	vs := make([]int, 0, len(configured))
+	for _, v := range configured {
+		vs = append(vs, v.(int))
+	}
+	return vs
+}
+
 func InterfacesIntInt64Point(configured []interface{}) []*int64 {
 	vs := make([]*int64, 0, len(configured))
 	for _, v := range configured {

tencentcloud/resource_tc_redis_instance.go

@@ -124,14 +124,12 @@ func resourceTencentCloudRedisInstance() *schema.Resource {
 			"redis_replicas_num": {
 				Type:        schema.TypeInt,
 				Optional:    true,
-				ForceNew:    true,
 				Default:     1,
 				Description: "The number of instance copies. This is not required for standalone and master slave versions.",
 			},
 			"replica_zone_ids": {
 				Type:        schema.TypeList,
 				Optional:    true,
-				ForceNew:    true,
 				Description: "ID of replica nodes available zone. This is not required for standalone and master slave versions.",
 				Elem:        &schema.Schema{Type: schema.TypeInt},
 			},
@@ -578,17 +576,25 @@ func resourceTencentCloudRedisInstanceRead(d *schema.ResourceData, meta interfac
 	}
 
 	if info.NodeSet != nil {
-		var zoneIds []uint64
+		var zoneIds []int
 		for i := range info.NodeSet {
 			nodeInfo := info.NodeSet[i]
 			if *nodeInfo.NodeType == 0 {
 				continue
 			}
-			zoneIds = append(zoneIds, *nodeInfo.ZoneId)
+			zoneIds = append(zoneIds, int(*nodeInfo.ZoneId))
+		}
+
+		var zoneIdsEqual = false
+
+		raw, ok := d.GetOk("replica_zone_ids")
+		if ok {
+			oldIds := helper.InterfacesIntegers(raw.([]interface{}))
+			zoneIdsEqual = checkZoneIdsEqual(oldIds, zoneIds)
 		}
 
-		if err := d.Set("replica_zone_ids", zoneIds); err != nil {
-			log.Printf("[WARN] replica_zone_ids set error: %s", err.Error())
+		if !zoneIdsEqual {
+			_ = d.Set("replica_zone_ids", zoneIds)
 		}
 	}
 
@@ -643,28 +649,29 @@ func resourceTencentCloudRedisInstanceUpdate(d *schema.ResourceData, meta interf
 		d.SetPartial("name")
 	}
 
+	// MemSize, ShardNum and ReplicaNum can only change one for each upgrade invoke
 	if d.HasChange("mem_size") {
 
 		_, newInter := d.GetChange("mem_size")
 		newMemSize := newInter.(int)
-		//oldMemSize := oldInter.(int)
-
-		//if oldMemSize >= newMemSize {
-		//	return fmt.Errorf("redis mem_size can only increase")
-		//}
+		oShard, _ := d.GetChange("redis_shard_num")
+		redisShardNum := oShard.(int)
+		oReplica, _ := d.GetChange("redis_replicas_num")
+		redisReplicasNum := oReplica.(int)
 
 		if newMemSize < 1 {
 			return fmt.Errorf("redis mem_size value cannot be set to less than 1")
 		}
-		redisShardNum := d.Get("redis_shard_num").(int)
-		redisReplicasNum := d.Get("redis_replicas_num").(int)
-		_, err := redisService.UpgradeInstance(ctx, id, int64(newMemSize), int64(redisShardNum), int64(redisReplicasNum))
+
+		_, err := redisService.UpgradeInstance(ctx, id, newMemSize, redisShardNum, redisReplicasNum, nil)
 
 		if err != nil {
-			log.Printf("[CRITAL]%s redis update mem size error, reason:%s\n", logId, err.Error())
+			log.Printf("[CRITAL]%s redis upgrade instance error, reason:%s\n", logId, err.Error())
 			return err
 		}
 
+		startUpdate := false
+
 		err = resource.Retry(4*readRetryTimeout, func() *resource.RetryError {
 			_, _, info, err := redisService.CheckRedisOnlineOk(ctx, id)
 
@@ -673,12 +680,17 @@ func resourceTencentCloudRedisInstanceUpdate(d *schema.ResourceData, meta interf
 				if status == "" {
 					return resource.NonRetryableError(fmt.Errorf("after update redis mem size, redis status is unknown ,status=%d", *info.Status))
 				}
-				if *info.Status == REDIS_STATUS_PROCESSING || *info.Status == REDIS_STATUS_INIT {
-					return resource.RetryableError(fmt.Errorf("redis update processing."))
+				if *info.Status == REDIS_STATUS_ONLINE && !startUpdate {
+					return resource.RetryableError(fmt.Errorf("waiting for status change to proccessing"))
 				}
-				if *info.Status == REDIS_STATUS_ONLINE {
+				if *info.Status == REDIS_STATUS_ONLINE && startUpdate {
 					return nil
 				}
+				startUpdate = true
+
+				if *info.Status == REDIS_STATUS_PROCESSING || *info.Status == REDIS_STATUS_INIT {
+					return resource.RetryableError(fmt.Errorf("redis update processing."))
+				}
 				return resource.NonRetryableError(fmt.Errorf("after update redis mem size, redis status is %s", status))
 			}
 
@@ -696,8 +708,143 @@ func resourceTencentCloudRedisInstanceUpdate(d *schema.ResourceData, meta interf
 			log.Printf("[CRITAL]%s redis update mem size fail , reason:%s\n", logId, err.Error())
 			return err
 		}
+	}
+
+	// MemSize, ShardNum and ReplicaNum can only change one for each upgrade invoke
+	if d.HasChange("redis_shard_num") {
+		redisShardNum := d.Get("redis_shard_num").(int)
+		oReplica, _ := d.GetChange("redis_replicas_num")
+		redisReplicasNum := oReplica.(int)
+		memSize := d.Get("mem_size").(int)
+		err := resource.Retry(writeRetryTimeout, func() *resource.RetryError {
+			_, err := redisService.UpgradeInstance(ctx, id, memSize, redisShardNum, redisReplicasNum, nil)
+			if err != nil {
+				// Upgrade memory will cause instance lock and cannot acknowledge by polling status, wait until lock release
+				return retryError(err, redis.FAILEDOPERATION_UNKNOWN, redis.FAILEDOPERATION_SYSTEMERROR)
+			}
+			return nil
+		})
+
+		if err != nil {
+			log.Printf("[CRITAL]%s redis upgrade instance error, reason:%s\n", logId, err.Error())
+			return err
+		}
+
+		startUpdate := false
+
+		err = resource.Retry(4*readRetryTimeout, func() *resource.RetryError {
+			_, _, info, err := redisService.CheckRedisOnlineOk(ctx, id)
+
+			if info != nil {
+				status := REDIS_STATUS[*info.Status]
+				if status == "" {
+					return resource.NonRetryableError(fmt.Errorf("after update redis shard num, redis status is unknown ,status=%d", *info.Status))
+				}
+				if *info.Status == REDIS_STATUS_ONLINE && !startUpdate {
+					return resource.RetryableError(fmt.Errorf("waiting for status change to proccessing"))
+				}
+				if *info.Status == REDIS_STATUS_ONLINE && startUpdate {
+					return nil
+				}
+				startUpdate = true
+
+				if *info.Status == REDIS_STATUS_PROCESSING || *info.Status == REDIS_STATUS_INIT {
+					return resource.RetryableError(fmt.Errorf("redis update processing."))
+				}
+				return resource.NonRetryableError(fmt.Errorf("after update redis shard num, redis status is %s", status))
+			}
+
+			if err != nil {
+				if _, ok := err.(*sdkErrors.TencentCloudSDKError); !ok {
+					return resource.RetryableError(err)
+				} else {
+					return resource.NonRetryableError(err)
+				}
+			}
+			return resource.NonRetryableError(fmt.Errorf("after update redis shard num, redis disappear"))
+		})
+
+		if err != nil {
+			log.Printf("[CRITAL]%s redis update shard num fail , reason:%s\n", logId, err.Error())
+			return err
+		}
+	}
+
+	// MemSize, ShardNum and ReplicaNum can only change one for each upgrade invoke
+	if d.HasChange("redis_replicas_num") || d.HasChange("replica_zone_ids") {
+		//availabilityZone := d.Get("availability_zone").(string)
+		o, n := d.GetChange("replica_zone_ids") // Only pass zone id increments
+		ov := helper.InterfacesIntegers(o.([]interface{}))
+		nv := helper.InterfacesIntegers(n.([]interface{}))
+		zoneIds, err := GetListIncrement(ov, nv)
+
+		if err != nil {
+			return fmt.Errorf("get diff replica error: %s", err.Error())
+		}
+
+		var nodeInfo []*redis.RedisNodeInfo
+
+		for _, id := range zoneIds {
+			nodeInfo = append(nodeInfo, &redis.RedisNodeInfo{
+				NodeType: helper.Int64(1),
+				ZoneId:   helper.IntUint64(id),
+			})
+		}
+		redisReplicasNum := d.Get("redis_replicas_num").(int)
+		memSize := d.Get("mem_size").(int)
+		redisShardNum := d.Get("redis_shard_num").(int)
+		err = resource.Retry(writeRetryTimeout, func() *resource.RetryError {
+			_, err = redisService.UpgradeInstance(ctx, id, memSize, redisShardNum, redisReplicasNum, nodeInfo)
+			if err != nil {
+				// Upgrade memory will cause instance lock and cannot acknowledge by polling status, wait until lock release
+				return retryError(err, redis.FAILEDOPERATION_UNKNOWN, redis.FAILEDOPERATION_SYSTEMERROR)
+			}
+			return nil
+		})
+
+		if err != nil {
+			return err
+		}
+
+		err = resource.Retry(4*readRetryTimeout, func() *resource.RetryError {
+			_, _, info, err := redisService.CheckRedisOnlineOk(ctx, id)
+
+			if info != nil {
+				status := REDIS_STATUS[*info.Status]
+				if status == "" {
+					return resource.NonRetryableError(fmt.Errorf("after update redis replicas, redis status is unknown ,status=%d", *info.Status))
+				}
+				actualNodeCount := len(info.NodeSet)
+				expectedNodeCount := len(nv) + 1
+				// wait until node set as expected or will cause status inconsistent
+				if *info.Status == REDIS_STATUS_ONLINE && actualNodeCount != expectedNodeCount {
+					return resource.RetryableError(fmt.Errorf("waiting for status change to proccessing"))
+				}
+				if *info.Status == REDIS_STATUS_ONLINE && actualNodeCount == expectedNodeCount {
+					return nil
+				}
+				if *info.Status == REDIS_STATUS_PROCESSING || *info.Status == REDIS_STATUS_INIT {
+					return resource.RetryableError(fmt.Errorf("redis update processing."))
+				}
+
+				return resource.NonRetryableError(fmt.Errorf("after update redis replicas, redis status is %s", status))
+			}
+
+			if err != nil {
+				if _, ok := err.(*sdkErrors.TencentCloudSDKError); !ok {
+					return resource.RetryableError(err)
+				} else {
+					return resource.NonRetryableError(err)
+				}
+			}
+			return resource.NonRetryableError(fmt.Errorf("after update redis replicas, redis disappear"))
+		})
+
+		if err != nil {
+			log.Printf("[CRITAL]%s redis update replicas fail , reason:%s\n", logId, err.Error())
+			return err
+		}
 
-		d.SetPartial("mem_size")
 	}
 
 	if d.HasChange("password") {
@@ -880,3 +1027,19 @@ func resourceTencentCloudRedisInstanceDelete(d *schema.ResourceData, meta interf
 		return nil
 	}
 }
+
+func checkZoneIdsEqual(o []int, n []int) bool {
+	if len(o) != len(n) {
+		return false
+	}
+
+	sort.Ints(o)
+	sort.Ints(n)
+
+	for i, v := range o {
+		if v != n[i] {
+			return false
+		}
+	}
+	return true
+}