fix(cubemaster): clean job rows after snapshot delete

[xiaojunxiang2023]

Summary

Fixes a bug where deleted snapshots still appeared in template list APIs until a second delete.

After the first DELETE /templates/{snapshotID}:

• Single-snapshot lookup correctly returned not found
• GET /templates and cubemastercli tpl ls still listed the deleted snapshot
• A second delete was required to remove it from the list

Root cause

Snapshot delete (runSnapshotDeleteJob) removed template_definition metadata but did not remove related template_image_job rows.

ListTemplates appends orphan job records when no definition exists:

for _, job := range jobs {
    if _, ok := seen[job.TemplateID]; ok {
        continue
    }
    out = append(out, templateInfoFromJob(&job))
}

So the deleted snapshot was resurrected in list responses via job fallback entries.

On the second delete, has_snapshot returned false (definition gone, job fallback has no Kind), routing the request through template delete (cleanupTemplateJobs) instead of snapshot delete.

Fix

• Call cleanupTemplateJobs after successful snapshot metadata cleanup, matching template delete behavior
• Return terminal READY job info from executeSnapshotDeleteJob without re-querying job rows that were just deleted

Deploy And Verify

[cubesandboxbot]

PR #559 Review: fix(cubemaster): clean job rows after snapshot delete

Overview

This PR correctly fixes a bug where deleted snapshots reappeared in ListTemplates due to orphan template_image_job rows creating fallback entries. The fix replaces the old updateTemplateImageJob (set job row to READY) with runTemplateJobCleanup (DELETE job rows) and returns a terminal READY view from cloneTemplateImageJobInfo instead of re-querying deleted rows.

The changes are well-scoped, the root-cause analysis in the PR description is accurate, and the test suite covers the new behavior effectively. After reviewing across code quality, performance, test coverage, security, and documentation dimensions, I found a few areas worth addressing:

Notable Findings

1. Retry-failure returns success silently - snapshot_ops.go:730-738
When both the initial runTemplateJobCleanup and its retry fail, the function logs WARN and returns nil (success). While the snapshot metadata is already gone so returning success is correct, orphaned job rows remain unreconciled. The second failure should be logged at Errorf rather than Warnf so it is visible in production triage.

2. Empty-slice inconsistency in cloneTemplateImageJobInfo - job_dto.go:60
The len(src.RedoScope) > 0 guard means a non-nil empty slice []string{} is not deep-copied. Removing the guard makes the copy consistent for all slice states at negligible allocation cost.

3. Missing doc on terminalSnapshotDeleteJobInfo - snapshot_ops.go:694
This function constructs the terminal response without re-querying deleted DB rows but lacks a doc comment explaining why it exists.

Overall Assessment

The fix is correct and well-implemented. The clone-and-stamp pattern in executeSnapshotDeleteJob is the right approach - it avoids a DB ound-trip for a record that has just been deleted. The test coverage is comprehensive for the PR scope, and the performance impact is positive (fewer DB calls per delete operation). None of the findings above are blocking; they are refinements for production robustness and maintainability.