feat(clone): reap the internal snapshot a clone took on its source

internal/cli/definition.go

@@ -29,6 +29,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	apiv1 "github.com/cozystack/blockstor/pkg/api/v1"
+	"github.com/cozystack/blockstor/pkg/store"
 
 	"github.com/cozystack/blockstor/internal/cli/command"
 	"github.com/cozystack/blockstor/internal/cli/view"
@@ -156,7 +157,7 @@ func resourceDefinitionClone(ctx context.Context, run *runContext) error {
 
 	snapName := cloneSnapshotName(target)
 
-	err = ensureCloneSnapshot(ctx, run, &src, snapName)
+	err = ensureCloneSnapshot(ctx, run, &src, snapName, target)
 	if err != nil {
 		return err
 	}
@@ -234,7 +235,9 @@ func checkCloneSnapshotIsCurrent(
 	return nil
 }
 
-func ensureCloneSnapshot(ctx context.Context, run *runContext, src *apiv1.ResourceDefinition, snapName string) error {
+func ensureCloneSnapshot(
+	ctx context.Context, run *runContext, src *apiv1.ResourceDefinition, snapName, target string,
+) error {
 	existing, err := run.Store.Snapshots().Get(ctx, src.Name, snapName)
 	if err == nil {
 		return checkCloneSnapshotIsCurrent(ctx, run, src, &existing)
@@ -255,6 +258,14 @@ func ensureCloneSnapshot(ctx context.Context, run *runContext, src *apiv1.Resour
 		return err
 	}
 
+	// Stamped as the clone's own, the way the REST door stamps it, so that
+	// deleting the clone may reap it; see store.CloneSnapshotOwnerProp.
+	if snap.Props == nil {
+		snap.Props = map[string]string{}
+	}
+
+	snap.Props[store.CloneSnapshotOwnerProp] = target
+
 	err = run.Store.Snapshots().Create(ctx, snap)
 	if err != nil {
 		return fmt.Errorf("create clone snapshot %s: %w", snapName, err)

internal/cli/rd_delete_clone_snapshot_test.go

@@ -0,0 +1,112 @@
+// SPDX-License-Identifier: Apache-2.0
+
+package cli_test
+
+import (
+	"context"
+	"errors"
+	"strings"
+	"testing"
+
+	apiv1 "github.com/cozystack/blockstor/pkg/api/v1"
+	"github.com/cozystack/blockstor/pkg/store"
+)
+
+// `rd clone` takes the same internal snapshot the REST door takes, and `rd d`
+// of the clone reaped nothing, so the source could never be deleted through
+// the CLI either.
+func TestResourceDefinitionDeleteOfACloneReapsItsSnapshot(t *testing.T) {
+	t.Parallel()
+
+	app, _, errBuf := newApp(t, seedSnapshotSource)
+
+	if got := app.Run(t.Context(), []string{"rd", "clone", "pvc-x", "pvc-reap"}); got != 0 {
+		t.Fatalf("clone exit = %d (stderr: %s)", got, errBuf.String())
+	}
+
+	// The owner prop is the snapshot's. Carried onto the clone, it would be
+	// copied into every snapshot later taken of the clone as a claim of
+	// ownership nobody made.
+	clone, err := appStore(t, app).ResourceDefinitions().Get(t.Context(), "pvc-reap")
+	if err != nil {
+		t.Fatalf("get the clone: %v", err)
+	}
+
+	if owner, ok := clone.Props[store.CloneSnapshotOwnerProp]; ok {
+		t.Errorf("the clone carries the snapshot's owner prop %q", owner)
+	}
+
+	if got := app.Run(t.Context(), []string{"rd", "d", "pvc-reap"}); got != 0 {
+		t.Fatalf("delete of the clone exit = %d (stderr: %s)", got, errBuf.String())
+	}
+
+	backend := appStore(t, app)
+
+	if _, err := backend.Snapshots().Get(t.Context(), "pvc-x", "clone-pvc-reap"); !errors.Is(err, store.ErrNotFound) {
+		t.Errorf("the internal snapshot outlived the clone it backed: %v", err)
+	}
+
+	// seedSnapshotSource also leaves an operator snapshot on pvc-x, which the
+	// delete has to keep refusing over. The clone's own must not be among what
+	// it names.
+	app.Run(t.Context(), []string{"rd", "d", "pvc-x"})
+
+	if strings.Contains(errBuf.String(), "clone-pvc-reap") {
+		t.Errorf("the source delete still meets the clone's snapshot: %s", errBuf.String())
+	}
+}
+
+// An operator can name a snapshot anything, `clone-<target>` included, and
+// restore it under that target. Only the prop the clone path writes makes a
+// snapshot the clone's to reap.
+func TestResourceDefinitionDeleteKeepsAnOperatorSnapshotNamedLikeAClone(t *testing.T) {
+	t.Parallel()
+
+	app, _, errBuf := newApp(t, func(ctx context.Context, backend store.Store) {
+		seedSnapshotSource(ctx, backend)
+		_ = backend.Snapshots().Create(ctx, &apiv1.Snapshot{
+			Name: "clone-pvc-op", ResourceName: "pvc-x",
+			Nodes:             []string{"node-1", "node-2"},
+			VolumeDefinitions: []apiv1.SnapshotVolumeDef{{VolumeNumber: 0, SizeKib: 1 << 20}},
+		})
+	})
+
+	argv := []string{
+		"s", "resource", "restore",
+		"--from-resource", "pvc-x", "--from-snapshot", "clone-pvc-op", "--to-resource", "pvc-op",
+	}
+
+	if got := app.Run(t.Context(), argv); got != 0 {
+		t.Fatalf("restore exit = %d (stderr: %s)", got, errBuf.String())
+	}
+
+	if got := app.Run(t.Context(), []string{"rd", "d", "pvc-op"}); got != 0 {
+		t.Fatalf("delete exit = %d (stderr: %s)", got, errBuf.String())
+	}
+
+	if _, err := appStore(t, app).Snapshots().Get(t.Context(), "pvc-x", "clone-pvc-op"); err != nil {
+		t.Errorf("the operator's snapshot was reaped because of its name: %v", err)
+	}
+}
+
+// The CLI refusal over a source still carrying a snapshot its clone left had no
+// case that reached it: every fixture's list came back empty.
+func TestResourceDefinitionDeleteNamesACloneSnapshotLeftBehind(t *testing.T) {
+	t.Parallel()
+
+	app, _, errBuf := newApp(t, func(ctx context.Context, backend store.Store) {
+		_ = backend.ResourceDefinitions().Create(ctx, &apiv1.ResourceDefinition{Name: "pvc-left"})
+		_ = backend.Snapshots().Create(ctx, &apiv1.Snapshot{
+			Name: "clone-pvc-gone", ResourceName: "pvc-left",
+			Props: map[string]string{store.CloneSnapshotOwnerProp: "pvc-gone"},
+		})
+	})
+
+	if got := app.Run(t.Context(), []string{"rd", "d", "pvc-left"}); got == 0 {
+		t.Fatal("delete of a source still carrying a snapshot succeeded")
+	}
+
+	if !strings.Contains(errBuf.String(), "clone-pvc-gone") {
+		t.Errorf("refusal %q does not name the snapshot the clone left", errBuf.String())
+	}
+}

internal/cli/rd_delete_clone_snapshot_unreadable_test.go

@@ -0,0 +1,80 @@
+// SPDX-License-Identifier: Apache-2.0
+
+/*
+Copyright 2026 Cozystack contributors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package cli_test
+
+import (
+	"bytes"
+	"context"
+	"errors"
+	"testing"
+
+	"github.com/cozystack/blockstor/internal/cli"
+	apiv1 "github.com/cozystack/blockstor/pkg/api/v1"
+	"github.com/cozystack/blockstor/pkg/store"
+)
+
+var errSourceSnapshotsUnreadable = errors.New("apiserver unavailable")
+
+type sourceSnapshotsUnreadable struct{ store.SnapshotStore }
+
+func (u sourceSnapshotsUnreadable) ListByDefinitionUncached(ctx context.Context, rdName string) ([]apiv1.Snapshot, error) {
+	if rdName == "src" {
+		return nil, errSourceSnapshotsUnreadable
+	}
+
+	return u.SnapshotStore.ListByDefinitionUncached(ctx, rdName) //nolint:wrapcheck // pass-through test double
+}
+
+type sourceSnapshotsUnreadableStore struct{ store.Store }
+
+func (s sourceSnapshotsUnreadableStore) Snapshots() store.SnapshotStore {
+	return sourceSnapshotsUnreadable{SnapshotStore: s.Store.Snapshots()}
+}
+
+// The CLI door stops the same way the REST one does: a clone whose
+// snapshot cannot be read is left in place for a retry to reap.
+func TestRDDeleteCLIStopsWhenTheCloneSnapshotCannotBeRead(t *testing.T) {
+	t.Parallel()
+
+	backend := store.NewInMemory()
+	if err := backend.ResourceDefinitions().Create(t.Context(), &apiv1.ResourceDefinition{
+		Name:  "dst-unread",
+		Props: map[string]string{store.RestoreFromSnapshotProp: "src:clone-dst-unread"},
+	}); err != nil {
+		t.Fatalf("seed: %v", err)
+	}
+
+	var out, errBuf bytes.Buffer
+
+	app := &cli.App{
+		Out: &out,
+		Err: &errBuf,
+		StoreFor: func(context.Context) (store.Store, error) {
+			return sourceSnapshotsUnreadableStore{Store: backend}, nil
+		},
+	}
+
+	if code := app.Run(t.Context(), []string{"resource-definition", "delete", "dst-unread"}); code == 0 {
+		t.Errorf("rd d exited 0 over an unreadable clone snapshot (stderr %q)", errBuf.String())
+	}
+
+	if _, err := backend.ResourceDefinitions().Get(t.Context(), "dst-unread"); err != nil {
+		t.Errorf("the clone was deleted although its snapshot could not be read: %v", err)
+	}
+}

internal/cli/snapshot.go

@@ -350,6 +350,13 @@ func snapshotRestoreResource(ctx context.Context, run *runContext) error {
 		return fmt.Errorf("create resource definition %s: %w", def.Name, err)
 	}
 
+	// The snapshot was read before the definition existed; see
+	// store.ReapClonedSnapshot.
+	err = store.RestoreSourceWithdrawn(ctx, run.Store, snap.ResourceName, snap.Name)
+	if err != nil {
+		return rollbackRestore(ctx, run, def.Name, planned, err)
+	}
+
 	// The definition is this command's own, but a concurrent run may already
 	// have adopted it and be hydrating the same volumes. A volume that is
 	// already there at the snapshot's size is the restore's, not a collision
@@ -411,6 +418,8 @@ func newRestoredDefinition(
 	// the placer looks the source half up as a store key.
 	def.Props = store.WithRestoreMarker(def.Props, snap)
 
+	store.DropCloneSnapshotMarks(def.Props)
+
 	return def, nil
 }
 
@@ -493,6 +502,21 @@ func restoreIntoExistingTarget(
 		return finishConcurrentRestore(ctx, run, args, existing.Name, snap, createErr)
 	}
 
+	return placeOverMarkedTarget(ctx, run, existing.Name, snap, planned)
+}
+
+// placeOverMarkedTarget places the planned replicas on a target that carries
+// this restore's marker. The snapshot was read before the marker went on, or
+// before this run found it there, so it is read back first; see
+// store.ReapClonedSnapshot.
+func placeOverMarkedTarget(
+	ctx context.Context, run *runContext, rdName string, snap *apiv1.Snapshot, planned []apiv1.Resource,
+) error {
+	err := store.RestoreSourceWithdrawn(ctx, run.Store, snap.ResourceName, snap.Name)
+	if err != nil {
+		return fmt.Errorf("restore into %s: %w", rdName, err)
+	}
+
 	return placeOverExisting(ctx, run, planned)
 }
 
@@ -654,7 +678,7 @@ func finishRestoreLeftover(
 		return err
 	}
 
-	return placeOverExisting(ctx, run, planned)
+	return placeOverMarkedTarget(ctx, run, rdName, snap, planned)
 }
 
 // answerFinishedLeftover reports a finished leftover of this restore as