data disk: add deletionPolicy logic

It adds logic to allow specifying the deletionPolicy field for a Data Disk and pass it down to the provider. It's part of the changes proposed in openshift/enhancements#1070
damdo · Mar 24, 2022 · 15161f5 · 15161f5
1 parent c7c196c
commit 15161f5
Show file tree

Hide file tree

Showing 2 changed files with 116 additions and 5 deletions.
diff --git a/pkg/cloud/azure/services/virtualmachines/virtualmachines.go b/pkg/cloud/azure/services/virtualmachines/virtualmachines.go
@@ -502,6 +502,15 @@ func generateDataDisks(vmSpec *Spec) ([]compute.DataDisk, error) {
 				dataDiskName, vmSpec.Name, disk.Lun)
 		}
 
+		switch disk.DeletionPolicy {
+		case machinev1.DiskDeletionPolicyTypeDelete, machinev1.DiskDeletionPolicyTypeDetach:
+			// valid
+		default:
+			return nil, apierrors.InvalidMachineConfiguration("failed to create Data Disk: %s for vm %s. "+
+				"Invalid value `deletionPolicy`: \"%s\". Valid values are \"%s\",\"%s\".",
+				dataDiskName, vmSpec.Name, disk.DeletionPolicy, machinev1.DiskDeletionPolicyTypeDelete, machinev1.DiskDeletionPolicyTypeDetach)
+		}
+
 		seenDataDiskNames[disk.NameSuffix] = struct{}{}
 		seenDataDiskLuns[disk.Lun] = struct{}{}
 
@@ -511,6 +520,7 @@ func generateDataDisks(vmSpec *Spec) ([]compute.DataDisk, error) {
 			Lun:          to.Int32Ptr(disk.Lun),
 			Name:         to.StringPtr(dataDiskName),
 			Caching:      compute.CachingTypes(disk.CachingType),
+			DeleteOption: compute.DiskDeleteOptionTypes(disk.DeletionPolicy),
 		}
 
 		dataDisks[i].ManagedDisk = &compute.ManagedDiskParameters{

diff --git a/pkg/cloud/azure/services/virtualmachines/virtualmachines_test.go b/pkg/cloud/azure/services/virtualmachines/virtualmachines_test.go
@@ -122,6 +122,7 @@ func TestDeriveVirtualMachineParameters(t *testing.T) {
 						ManagedDisk: machinev1.DataDiskManagedDiskParameters{
 							StorageAccountType: machinev1.StorageAccountUltraSSDLRS,
 						},
+						DeletionPolicy: machinev1.DiskDeletionPolicyTypeDelete,
 					},
 				}
 			},
@@ -140,6 +141,7 @@ func TestDeriveVirtualMachineParameters(t *testing.T) {
 						ManagedDisk: machinev1.DataDiskManagedDiskParameters{
 							StorageAccountType: machinev1.StorageAccountUltraSSDLRS,
 						},
+						DeletionPolicy: machinev1.DiskDeletionPolicyTypeDelete,
 					},
 				}
 			},
@@ -177,6 +179,7 @@ func TestDeriveVirtualMachineParameters(t *testing.T) {
 						ManagedDisk: machinev1.DataDiskManagedDiskParameters{
 							StorageAccountType: machinev1.StorageAccountUltraSSDLRS,
 						},
+						DeletionPolicy: machinev1.DiskDeletionPolicyTypeDelete,
 					},
 				}
 			},
@@ -195,6 +198,7 @@ func TestDeriveVirtualMachineParameters(t *testing.T) {
 						ManagedDisk: machinev1.DataDiskManagedDiskParameters{
 							StorageAccountType: machinev1.StorageAccountPremiumLRS,
 						},
+						DeletionPolicy: machinev1.DiskDeletionPolicyTypeDelete,
 					},
 				}
 			},
@@ -206,6 +210,48 @@ func TestDeriveVirtualMachineParameters(t *testing.T) {
 				}
 			},
 		},
