Skip to content

Commit 5b280bd

Browse files
authored
Lock upgrade marker (#8254)
* Lock upgrade marker at creation * Lock update marker file when reading/writing from watcher * Add test with external locking process * Introduce yet another file locker * Run update marker lock unit tests in parallel * Unify AppLock and FileLock implementations * linting * Add godoc for FileLocker * Handle possible error at AppLocker creation * Expose Locked() function through FileLocker and add tests for double locking * split lockMarkerFile() creation and locking * changelog
1 parent 1755799 commit 5b280bd

File tree

12 files changed

+869
-27
lines changed

12 files changed

+869
-27
lines changed
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
# Kind can be one of:
2+
# - breaking-change: a change to previously-documented behavior
3+
# - deprecation: functionality that is being removed in a later release
4+
# - bug-fix: fixes a problem in a previous version
5+
# - enhancement: extends functionality but does not break or fix existing behavior
6+
# - feature: new functionality
7+
# - known-issue: problems that we are aware of in a given version
8+
# - security: impacts on the security of a product or a user’s deployment.
9+
# - upgrade: important information for someone upgrading from a prior version
10+
# - other: does not fit into any of the other categories
11+
kind: feature
12+
13+
# Change summary; a 80ish characters long description of the change.
14+
summary: Introduce update marker file locking during read/write operations
15+
16+
# Long description; in case the summary is not enough to describe the change
17+
# this field accommodate a description without length limits.
18+
# NOTE: This field will be rendered only for breaking-change and known-issue kinds at the moment.
19+
#description:
20+
21+
# Affected component; a word indicating the component this changeset affects.
22+
component: elastic-agent
23+
24+
# PR URL; optional; the PR number that added the changeset.
25+
# If not present is automatically filled by the tooling finding the PR where this changelog fragment has been added.
26+
# NOTE: the tooling supports backports, so it's able to fill the original PR number instead of the backport PR number.
27+
# Please provide it if you are adding a fragment for a different PR.
28+
pr: https://github.com/elastic/elastic-agent/pull/8254
29+
30+
# Issue URL; optional; the GitHub issue related to this changeset (either closes or is part of).
31+
# If not present is automatically filled by the tooling with the issue linked to the PR number.
32+
issue: https://github.com/elastic/elastic-agent/issues/6878

internal/pkg/agent/application/filelock/locker.go renamed to internal/pkg/agent/application/filelock/app_locker.go

Lines changed: 10 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -8,41 +8,33 @@ import (
88
"fmt"
99
"os"
1010
"path/filepath"
11-
12-
"github.com/gofrs/flock"
1311
)
1412

1513
// ErrAppAlreadyRunning error returned when another elastic-agent is already holding the lock.
1614
var ErrAppAlreadyRunning = fmt.Errorf("another elastic-agent is already running")
1715

1816
// AppLocker locks the agent.lock file inside the provided directory.
1917
type AppLocker struct {
20-
lock *flock.Flock
18+
*FileLocker
2119
}
2220

2321
// NewAppLocker creates an AppLocker that locks the agent.lock file inside the provided directory.
2422
func NewAppLocker(dir, lockFileName string) *AppLocker {
2523
if _, err := os.Stat(dir); os.IsNotExist(err) {
2624
_ = os.Mkdir(dir, 0755)
2725
}
28-
return &AppLocker{
29-
lock: flock.New(filepath.Join(dir, lockFileName)),
30-
}
31-
}
3226

33-
// TryLock tries to grab the lock file and returns error if it cannot.
34-
func (a *AppLocker) TryLock() error {
35-
locked, err := a.lock.TryLock()
27+
lockFilePath := filepath.Join(dir, lockFileName)
28+
lock, err := NewFileLocker(lockFilePath, WithCustomNotLockedError(ErrAppAlreadyRunning))
3629
if err != nil {
37-
return err
30+
// should never happen, if it does something is seriously wrong. Better to abort here and let a human take over.
31+
panic(fmt.Errorf("creating new file locker %s: %w", lockFilePath, err))
3832
}
39-
if !locked {
40-
return ErrAppAlreadyRunning
41-
}
42-
return nil
33+
34+
return &AppLocker{FileLocker: lock}
4335
}
4436

45-
// Unlock releases the lock file.
46-
func (a *AppLocker) Unlock() error {
47-
return a.lock.Unlock()
37+
// TryLock tries to grab the lock file and returns error if it cannot.
38+
func (a *AppLocker) TryLock() error {
39+
return a.Lock()
4840
}

internal/pkg/agent/application/filelock/locker_test.go renamed to internal/pkg/agent/application/filelock/app_locker_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,9 @@ func TestAppLocker(t *testing.T) {
2020
locker2 := NewAppLocker(tmp, testLockFile)
2121

2222
require.NoError(t, locker1.TryLock())
23-
assert.Error(t, locker2.TryLock())
23+
assert.ErrorIs(t, locker2.TryLock(), ErrAppAlreadyRunning)
2424
require.NoError(t, locker1.Unlock())
2525
require.NoError(t, locker2.TryLock())
26-
assert.Error(t, locker1.TryLock())
26+
assert.ErrorIs(t, locker1.TryLock(), ErrAppAlreadyRunning)
2727
require.NoError(t, locker2.Unlock())
2828
}
Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
2+
// or more contributor license agreements. Licensed under the Elastic License 2.0;
3+
// you may not use this file except in compliance with the Elastic License 2.0.
4+
5+
package filelock
6+
7+
import (
8+
"context"
9+
"errors"
10+
"fmt"
11+
"time"
12+
13+
"github.com/gofrs/flock"
14+
)
15+
16+
var (
17+
ErrZeroTimeout = errors.New("must specify a non-zero timeout for a blocking file locker")
18+
ErrNotLocked = errors.New("file not locked")
19+
)
20+
21+
// FileLocker is a thin wrapper around "github.com/gofrs/flock" Flock providing both blocking and non-blocking file locking.
22+
// It exposes a simplified Lock*/Unlock interface and by default is non-blocking.
23+
// If it's not possible to acquire a lock on the specified file ErrNotLocked (directly or wrapped in another error) is returned by default.
24+
// It's possible to customize FileLocker behavior specifying one or more FileLockerOption at creation time.
25+
type FileLocker struct {
26+
fileLock *flock.Flock
27+
blocking bool
28+
timeout time.Duration
29+
customNotLockedError error
30+
}
31+
32+
func NewFileLocker(lockFilePath string, opts ...FileLockerOption) (*FileLocker, error) {
33+
flocker := &FileLocker{fileLock: flock.New(lockFilePath)}
34+
for _, opt := range opts {
35+
if err := opt(flocker); err != nil {
36+
return nil, fmt.Errorf("applying options to new file locker: %w", err)
37+
}
38+
}
39+
return flocker, nil
40+
}
41+
42+
// Lock() will attempt to lock the configured lockfile. Depending on the options specified at FileLocker creation this
43+
// call can be blocking or non-blocking. In order to use a blocking FileLocker a timeout must be specified at creation
44+
// specifying WithTimeout() option.
45+
// Even in case of a blocking FileLocker the maximum duration of the locking attempt will be the timeout specified at creation.
46+
// If no lock can be acquired ErrNotLocked error will be returned by default, unless a custom "not locked" error has been
47+
// specified with WithCustomNotLockedError at creation.
48+
func (fl *FileLocker) Lock() error {
49+
return fl.LockContext(context.Background())
50+
}
51+
52+
// LockWithContext() will attempt to lock the configured lockfile. It has the same semantics as Lock(), additionally it
53+
// allows passing a context as an argument to back out of locking attempts when context expires (useful in case of a
54+
// blocking FileLocker)
55+
func (fl *FileLocker) LockContext(ctx context.Context) error {
56+
var locked bool
57+
var err error
58+
59+
if fl.blocking {
60+
timeoutCtx, cancel := context.WithTimeout(ctx, fl.timeout)
61+
defer cancel()
62+
locked, err = fl.fileLock.TryLockContext(timeoutCtx, time.Second)
63+
} else {
64+
locked, err = fl.fileLock.TryLock()
65+
}
66+
67+
if err != nil {
68+
return fmt.Errorf("locking %s: %w", fl.fileLock.Path(), err)
69+
}
70+
if !locked {
71+
if fl.customNotLockedError != nil {
72+
return fmt.Errorf("failed locking %s: %w", fl.fileLock.Path(), fl.customNotLockedError)
73+
}
74+
return fmt.Errorf("failed locking %s: %w", fl.fileLock.Path(), ErrNotLocked)
75+
}
76+
return nil
77+
}
78+
79+
func (fl *FileLocker) Unlock() error {
80+
return fl.fileLock.Unlock()
81+
}
82+
83+
func (fl *FileLocker) Locked() bool {
84+
return fl.fileLock.Locked()
85+
}
86+
87+
type FileLockerOption func(locker *FileLocker) error
88+
89+
// WithCustomNotLockedError will set a custom error to be returned when it's not possible to acquire a lock
90+
func WithCustomNotLockedError(customError error) FileLockerOption {
91+
return func(locker *FileLocker) error {
92+
locker.customNotLockedError = customError
93+
return nil
94+
}
95+
}
96+
97+
// WithTimeout will set the FileLocker to be blocking and will enforce a non-zero timeout.
98+
// If a zero timeout is passed this option will error out, failing the FileLocker creation.
99+
func WithTimeout(timeout time.Duration) FileLockerOption {
100+
return func(locker *FileLocker) error {
101+
102+
if timeout == 0 {
103+
return ErrZeroTimeout
104+
}
105+
106+
locker.blocking = true
107+
locker.timeout = timeout
108+
109+
return nil
110+
}
111+
}

0 commit comments

Comments
 (0)