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

SDKv12: Migrating over to the new Compute SDK #744

Merged
merged 9 commits into from
Jan 24, 2018
Merged

SDKv12: Migrating over to the new Compute SDK #744

merged 9 commits into from
Jan 24, 2018

Conversation

tombuildsstuff
Copy link
Contributor

@tombuildsstuff tombuildsstuff commented Jan 23, 2018 •
edited
Loading

Note the source_uri field is no longer being returned from the API for Snapshots - so I've intentionally unset both it and the SourceResourceID field. I've also made this ForceNew due to this.

  • Migrate off the old Compute SDK
  • Migrate off the old Disk SDK

Tests:

  • Data Sources
  • Availability Sets
  • Image
  • Managed Disk
  • Snapshot
  • VM
  • VMSS
  • VM Extension
  • Virtual Networking

@tombuildsstuff tombuildsstuff added this to the 1.0.2 milestone Jan 23, 2018
@tombuildsstuff tombuildsstuff force-pushed the sdk-migration-compute branch 3 times, most recently from 91c5a23 to 5bd0de7 Compare January 23, 2018 14:59
@tombuildsstuff tombuildsstuff requested review from mbfrahry and paultyng and removed request for mbfrahry January 23, 2018 15:03
@tombuildsstuff
Copy link
Contributor Author

Availability Sets tests pass:

screen shot 2018-01-23 at 16 09 46

@tombuildsstuff
Copy link
Contributor Author

Managed Disk tests pass:

screen shot 2018-01-23 at 16 55 19

@tombuildsstuff
Copy link
Contributor Author

Virtual Networking tests pass:

screen shot 2018-01-23 at 16 59 19

@tombuildsstuff
Copy link
Contributor Author

tombuildsstuff commented Jan 24, 2018 •
edited
Loading

VM/VMSS/VM Extension tests pass, ignoring a transient service error:

screen shot 2018-01-24 at 08 38 41

Image tests pass:

screen shot 2018-01-24 at 08 38 37

@tombuildsstuff
Copy link
Contributor Author

Data Source tests pass:

screen shot 2018-01-24 at 14 00 05

@tombuildsstuff
Copy link
Contributor Author

Snapshot tests now pass:

screen shot 2018-01-24 at 14 41 57

Re-running the Data Source tests given this will impact them too

paultyng
paultyng approved these changes Jan 24, 2018
View reviewed changes
Copy link
Contributor

@paultyng paultyng left a comment

Choose a reason for hiding this comment

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

lgtm

paultyng
paultyng reviewed Jan 24, 2018
View reviewed changes
azurerm/data_source_image.go
if r.Match(([]byte)(*ri.Name)) {
list = append(list, ri)

for resp.NotDone() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you know if the resp object will abort if the cancel on the StopContext is invoked? If not, we may want a select in here on the StopContext.Done, but not a big deal, I don't think it should hold up this pr as its an edge case to have enough images where it would be an issue I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not from what I can see, whilst it's a valid concern it's such an edge case (and minor issue in terms of iterations) I think we can safely ignore it for the moment

@tombuildsstuff
Removing the old Compute SDK
fdd03d1
@tombuildsstuff
Migrating over to the new Compute SDK
cf40e38
@tombuildsstuff
Removing the old Disk SDK
c3df0ee
@tombuildsstuff
Migrating over to the new Disks SDK
b31940b
@tombuildsstuff
Ensuring we pull a complete image list
a11d036
@tombuildsstuff
Handling the Storage Account Type being case insensitive
c02ee8c
@tombuildsstuff
Fixing the broken image test
09cd25d 
```
$ acctests azurerm TestAccAzureRMImage_customImageVMFromVHD
=== RUN   TestAccAzureRMImage_customImageVMFromVHD
--- PASS: TestAccAzureRMImage_customImageVMFromVHD (860.22s)
PASS
ok  	github.com/terraform-providers/terraform-provider-azurerm/azurerm	860.253s
```
@tombuildsstuff
Making the Source Resource ID computed
d46a134
@tombuildsstuff
SourceUri is no longer returned from the API
39a3172
@tombuildsstuff tombuildsstuff merged commit 999c79d into master Jan 24, 2018
@tombuildsstuff tombuildsstuff deleted the sdk-migration-compute branch January 24, 2018 15:13
tombuildsstuff added a commit that referenced this pull request Jan 26, 2018
@tombuildsstuff
Updating to include the forcenew change in #744
173891c
@ghost
Copy link

ghost commented Mar 31, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks!

@ghost ghost locked and limited conversation to collaborators Mar 31, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@paultyng paultyng paultyng approved these changes

Assignees
No one assigned
Labels
dependencies
Projects
None yet
Milestone
v1.1.0
Development

Successfully merging this pull request may close these issues.
2 participants
@tombuildsstuff @paultyng