Mark faulty device in one transaction

mxpv · mxpv · commit 4d5a0e19ebe1 · 2019-07-30T16:26:55.000-07:00
Signed-off-by: Maksym Pavlenko &lt;makpav@amazon.com&gt;
diff --git a/snapshots/devmapper/metadata.go b/snapshots/devmapper/metadata.go
@@ -24,7 +24,6 @@ import (
 	"fmt"
 	"strconv"
 
-	"github.com/hashicorp/go-multierror"
 	"github.com/pkg/errors"
 	bolt "go.etcd.io/bbolt"
 )
@@ -127,23 +126,25 @@ func (m *PoolMetadata) AddDevice(ctx context.Context, info *DeviceInfo) error {
 // The snapshotter might attempt to recreate a device in 'Faulty' state with another devmapper ID in
 // subsequent calls, and in case of success it's status will be changed to 'Created' or 'Activated'.
 // The devmapper dev ID will remain in 'deviceFaulty' state until manually handled by a user.
-func (m *PoolMetadata) MarkFaulty(ctx context.Context, info *DeviceInfo) error {
-	var result error
+func (m *PoolMetadata) MarkFaulty(ctx context.Context, name string) error {
+	return m.db.Update(func(tx *bolt.Tx) error {
+		var (
+			device    = DeviceInfo{}
+			devBucket = tx.Bucket(devicesBucketName)
+		)
 
-	if err := m.UpdateDevice(ctx, info.Name, func(deviceInfo *DeviceInfo) error {
-		deviceInfo.State = Faulty
-		return nil
-	}); err != nil {
-		result = multierror.Append(result, err)
-	}
+		if err := getObject(devBucket, name, &device); err != nil {
+			return err
+		}
 
-	if err := m.db.Update(func(tx *bolt.Tx) error {
-		return markDeviceID(tx, info.DeviceID, deviceFaulty)
-	}); err != nil {
-		result = multierror.Append(result, err)
-	}
+		device.State = Faulty
+
+		if err := putObject(devBucket, name, &device, true); err != nil {
+			return err
+		}
 
-	return result
+		return markDeviceID(tx, device.DeviceID, deviceFaulty)
+	})
 }
 
 // getNextDeviceID finds the next free device ID by taking a cursor
diff --git a/snapshots/devmapper/metadata_test.go b/snapshots/devmapper/metadata_test.go
@@ -162,7 +162,7 @@ func TestPoolMetadata_MarkFaulty(t *testing.T) {
 	err := store.AddDevice(testCtx, info)
 	assert.NilError(t, err)
 
-	err = store.MarkFaulty(testCtx, info)
+	err = store.MarkFaulty(testCtx, "test")
 	assert.NilError(t, err)
 
 	saved, err := store.GetDevice(testCtx, info.Name)
diff --git a/snapshots/devmapper/pool_device.go b/snapshots/devmapper/pool_device.go
@@ -132,7 +132,7 @@ func (p *PoolDevice) CreateThinDevice(ctx context.Context, deviceName string, vi
 			if delErr != nil {
 				// Failed to rollback, mark the device as faulty and keep metadata in order to
 				// preserve the faulty device ID
-				retErr = multierror.Append(retErr, delErr, p.metadata.MarkFaulty(ctx, info))
+				retErr = multierror.Append(retErr, delErr, p.metadata.MarkFaulty(ctx, info.Name))
 				return
 			}
 
@@ -147,7 +147,7 @@ func (p *PoolDevice) CreateThinDevice(ctx context.Context, deviceName string, vi
 
 		// We're unable to create the devmapper device, most likely something wrong with the deviceID
 		if devErr != nil {
-			retErr = multierror.Append(retErr, p.metadata.MarkFaulty(ctx, info))
+			retErr = multierror.Append(retErr, p.metadata.MarkFaulty(ctx, info.Name))
 			return
 		}
 	}()
@@ -223,7 +223,7 @@ func (p *PoolDevice) CreateSnapshotDevice(ctx context.Context, deviceName string
 			if delErr != nil {
 				// Failed to rollback, mark the device as faulty and keep metadata in order to
 				// preserve the faulty device ID
-				retErr = multierror.Append(retErr, delErr, p.metadata.MarkFaulty(ctx, snapInfo))
+				retErr = multierror.Append(retErr, delErr, p.metadata.MarkFaulty(ctx, snapInfo.Name))
 				return
 			}
 
@@ -238,7 +238,7 @@ func (p *PoolDevice) CreateSnapshotDevice(ctx context.Context, deviceName string
 
 		// We're unable to create the devmapper device, most likely something wrong with the deviceID
 		if devErr != nil {
-			retErr = multierror.Append(retErr, p.metadata.MarkFaulty(ctx, snapInfo))
+			retErr = multierror.Append(retErr, p.metadata.MarkFaulty(ctx, snapInfo.Name))
 			return
 		}
 	}()
