Skip to content

Commit

Permalink
cli: fixed snapshot delete to support deleting file (not directory) s…
Browse files Browse the repository at this point in the history
…napshots by object ID (#613)
  • Loading branch information
jkowalski committed Sep 13, 2020
1 parent f0b97b9 commit 9faf6b3
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 12 deletions.
20 changes: 8 additions & 12 deletions cli/command_snapshot_delete.go
Expand Up @@ -3,7 +3,6 @@ package cli
import (
"context"
"fmt"
"strings"

"github.com/pkg/errors"

Expand All @@ -21,19 +20,16 @@ var (

func runDeleteCommand(ctx context.Context, rep repo.Repository) error {
for _, id := range *snapshotDeleteIDs {
if strings.HasPrefix(id, "k") {
if err := deleteSnapshotsByRootObjectID(ctx, rep, object.ID(id)); err != nil {
return errors.Wrapf(err, "error deleting snapshots by root ID %v", id)
}
} else {
m, err := snapshot.LoadSnapshot(ctx, rep, manifest.ID(id))
if err != nil {
return errors.Wrapf(err, "error loading snapshot %v", id)
}

if err := deleteSnapshot(ctx, rep, m); err != nil {
m, err := snapshot.LoadSnapshot(ctx, rep, manifest.ID(id))
if err == nil {
// snapshot found by manifest ID, delete it directly.
if err = deleteSnapshot(ctx, rep, m); err != nil {
return errors.Wrapf(err, "error deleting %v", id)
}
} else if !errors.Is(err, snapshot.ErrSnapshotNotFound) {
return errors.Wrapf(err, "error loading snapshot %v", id)
} else if err := deleteSnapshotsByRootObjectID(ctx, rep, object.ID(id)); err != nil {
return errors.Wrapf(err, "error deleting snapshots by root ID %v", id)
}
}

Expand Down
7 changes: 7 additions & 0 deletions snapshot/manager.go
Expand Up @@ -14,6 +14,9 @@ import (
// ManifestType is the value of the "type" label for snapshot manifests.
const ManifestType = "snapshot"

// ErrSnapshotNotFound is returned when a snapshot is not found.
var ErrSnapshotNotFound = errors.Errorf("snapshot not found")

const (
typeKey = manifest.TypeLabelKey

Expand Down Expand Up @@ -73,6 +76,10 @@ func LoadSnapshot(ctx context.Context, rep repo.Repository, manifestID manifest.

em, err := rep.GetManifest(ctx, manifestID, sm)
if err != nil {
if errors.Is(err, manifest.ErrNotFound) {
return nil, ErrSnapshotNotFound
}

return nil, errors.Wrap(err, "unable to find manifest entries")
}

Expand Down
5 changes: 5 additions & 0 deletions snapshot/snapshot_test.go
@@ -1,6 +1,7 @@
package snapshot_test

import (
"errors"
"os"
"path/filepath"
"reflect"
Expand Down Expand Up @@ -32,6 +33,10 @@ func TestSnapshotsAPI(t *testing.T) {
Path: "/some/other/path",
}

if _, err := snapshot.LoadSnapshot(ctx, env.Repository, "no-such-manifest-id"); !errors.Is(err, snapshot.ErrSnapshotNotFound) {
t.Errorf("unexpected error when deleting snapshot for manifest that does not exist: %v", err)
}

verifySnapshotManifestIDs(t, env.Repository, nil, nil)
verifySnapshotManifestIDs(t, env.Repository, &src1, nil)
verifySnapshotManifestIDs(t, env.Repository, &src2, nil)
Expand Down

0 comments on commit 9faf6b3

Please sign in to comment.