Skip to content

Commit 5df2a8f

Browse files
authored
[Disk Manager] fix idempotence of sync deletion of non-existent disk (#5157)
If we delete non-existent disk twice using `sync` flag, then the second deletions ends with an error: ``` disk_service_test.go:1275: Error Trace: /-S/cloud/disk_manager/internal/pkg/facade/disk_service_test/disk_service_test.go:1275 /-S/cloud/disk_manager/internal/pkg/facade/disk_service_test/disk_service_test.go:1285 Error: Received unexpected error: rpc error: code = Unknown desc = Non retriable error, Silent=false: unknown zone "", available zones: ["zone-a" "zone-b" "zone-c" "no_dataplane" "zone-d" "zone-d-shard1"] ``` Errors occures here, in `setEstimate`: https://github.com/ydb-platform/nbs/blob/main/cloud/disk_manager/internal/pkg/services/disks/delete_disk_task.go#L216 - After the first deletion tombstone is insterted into the database - On the second deletion, we call GetDiskMeta here https://github.com/ydb-platform/nbs/blob/main/cloud/disk_manager/internal/pkg/services/disks/delete_disk_task.go#L206 and get a tombstone. - The tombstone has empty zone id, so we can't create a Blockstore client with empty zone.
1 parent 1882cff commit 5df2a8f

File tree

2 files changed

+46
-1
lines changed

2 files changed

+46
-1
lines changed

cloud/disk_manager/internal/pkg/facade/disk_service_test/disk_service_test.go

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1218,3 +1218,48 @@ func TestDiskServiceShouldCreateSsdNonreplWithRootKmsEncryptionByFolderID(
12181218
nil, // encryptionDesc
12191219
)
12201220
}
1221+
1222+
func testDiskServiceDeleteNonExistentDisk(t *testing.T, sync bool) {
1223+
ctx := testcommon.NewContext()
1224+
1225+
client, err := testcommon.NewClient(ctx)
1226+
require.NoError(t, err)
1227+
defer client.Close()
1228+
1229+
diskID := t.Name() + "_NonExistent"
1230+
1231+
reqCtx := testcommon.GetRequestContext(t, ctx)
1232+
operation, err := client.DeleteDisk(reqCtx, &disk_manager.DeleteDiskRequest{
1233+
DiskId: &disk_manager.DiskId{
1234+
DiskId: diskID,
1235+
},
1236+
Sync: sync,
1237+
})
1238+
require.NoError(t, err)
1239+
require.NotEmpty(t, operation)
1240+
err = internal_client.WaitOperation(ctx, client, operation.Id)
1241+
require.NoError(t, err)
1242+
1243+
// Should be idempotent.
1244+
reqCtx = testcommon.GetRequestContext(t, ctx)
1245+
operation, err = client.DeleteDisk(reqCtx, &disk_manager.DeleteDiskRequest{
1246+
DiskId: &disk_manager.DiskId{
1247+
DiskId: diskID,
1248+
},
1249+
Sync: sync,
1250+
})
1251+
require.NoError(t, err)
1252+
require.NotEmpty(t, operation)
1253+
err = internal_client.WaitOperation(ctx, client, operation.Id)
1254+
require.NoError(t, err)
1255+
1256+
testcommon.CheckConsistency(t, ctx)
1257+
}
1258+
1259+
func TestDiskServiceDeleteNonExistentDisk(t *testing.T) {
1260+
testDiskServiceDeleteNonExistentDisk(t, false /* sync */)
1261+
}
1262+
1263+
func TestDiskServiceDeleteNonExistentDiskSync(t *testing.T) {
1264+
testDiskServiceDeleteNonExistentDisk(t, true /* sync */)
1265+
}

cloud/disk_manager/internal/pkg/services/disks/delete_disk_task.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ func (t *deleteDiskTask) setEstimate(
208208
return err
209209
}
210210

211-
if diskMeta == nil {
211+
if diskMeta == nil || len(diskMeta.ZoneID) == 0 {
212212
// Should be idempotent.
213213
return nil
214214
}

0 commit comments

Comments
 (0)