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

provider/azurerm: Add disk_size_gb param to VM storage_os_disk #9200

Merged
merged 1 commit into from
Oct 25, 2016

Conversation

titanous
Copy link
Contributor

@titanous titanous commented Oct 4, 2016

TF_ACC=1 go test ./builtin/providers/azurerm -v -run=TestAccAzureRMVirtualMachine_basicLinuxMachine -timeout 120m
=== RUN   TestAccAzureRMVirtualMachine_basicLinuxMachine
--- PASS: TestAccAzureRMVirtualMachine_basicLinuxMachine (540.83s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/azurerm    540.841s

TF_ACC=1 go test ./builtin/providers/azurerm -v -run=TestAccAzureRMVirtualMachine_withDataDisk -timeout 120m
=== RUN   TestAccAzureRMVirtualMachine_withDataDisk
--- PASS: TestAccAzureRMVirtualMachine_withDataDisk (431.19s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/azurerm    431.203s

@stack72
Copy link
Contributor

stack72 commented Oct 6, 2016

Hi @titanous

Thanks for the work here -- 2 small things:

  1. Can you rebase from master - i have just merged the latest SDK changes
  2. Will the disk_size_in_gb not have to be computed? If there is a disk_size in the api response from Azure and it's not classed as computed in our schema, then Terraform will try and change it

Paul

@stack72 stack72 added the waiting-response An issue/pull request is waiting for a response from the community label Oct 6, 2016
@titanous titanous force-pushed the azurerm-add-os-disk-size branch from e95881c to 669df98 Compare October 10, 2016 14:22
@titanous
Copy link
Contributor Author

@stack72

Can you rebase from master - i have just merged the latest SDK changes

Done.

Will the disk_size_in_gb not have to be computed? If there is a disk_size in the api response from Azure and it's not classed as computed in our schema, then Terraform will try and change it

The storage disk disk_size_in_gb is not marked as computed and no diff shows up when I run a subsequent terraform plan.

@@ -204,11 +206,11 @@ func testCheckAzureRMAvailabilitySetDestroy(s *terraform.State) error {
var testAccAzureRMVAvailabilitySet_basic = `
resource "azurerm_resource_group" "test" {
name = "acctestRG-%d"
location = "West US"
location = "Canada Central"
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you mean to change the default test location?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, see the commit message here: 14169d8

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, it looks like this commit is supposed to be on a separate PR. Fixing...

@titanous titanous force-pushed the azurerm-add-os-disk-size branch from 669df98 to 0ade217 Compare October 11, 2016 13:58
"disk_size_gb": {
Type: schema.TypeInt,
Optional: true,
ValidateFunc: func(v interface{}, k string) (ws []string, errors []error) {
Copy link
Contributor

@carinadigital carinadigital Oct 11, 2016

Choose a reason for hiding this comment

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

This is exactly the same function as for storage_os_disk:disk_size_gb . Consider replacing both with a single common function.

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.

@titanous titanous force-pushed the azurerm-add-os-disk-size branch from 0ade217 to 6c34169 Compare October 11, 2016 14:19
    TF_ACC=1 go test ./builtin/providers/azurerm -v -run=TestAccAzureRMVirtualMachine_basicLinuxMachine -timeout 120m
    === RUN   TestAccAzureRMVirtualMachine_basicLinuxMachine
    --- PASS: TestAccAzureRMVirtualMachine_basicLinuxMachine (540.83s)
    PASS
    ok  	github.com/hashicorp/terraform/builtin/providers/azurerm	540.841s

    TF_ACC=1 go test ./builtin/providers/azurerm -v -run=TestAccAzureRMVirtualMachine_withDataDisk -timeout 120m
    === RUN   TestAccAzureRMVirtualMachine_withDataDisk
    --- PASS: TestAccAzureRMVirtualMachine_withDataDisk (431.19s)
    PASS
    ok  	github.com/hashicorp/terraform/builtin/providers/azurerm	431.203s
@titanous titanous force-pushed the azurerm-add-os-disk-size branch from 6c34169 to f385408 Compare October 11, 2016 14:20
@carinadigital
Copy link
Contributor

I can confirm @titanous findings about disk_size_in_gb on storage_os_disk. The other optional property image_uri also does not produce a diff when you update it's value.

@stack72
Copy link
Contributor

stack72 commented Oct 25, 2016

@carinadigital not sure what you mean here ?

@stack72
Copy link
Contributor

stack72 commented Oct 25, 2016

Hi @titanous

Thanks for the work here - this is working as you suggested :)

% make testacc TEST=./builtin/providers/azurerm TESTARGS='-run=TestAccAzureRMVirtualMachine_basicLinuxMachine'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/10/25 17:11:46 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/azurerm -v -run=TestAccAzureRMVirtualMachine_basicLinuxMachine -timeout 120m
=== RUN   TestAccAzureRMVirtualMachine_basicLinuxMachine
--- PASS: TestAccAzureRMVirtualMachine_basicLinuxMachine (567.20s)
=== RUN   TestAccAzureRMVirtualMachine_basicLinuxMachine_disappears
--- PASS: TestAccAzureRMVirtualMachine_basicLinuxMachine_disappears (560.96s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/azurerm    1128.173s
[stacko@Pauls-MacBook-Pro:~/Code/go/src/github.com/hashicorp/terraform on titanous-azurerm-add-os-disk-size]
% make testacc TEST=./builtin/providers/azurerm TESTARGS='-run=TestAccAzureRMVirtualMachine_withDataDisk'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/10/25 17:34:28 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/azurerm -v -run=TestAccAzureRMVirtualMachine_withDataDisk -timeout 120m
=== RUN   TestAccAzureRMVirtualMachine_withDataDisk
--- PASS: TestAccAzureRMVirtualMachine_withDataDisk (683.58s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/azurerm    683.596s

@stack72 stack72 merged commit d265a6f into hashicorp:master Oct 25, 2016
@titanous titanous deleted the azurerm-add-os-disk-size branch November 4, 2016 17:20
@ghost
Copy link

ghost commented Apr 20, 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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement provider/azurerm waiting-response An issue/pull request is waiting for a response from the community
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants