fix: get private ip with retry #100
Conversation
There was a problem hiding this comment.
Hi @alexyueer,
Sorry for the delay, taking a look at your PR now.
The code overall looks fine, I've left two comments, one that is more a curiosity question than anything, and another that concerns state.
I'd like to hear your input on the state one especially, since this may change the behaviour for clients that define associate_public_ip_address, unless I'm mistaken in this case the ipaddress will not be set, and this may have some altering effects on the following steps.
Other than this, LGTM, thanks for the contribution!
| @@ -13,7 +13,8 @@ | |||
| "ssh_username":"root", | |||
| "instance_type":"ecs.n1.tiny", | |||
| "internet_charge_type":"PayByTraffic", | |||
| "io_optimized":"true" | |||
| "io_optimized":"true", | |||
| "associate_public_ip_address":"true" | |||
There was a problem hiding this comment.
Out of curiosity since the code modifies the private IP address path, why do we change this in the basic example?
There was a problem hiding this comment.
Basic example not set associate_public_ip_address or ssh_private_ip, it not work since this PR：https://github.com/hashicorp/packer-plugin-alicloud/pull/52/commits
builder/ecs/step_config_eip.go
Outdated
| return WaitForExpectToRetry | ||
| } | ||
| ipaddress = ipAddresses[0] | ||
| state.Put("ipaddress", ipaddress) |
There was a problem hiding this comment.
I think this should stay in its previous position, otherwise only the private IP will be set in state, and not the public one if that's what is chosen.
There was a problem hiding this comment.
Thanks for the comment, you are right.
There was a problem hiding this comment.
With the latest reroll, the code looks good to me, thanks @alexyueer!
Merging this
Support to get private ip of instance with retry.