Skip to content

Commit

Permalink
Merge pull request #6162 from shurikk/only-reboot-on-disk-resize
Browse files Browse the repository at this point in the history 
reboot a VM only on managed disk resize
  • Loading branch information
@tombuildsstuff
tombuildsstuff authored Mar 25, 2020
2 parents 368e3cf + 8456987 commit 9ff4a7e
Show file tree
Hide file tree
Showing 4 changed files with 151 additions and 88 deletions.
14 changes: 11 additions & 3 deletions azurerm/internal/services/compute/resource_arm_managed_disk.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -292,14 +292,15 @@ func resourceArmManagedDiskCreateUpdate(d *schema.ResourceData, meta interface{}

func resourceArmManagedDiskUpdate(d *schema.ResourceData, meta interface{}) error {
client := meta.(*clients.Client).Compute.DisksClient
ctx, cancel := timeouts.ForRead(meta.(*clients.Client).StopContext, d)
ctx, cancel := timeouts.ForUpdate(meta.(*clients.Client).StopContext, d)
defer cancel()

log.Printf("[INFO] preparing arguments for Azure ARM Managed Disk update.")

name := d.Get("name").(string)
resourceGroup := d.Get("resource_group_name").(string)
storageAccountType := d.Get("storage_account_type").(string)
shouldShutDown := false

disk, err := client.Get(ctx, resourceGroup, name)
if err != nil {
Expand All @@ -320,6 +321,7 @@ func resourceArmManagedDiskUpdate(d *schema.ResourceData, meta interface{}) erro
}

if d.HasChange("storage_account_type") {
shouldShutDown = true
var skuName compute.DiskStorageAccountTypes
for _, v := range compute.PossibleDiskStorageAccountTypesValues() {
if strings.EqualFold(storageAccountType, string(v)) {
Expand Down Expand Up @@ -355,13 +357,15 @@ func resourceArmManagedDiskUpdate(d *schema.ResourceData, meta interface{}) erro

if d.HasChange("disk_size_gb") {
if old, new := d.GetChange("disk_size_gb"); new.(int) > old.(int) {
shouldShutDown = true
diskUpdate.DiskUpdateProperties.DiskSizeGB = utils.Int32(int32(new.(int)))
} else {
return fmt.Errorf("Error - New size must be greater than original size. Shrinking disks is not supported on Azure")
}
}

if d.HasChange("disk_encryption_set_id") {
shouldShutDown = true
if diskEncryptionSetId := d.Get("disk_encryption_set_id").(string); diskEncryptionSetId != "" {
diskUpdate.Encryption = &compute.Encryption{
Type: compute.EncryptionAtRestWithCustomerKey,
Expand All @@ -372,8 +376,13 @@ func resourceArmManagedDiskUpdate(d *schema.ResourceData, meta interface{}) erro
}
}

// whilst we need to shut this down, if we're not attached to anything there's no point
if shouldShutDown && disk.ManagedBy == nil {
shouldShutDown = false
}

// if we are attached to a VM we bring down the VM as necessary for the operations which are not allowed while it's online
if disk.ManagedBy != nil {
if shouldShutDown {
virtualMachine, err := ParseVirtualMachineID(*disk.ManagedBy)
if err != nil {
return fmt.Errorf("Error parsing VMID %q for disk attachment: %+v", *disk.ManagedBy, err)
Expand All @@ -390,7 +399,6 @@ func resourceArmManagedDiskUpdate(d *schema.ResourceData, meta interface{}) erro
}

shouldTurnBackOn := true
shouldShutDown := true
shouldDeallocate := true

if instanceView.Statuses != nil {
Expand Down
221 changes: 138 additions & 83 deletions azurerm/internal/services/compute/tests/resource_arm_managed_disk_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -317,14 +317,7 @@ func TestAccAzureRMManagedDisk_diskEncryptionSet(t *testing.T) {
CheckDestroy: testCheckAzureRMManagedDiskDestroy,
Steps: []resource.TestStep{
{
// TODO: After applying soft-delete and purge-protection in keyVault, this extra step can be removed.
Config: testAccAzureRMManagedDisk_diskEncryptionSetDependencies(data),
Check: resource.ComposeTestCheckFunc(
enableSoftDeleteAndPurgeProtectionForKeyVault("azurerm_key_vault.test"),
),
},
{
Config: testAccAzureRMManagedDisk_diskEncryptionSet(data, true),
Config: testAccAzureRMManagedDisk_diskEncryptionSetEncrypted(data),
Check: resource.ComposeTestCheckFunc(
testCheckAzureRMManagedDiskExists(data.ResourceName, &d, true),
),
Expand All @@ -344,21 +337,14 @@ func TestAccAzureRMManagedDisk_diskEncryptionSet_update(t *testing.T) {
CheckDestroy: testCheckAzureRMManagedDiskDestroy,
Steps: []resource.TestStep{
{
// TODO: After applying soft-delete and purge-protection in keyVault, this extra step can be removed.
Config: testAccAzureRMManagedDisk_diskEncryptionSetDependencies(data),
Check: resource.ComposeTestCheckFunc(
enableSoftDeleteAndPurgeProtectionForKeyVault("azurerm_key_vault.test"),
),
},
{
Config: testAccAzureRMManagedDisk_diskEncryptionSet(data, false),
Config: testAccAzureRMManagedDisk_diskEncryptionSetUnencrypted(data),
Check: resource.ComposeTestCheckFunc(
testCheckAzureRMManagedDiskExists(data.ResourceName, &d, true),
),
},
data.ImportStep(),
{
Config: testAccAzureRMManagedDisk_diskEncryptionSet(data, true),
Config: testAccAzureRMManagedDisk_diskEncryptionSetEncrypted(data),
Check: resource.ComposeTestCheckFunc(
testCheckAzureRMManagedDiskExists(data.ResourceName, &d, true),
),
Expand Down Expand Up @@ -396,7 +382,32 @@ func TestAccAzureRMManagedDisk_attachedDiskUpdate(t *testing.T) {
})
}

// TODO: More property update tests?
func TestAccAzureRMManagedDisk_attachedStorageTypeUpdate(t *testing.T) {
data := acceptance.BuildTestData(t, "azurerm_managed_disk", "test")
var d compute.Disk

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acceptance.PreCheck(t) },
Providers: acceptance.SupportedProviders,
CheckDestroy: testCheckAzureRMManagedDiskDestroy,
Steps: []resource.TestStep{
{
Config: testAccAzureRMManagedDisk_storageTypeUpdateWhilstAttached(data, "Standard_LRS"),
Check: resource.ComposeTestCheckFunc(
testCheckAzureRMManagedDiskExists(data.ResourceName, &d, true),
),
},
data.ImportStep(),
{
Config: testAccAzureRMManagedDisk_storageTypeUpdateWhilstAttached(data, "Premium_LRS"),
Check: resource.ComposeTestCheckFunc(
testCheckAzureRMManagedDiskExists(data.ResourceName, &d, true),
),
},
data.ImportStep(),
},
})
}

func testCheckAzureRMManagedDiskExists(resourceName string, d *compute.Disk, shouldExist bool) resource.TestCheckFunc {
return func(s *terraform.State) error {
Expand Down Expand Up @@ -731,7 +742,7 @@ resource "azurerm_managed_disk" "test" {
resource_group_name = azurerm_resource_group.test.name
os_type = "Linux"
create_option = "FromImage"
disk_size_gb = "0"
disk_size_gb = 0
image_reference_id = data.azurerm_platform_image.test.id
storage_account_type = "Standard_LRS"
}
Expand All @@ -756,10 +767,7 @@ resource "azurerm_key_vault" "test" {
location = "${azurerm_resource_group.test.location}"
resource_group_name = "${azurerm_resource_group.test.name}"
tenant_id = "${data.azurerm_client_config.current.tenant_id}"
sku {
name = "premium"
}
sku_name = "premium"
access_policy {
tenant_id = "${data.azurerm_client_config.current.tenant_id}"
Expand Down Expand Up @@ -940,6 +948,8 @@ resource "azurerm_key_vault" "test" {
tenant_id = data.azurerm_client_config.current.tenant_id
sku_name = "premium"
enabled_for_disk_encryption = true
soft_delete_enabled = true
purge_protection_enabled = true
}
resource "azurerm_key_vault_access_policy" "service-principal" {
Expand Down Expand Up @@ -978,18 +988,6 @@ resource "azurerm_key_vault_key" "test" {
depends_on = ["azurerm_key_vault_access_policy.service-principal"]
}
`, data.RandomInteger, location, data.RandomString)
}

func testAccAzureRMManagedDisk_diskEncryptionSet(data acceptance.TestData, complete bool) string {
template := testAccAzureRMManagedDisk_diskEncryptionSetDependencies(data)
diskEncryptionSetLine := ""
if complete {
diskEncryptionSetLine = "disk_encryption_set_id = azurerm_disk_encryption_set.test.id"
}

return fmt.Sprintf(`
%s
resource "azurerm_disk_encryption_set" "test" {
name = "acctestdes-%d"
Expand Down Expand Up @@ -1020,6 +1018,36 @@ resource "azurerm_role_assignment" "disk-encryption-read-keyvault" {
role_definition_name = "Reader"
principal_id = azurerm_disk_encryption_set.test.identity.0.principal_id
}
`, data.RandomInteger, location, data.RandomString, data.RandomInteger)
}

func testAccAzureRMManagedDisk_diskEncryptionSetEncrypted(data acceptance.TestData) string {
template := testAccAzureRMManagedDisk_diskEncryptionSetDependencies(data)
return fmt.Sprintf(`
%s
resource "azurerm_managed_disk" "test" {
name = "acctestd-%d"
location = azurerm_resource_group.test.location
resource_group_name = azurerm_resource_group.test.name
storage_account_type = "Standard_LRS"
create_option = "Empty"
disk_size_gb = 1
disk_encryption_set_id = azurerm_disk_encryption_set.test.id
depends_on = [
"azurerm_role_assignment.disk-encryption-read-keyvault",
"azurerm_key_vault_access_policy.disk-encryption",
]
}
`, template, data.RandomInteger)
}

func testAccAzureRMManagedDisk_diskEncryptionSetUnencrypted(data acceptance.TestData) string {
template := testAccAzureRMManagedDisk_diskEncryptionSetDependencies(data)

return fmt.Sprintf(`
%s
resource "azurerm_managed_disk" "test" {
name = "acctestd-%d"
Expand All @@ -1028,43 +1056,92 @@ resource "azurerm_managed_disk" "test" {
storage_account_type = "Standard_LRS"
create_option = "Empty"
disk_size_gb = 1
%s
depends_on = [
"azurerm_role_assignment.disk-encryption-read-keyvault",
"azurerm_key_vault_access_policy.disk-encryption",
]
}
`, template, data.RandomInteger, data.RandomInteger, diskEncryptionSetLine)
`, template, data.RandomInteger)
}

func testAccAzureRMManagedDisk_managedDiskAttached(data acceptance.TestData, diskSize int) string {
template := testAccAzureRMManagedDisk_templateAttached(data)
return fmt.Sprintf(`
provider "azurerm" {
features {}
}
%s
resource "azurerm_managed_disk" "test" {
name = "%d-disk1"
location = azurerm_resource_group.test.location
resource_group_name = azurerm_resource_group.test.name
storage_account_type = "Standard_LRS"
create_option = "Empty"
disk_size_gb = %d
}
resource "azurerm_virtual_machine_data_disk_attachment" "test" {
managed_disk_id = azurerm_managed_disk.test.id
virtual_machine_id = azurerm_linux_virtual_machine.test.id
lun = "0"
caching = "None"
}
`, template, data.RandomInteger, diskSize)
}

func testAccAzureRMManagedDisk_storageTypeUpdateWhilstAttached(data acceptance.TestData, storageAccountType string) string {
template := testAccAzureRMManagedDisk_templateAttached(data)
return fmt.Sprintf(`
provider "azurerm" {
features {}
}
%s
resource "azurerm_managed_disk" "test" {
name = "acctestdisk-%d"
location = azurerm_resource_group.test.location
resource_group_name = azurerm_resource_group.test.name
storage_account_type = "%s"
create_option = "Empty"
disk_size_gb = 10
}
resource "azurerm_virtual_machine_data_disk_attachment" "test" {
managed_disk_id = azurerm_managed_disk.test.id
virtual_machine_id = azurerm_linux_virtual_machine.test.id
lun = "0"
caching = "None"
}
`, template, data.RandomInteger, storageAccountType)
}

func testAccAzureRMManagedDisk_templateAttached(data acceptance.TestData) string {
return fmt.Sprintf(`
resource "azurerm_resource_group" "test" {
name = "acctestRG-%[1]d"
location = "%[2]s"
name = "acctestRG-%d"
location = "%s"
}
resource "azurerm_virtual_network" "test" {
name = "acctvn-%[1]d"
name = "acctvn-%d"
address_space = ["10.0.0.0/16"]
location = azurerm_resource_group.test.location
resource_group_name = azurerm_resource_group.test.name
}
resource "azurerm_subnet" "test" {
name = "acctsub-%[1]d"
name = "internal"
resource_group_name = azurerm_resource_group.test.name
virtual_network_name = azurerm_virtual_network.test.name
address_prefix = "10.0.2.0/24"
}
resource "azurerm_network_interface" "test" {
name = "acctni-%[1]d"
name = "acctni-%d"
location = azurerm_resource_group.test.location
resource_group_name = azurerm_resource_group.test.name
Expand All @@ -1075,52 +1152,30 @@ resource "azurerm_network_interface" "test" {
}
}
resource "azurerm_virtual_machine" "test" {
name = "acctvm-%[1]d"
location = azurerm_resource_group.test.location
resource_group_name = azurerm_resource_group.test.name
network_interface_ids = [azurerm_network_interface.test.id]
vm_size = "Standard_F2"
resource "azurerm_linux_virtual_machine" "test" {
name = "acctestvm-%d"
resource_group_name = azurerm_resource_group.test.name
location = azurerm_resource_group.test.location
size = "Standard_D2s_v3"
admin_username = "adminuser"
admin_password = "Password1234!"
disable_password_authentication = false
storage_image_reference {
network_interface_ids = [
azurerm_network_interface.test.id,
]
os_disk {
caching = "ReadWrite"
storage_account_type = "Standard_LRS"
}
source_image_reference {
publisher = "Canonical"
offer = "UbuntuServer"
sku = "16.04-LTS"
version = "latest"
}
storage_os_disk {
name = "myosdisk1"
caching = "ReadWrite"
create_option = "FromImage"
managed_disk_type = "Standard_LRS"
}
os_profile {
computer_name = "hn%[1]d"
admin_username = "testadmin"
admin_password = "Password1234!"
}
os_profile_linux_config {
disable_password_authentication = false
}
}
resource "azurerm_managed_disk" "test" {
name = "%[1]d-disk1"
location = azurerm_resource_group.test.location
resource_group_name = azurerm_resource_group.test.name
storage_account_type = "Standard_LRS"
create_option = "Empty"
disk_size_gb = %[3]d
}
resource "azurerm_virtual_machine_data_disk_attachment" "test" {
managed_disk_id = azurerm_managed_disk.test.id
virtual_machine_id = azurerm_virtual_machine.test.id
lun = "0"
caching = "None"
}
`, data.RandomInteger, data.Locations.Primary, diskSize)
`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.RandomInteger, data.RandomInteger)
}
Loading

0 comments on commit 9ff4a7e

Please sign in to comment.