+		{
+			name: "When Data Disk deletionPolicy is Detach",
+			updateSpec: func(vmSpec *Spec) {
+				vmSpec.Name = "testvm"
+				vmSpec.DataDisks = []machinev1.DataDisk{
+					{
+						NameSuffix: "datadisk-test",
+						DiskSizeGB: 4,
+						Lun:        0,
+						ManagedDisk: machinev1.DataDiskManagedDiskParameters{
+							StorageAccountType: machinev1.StorageAccountUltraSSDLRS,
+						},
+						DeletionPolicy: machinev1.DiskDeletionPolicyTypeDetach,
+					},
+				}
+			},
+			validate: func(g *WithT, vm *compute.VirtualMachine) {
+				g.Expect(vm.StorageProfile.DataDisks).ToNot(BeNil())
+				g.Expect((*vm.StorageProfile.DataDisks)[0].DeleteOption).To(BeIdenticalTo(compute.DiskDeleteOptionTypesDetach))
+			},
+		},
+		{
+			name: "When Data Disk deletionPolicy is Delete",
+			updateSpec: func(vmSpec *Spec) {
+				vmSpec.Name = "testvm"
+				vmSpec.DataDisks = []machinev1.DataDisk{
+					{
+						NameSuffix: "datadisk-test",
+						DiskSizeGB: 4,
+						Lun:        0,
+						ManagedDisk: machinev1.DataDiskManagedDiskParameters{
+							StorageAccountType: machinev1.StorageAccountUltraSSDLRS,
+						},
+						DeletionPolicy: machinev1.DiskDeletionPolicyTypeDelete,
+					},
+				}
+			},
+			validate: func(g *WithT, vm *compute.VirtualMachine) {
+				g.Expect(vm.StorageProfile.DataDisks).ToNot(BeNil())
+				g.Expect((*vm.StorageProfile.DataDisks)[0].DeleteOption).To(BeIdenticalTo(compute.DiskDeleteOptionTypesDelete))
+			},
+		},
 		{
 			name: "Error when Data Disk lun is too high",
 			updateSpec: func(vmSpec *Spec) {
@@ -218,6 +264,7 @@ func TestDeriveVirtualMachineParameters(t *testing.T) {
 						ManagedDisk: machinev1.DataDiskManagedDiskParameters{
 							StorageAccountType: machinev1.StorageAccountPremiumLRS,
 						},
+						DeletionPolicy: machinev1.DiskDeletionPolicyTypeDelete,
 					},
 				}
 			},
@@ -237,6 +284,7 @@ func TestDeriveVirtualMachineParameters(t *testing.T) {
 						ManagedDisk: machinev1.DataDiskManagedDiskParameters{
 							StorageAccountType: machinev1.StorageAccountPremiumLRS,
 						},
+						DeletionPolicy: machinev1.DiskDeletionPolicyTypeDelete,
 					},
 				}
 			},
@@ -256,6 +304,7 @@ func TestDeriveVirtualMachineParameters(t *testing.T) {
 						ManagedDisk: machinev1.DataDiskManagedDiskParameters{
 							StorageAccountType: machinev1.StorageAccountPremiumLRS,
 						},
+						DeletionPolicy: machinev1.DiskDeletionPolicyTypeDelete,
 					},
 					{
 						NameSuffix: "datadisk-test-2",
@@ -264,6 +313,7 @@ func TestDeriveVirtualMachineParameters(t *testing.T) {
 						ManagedDisk: machinev1.DataDiskManagedDiskParameters{
 							StorageAccountType: machinev1.StorageAccountPremiumLRS,
 						},
+						DeletionPolicy: machinev1.DiskDeletionPolicyTypeDelete,
 					},
 				}
 			},
@@ -284,6 +334,7 @@ func TestDeriveVirtualMachineParameters(t *testing.T) {
 						ManagedDisk: machinev1.DataDiskManagedDiskParameters{
 							StorageAccountType: machinev1.StorageAccountPremiumLRS,
 						},
+						DeletionPolicy: machinev1.DiskDeletionPolicyTypeDelete,
 					},
 					{
 						NameSuffix: "datadisk-test",
@@ -292,6 +343,7 @@ func TestDeriveVirtualMachineParameters(t *testing.T) {
 						ManagedDisk: machinev1.DataDiskManagedDiskParameters{
 							StorageAccountType: machinev1.StorageAccountPremiumLRS,
 						},
+						DeletionPolicy: machinev1.DiskDeletionPolicyTypeDelete,
 					},
 				}
 			},
@@ -312,6 +364,7 @@ func TestDeriveVirtualMachineParameters(t *testing.T) {
 						ManagedDisk: machinev1.DataDiskManagedDiskParameters{
 							StorageAccountType: machinev1.StorageAccountPremiumLRS,
 						},
+						DeletionPolicy: machinev1.DiskDeletionPolicyTypeDelete,
 					},
 				}
 			},
