<fix>[storage]: ZSTAC-86038 recover stale XInfini image cache

plugin/xinfini/src/main/java/org/zstack/xinfini/XInfiniApiHelper.java

@@ -9,6 +9,7 @@
 import org.zstack.core.retry.RetryCondition;
 import org.zstack.header.core.Completion;
 import org.zstack.header.errorcode.OperationFailureException;
+import org.zstack.header.errorcode.SysErrors;
 import org.zstack.header.volume.VolumeConfigs;
 import org.zstack.header.volume.VolumeVO;
 import org.zstack.header.volume.VolumeVO_;
@@ -36,6 +37,7 @@
 import java.util.concurrent.TimeUnit;
 import java.util.stream.Collectors;
 
+import static org.zstack.core.Platform.err;
 import static org.zstack.core.Platform.operr;
 import static org.zstack.utils.clouderrorcode.CloudOperationsErrorCode.*;
 
@@ -102,6 +104,11 @@ public <T extends XInfiniResponse> void call(XInfiniRequest req, Completion comp
     }
 
     public void errorOut(XInfiniResponse rsp) {
+        if (rsp.resourceIsDeleted()) {
+            throw new OperationFailureException(err(ORG_ZSTACK_XINFINI_10002, SysErrors.RESOURCE_NOT_FOUND,
+                    "xinfini request failed, message: %s.", rsp.getMessage()));
+        }
+
         if (!rsp.isSuccess()) {
             throw new OperationFailureException(operr(ORG_ZSTACK_XINFINI_10002, "xinfini request failed, message: %s.", rsp.getMessage()));
         }

plugin/xinfini/src/main/java/org/zstack/xinfini/XInfiniStorageController.java

@@ -852,6 +852,10 @@ public void flattenVolume(String installPath, ReturnValueCompletion<VolumeStats>
 
     @Override
     public void stats(String installPath, ReturnValueCompletion<VolumeStats> comp) {
+        if (installPath.contains("@")) {
+            apiHelper.getVolumeSnapshot(getSnapIdFromPath(installPath));
+        }
+
         VolumeModule vol = apiHelper.getVolume(getVolIdFromPath(installPath));
         VolumeStats stats = new VolumeStats();
         stats.setInstallPath(installPath);

storage/src/main/java/org/zstack/storage/addon/primary/ExternalPrimaryStorage.java

@@ -28,6 +28,7 @@
 import org.zstack.header.errorcode.ErrorCode;
 import org.zstack.header.errorcode.ErrorCodeList;
 import org.zstack.header.errorcode.OperationFailureException;
+import org.zstack.header.errorcode.SysErrors;
 import org.zstack.header.host.HostInventory;
 import org.zstack.header.host.HostState;
 import org.zstack.header.host.HostStatus;
@@ -1397,19 +1398,67 @@ private void doDownloadImageCache(ImageInventory image, String allocatedUrl, Ret
                 .eq(ImageCacheVO_.imageUuid, image.getUuid())
                 .list();
 
+        ImageCacheVO candidate = null;
         if (!controller.reportCapabilities().isSupportMultiSpace() && !caches.isEmpty()) {
-            // TODO check exists in ps
-            completion.success(caches.get(0).toInventory());
-            return;
+            candidate = caches.get(0);
+        } else {
+            for (ImageCacheVO cache : caches) {
+                ImageCacheInventory inv = cache.toInventory();
+                // TODO: suppose that path always starts with allocatedUrl
+                if (allocatedUrl != null && ImageCacheUtil.getImageCachePath(inv).startsWith(allocatedUrl)) {
+                    candidate = cache;
+                    break;
+                }
+            }
         }
 
-        for (ImageCacheVO cache : caches) {
-            ImageCacheInventory inv = cache.toInventory();
-            // TODO: suppose that path always starts with allocatedUrl
-            if (allocatedUrl != null && ImageCacheUtil.getImageCachePath(inv).startsWith(allocatedUrl)) {
-                completion.success(inv);
-                return;
+        if (candidate != null) {
+            ImageCacheVO cache = candidate;
+            String cachePath = ImageCacheUtil.getImageCachePath(cache.getInstallUrl());
+            ReturnValueCompletion<VolumeStats> statsCompletion = new ReturnValueCompletion<VolumeStats>(completion) {
+                @Override
+                public void success(VolumeStats ignored) {
+                    completion.success(cache.toInventory());
+                }
+
+                @Override
+                public void fail(ErrorCode errorCode) {
+                    if (!errorCode.isError(SysErrors.RESOURCE_NOT_FOUND)) {
+                        completion.fail(errorCode);
+                        return;
+                    }
+
+                    if (cache.getInstallUrl().startsWith(ImageConstant.SNAPSHOT_REUSE_IMAGE_SCHEMA)) {
+                        completion.fail(operr("image cache[imageUuid:%s, primaryStorageUuid:%s] refers to a missing reused volume snapshot",
+                                cache.getImageUuid(), cache.getPrimaryStorageUuid()));
+                        return;
+                    }
+
+                    try {
+                        controller.deleteVolumeAndSnapshot(cachePath, new Completion(completion) {
+                            @Override
+                            public void success() {
+                                dbf.removeByPrimaryKey(cache.getId(), ImageCacheVO.class);
+                                doDownloadImageCache(image, allocatedUrl, completion);
+                            }
+
+                            @Override
+                            public void fail(ErrorCode errorCode) {
+                                completion.fail(errorCode);
+                            }
+                        });
+                    } catch (OperationFailureException e) {
+                        completion.fail(e.getErrorCode());
+                    }
+                }
+            };
+
+            try {
+                controller.stats(cachePath, statsCompletion);
+            } catch (OperationFailureException e) {
+                statsCompletion.fail(e.getErrorCode());
             }
+            return;
         }
 
         downloadImageTo(image, spec, ImageCacheVO.class.getSimpleName(), new ReturnValueCompletion<VolumeStats>(completion) {

test/src/test/groovy/org/zstack/test/integration/storage/primary/addon/xinfini/XinfiniPrimaryStorageCase.groovy

@@ -19,6 +19,7 @@ import org.zstack.header.host.HostConstant
 import org.zstack.header.host.HostVO
 import org.zstack.header.host.HostVO_
 import org.zstack.header.host.PingHostMsg
+import org.zstack.header.image.ImageConstant
 import org.zstack.header.message.MessageReply
 import org.zstack.header.storage.backup.DownloadImageFromRemoteTargetMsg
 import org.zstack.header.storage.backup.DownloadImageFromRemoteTargetReply
@@ -195,6 +196,8 @@ class XinfiniPrimaryStorageCase extends SubCase {
             testCreateDataVolume()
             testCreateSnapshot()
             testCreateTemplate()
+            testRecreateVmWhenImageCacheSnapshotMissing()
+            testMissingReusedSnapshotDoesNotDeleteSourceVolume()
             testClean()
             testImageCacheClean()
             testDeletePs()
@@ -283,12 +286,7 @@ class XinfiniPrimaryStorageCase extends SubCase {
 
         assert result.getRootVolumePrimaryStorages().size() == 1
 
-        env.message(UploadImageToRemoteTargetMsg.class) { UploadImageToRemoteTargetMsg msg, CloudBus bus ->
-            UploadImageToRemoteTargetReply r = new UploadImageToRemoteTargetReply()
-            assert msg.getRemoteTargetUrl().startsWith(exportProtocol)
-            assert msg.getFormat() == "raw"
-            bus.reply(msg, r)
-        }
+        mockUploadImageToRemoteTarget()
 
         env.afterSimulator(KVMConstant.KVM_START_VM_PATH) { rsp, HttpEntity<String> e ->
             def cmd = JSONObjectUtil.toObject(e.body, KVMAgentCommands.StartVmCmd.class)
@@ -351,6 +349,125 @@ class XinfiniPrimaryStorageCase extends SubCase {
         deleteVm(vm2.uuid)
     }
 
+    void mockUploadImageToRemoteTarget() {
+        env.message(UploadImageToRemoteTargetMsg.class) { UploadImageToRemoteTargetMsg msg, CloudBus bus ->
+            UploadImageToRemoteTargetReply r = new UploadImageToRemoteTargetReply()
+            assert msg.getRemoteTargetUrl().startsWith(exportProtocol)
+            assert msg.getFormat() == "raw"
+            bus.reply(msg, r)
+        }
+    }
+
+    void testRecreateVmWhenImageCacheSnapshotMissing() {
+        List<ImageCacheVO> imageCaches = Q.New(ImageCacheVO.class)
+                .eq(ImageCacheVO_.imageUuid, image.uuid)
+                .eq(ImageCacheVO_.primaryStorageUuid, ps.uuid)
+                .list()
+        assert imageCaches.size() == 1 :
+                "image cache precondition is invalid: expected=1 actual=${imageCaches.size()} imageUuid=${image.uuid} primaryStorageUuid=${ps.uuid}"
+
+        ImageCacheVO oldImageCache = imageCaches[0]
+        int oldSnapshotId = XInfiniPathHelper.getSnapIdFromPath(oldImageCache.installUrl)
+        int backingVolumeId = XInfiniPathHelper.getVolIdFromPath(oldImageCache.installUrl)
+        assert ExternalPrimaryStorageSpec.XinfiniSimulators.snapshots.containsKey(oldSnapshotId) :
+                "image cache snapshot precondition is invalid: expected snapshotId=${oldSnapshotId} to exist actualSnapshotIds=${ExternalPrimaryStorageSpec.XinfiniSimulators.snapshots.keySet()} installUrl=${oldImageCache.installUrl}"
+
+        ExternalPrimaryStorageSpec.XinfiniSimulators.snapshots.remove(oldSnapshotId)
+        long retainedImageCacheCount = Q.New(ImageCacheVO.class)
+                .eq(ImageCacheVO_.imageUuid, image.uuid)
+                .eq(ImageCacheVO_.primaryStorageUuid, ps.uuid)
+                .count()
+        assert !ExternalPrimaryStorageSpec.XinfiniSimulators.snapshots.containsKey(oldSnapshotId) :
+                "stale image cache setup must remove only its snapshot: expected snapshotId=${oldSnapshotId} to be absent actualSnapshotIds=${ExternalPrimaryStorageSpec.XinfiniSimulators.snapshots.keySet()}"
+        assert retainedImageCacheCount == 1 :
+                "stale image cache setup must retain its database record: expected=1 actual=${retainedImageCacheCount} imageUuid=${image.uuid} primaryStorageUuid=${ps.uuid}"
+        assert ExternalPrimaryStorageSpec.XinfiniSimulators.volumes.containsKey(backingVolumeId) :
+                "stale image cache setup must retain its backing volume: expected volumeId=${backingVolumeId} to exist actualVolumeIds=${ExternalPrimaryStorageSpec.XinfiniSimulators.volumes.keySet()} installUrl=${oldImageCache.installUrl}"
+
+        mockUploadImageToRemoteTarget()
+
+        def recreatedVm = createVmInstance {
+            name = "vm-from-stale-image-cache"
+            instanceOfferingUuid = instanceOffering.uuid
+            imageUuid = image.uuid
+            l3NetworkUuids = [l3.uuid]
+            hostUuid = host1.uuid
+            primaryStorageUuidForRootVolume = ps.uuid
+        } as VmInstanceInventory
+
+        List<ImageCacheVO> refreshedImageCaches = Q.New(ImageCacheVO.class)
+                .eq(ImageCacheVO_.imageUuid, image.uuid)
+                .eq(ImageCacheVO_.primaryStorageUuid, ps.uuid)
+                .list()
+        assert refreshedImageCaches.size() == 1 :
+                "stale image cache must be replaced in place: expected=1 actual=${refreshedImageCaches.size()} imageUuid=${image.uuid} primaryStorageUuid=${ps.uuid}"
+
+        int newSnapshotId = XInfiniPathHelper.getSnapIdFromPath(refreshedImageCaches[0].installUrl)
+        assert newSnapshotId != oldSnapshotId :
+                "stale image cache snapshot must be recreated: expectedSnapshotId!=${oldSnapshotId} actualSnapshotId=${newSnapshotId} installUrl=${refreshedImageCaches[0].installUrl}"
+        assert ExternalPrimaryStorageSpec.XinfiniSimulators.snapshots.containsKey(newSnapshotId) :
+                "refreshed image cache must reference an existing snapshot: expected snapshotId=${newSnapshotId} to exist actualSnapshotIds=${ExternalPrimaryStorageSpec.XinfiniSimulators.snapshots.keySet()} installUrl=${refreshedImageCaches[0].installUrl}"
+        assert !ExternalPrimaryStorageSpec.XinfiniSimulators.volumes.containsKey(backingVolumeId) :
+                "stale image cache backing volume must be deleted before redownload: expected volumeId=${backingVolumeId} to be absent actualVolumeIds=${ExternalPrimaryStorageSpec.XinfiniSimulators.volumes.keySet()}"
+
+        deleteVm(recreatedVm.uuid)
+    }
+
+    void testMissingReusedSnapshotDoesNotDeleteSourceVolume() {
+        ImageCacheVO cache = Q.New(ImageCacheVO.class)
+                .eq(ImageCacheVO_.imageUuid, image.uuid)
+                .eq(ImageCacheVO_.primaryStorageUuid, ps.uuid)
+                .find()
+        assert cache != null
+
+        def sourceSnapshot = createVolumeSnapshot {
+            name = "snapshot-reuse-source"
+            volumeUuid = vol.uuid
+        } as VolumeSnapshotInventory
+        int sourceVolumeId = XInfiniPathHelper.getVolIdFromPath(sourceSnapshot.primaryStorageInstallPath)
+        int sourceSnapshotId = XInfiniPathHelper.getSnapIdFromPath(sourceSnapshot.primaryStorageInstallPath)
+        String originalInstallUrl = cache.installUrl
+        String reuseInstallUrl = ImageConstant.SNAPSHOT_REUSE_IMAGE_SCHEMA + sourceSnapshot.uuid
+
+        try {
+            SQL.New(ImageCacheVO.class)
+                    .set(ImageCacheVO_.installUrl, reuseInstallUrl)
+                    .eq(ImageCacheVO_.id, cache.id)
+                    .update()
+            ExternalPrimaryStorageSpec.XinfiniSimulators.snapshots.remove(sourceSnapshotId)
+
+            CreateVmInstanceAction action = new CreateVmInstanceAction()
+            action.name = "vm-from-missing-reused-snapshot"
+            action.instanceOfferingUuid = instanceOffering.uuid
+            action.imageUuid = image.uuid
+            action.l3NetworkUuids = [l3.uuid]
+            action.hostUuid = host1.uuid
+            action.primaryStorageUuidForRootVolume = ps.uuid
+            action.sessionId = adminSession()
+
+            CreateVmInstanceAction.Result result = action.call()
+            assert result.error != null
+            assert JSONObjectUtil.toJsonString(result.error).contains("missing reused volume snapshot")
+
+            ImageCacheVO retainedCache = Q.New(ImageCacheVO.class).eq(ImageCacheVO_.id, cache.id).find()
+            assert retainedCache != null
+            assert retainedCache.installUrl == reuseInstallUrl
+            assert ExternalPrimaryStorageSpec.XinfiniSimulators.volumes.containsKey(sourceVolumeId)
+            assert Q.New(ImageCacheVO.class)
+                    .eq(ImageCacheVO_.imageUuid, image.uuid)
+                    .eq(ImageCacheVO_.primaryStorageUuid, ps.uuid)
+                    .count() == 1
+        } finally {
+            SQL.New(ImageCacheVO.class)
+                    .set(ImageCacheVO_.installUrl, originalInstallUrl)
+                    .eq(ImageCacheVO_.id, cache.id)
+                    .update()
+            deleteVolumeSnapshot {
+                uuid = sourceSnapshot.uuid
+            }
+        }
+    }
+
     // when the old host's storage client cannot be isolated, deactivate must report
     // a failure instead of silently succeeding, otherwise the VM gets started on a
     // new host while the old QEMU still holds the volume (split-brain).

testlib/src/main/java/org/zstack/testlib/ExternalPrimaryStorageSpec.groovy

@@ -1090,16 +1090,14 @@ class ExternalPrimaryStorageSpec extends PrimaryStorageSpec {
             simulator("/afa/v1/bs-volumes/:clone") { HttpServletRequest req, HttpEntity<String> e, EnvSpec spec ->
                 def body = JSONObjectUtil.toObject(e.body, LinkedHashMap.class)
                 def specData = body?.spec ?: body
-                int volId = volumeCounter.incrementAndGet()
-                String volName = specData?.name ?: "clone-${volId}"
                 int bsSnapId = specData?.bs_snap_id ?: 0
-
-                // Get size from source snapshot if available
-                long sizeMb = 1024
                 def srcSnap = snapshots.get(bsSnapId)
-                if (srcSnap != null) {
-                    sizeMb = srcSnap.spec.size_mb ?: 1024
+                if (srcSnap == null) {
+                    throw new HttpError(404, "BsSnap not found")
                 }
+                int volId = volumeCounter.incrementAndGet()
+                String volName = specData?.name ?: "clone-${volId}"
+                long sizeMb = srcSnap.spec.size_mb ?: 1024
 
                 def volSpec = [
                     id: volId, name: volName, pool_id: POOL_ID, size_mb: sizeMb,