Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

resource/alicloud_db_instance: add new attributes:bursting_enabled and modify sql_collector_config_value #7840

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

chaitiansheng0524
Copy link
Contributor

No description provided.

@chaitiansheng0524 chaitiansheng0524 force-pushed the fix_Account_Modification branch 5 times, most recently from 2ccde41 to dc35b04 Compare December 13, 2024 05:35
@chaitiansheng0524 chaitiansheng0524 changed the title resource/alicloud_db_instance: fix_Account_Modification resource/alicloud_rds_account: fix_Account_Modification Dec 13, 2024
@@ -606,6 +611,11 @@ func resourceAliCloudDBInstance() *schema.Resource {
Optional: true,
ValidateFunc: StringInSlice([]string{"Up", "Down", "TempUpgrade", "Serverless"}, false),
},
"bursting_enabled": {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

不建议显式设置默认值,在tf中,针对bool类型的字段,除非后端返回的是true,否则默认值是false

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

该参数未开启时会默认为false 记得这种情况好像会影响创建实例吧
image

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

什么意思?如果集成bursting_enabled字段,资源都必须给他赋值为true,才能让实例创建成功吗?如果是,影响存量资源。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已去掉显式默认值设置

if d.HasChange("bursting_enabled") {
update = true
}
request["BurstingEnabled"] = d.Get("bursting_enabled")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BurstingEnabled是必传的吗?针对bool类型的字段,应该使用d.GetOkExists,参考
if v, ok := d.GetOkExists("bursting_enabled"); ok {
request["BurstingEnabled"] = v
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已修改

@@ -1966,6 +1982,9 @@ func buildDBCreateRequest(d *schema.ResourceData, meta interface{}) (map[string]
if v, ok := d.GetOk("port"); ok && v.(string) != "" {
request["Port"] = v
}
if v, ok := d.GetOk("bursting_enabled"); ok && v.(string) != "" {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bursting_enabled是bool类型的字段,不是string类型的,不要断言,使用下面的方式设置值
if v, ok := d.GetOkExists("bursting_enabled"); ok {
request["BurstingEnabled"] = v
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已修改

@@ -360,7 +360,12 @@ func resourceAliCloudDBInstance() *schema.Resource {
Type: schema.TypeInt,
Optional: true,
ValidateFunc: IntInSlice([]int{30, 180, 365, 1095, 1825}),
Default: 30,
DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

为什么删除默认值,并加上压制函数?

Copy link
Contributor Author

@chaitiansheng0524 chaitiansheng0524 Dec 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

因为DescribeSQLCollectorRetention接口的返回值有改动,之前没这个参数,现在有该参数的返回值了,导致线上存量实例产生了diff,所以要删除默认值和加上压制函数

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -241,6 +241,26 @@ func TestAccAliCloudRdsDBInstance_Mysql_8_0(t *testing.T) {
}),
),
},
{
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

单测覆盖不完整,只覆盖了更新操作,缺少创建时指定字段的单测,新增一个单测,用来测试创建时指定bursting_enabled

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已修改

@MrWolong
Copy link
Collaborator

MrWolong commented Dec 13, 2024

PR title要与本地commit保持一致。当前的pr title描述与实际不一致。实际上这个pr涉及到两个资源的变动,第一个是alicloud_db_instance,他主要新增了字段bursting_enabled,并且给字段sql_collector_config_value增加了压制函数;第二个资源是alicloud_rds_account,他主要是修改了字段account_name和name的校验条件

@chaitiansheng0524 chaitiansheng0524 changed the title resource/alicloud_rds_account: fix_Account_Modification resource/alicloud_rds_account: add new attributes:bursting_enabled and modify sql_collector_config_value Dec 31, 2024
@chaitiansheng0524 chaitiansheng0524 changed the title resource/alicloud_rds_account: add new attributes:bursting_enabled and modify sql_collector_config_value resource/alicloud_db_instance: add new attributes:bursting_enabled and modify sql_collector_config_value Dec 31, 2024
@chaitiansheng0524 chaitiansheng0524 force-pushed the fix_Account_Modification branch 4 times, most recently from 8d4512e to 612b793 Compare January 2, 2025 08:22
@chaitiansheng0524 chaitiansheng0524 force-pushed the fix_Account_Modification branch from 612b793 to c71c330 Compare January 2, 2025 08:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants