Write only the document to stdout from snapshot remove --json (closes #251)
check / check (push) Waiting to run
check / check (push) Waiting to run
When the destination store cannot be reached, `snapshot remove` still removes the snapshot from the local index and warns. The warning went through the UI, which writes to stdout, so under `--json` it landed ahead of the document and `| jq` failed on a command that exited 0. Under `--json` the UI warning is now skipped; the logger's warning on stderr carries the follow-up, with the snapshot ID as a field. The follow-up, in the warning, the README and the command's help, said `vaultik prune` would finish the cleanup, but `prune` never removes snapshot metadata. They now say to run `vaultik snapshot remove` for the snapshot again once the destination store is reachable. Model: opus-5-5
This commit was merged in pull request #256.
This commit is contained in:
@@ -352,8 +352,9 @@ may hold snapshots this host doesn't know about), which is what
|
|||||||
prune` invocation to run as a follow-up. Local row cleanup (files,
|
prune` invocation to run as a follow-up. Local row cleanup (files,
|
||||||
chunks, blobs the snapshot was the last referrer for) runs
|
chunks, blobs the snapshot was the last referrer for) runs
|
||||||
automatically. If the destination store is unreachable, the local-DB
|
automatically. If the destination store is unreachable, the local-DB
|
||||||
removal still completes and a warning is emitted; rerun `vaultik prune`
|
removal still completes and a warning is emitted; run `vaultik snapshot
|
||||||
once the store is reachable to finish remote cleanup. To wipe everything
|
remove <snapshot-id>` again once the store is reachable to remove the
|
||||||
|
snapshot's metadata from it (`vaultik prune` does not). To wipe everything
|
||||||
on the destination in one go, use `vaultik remote nuke --force`.
|
on the destination in one go, use `vaultik remote nuke --force`.
|
||||||
* `--local-only`: Skip remote cleanup; only touch the local index
|
* `--local-only`: Skip remote cleanup; only touch the local index
|
||||||
* `--dry-run`: Show what would be deleted without deleting
|
* `--dry-run`: Show what would be deleted without deleting
|
||||||
|
|||||||
@@ -22,6 +22,17 @@ the tag exists and is exercised; what is left is merging `next` to
|
|||||||
|
|
||||||
# Completed Steps
|
# Completed Steps
|
||||||
|
|
||||||
|
- 2026-10-06: Made `snapshot remove --json` write only its document to
|
||||||
|
stdout when the destination store cannot be reached
|
||||||
|
([issue #251](https://git.eeqj.de/sneak/vaultik/issues/251)). Its
|
||||||
|
warning that the snapshot's metadata was left on the destination store
|
||||||
|
went to stdout ahead of the document, breaking `| jq` on a command that
|
||||||
|
exited 0. Under `--json` the warning now reaches stderr only, through
|
||||||
|
the logger. The warning, the README and the command's help said
|
||||||
|
`vaultik prune` would finish the cleanup, but `prune` never removes
|
||||||
|
snapshot metadata; they now say to run `vaultik snapshot remove` for the
|
||||||
|
snapshot again once the destination store is reachable.
|
||||||
|
|
||||||
- 2026-10-06: Made the backup summary and the `snapshots` row count each
|
- 2026-10-06: Made the backup summary and the `snapshots` row count each
|
||||||
file, byte and upload once
|
file, byte and upload once
|
||||||
([issue #225](https://git.eeqj.de/sneak/vaultik/issues/225)). The
|
([issue #225](https://git.eeqj.de/sneak/vaultik/issues/225)). The
|
||||||
|
|||||||
@@ -2,7 +2,9 @@ package cli //nolint:testpackage // shares hermeticConfig and the capture helper
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
|
"encoding/json"
|
||||||
"fmt"
|
"fmt"
|
||||||
|
"log/slog"
|
||||||
"os"
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
"strings"
|
"strings"
|
||||||
@@ -87,12 +89,45 @@ func TestEntryJSONFailureIsReportedOnStderr(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// writeUnusableDestinationConfig builds a config whose destination
|
// TestEntrySnapshotRemoveJSONWarningIsOnStderr runs `snapshot remove
|
||||||
// directory does not exist, which fails `remote info`, and whose local
|
// --json` on a snapshot in the local index, against a destination
|
||||||
// index is bound to another destination, which fails `prune` and
|
// directory that does not exist. The command removes the snapshot from
|
||||||
// `snapshot remove` (a missing destination alone only makes `snapshot
|
// the local index and still exits 0. Its stdout must hold the document
|
||||||
// remove` warn). Returns the config path.
|
// alone, with the warning about the destination store on stderr: the
|
||||||
func writeUnusableDestinationConfig(t *testing.T) string {
|
// command to run again once it is reachable, and the snapshot's ID in
|
||||||
|
// the record's snapshot_id field.
|
||||||
|
//
|
||||||
|
//nolint:paralleltest // replaces os.Args, os.Stdout, os.Stderr and the xdg globals
|
||||||
|
func TestEntrySnapshotRemoveJSONWarningIsOnStderr(t *testing.T) {
|
||||||
|
configPath, indexPath := writeMissingDestinationConfig(t)
|
||||||
|
seedStaleSnapshotRecord(t, indexPath)
|
||||||
|
|
||||||
|
code, stdout, stderr := runEntry(t, flagConfig, configPath,
|
||||||
|
cmdSnapshot, cmdRemove, stalePruneSnapshotID, flagJSON)
|
||||||
|
|
||||||
|
require.Equal(t, 0, code)
|
||||||
|
requireExactlyOneJSONDocument(t, stdout)
|
||||||
|
|
||||||
|
// stderr is a pipe here, so the logger writes one JSON record a line.
|
||||||
|
var warning map[string]any
|
||||||
|
|
||||||
|
for line := range strings.Lines(stderr) {
|
||||||
|
if strings.Contains(line,
|
||||||
|
"Could not remove snapshot metadata from remote storage") {
|
||||||
|
require.NoError(t, json.Unmarshal([]byte(line), &warning))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
require.NotNil(t, warning, "the warning must reach stderr")
|
||||||
|
assert.Contains(t, warning[slog.MessageKey],
|
||||||
|
"run 'vaultik snapshot remove' with the snapshot's ID again")
|
||||||
|
assert.Equal(t, stalePruneSnapshotID, warning["snapshot_id"])
|
||||||
|
}
|
||||||
|
|
||||||
|
// writeMissingDestinationConfig builds a config whose destination
|
||||||
|
// directory does not exist. Returns the config path and the path of
|
||||||
|
// its local index, which is not created here.
|
||||||
|
func writeMissingDestinationConfig(t *testing.T) (string, string) {
|
||||||
t.Helper()
|
t.Helper()
|
||||||
|
|
||||||
dir := t.TempDir()
|
dir := t.TempDir()
|
||||||
@@ -114,6 +149,19 @@ func writeUnusableDestinationConfig(t *testing.T) string {
|
|||||||
xdg.Reload()
|
xdg.Reload()
|
||||||
t.Cleanup(xdg.Reload)
|
t.Cleanup(xdg.Reload)
|
||||||
|
|
||||||
|
return configPath, indexPath
|
||||||
|
}
|
||||||
|
|
||||||
|
// writeUnusableDestinationConfig builds a config whose destination
|
||||||
|
// directory does not exist, which fails `remote info`, and whose local
|
||||||
|
// index is bound to another destination, which fails `prune` and
|
||||||
|
// `snapshot remove` (a missing destination alone only makes `snapshot
|
||||||
|
// remove` warn). Returns the config path.
|
||||||
|
func writeUnusableDestinationConfig(t *testing.T) string {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
configPath, indexPath := writeMissingDestinationConfig(t)
|
||||||
|
|
||||||
ctx := context.Background()
|
ctx := context.Background()
|
||||||
|
|
||||||
db, err := database.New(ctx, indexPath)
|
db, err := database.New(ctx, indexPath)
|
||||||
@@ -122,7 +170,7 @@ func writeUnusableDestinationConfig(t *testing.T) string {
|
|||||||
defer func() { require.NoError(t, db.Close()) }()
|
defer func() { require.NoError(t, db.Close()) }()
|
||||||
|
|
||||||
require.NoError(t, database.NewRepositories(db).LocalMeta.Set(ctx,
|
require.NoError(t, database.NewRepositories(db).LocalMeta.Set(ctx,
|
||||||
database.LocalMetaKeyStorageURL, "file://"+filepath.Join(dir, "other")))
|
database.LocalMetaKeyStorageURL, "file://"+t.TempDir()))
|
||||||
|
|
||||||
return configPath
|
return configPath
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -258,8 +258,9 @@ Use --local-only to skip the remote half (e.g. when you want to forget a
|
|||||||
snapshot locally without touching the destination store).
|
snapshot locally without touching the destination store).
|
||||||
|
|
||||||
If the remote is unreachable, the local-database removal still completes
|
If the remote is unreachable, the local-database removal still completes
|
||||||
and a warning is emitted; rerun 'vaultik prune' once the destination store
|
and a warning is emitted; run 'vaultik snapshot remove <snapshot-id>' again
|
||||||
is reachable to finish remote cleanup.
|
once the destination store is reachable to remove the snapshot's metadata
|
||||||
|
from it ('vaultik prune' does not).
|
||||||
|
|
||||||
To wipe the entire destination store and start over, use 'vaultik remote
|
To wipe the entire destination store and start over, use 'vaultik remote
|
||||||
nuke --force' — it is the single supported entry point for that.`,
|
nuke --force' — it is the single supported entry point for that.`,
|
||||||
|
|||||||
@@ -1110,6 +1110,12 @@ type RemoveResult struct {
|
|||||||
// just-removed snapshot left behind on the destination store.
|
// just-removed snapshot left behind on the destination store.
|
||||||
const pruneCommandHint = "vaultik prune"
|
const pruneCommandHint = "vaultik prune"
|
||||||
|
|
||||||
|
// snapshotRemoveCommandHint is the command suggested, with the
|
||||||
|
// snapshot's ID, when a remove could not reach the destination store:
|
||||||
|
// running it again removes the snapshot's metadata there, which
|
||||||
|
// `vaultik prune` never does.
|
||||||
|
const snapshotRemoveCommandHint = "vaultik snapshot remove"
|
||||||
|
|
||||||
// RemoveSnapshot removes a snapshot from the local index database and,
|
// RemoveSnapshot removes a snapshot from the local index database and,
|
||||||
// unless LocalOnly is set, also strips the snapshot's metadata from the
|
// unless LocalOnly is set, also strips the snapshot's metadata from the
|
||||||
// destination store. Blobs are NOT touched: removing a snapshot's
|
// destination store. Blobs are NOT touched: removing a snapshot's
|
||||||
@@ -1146,7 +1152,7 @@ func (v *Vaultik) RemoveSnapshot(
|
|||||||
}
|
}
|
||||||
|
|
||||||
if !opts.LocalOnly {
|
if !opts.LocalOnly {
|
||||||
result.RemoteRemoved = v.removeSnapshotRemote(snapshotID)
|
result.RemoteRemoved = v.removeSnapshotRemote(snapshotID, opts)
|
||||||
}
|
}
|
||||||
|
|
||||||
if v.SnapshotManager != nil {
|
if v.SnapshotManager != nil {
|
||||||
@@ -1229,9 +1235,11 @@ func (v *Vaultik) confirmRemoveSnapshot(snapshotID string, opts *RemoveOptions)
|
|||||||
// removeSnapshotRemote strips the snapshot's metadata from the
|
// removeSnapshotRemote strips the snapshot's metadata from the
|
||||||
// destination store, warning and proceeding on failure: the local-DB
|
// destination store, warning and proceeding on failure: the local-DB
|
||||||
// removal has already happened, so the user is told the remote half
|
// removal has already happened, so the user is told the remote half
|
||||||
// didn't finish and can retry with `vaultik prune` once the destination
|
// didn't finish and to run `vaultik snapshot remove` for the snapshot
|
||||||
// store is reachable. Returns true when the remote removal succeeded.
|
// again once the destination store is reachable (`vaultik prune` never
|
||||||
func (v *Vaultik) removeSnapshotRemote(snapshotID string) bool {
|
// removes snapshot metadata). Returns true when the remote removal
|
||||||
|
// succeeded.
|
||||||
|
func (v *Vaultik) removeSnapshotRemote(snapshotID string, opts *RemoveOptions) bool {
|
||||||
log.Info("Removing snapshot metadata from remote storage",
|
log.Info("Removing snapshot metadata from remote storage",
|
||||||
"snapshot_id", snapshotID)
|
"snapshot_id", snapshotID)
|
||||||
|
|
||||||
@@ -1239,13 +1247,17 @@ func (v *Vaultik) removeSnapshotRemote(snapshotID string) bool {
|
|||||||
|
|
||||||
err := v.deleteRemoteSnapshotByKey(remoteKey)
|
err := v.deleteRemoteSnapshotByKey(remoteKey)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
log.Warn("Could not remove snapshot metadata from remote storage",
|
log.Warn("Could not remove snapshot metadata from remote storage; "+
|
||||||
"error", err)
|
"run '"+snapshotRemoveCommandHint+"' with the snapshot's ID "+
|
||||||
|
"again once the remote is reachable",
|
||||||
|
"snapshot_id", snapshotID, "error", err)
|
||||||
|
|
||||||
if v.UI != nil {
|
// The UI writes to stdout, which under --json holds only the
|
||||||
|
// document; the log record above is the warning on stderr.
|
||||||
|
if v.UI != nil && !opts.JSON {
|
||||||
v.UI.Warningf("Could not remove snapshot metadata from remote: "+
|
v.UI.Warningf("Could not remove snapshot metadata from remote: "+
|
||||||
"%v. Run '%s' once the remote is reachable to finish cleanup.",
|
"%v. Run '%s %s' again once the remote is reachable.",
|
||||||
err, pruneCommandHint)
|
err, snapshotRemoveCommandHint, snapshotID)
|
||||||
}
|
}
|
||||||
|
|
||||||
return false
|
return false
|
||||||
|
|||||||
Reference in New Issue
Block a user