Merge pull request #1864 from yanxuean/withSnapshotBucket

crosbymichael · web-flow · commit 617c63de06a2 · 2017-12-04T17:25:23.000-05:00
get wrong parents bucket in withSnapshotBucket
diff --git a/snapshots/overlay/overlay.go b/snapshots/overlay/overlay.go
@@ -287,9 +287,9 @@ func (o *snapshotter) createSnapshot(ctx context.Context, kind snapshots.Kind, k
 	if err != nil {
 		return nil, err
 	}
-	isRollback := true
+	rollback := true
 	defer func() {
-		if isRollback {
+		if rollback {
 			if rerr := t.Rollback(); rerr != nil {
 				log.G(ctx).WithError(rerr).Warn("failed to rollback transaction")
 			}
@@ -317,9 +317,9 @@ func (o *snapshotter) createSnapshot(ctx context.Context, kind snapshots.Kind, k
 	if err = os.Rename(td, path); err != nil {
 		return nil, errors.Wrap(err, "failed to rename")
 	}
-	isRollback = false
 	td = ""
 
+	rollback = false
 	if err = t.Commit(); err != nil {
 		return nil, errors.Wrap(err, "commit failed")
 	}
diff --git a/snapshots/storage/bolt.go b/snapshots/storage/bolt.go
@@ -341,16 +341,16 @@ func CommitActive(ctx context.Context, key, name string, usage snapshots.Usage,
 		}
 		sbkt := bkt.Bucket([]byte(key))
 		if sbkt == nil {
-			return errors.Wrap(errdefs.ErrNotFound, "failed to get active snapshot")
+			return errors.Wrapf(errdefs.ErrNotFound, "failed to get active snapshot %q", key)
 		}
 
 		var si snapshots.Info
 		if err := readSnapshot(sbkt, &id, &si); err != nil {
-			return errors.Wrap(err, "failed to read snapshot")
+			return errors.Wrapf(err, "failed to read active snapshot %q", key)
 		}
 
 		if si.Kind != snapshots.KindActive {
-			return errors.Wrapf(errdefs.ErrFailedPrecondition, "snapshot %v is not active", name)
+			return errors.Wrapf(errdefs.ErrFailedPrecondition, "snapshot %q is not active", key)
 		}
 		si.Kind = snapshots.KindCommitted
 		si.Created = time.Now().UTC()
@@ -366,18 +366,18 @@ func CommitActive(ctx context.Context, key, name string, usage snapshots.Usage,
 			return err
 		}
 		if err := bkt.DeleteBucket([]byte(key)); err != nil {
-			return errors.Wrap(err, "failed to delete active")
+			return errors.Wrapf(err, "failed to delete active snapshot %q", key)
 		}
 		if si.Parent != "" {
 			spbkt := bkt.Bucket([]byte(si.Parent))
 			if spbkt == nil {
-				return errors.Wrap(errdefs.ErrNotFound, "missing parent")
+				return errors.Wrapf(errdefs.ErrNotFound, "missing parent snapshot of %q", key)
 			}
 			pid := readID(spbkt)
 
 			// Updates parent back link to use new key
 			if err := pbkt.Put(parentKey(pid, id), []byte(name)); err != nil {
-				return errors.Wrap(err, "failed to update parent link")
+				return errors.Wrapf(err, "failed to update parent link from %q to %q", key, name)
 			}
 		}
 
@@ -394,11 +394,11 @@ func withSnapshotBucket(ctx context.Context, key string, fn func(context.Context
 	if !ok {
 		return ErrNoTransaction
 	}
-	bkt := tx.Bucket(bucketKeyStorageVersion)
-	if bkt == nil {
+	vbkt := tx.Bucket(bucketKeyStorageVersion)
+	if vbkt == nil {
 		return errors.Wrap(errdefs.ErrNotFound, "bucket does not exist")
 	}
-	bkt = bkt.Bucket(bucketKeySnapshot)
+	bkt := vbkt.Bucket(bucketKeySnapshot)
 	if bkt == nil {
 		return errors.Wrap(errdefs.ErrNotFound, "snapshots bucket does not exist")
 	}
@@ -407,7 +407,7 @@ func withSnapshotBucket(ctx context.Context, key string, fn func(context.Context
 		return errors.Wrap(errdefs.ErrNotFound, "snapshot does not exist")
 	}
 
-	return fn(ctx, bkt, bkt.Bucket(bucketKeyParents))
+	return fn(ctx, bkt, vbkt.Bucket(bucketKeyParents))
 }
 
 func withBucket(ctx context.Context, fn func(context.Context, *bolt.Bucket, *bolt.Bucket) error) error {

Original file line number	Diff line number	Diff line change
`@@ -287,9 +287,9 @@ func (o *snapshotter) createSnapshot(ctx context.Context, kind snapshots.Kind, k`
`287`	`287`	`if err != nil {`
`288`	`288`	`return nil, err`
`289`	`289`	`}`
`290`		`- isRollback := true`
	`290`	`+ rollback := true`
`291`	`291`	`defer func() {`
`292`		`- if isRollback {`
	`292`	`+ if rollback {`
`293`	`293`	`if rerr := t.Rollback(); rerr != nil {`
`294`	`294`	`log.G(ctx).WithError(rerr).Warn("failed to rollback transaction")`
`295`	`295`	`}`
`@@ -317,9 +317,9 @@ func (o *snapshotter) createSnapshot(ctx context.Context, kind snapshots.Kind, k`
`317`	`317`	`if err = os.Rename(td, path); err != nil {`
`318`	`318`	`return nil, errors.Wrap(err, "failed to rename")`
`319`	`319`	`}`
`320`		`- isRollback = false`
`321`	`320`	`td = ""`
`322`	`321`
	`322`	`+ rollback = false`
`323`	`323`	`if err = t.Commit(); err != nil {`
`324`	`324`	`return nil, errors.Wrap(err, "commit failed")`
`325`	`325`	`}`
Original file line number	Diff line number	Diff line change
`@@ -341,16 +341,16 @@ func CommitActive(ctx context.Context, key, name string, usage snapshots.Usage,`
`341`	`341`	`}`
`342`	`342`	`sbkt := bkt.Bucket([]byte(key))`
`343`	`343`	`if sbkt == nil {`
`344`		`- return errors.Wrap(errdefs.ErrNotFound, "failed to get active snapshot")`
	`344`	`+ return errors.Wrapf(errdefs.ErrNotFound, "failed to get active snapshot %q", key)`
`345`	`345`	`}`
`346`	`346`
`347`	`347`	`var si snapshots.Info`
`348`	`348`	`if err := readSnapshot(sbkt, &id, &si); err != nil {`
`349`		`- return errors.Wrap(err, "failed to read snapshot")`
	`349`	`+ return errors.Wrapf(err, "failed to read active snapshot %q", key)`
`350`	`350`	`}`
`351`	`351`
`352`	`352`	`if si.Kind != snapshots.KindActive {`
`353`		`- return errors.Wrapf(errdefs.ErrFailedPrecondition, "snapshot %v is not active", name)`
	`353`	`+ return errors.Wrapf(errdefs.ErrFailedPrecondition, "snapshot %q is not active", key)`
`354`	`354`	`}`
`355`	`355`	`si.Kind = snapshots.KindCommitted`
`356`	`356`	`si.Created = time.Now().UTC()`
`@@ -366,18 +366,18 @@ func CommitActive(ctx context.Context, key, name string, usage snapshots.Usage,`
`366`	`366`	`return err`
`367`	`367`	`}`
`368`	`368`	`if err := bkt.DeleteBucket([]byte(key)); err != nil {`
`369`		`- return errors.Wrap(err, "failed to delete active")`
	`369`	`+ return errors.Wrapf(err, "failed to delete active snapshot %q", key)`
`370`	`370`	`}`
`371`	`371`	`if si.Parent != "" {`
`372`	`372`	`spbkt := bkt.Bucket([]byte(si.Parent))`
`373`	`373`	`if spbkt == nil {`
`374`		`- return errors.Wrap(errdefs.ErrNotFound, "missing parent")`
	`374`	`+ return errors.Wrapf(errdefs.ErrNotFound, "missing parent snapshot of %q", key)`
`375`	`375`	`}`
`376`	`376`	`pid := readID(spbkt)`
`377`	`377`
`378`	`378`	`// Updates parent back link to use new key`
`379`	`379`	`if err := pbkt.Put(parentKey(pid, id), []byte(name)); err != nil {`
`380`		`- return errors.Wrap(err, "failed to update parent link")`
	`380`	`+ return errors.Wrapf(err, "failed to update parent link from %q to %q", key, name)`
`381`	`381`	`}`
`382`	`382`	`}`
`383`	`383`
`@@ -394,11 +394,11 @@ func withSnapshotBucket(ctx context.Context, key string, fn func(context.Context`
`394`	`394`	`if !ok {`
`395`	`395`	`return ErrNoTransaction`
`396`	`396`	`}`
`397`		`- bkt := tx.Bucket(bucketKeyStorageVersion)`
`398`		`- if bkt == nil {`
	`397`	`+ vbkt := tx.Bucket(bucketKeyStorageVersion)`
	`398`	`+ if vbkt == nil {`
`399`	`399`	`return errors.Wrap(errdefs.ErrNotFound, "bucket does not exist")`
`400`	`400`	`}`
`401`		`- bkt = bkt.Bucket(bucketKeySnapshot)`
	`401`	`+ bkt := vbkt.Bucket(bucketKeySnapshot)`
`402`	`402`	`if bkt == nil {`
`403`	`403`	`return errors.Wrap(errdefs.ErrNotFound, "snapshots bucket does not exist")`
`404`	`404`	`}`
`@@ -407,7 +407,7 @@ func withSnapshotBucket(ctx context.Context, key string, fn func(context.Context`
`407`	`407`	`return errors.Wrap(errdefs.ErrNotFound, "snapshot does not exist")`
`408`	`408`	`}`
`409`	`409`
`410`		`- return fn(ctx, bkt, bkt.Bucket(bucketKeyParents))`
	`410`	`+ return fn(ctx, bkt, vbkt.Bucket(bucketKeyParents))`
`411`	`411`	`}`
`412`	`412`
`413`	`413`	`func withBucket(ctx context.Context, fn func(context.Context, bolt.Bucket, bolt.Bucket) error) error {`