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.

Sign up for GitHub

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

Jump to bottom

update alicloud provider version #39

Merged
merged 1 commit into from
Jun 3, 2020
Merged

update alicloud provider version #39

merged 1 commit into from
Jun 3, 2020

Conversation

minchaow
Copy link
Contributor

@minchaow minchaow commented May 28, 2020 •
edited by ialidzhikov
Loading

What this PR does / why we need it:
This PR is to update alicloud provider version to 1.84.0, because in this version it fix the the issue:

https://github.com/terraform-providers/terraform-provider-alicloud/issues/2399.

The fix is necessary for the enhancement:

https://github.com/terraform-providers/terraform-provider-alicloud/issues/2399

Which issue(s) this PR fixes:
Fixes #
N/A
Special notes for your reviewer:
N/A
Release note:

Terraform version has been upgraded to `0.12.20`.

Provider `alicloud` version has been upgraded to `1.84.0`.

@minchaow minchaow requested a review from a team as a code owner May 28, 2020 03:42
@gardener-robot-ci-1 gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label May 28, 2020
@gardener-robot-ci-3 gardener-robot-ci-3 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels May 28, 2020
rfranzke
rfranzke approved these changes May 28, 2020
View reviewed changes
Copy link
Member

@rfranzke rfranzke left a comment

Choose a reason for hiding this comment

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

/lgtm

@ghost ghost added the reviewed/lgtm Has approval for merging label May 28, 2020
@rfranzke
Copy link
Member

/cc @ialidzhikov

@gardener gardener deleted a comment May 28, 2020
@rfranzke
Copy link
Member

/review @ialidzhikov

@ghost ghost requested a review from ialidzhikov May 28, 2020 04:23
@ghost ghost added needs/review Needs review and removed reviewed/lgtm Has approval for merging labels May 28, 2020
@minchaow minchaow changed the title update alicloud provider version [Do not Merge] update alicloud provider version May 28, 2020
@minchaow minchaow force-pushed the alicloud-provider branch from f2bad33 to 1bfbad1 Compare May 28, 2020 15:08
@gardener-robot-ci-1 gardener-robot-ci-1 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels May 28, 2020
@minchaow
Copy link
Contributor Author

Not only updating the version of alicloud provider, but also need to update terraform version to 0.12.20. Because current version 0.12.9 causes the bug, alicloud_eip_association is blocked and cannot be destroyed. In the version 0.12.20, this bug has been fixed.

@minchaow minchaow requested a review from rfranzke May 28, 2020 15:14
@rfranzke
Copy link
Member

/title update alicloud provider version

@ghost ghost changed the title [Do not Merge] update alicloud provider version update alicloud provider version May 28, 2020
@rfranzke
Copy link
Member

Just bumping the patch version of TF should be no risk, right @ialidzhikov ?

@ialidzhikov
Copy link
Member

Just bumping the patch version of TF should be no risk, right @ialidzhikov ?

We need to check the changelogs and probably test with all providers just to be sure.

@minchaow , can you update the issue you reference as in it you say that the affected terraform version is v0.12.20 and in this PR you update to that version.

@rfranzke
Copy link
Member

Yes, makes sense, thanks.
Once you approved @ialidzhikov we can go ahead with the merge + release. Before merging the update PR in the respective provider extension repositories we should do some testing with it.

@minchaow minchaow mentioned this pull request May 29, 2020
Merged
@rfranzke
Copy link
Member

rfranzke commented Jun 2, 2020

/assign @ialidzhikov

@ghost ghost assigned ialidzhikov Jun 2, 2020
@ialidzhikov
Copy link
Member

For terraform-providers/terraform-provider-alicloud#2399 update

/status author-action

@ghost ghost added the status/author-action Issue requires issue author action label Jun 2, 2020
@minchaow
Copy link
Contributor Author

minchaow commented Jun 2, 2020

@ialidzhikov: for issue terraform-providers/terraform-provider-alicloud#2399, I tested with latest version of provider, it has already been fixed. So I just closed this issue.

@ialidzhikov
Copy link
Member

@ialidzhikov: for issue terraform-providers/terraform-provider-alicloud#2399, I tested with latest version of provider, it has already been fixed. So I just closed this issue.

Thank you.

ialidzhikov
ialidzhikov approved these changes Jun 2, 2020
View reviewed changes
Copy link
Member

@ialidzhikov ialidzhikov left a comment

Choose a reason for hiding this comment

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

I tested some basic operations with the tag from the PR v1.1.0-dev-1bfbad1c5f50618b7be1ac85489b208bec97f144 - it looks good to me.

/lgtm

@ghost ghost added reviewed/lgtm Has approval for merging and removed needs/review Needs review status/author-action Issue requires issue author action labels Jun 2, 2020
@ialidzhikov
Copy link
Member

/remove status/author-action

@rfranzke rfranzke merged commit 46f8a42 into gardener:master Jun 3, 2020
@ialidzhikov
Copy link
Member

@minchaow , I actually reproduce the issue your described in https://github.com/terraform-providers/terraform-provider-alicloud/issues/2399. The first attempt to destroy always fails to delete alicloud_eip_association.eip_natgw_asso_z0 with the same error from the issue and the second one succeeds. Was this the case also before this version bump?

@minchaow
Copy link
Contributor Author

minchaow commented Jun 4, 2020

@ialidzhikov , For the problem in which cannot destroy alicloud_eip_association.eip_natgw_asso_z0, I think it is affected by terraform, not the alicloud provider. I can destroy the resources successfully with the version 0.12.20, before the fix of issue terraform-providers/terraform-provider-alicloud#2399.

@ialidzhikov
Copy link
Member

We had a short sync with @minchaow and he will try to reproduce.

@minchaow
Copy link
Contributor Author

minchaow commented Jun 4, 2020

affected by issue gardener/gardener-extension-provider-alicloud#94

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ialidzhikov ialidzhikov ialidzhikov approved these changes

@rfranzke rfranzke Awaiting requested review from rfranzke

Assignees

@ialidzhikov ialidzhikov

Labels
needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) reviewed/lgtm Has approval for merging
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@minchaow @rfranzke @ialidzhikov @gardener-robot-ci-1 @gardener-robot-ci-3