@@ -333,6 +386,7 @@ func TestDeriveVirtualMachineParameters(t *testing.T) {
 						ManagedDisk: machinev1.DataDiskManagedDiskParameters{
 							StorageAccountType: machinev1.StorageAccountPremiumLRS,
 						},
+						DeletionPolicy: machinev1.DiskDeletionPolicyTypeDelete,
 					},
 				}
 			},
@@ -353,7 +407,8 @@ func TestDeriveVirtualMachineParameters(t *testing.T) {
 						ManagedDisk: machinev1.DataDiskManagedDiskParameters{
 							StorageAccountType: machinev1.StorageAccountUltraSSDLRS,
 						},
-						CachingType: machinev1.CachingTypeReadOnly,
+						CachingType:    machinev1.CachingTypeReadOnly,
+						DeletionPolicy: machinev1.DiskDeletionPolicyTypeDelete,
 					},
 				}
 			},
@@ -376,7 +431,8 @@ func TestDeriveVirtualMachineParameters(t *testing.T) {
 						ManagedDisk: machinev1.DataDiskManagedDiskParameters{
 							StorageAccountType: machinev1.StorageAccountUltraSSDLRS,
 						},
-						CachingType: machinev1.CachingTypeReadOnly,
+						CachingType:    machinev1.CachingTypeReadOnly,
+						DeletionPolicy: machinev1.DiskDeletionPolicyTypeDelete,
 					},
 				}
 			},
@@ -385,6 +441,50 @@ func TestDeriveVirtualMachineParameters(t *testing.T) {
 					"`diskSizeGB`: %d, is invalid, disk size must be greater or equal than 4.",
 					"testvm"+"_"+"datadisk-test", "testvm", 3)),
 		},
+		{
+			name: "Error when Data Disk deletionPolicy is invalid",
+			updateSpec: func(vmSpec *Spec) {
+				vmSpec.Name = "testvm"
+				vmSpec.DataDisks = []machinev1.DataDisk{
+					{
+						NameSuffix: "datadisk-test",
+						DiskSizeGB: 4,
+						Lun:        0,
+						ManagedDisk: machinev1.DataDiskManagedDiskParameters{
+							StorageAccountType: machinev1.StorageAccountUltraSSDLRS,
+						},
+						DeletionPolicy: "bla",
+					},
+				}
+			},
+			expectedError: fmt.Errorf("failed to generate data disk spec: %w",
+				apierrors.InvalidMachineConfiguration("failed to create Data Disk: %s for vm %s. "+
+					"Invalid value `deletionPolicy`: \"%s\". Valid values are \"%s\",\"%s\".",
+					"testvm"+"_"+"datadisk-test", "testvm",
+					"bla", machinev1.DiskDeletionPolicyTypeDelete, machinev1.DiskDeletionPolicyTypeDetach)),
+		},
+		{
+			name: "Error when Data Disk deletionPolicy is empty",
+			updateSpec: func(vmSpec *Spec) {
+				vmSpec.Name = "testvm"
+				vmSpec.DataDisks = []machinev1.DataDisk{
+					{
+						NameSuffix: "datadisk-test",
+						DiskSizeGB: 4,
+						Lun:        0,
+						ManagedDisk: machinev1.DataDiskManagedDiskParameters{
+							StorageAccountType: machinev1.StorageAccountUltraSSDLRS,
+						},
+						DeletionPolicy: "",
+					},
+				}
+			},
+			expectedError: fmt.Errorf("failed to generate data disk spec: %w",
+				apierrors.InvalidMachineConfiguration("failed to create Data Disk: %s for vm %s. "+
+					"Invalid value `deletionPolicy`: \"%s\". Valid values are \"%s\",\"%s\".",
+					"testvm"+"_"+"datadisk-test", "testvm",
+					"", machinev1.DiskDeletionPolicyTypeDelete, machinev1.DiskDeletionPolicyTypeDetach)),
+		},
 	}
 
 	for _, tc := range testCases {
@@ -450,9 +550,10 @@ func getTestVMSpec(updateSpec func(*Spec)) *Spec {
 		},
 		DataDisks: []machinev1.DataDisk{
 			{
-				DiskSizeGB: 4,
-				NameSuffix: "testdata",
-				Lun:        0,
+				DiskSizeGB:     4,
+				NameSuffix:     "testdata",
+				Lun:            0,
+				DeletionPolicy: machinev1.DiskDeletionPolicyTypeDelete,
 			},
 		},
 		CustomData:      "",