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

New Resource: alicloud_ess_server_group_attachment. #7696

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

Conversation

fuliu-zln
Copy link
Contributor

No description provided.

@fuliu-zln fuliu-zln changed the title New Resource: alicloud_ess_nlb_alb_server_group_attachment. New Resource: alicloud_ess_server_group_attachment. Sep 24, 2024
@fuliu-zln fuliu-zln force-pushed the feat/newResource branch 3 times, most recently from 4788536 to 0341a0f Compare September 24, 2024 04:50
Create: resourceAliyunEssServerGroupAttachmentCreate,
Read: resourceAliyunEssServerGroupAttachmentRead,
Update: resourceAliyunEssServerGroupAttachmentUpdate,
Delete: resourceAliyunEssServerGroupAttachmentDelete,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Timeouts 加上

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done


var raw interface{}
var err error
err = resource.Retry(5*time.Minute, func() *resource.RetryError {
Copy link
Collaborator

Choose a reason for hiding this comment

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

这里使用 schema.TimeoutCreate

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

},
})
}

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

@fuliu-zln fuliu-zln Sep 24, 2024

Choose a reason for hiding this comment

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

补充了冲突重试 + 测试并发

@fuliu-zln fuliu-zln force-pushed the feat/newResource branch 2 times, most recently from ef5fcf0 to 4809649 Compare September 24, 2024 08:36

var raw interface{}
var err error
err = resource.Retry(d.Timeout(schema.TimeoutCreate), func() *resource.RetryError {
Copy link
Collaborator

Choose a reason for hiding this comment

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

err = resource.Retry(client.GetRetryTimeout(d.Timeout(schema.TimeoutCreate)), func() *resource.RetryError {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

if activityId == "" {
return nil
}
stateConf := BuildStateConf([]string{}, []string{"Successful"}, d.Timeout(schema.TimeoutCreate), 1*time.Minute, essService.ActivityStateRefreshFunc(activityId, []string{"Failed", "Rejected"}))
Copy link
Collaborator

Choose a reason for hiding this comment

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

schema.TimeoutDelete
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

})
}

func TestAccAliCloudEssServerGroupAttachment_nonForceAttach_mutil(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.

TestAccAliCloudEssServerGroupAttachment_nonForceAttach_mutil单测删了,没有存在的意义,单测冗余,根本不是测试创建多个资源的单测。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这个是chengyi 要求的


-> **NOTE:** If scaling group's network type is `VPC`, the alb server groups must be in the same `VPC`.

-> **NOTE:** Alb server group attachment is defined uniquely by `scaling_group_id`, `server_group_id`,`type`, `port`.
Copy link
Collaborator

@MrWolong MrWolong Sep 24, 2024

Choose a reason for hiding this comment

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

不要去完全复制粘贴ess_alb_server_group_attachment.html.markdown文档的内容,这个资源是ess_server_group_attachment,不是ess_alb_server_group_attachment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done


## Example Usage

<div style="display: block;margin-bottom: 40px;"><div class="oics-button" style="float: right;position: absolute;margin-bottom: 10px;">
Copy link
Collaborator

Choose a reason for hiding this comment

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

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done


The following attributes are exported:

* `id` - (Required, ForceNew) The ESS alb server group attachment resource ID,in the follwing format: scaling_group_id:
Copy link
Collaborator

@MrWolong MrWolong Sep 24, 2024

Choose a reason for hiding this comment

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

出参id为什么要带(Required, ForceNew)?还有,这个资源是alb server group attachment的吗?改!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

ESS server groups can be imported using the id, e.g.

```shell
$ terraform import alicloud_ess_server_group_attachment.example asg-xxx:sgp-xxx:alb:5000
Copy link
Collaborator

Choose a reason for hiding this comment

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

你看过这个资源的id吗?这个资源的type的入参是大写!
image
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -1059,6 +1059,7 @@ func Provider() terraform.ResourceProvider {
"alicloud_ess_alarm": resourceAlicloudEssAlarm(),
"alicloud_ess_scalinggroup_vserver_groups": resourceAlicloudEssScalingGroupVserverGroups(),
"alicloud_ess_alb_server_group_attachment": resourceAlicloudEssAlbServerGroupAttachment(),
"alicloud_ess_server_group_attachment": resourceAlicloudEssServerGroupAttachment(),
Copy link
Collaborator

Choose a reason for hiding this comment

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

resourceAliCloudEssServerGroupAttachment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@fuliu-zln fuliu-zln force-pushed the feat/newResource branch 2 times, most recently from 12dcc8f to 42aff53 Compare September 25, 2024 06:24
Importer: &schema.ResourceImporter{
State: schema.ImportStatePassthrough,
},
Timeouts: &schema.ResourceTimeout{
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

Choose a reason for hiding this comment

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

done

"scaling_group_id": "${alicloud_ess_scaling_group.default.id}",
}),
Check: resource.ComposeTestCheckFunc(
testAccCheck(map[string]string{
Copy link
Collaborator

Choose a reason for hiding this comment

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

这几个字段为什么不校验?
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

增加了

"scaling_group_id": "${alicloud_ess_scaling_group.default.id}",
}),
Check: resource.ComposeTestCheckFunc(
testAccCheck(nil),
Copy link
Collaborator

@MrWolong MrWolong Sep 25, 2024

Choose a reason for hiding this comment

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

为什么不校验可查入参?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

增加了

"scaling_group_id": "${alicloud_ess_scaling_group.default.id}",
}),
Check: resource.ComposeTestCheckFunc(
testAccCheck(map[string]string{
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

Choose a reason for hiding this comment

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

增加了

"scaling_group_id": "${alicloud_ess_scaling_group.default.id}",
}),
Check: resource.ComposeTestCheckFunc(
testAccCheck(map[string]string{
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

Choose a reason for hiding this comment

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

增加了

* `scaling_group_id` - (Required, ForceNew) ID of the scaling group.
* `server_group_id` - (Required, ForceNew) ID of Server Group.
* `type` - (Required, ForceNew) The type of server group N. Valid values: ALB, NLB.
* `port` - (Required, ForceNew) - The port will be used for Alb Server Group backend server.
Copy link
Collaborator

Choose a reason for hiding this comment

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

这个资源是只针对alb的吗?
image

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 len(scalingGroup.ServerGroups.ServerGroup) > 0 {
return WrapError(fmt.Errorf("There are still attached alb server groups."))
Copy link
Collaborator

Choose a reason for hiding this comment

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

testAccCheckEssServerGroupsDestroy这个函数只能用来校验alb的,对吗?

Copy link
Contributor Author

@fuliu-zln fuliu-zln Sep 25, 2024

Choose a reason for hiding this comment

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

不是 alb 和nlb 通用的 将alb描述 去掉了

@fuliu-zln fuliu-zln force-pushed the feat/newResource branch 3 times, most recently from d17d794 to 42f1dbc Compare September 25, 2024 07:39
Copy link
Collaborator

@MrWolong MrWolong left a comment

Choose a reason for hiding this comment

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

approved

Copy link
Collaborator

@ChenHanZhang ChenHanZhang left a comment

Choose a reason for hiding this comment

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

approved

Copy link
Collaborator

@MrWolong MrWolong left a comment

Choose a reason for hiding this comment

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

approved

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.

3 participants