snapshot: ignore stale lock delete failures

snapshot/builder.go

@@ -279,10 +279,9 @@ func (snap *Builder) Lock() (chan bool, error) {
 
 		/* Kick out stale locks */
 		if lock.IsStale() {
-			err := snap.repository.DeleteLock(lockID)
-			if err != nil {
-				snap.repository.DeleteLock(snap.Header.Identifier)
-				return nil, err
+			if err := snap.repository.DeleteLock(lockID); err != nil {
+				// Stale lock cleanup is opportunistic: stores may allow read
+				// and write operations without granting delete permission.
 			}
 
 			continue

snapshot/builder_test.go

@@ -1,7 +1,9 @@
 package snapshot_test
 
 import (
+	"bytes"
 	"testing"
+	"time"
 
 	"github.com/PlakarKorp/kloset/objects"
 	"github.com/PlakarKorp/kloset/repository"
@@ -44,3 +46,29 @@ func TestBuilderOptionsDataClassesDefault(t *testing.T) {
 	require.NotNil(t, builder.Header.DataClasses)
 	require.Empty(t, builder.Header.DataClasses)
 }
+
+func TestCreateIgnoresStaleLockDeleteFailure(t *testing.T) {
+	const staleLockHostname = "stale-lock-host"
+
+	repo := ptesting.GenerateRepository(t, nil, nil, nil)
+	staleLockID := objects.MAC{0x16, 0x70}
+	staleLock := repository.NewSharedLock(staleLockHostname)
+	staleLock.Timestamp = time.Now().Add(-repository.LOCK_TTL)
+
+	var buf bytes.Buffer
+	require.NoError(t, staleLock.SerializeToStream(&buf))
+	_, err := repo.PutLock(staleLockID, &buf)
+	require.NoError(t, err)
+
+	mockStore, ok := repo.Store().(*ptesting.MockBackend)
+	require.True(t, ok)
+	mockStore.RefuseLockDeletes()
+
+	builder, err := snapshot.Create(repo, repository.DefaultType, "", objects.NilMac, &snapshot.BuilderOptions{
+		Name:         "stale-lock-delete-test",
+		NoCheckpoint: true,
+	})
+	require.NoError(t, err)
+	require.NotNil(t, builder)
+	require.NoError(t, builder.Close())
+}

testing/backend.go

@@ -29,6 +29,10 @@ type mockedBackendBehavior struct {
 	packfile      string
 }
 
+const mockLockDeleteDenied = "mock lock delete denied"
+
+var ErrMockLockDeleteDenied = errors.New(mockLockDeleteDenied)
+
 var behaviors = map[string]mockedBackendBehavior{
 	"default": {
 		statesMACs:    nil,
@@ -71,7 +75,8 @@ type MockBackend struct {
 	stateMACs     map[objects.MAC][]byte
 	packfileMACs  map[objects.MAC][]byte
 
-	packfileMutex sync.Mutex
+	packfileMutex    sync.Mutex
+	refuseLockDelete bool
 	// used to trigger different behaviors during tests
 	behavior string
 }
@@ -80,6 +85,10 @@ func NewMockBackend(storeConfig map[string]string) *MockBackend {
 	return &MockBackend{location: storeConfig["location"], locks: make(map[objects.MAC][]byte), stateMACs: make(map[objects.MAC][]byte), packfileMACs: make(map[objects.MAC][]byte)}
 }
 
+func (mb *MockBackend) RefuseLockDeletes() {
+	mb.refuseLockDelete = true
+}
+
 func (mb *MockBackend) Create(ctx context.Context, configuration []byte) error {
 	if strings.Contains(mb.location, "musterror") {
 		return errors.New("creating error")
@@ -216,6 +225,9 @@ func (mb *MockBackend) Delete(ctx context.Context, res storage.StorageResource,
 	case storage.StorageResourceState:
 		delete(mb.stateMACs, mac)
 	case storage.StorageResourceLock:
+		if mb.refuseLockDelete {
+			return ErrMockLockDeleteDenied
+		}
 		delete(mb.locks, mac)
 	default:
 		return errors.ErrUnsupported