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

Add support for no-downtime disk resizes. #17245

Merged
merged 21 commits into from
Nov 9, 2022
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions internal/features/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ func Default() UserFeatures {
LogAnalyticsWorkspace: LogAnalyticsWorkspaceFeatures{
PermanentlyDeleteOnDestroy: true,
},
ManagedDisk: ManagedDiskFeatures{
NoDowntimeResize: true,
},
ResourceGroup: ResourceGroupFeatures{
PreventDeletionIfContainsResources: true,
},
Expand Down
5 changes: 5 additions & 0 deletions internal/features/user_flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ type UserFeatures struct {
TemplateDeployment TemplateDeploymentFeatures
LogAnalyticsWorkspace LogAnalyticsWorkspaceFeatures
ResourceGroup ResourceGroupFeatures
ManagedDisk ManagedDiskFeatures
}

type CognitiveAccountFeatures struct {
Expand Down Expand Up @@ -60,3 +61,7 @@ type ApiManagementFeatures struct {
type ApplicationInsightFeatures struct {
DisableGeneratedRule bool
}

type ManagedDiskFeatures struct {
NoDowntimeResize bool
}
25 changes: 25 additions & 0 deletions internal/provider/features.go
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,21 @@ func schemaFeatures(supportLegacyTestSuite bool) *pluginsdk.Schema {
},
},
},

"managed_disk": {
Type: pluginsdk.TypeList,
Optional: true,
MaxItems: 1,
Elem: &pluginsdk.Resource{
Schema: map[string]*pluginsdk.Schema{
"no_downtime_resize": {
Copy link
Member

Choose a reason for hiding this comment

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

it appears the product name for this has settled on Expand Without Downtime as such we should make this:

Suggested change
"no_downtime_resize": {
"expand_without_downtime": {

Type: pluginsdk.TypeBool,
Optional: true,
Default: true,
},
},
},
},
}

// this is a temporary hack to enable us to gradually add provider blocks to test configurations
Expand Down Expand Up @@ -407,5 +422,15 @@ func expandFeatures(input []interface{}) features.UserFeatures {
}
}

if raw, ok := val["managed_disk"]; ok {
items := raw.([]interface{})
if len(items) > 0 {
managedDiskRaw := items[0].(map[string]interface{})
if v, ok := managedDiskRaw["no_downtime_resize"]; ok {
featuresMap.ManagedDisk.NoDowntimeResize = v.(bool)
}
}
}

return featuresMap
}
84 changes: 84 additions & 0 deletions internal/provider/features_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ func TestExpandFeatures(t *testing.T) {
LogAnalyticsWorkspace: features.LogAnalyticsWorkspaceFeatures{
PermanentlyDeleteOnDestroy: true,
},
ManagedDisk: features.ManagedDiskFeatures{
NoDowntimeResize: true,
},
TemplateDeployment: features.TemplateDeploymentFeatures{
DeleteNestedItemsDuringDeletion: true,
},
Expand Down Expand Up @@ -98,6 +101,11 @@ func TestExpandFeatures(t *testing.T) {
"permanently_delete_on_destroy": true,
},
},
"managed_disk": []interface{}{
map[string]interface{}{
"no_downtime_resize": true,
},
},
"network": []interface{}{
map[string]interface{}{
"relaxed_locking": true,
Expand Down Expand Up @@ -154,6 +162,9 @@ func TestExpandFeatures(t *testing.T) {
LogAnalyticsWorkspace: features.LogAnalyticsWorkspaceFeatures{
PermanentlyDeleteOnDestroy: true,
},
ManagedDisk: features.ManagedDiskFeatures{
NoDowntimeResize: true,
},
ResourceGroup: features.ResourceGroupFeatures{
PreventDeletionIfContainsResources: true,
},
Expand Down Expand Up @@ -210,6 +221,11 @@ func TestExpandFeatures(t *testing.T) {
"permanently_delete_on_destroy": false,
},
},
"managed_disk": []interface{}{
map[string]interface{}{
"no_downtime_resize": false,
},
},
"network_locking": []interface{}{
map[string]interface{}{
"relaxed_locking": false,
Expand Down Expand Up @@ -266,6 +282,9 @@ func TestExpandFeatures(t *testing.T) {
LogAnalyticsWorkspace: features.LogAnalyticsWorkspaceFeatures{
PermanentlyDeleteOnDestroy: false,
},
ManagedDisk: features.ManagedDiskFeatures{
NoDowntimeResize: false,
},
ResourceGroup: features.ResourceGroupFeatures{
PreventDeletionIfContainsResources: false,
},
Expand Down Expand Up @@ -1025,3 +1044,68 @@ func TestExpandFeaturesResourceGroup(t *testing.T) {
}
}
}

func TestExpandFeaturesManagedDisk(t *testing.T) {
testData := []struct {
Name string
Input []interface{}
EnvVars map[string]interface{}
Expected features.UserFeatures
}{
{
Name: "Empty Block",
Input: []interface{}{
map[string]interface{}{
"managed_disk": []interface{}{},
},
},
Expected: features.UserFeatures{
ManagedDisk: features.ManagedDiskFeatures{
NoDowntimeResize: true,
},
},
},
{
Name: "No Downtime Resize Enabled",
Input: []interface{}{
map[string]interface{}{
"managed_disk": []interface{}{
map[string]interface{}{
"no_downtime_resize": true,
},
},
},
},
Expected: features.UserFeatures{
ManagedDisk: features.ManagedDiskFeatures{
NoDowntimeResize: true,
},
},
},
{
Name: "No Downtime Resize Disabled",
Input: []interface{}{
map[string]interface{}{
"managed_disk": []interface{}{
map[string]interface{}{
"no_downtime_resize": false,
},
},
},
},
Expected: features.UserFeatures{
ManagedDisk: features.ManagedDiskFeatures{
NoDowntimeResize: false,
},
},
},
}

for _, testCase := range testData {
t.Logf("[DEBUG] Test Case: %q", testCase.Name)
result := expandFeatures(testCase.Input)
if !reflect.DeepEqual(result.ManagedDisk, testCase.Expected.ManagedDisk) {
t.Fatalf("Expected %+v but got %+v", result.ManagedDisk, testCase.Expected.ManagedDisk)
}
}
}
28 changes: 27 additions & 1 deletion internal/services/compute/managed_disk_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -673,7 +673,9 @@ func resourceManagedDiskUpdate(d *pluginsdk.ResourceData, meta interface{}) erro

if d.HasChange("disk_size_gb") {
if old, new := d.GetChange("disk_size_gb"); new.(int) > old.(int) {
shouldShutDown = true
if !meta.(*clients.Client).Features.ManagedDisk.NoDowntimeResize || shutDownOnResize(disk, old.(int), new.(int)) {
shouldShutDown = true
}
diskUpdate.Properties.DiskSizeGB = utils.Int64(int64(new.(int)))
} else {
return fmt.Errorf("- New size must be greater than original size. Shrinking disks is not supported on Azure")
Expand Down Expand Up @@ -1000,3 +1002,27 @@ func resourceManagedDiskDelete(d *pluginsdk.ResourceData, meta interface{}) erro

return nil
}

// shutDownOnResize implements live resize restrictions according to https://docs.microsoft.com/en-us/azure/virtual-machines/linux/expand-disks#expand-without-downtime
func shutDownOnResize(disk compute.Disk, oldSizeGB, newSizeGB int) bool {
// OS disks can't be expanded without downtime.
if disk.OsType != "" {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tombuildsstuff - let me know if you have suggestions for a better way of detecting data disks.

return true
}
// Disks smaller than 4 TiB can't be expanded to 4 TiB or larger without downtime.
if oldSizeGB < 4096 && newSizeGB >= 4096 {
return true
}
// Only Premium SSD v1 and Standard SSD disks support live resize
for _, diskType := range []compute.DiskStorageAccountTypes{
compute.DiskStorageAccountTypesPremiumLRS,
compute.DiskStorageAccountTypesPremiumZRS,
compute.DiskStorageAccountTypesStandardSSDLRS,
compute.DiskStorageAccountTypesStandardSSDZRS,
} {
if disk.Sku.Name == diskType {
return false
}
}
return true
}
12 changes: 12 additions & 0 deletions website/docs/guides/features-block.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ provider "azurerm" {
permanently_delete_on_destroy = true
}

managed_disk {
no_downtime_resize = true
}

resource_group {
prevent_deletion_if_contains_resources = true
}
Expand Down Expand Up @@ -86,6 +90,8 @@ The `features` block supports the following:

* `log_analytics_workspace` - (Optional) A `log_analytics_workspace` block as defined below.

* `managed_disk` - (Optional) A `managed_disk` block as defined below.

* `resource_group` - (Optional) A `resource_group` block as defined below.

* `template_deployment` - (Optional) A `template_deployment` block as defined below.
Expand Down Expand Up @@ -150,6 +156,12 @@ The `log_analytics_workspace` block supports the following:

---

The `managed_disk` block supports the following:

* `no_downtime_resize` (Optional) Specifies if no-downtime resizes are enabled for the managed disk resources. Defaults to `true`.
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe use the same name as azure does for this feature enable_live_resize or something like that?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah agreed - digging around this it appears that Expand Without Downtime is the final name for this feature? I'd argue that live_resize_data_disks_where_possible (or something) would be clearer where possible, but since this is only for expanding (and not shrinking, which Azure doesn't support regardless of if the associated VM is online/offline) - I think it's probably worth making this expand_without_downtime for now?


---

The `resource_group` block supports the following:

* `prevent_deletion_if_contains_resources` - (Optional) Should the `azurerm_resource_group` resource check that there are no Resources within the Resource Group during deletion? This means that all Resources within the Resource Group must be deleted prior to deleting the Resource Group. Defaults to `true`.
Expand Down