Skip to content

Commit

Permalink
mount,cache: lockfiles must not be part of users cache content
Browse files Browse the repository at this point in the history
`--mount=type=cache` must not add internal lockfiles to cache directory
created by users instead store it in a different central directory with
path as `/base/buildah-cache/buildah-lockfiles`.

There are use-cases where users can wipe cache between the builds so
lockfiles will be removed in unexpected manner and also its not okay to
mix buildah's internal construct with user's cache content.

Helps in: containers#4342

Signed-off-by: Aditya R <arajan@redhat.com>
  • Loading branch information
flouthoc committed Oct 19, 2022
1 parent 7f2e17e commit f2e0af5
Showing 1 changed file with 15 additions and 2 deletions.
17 changes: 15 additions & 2 deletions internal/parse/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ const (
BuildahCacheDir = "buildah-cache"
// mount=type=cache allows users to lock a cache store while its being used by another build
BuildahCacheLockfile = "buildah-cache-lockfile"
// All the lockfiles are stored in a separate directory inside `BuildahCacheDir`
// Example `/var/tmp/buildah-cache/<target>/buildah-cache-lockfile`
BuildahCacheLockfileDir = "buildah-cache-lockfiles"
)

var (
Expand Down Expand Up @@ -187,6 +190,7 @@ func GetBindMount(ctx *types.SystemContext, args []string, contextDir string, st
func GetCacheMount(args []string, store storage.Store, imageMountLabel string, additionalMountPoints map[string]internal.StageMountDetails) (specs.Mount, []string, error) {
var err error
var mode uint64
var buildahLockFilesDir string
lockedTargets := make([]string, 0)
var (
setDest bool
Expand Down Expand Up @@ -336,8 +340,10 @@ func GetCacheMount(args []string, store storage.Store, imageMountLabel string, a

if id != "" {
newMount.Source = filepath.Join(cacheParent, filepath.Clean(id))
buildahLockFilesDir = filepath.Join(BuildahCacheLockfileDir, filepath.Clean(id))
} else {
newMount.Source = filepath.Join(cacheParent, filepath.Clean(newMount.Destination))
buildahLockFilesDir = filepath.Join(BuildahCacheLockfileDir, filepath.Clean(newMount.Destination))
}
idPair := idtools.IDPair{
UID: uid,
Expand All @@ -348,18 +354,25 @@ func GetCacheMount(args []string, store storage.Store, imageMountLabel string, a
if err != nil {
return newMount, lockedTargets, fmt.Errorf("unable to change uid,gid of cache directory: %w", err)
}

// create a subdirectory inside `cacheParent` just to store lockfiles
buildahLockFilesDir = filepath.Join(cacheParent, buildahLockFilesDir)
err = os.MkdirAll(buildahLockFilesDir, os.FileMode(0700))
if err != nil {
return newMount, lockedTargets, fmt.Errorf("unable to create build cache lockfiles directory: %w", err)
}
}

switch sharing {
case "locked":
// lock parent cache
lockfile, err := lockfile.GetLockfile(filepath.Join(newMount.Source, BuildahCacheLockfile))
lockfile, err := lockfile.GetLockfile(filepath.Join(buildahLockFilesDir, BuildahCacheLockfile))
if err != nil {
return newMount, lockedTargets, fmt.Errorf("unable to acquire lock when sharing mode is locked: %w", err)
}
// Will be unlocked after the RUN step is executed.
lockfile.Lock()
lockedTargets = append(lockedTargets, filepath.Join(newMount.Source, BuildahCacheLockfile))
lockedTargets = append(lockedTargets, filepath.Join(buildahLockFilesDir, BuildahCacheLockfile))
case "shared":
// do nothing since default is `shared`
break
Expand Down

0 comments on commit f2e0af5

Please sign in to comment.