From 25252003cfd1121812710837548324c2ac664cb5 Mon Sep 17 00:00:00 2001
From: "xiangheng.zhao" <xiangheng.zhao@zstack.io>
Date: Thu, 13 Aug 2026 21:21:41 +0800
Subject: [PATCH] <fix>[ceph]: ZSTAC-85248 keep snapshot GC failed until
 backend cleanup

Root Cause:
Ceph snapshot deletion may fail while RBD clone children still exist in trash. The business delete path submits DeleteVolumeSnapshotGC and returns success with gcSubmitted=true, but the GC retry path sends DeleteSnapshotOnPrimaryStorageMsg with gcOnFailure=false. Ceph previously ignored this distinction and could submit another GC while returning success again, so the current GC was marked Done even though the snapshot remained on Ceph.

Solution:
Make Ceph snapshot deletion return failure when gcOnFailure is false. Business deletion still submits DeleteVolumeSnapshotGC on backend delete failure, while GC retry failure is reported as a real failure so the existing GC remains retryable instead of being completed incorrectly.

Test:
Updated CephGCCase to cover snapshot delete failure with Ceph trash-style dependencies. The case verifies through the existing delete snapshot and triggerGCJob DSL flow that snapshot GC is not marked Done while backend deletion still fails, Ceph DELETE_SNAPSHOT_PATH is called during GC retry, and no additional DeleteVolumeSnapshotGC job is created on that retry.

Verified with:
git diff --check
mvn compile -pl storage,plugin/ceph -am -Dmaven.test.skip=true -DskipJacoco=true
mvn test-compile -DskipJacoco=true from zstack/test
./runMavenProfile premium
mvn test -Dtest=CephGCCase -DfailIfNoTests=false -Dmanagement.server.ip=127.0.0.1 -DskipJacoco=true from zstack/test started CephGCCase but was blocked by local MySQL socket /run/mysqld/mysqld.sock before executing the scenario

Resolves: ZSTAC-85248

Change-Id: I85248ce796b74fa0a6d4b5a0c69d4f31e5e53838
---
 .../ceph/primary/CephPrimaryStorageBase.java  |  7 +++++++
 .../storage/primary/ceph/CephGCCase.groovy    | 20 +++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/plugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephPrimaryStorageBase.java b/plugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephPrimaryStorageBase.java
index 53816200465..54f6efea3d2 100755
--- a/plugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephPrimaryStorageBase.java
+++ b/plugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephPrimaryStorageBase.java
@@ -5594,6 +5594,13 @@ public void success(DeleteSnapshotRsp returnValue) {
 
             @Override
             public void fail(ErrorCode errorCode) {
+                if (!msg.isGcOnFailure()) {
+                    reply.setError(errorCode);
+                    bus.reply(msg, reply);
+                    completion.done();
+                    return;
+                }
+
                 // ceph has trash, so children may not be deleted immediately.
                 DeleteVolumeSnapshotGC snapshotGC = new DeleteVolumeSnapshotGC();
                 snapshotGC.NAME = String.format("gc-ceph-%s-volumesnapshot-path-%s", self.getUuid(), cmd.snapshotPath);
diff --git a/test/src/test/groovy/org/zstack/test/integration/storage/primary/ceph/CephGCCase.groovy b/test/src/test/groovy/org/zstack/test/integration/storage/primary/ceph/CephGCCase.groovy
index e5644c47072..8aecaf6ee06 100755
--- a/test/src/test/groovy/org/zstack/test/integration/storage/primary/ceph/CephGCCase.groovy
+++ b/test/src/test/groovy/org/zstack/test/integration/storage/primary/ceph/CephGCCase.groovy
@@ -151,8 +151,10 @@ class CephGCCase extends SubCase {
         } as VolumeSnapshotInventory
 
         def deleteFailed = true
+        def deleteSnapshotCalled = 0
         def deleteSucVol = []
         env.simulator(CephPrimaryStorageBase.DELETE_SNAPSHOT_PATH) { HttpEntity<String> e, EnvSpec spec ->
+            deleteSnapshotCalled++
             def rsp = new CephPrimaryStorageBase.DeleteSnapshotRsp()
             if (deleteFailed) {
                 rsp.setError("it's children in trash, cannot delete")
@@ -207,6 +209,24 @@ class CephGCCase extends SubCase {
         assert volGC.size() == 1
         assert volGC[0].status != GCStatus.Done.toString()
 
+        def deleteSnapshotCalledBeforeTriggerGC = deleteSnapshotCalled
+        def spGCBeforeTriggerGC = queryGCJob {
+            conditions = ["runnerClass=${DeleteVolumeSnapshotGC.class.name}".toString(), "context~=%${sp.primaryStorageInstallPath}%".toString()]
+        } as List<GarbageCollectorInventory>
+
+        triggerGCJob {
+            uuid = spGC[0].uuid
+        }
+        assert deleteSnapshotCalled == deleteSnapshotCalledBeforeTriggerGC + 1
+        assert queryGCJob {
+            conditions = ["runnerClass=${DeleteVolumeSnapshotGC.class.name}".toString(), "context~=%${sp.primaryStorageInstallPath}%".toString()]
+        }.size() == spGCBeforeTriggerGC.size()
+        assert !retryInSecs(3) {
+            queryGCJob {
+                conditions = ["runnerClass=${DeleteVolumeSnapshotGC.class.name}".toString(), "context~=%${sp.primaryStorageInstallPath}%".toString()]
+            }[0].status == GCStatus.Done.toString()
+        }
+
         deleteFailed = false
         triggerGCJob {
             uuid = spGC[0].uuid