Skip to content

Commit bef57a7

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes#91948 from andyzhangx/ToBeDetached
fix: use force detach for azure disk
2 parents 694566d + 5f71032 commit bef57a7

File tree

4 files changed

+25
-14
lines changed

4 files changed

+25
-14
lines changed

‎staging/src/k8s.io/legacy-cloud-providers/azure/azure_controller_standard.go‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,8 @@ func (as *availabilitySet) AttachDisk(isManagedDisk bool, diskName, diskURI stri
4444
return err
4545
}
4646

47-
disks := filterDetachingDisks(*vm.StorageProfile.DataDisks)
47+
disks := make([]compute.DataDisk, len(*vm.StorageProfile.DataDisks))
48+
copy(disks, *vm.StorageProfile.DataDisks)
4849

4950
if isManagedDisk {
5051
managedDisk := &compute.ManagedDiskParameters{ID: &diskURI}
@@ -131,7 +132,8 @@ func (as *availabilitySet) DetachDisk(diskName, diskURI string, nodeName types.N
131132
return err
132133
}
133134

134-
disks := filterDetachingDisks(*vm.StorageProfile.DataDisks)
135+
disks := make([]compute.DataDisk, len(*vm.StorageProfile.DataDisks))
136+
copy(disks, *vm.StorageProfile.DataDisks)
135137

136138
bFoundDisk := false
137139
for i, disk := range disks {
@@ -140,7 +142,7 @@ func (as *availabilitySet) DetachDisk(diskName, diskURI string, nodeName types.N
140142
(disk.ManagedDisk != nil && diskURI != "" && strings.EqualFold(*disk.ManagedDisk.ID, diskURI)) {
141143
// found the disk
142144
klog.V(2).Infof("azureDisk - detach disk: name %q uri %q", diskName, diskURI)
143-
disks = append(disks[:i], disks[i+1:]...)
145+
disks[i].ToBeDetached = to.BoolPtr(true)
144146
bFoundDisk = true
145147
break
146148
}

‎staging/src/k8s.io/legacy-cloud-providers/azure/azure_controller_standard_test.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,10 @@ func TestStandardDetachDisk(t *testing.T) {
163163

164164
err := vmSet.DetachDisk(test.diskName, "", test.nodeName)
165165
assert.Equal(t, test.expectedError, err != nil, "TestCase[%d]: %s", i, test.desc)
166+
if !test.expectedError && test.diskName != "" {
167+
dataDisks, err := vmSet.GetDataDisks(test.nodeName, azcache.CacheReadTypeDefault)
168+
assert.Equal(t, true, len(dataDisks) == 1, "TestCase[%d]: %s, err: %v", i, test.desc, err)
169+
}
166170
}
167171
}
168172

‎staging/src/k8s.io/legacy-cloud-providers/azure/azure_controller_vmss.go‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,8 @@ func (ss *scaleSet) AttachDisk(isManagedDisk bool, diskName, diskURI string, nod
4646

4747
disks := []compute.DataDisk{}
4848
if vm.StorageProfile != nil && vm.StorageProfile.DataDisks != nil {
49-
disks = filterDetachingDisks(*vm.StorageProfile.DataDisks)
49+
disks = make([]compute.DataDisk, len(*vm.StorageProfile.DataDisks))
50+
copy(disks, *vm.StorageProfile.DataDisks)
5051
}
5152
if isManagedDisk {
5253
managedDisk := &compute.ManagedDiskParameters{ID: &diskURI}
@@ -136,7 +137,8 @@ func (ss *scaleSet) DetachDisk(diskName, diskURI string, nodeName types.NodeName
136137

137138
disks := []compute.DataDisk{}
138139
if vm.StorageProfile != nil && vm.StorageProfile.DataDisks != nil {
139-
disks = filterDetachingDisks(*vm.StorageProfile.DataDisks)
140+
disks = make([]compute.DataDisk, len(*vm.StorageProfile.DataDisks))
141+
copy(disks, *vm.StorageProfile.DataDisks)
140142
}
141143
bFoundDisk := false
142144
for i, disk := range disks {
@@ -145,7 +147,7 @@ func (ss *scaleSet) DetachDisk(diskName, diskURI string, nodeName types.NodeName
145147
(disk.ManagedDisk != nil && diskURI != "" && strings.EqualFold(*disk.ManagedDisk.ID, diskURI)) {
146148
// found the disk
147149
klog.V(2).Infof("azureDisk - detach disk: name %q uri %q", diskName, diskURI)
148-
disks = append(disks[:i], disks[i+1:]...)
150+
disks[i].ToBeDetached = to.BoolPtr(true)
149151
bFoundDisk = true
150152
break
151153
}

‎staging/src/k8s.io/legacy-cloud-providers/azure/azure_controller_vmss_test.go‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,7 @@ func TestDetachDiskWithVMSS(t *testing.T) {
141141
defer ctrl.Finish()
142142

143143
fakeStatusNotFoundVMSSName := types.NodeName("FakeStatusNotFoundVMSSName")
144+
diskName := "disk-name"
144145
testCases := []struct {
145146
desc string
146147
vmList map[string]string
@@ -156,7 +157,7 @@ func TestDetachDiskWithVMSS(t *testing.T) {
156157
vmssVMList: []string{"vmss-vm-000001"},
157158
vmssName: "vm1",
158159
vmssvmName: "vm1",
159-
existedDisk: compute.Disk{Name: to.StringPtr("disk-name")},
160+
existedDisk: compute.Disk{Name: to.StringPtr(diskName)},
160161
expectedErr: true,
161162
expectedErrMsg: fmt.Errorf("not a vmss instance"),
162163
},
@@ -165,15 +166,15 @@ func TestDetachDiskWithVMSS(t *testing.T) {
165166
vmssVMList: []string{"vmss00-vm-000000", "vmss00-vm-000001", "vmss00-vm-000002"},
166167
vmssName: "vmss00",
167168
vmssvmName: "vmss00-vm-000000",
168-
existedDisk: compute.Disk{Name: to.StringPtr("disk-name")},
169+
existedDisk: compute.Disk{Name: to.StringPtr(diskName)},
169170
expectedErr: false,
170171
},
171172
{
172173
desc: "an error shall be returned if response StatusNotFound",
173174
vmssVMList: []string{"vmss00-vm-000000", "vmss00-vm-000001", "vmss00-vm-000002"},
174175
vmssName: fakeStatusNotFoundVMSSName,
175176
vmssvmName: "vmss00-vm-000000",
176-
existedDisk: compute.Disk{Name: to.StringPtr("disk-name")},
177+
existedDisk: compute.Disk{Name: to.StringPtr(diskName)},
177178
expectedErr: true,
178179
expectedErrMsg: fmt.Errorf("Retriable: false, RetryAfter: 0s, HTTPStatusCode: 404, RawError: instance not found"),
179180
},
@@ -213,7 +214,7 @@ func TestDetachDiskWithVMSS(t *testing.T) {
213214
},
214215
DataDisks: &[]compute.DataDisk{{
215216
Lun: to.Int32Ptr(0),
216-
Name: to.StringPtr("disk-name"),
217+
Name: to.StringPtr(diskName),
217218
}},
218219
}
219220
}
@@ -225,12 +226,14 @@ func TestDetachDiskWithVMSS(t *testing.T) {
225226
mockVMSSVMClient.EXPECT().Update(gomock.Any(), testCloud.ResourceGroup, scaleSetName, gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes()
226227
}
227228

228-
diskURI := fmt.Sprintf("/subscriptions/%s/resourceGroups/%s/providers/Microsoft.Compute/disks/%s",
229-
testCloud.SubscriptionID, testCloud.ResourceGroup, *test.existedDisk.Name)
230-
231-
err = ss.DetachDisk(*test.existedDisk.Name, diskURI, test.vmssvmName)
229+
err = ss.DetachDisk(*test.existedDisk.Name, diskName, test.vmssvmName)
232230
assert.Equal(t, test.expectedErr, err != nil, "TestCase[%d]: %s, err: %v", i, test.desc, err)
233231
assert.Equal(t, test.expectedErrMsg, err, "TestCase[%d]: %s, expected error: %v, return error: %v", i, test.desc, test.expectedErrMsg, err)
232+
233+
if !test.expectedErr {
234+
dataDisks, err := ss.GetDataDisks(test.vmssvmName, azcache.CacheReadTypeDefault)
235+
assert.Equal(t, true, len(dataDisks) == 1, "TestCase[%d]: %s, actual data disk num: %d, err: %v", i, test.desc, len(dataDisks), err)
236+
}
234237
}
235238
}
236239

0 commit comments

Comments
 (0)