diff --git a/snapshot/builder.go b/snapshot/builder.go index b327ffb7..39732f7c 100644 --- a/snapshot/builder.go +++ b/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 diff --git a/snapshot/builder_test.go b/snapshot/builder_test.go index 8c829859..686a72c1 100644 --- a/snapshot/builder_test.go +++ b/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()) +} diff --git a/testing/backend.go b/testing/backend.go index dbdd5e47..3629bd36 100644 --- a/testing/backend.go +++ b/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