Skip to content

Commit

Permalink
Fix: Local path should include bucket name for mountpoint s3 (#1012)
Browse files Browse the repository at this point in the history
If two buckets are mounted, they currently have the same local path.
This causes the last bucket to override the prior.
  • Loading branch information
dleviminzi authored Mar 5, 2025
1 parent 7ed1f2c commit de5897c
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 6 deletions.
6 changes: 3 additions & 3 deletions pkg/worker/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,15 +128,15 @@ func (c *ImageClient) PullLazy(ctx context.Context, request *types.ContainerRequ
if _, err := os.Stat(baseBlobFsContentPath); err == nil && c.cacheClient.IsPathCachedNearby(ctx, "/"+sourcePath) {
localCachePath = baseBlobFsContentPath
} else {
outputLogger.Info(fmt.Sprintf("Image <%s> not found in worker region, caching nearby", imageId))
outputLogger.Info(fmt.Sprintf("Image <%s> not found in worker region, caching nearby\n", imageId))

// Otherwise, lets cache it in a nearby blobcache host
_, err := c.cacheClient.StoreContentFromSource(sourcePath, sourceOffset)
if err == nil {
localCachePath = baseBlobFsContentPath
outputLogger.Info(fmt.Sprintf("Image <%s> cached in worker's region", imageId))
outputLogger.Info(fmt.Sprintf("Image <%s> cached in worker region\n", imageId))
} else {
outputLogger.Info(fmt.Sprintf("Failed to cache image in worker's region <%s>: %v", imageId, err))
outputLogger.Info(fmt.Sprintf("Failed to cache image in worker's region <%s>: %v\n", imageId, err))
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/worker/lifecycle.go
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ func (s *Worker) RunContainer(ctx context.Context, request *types.ContainerReque
InitialSpec: initialBundleSpec,
}

err = s.containerMountManager.SetupContainerMounts(request)
err = s.containerMountManager.SetupContainerMounts(request, outputLogger)
if err != nil {
s.containerLogger.Log(request.ContainerId, request.StubId, fmt.Sprintf("failed to setup container mounts: %v", err))
}
Expand Down
7 changes: 5 additions & 2 deletions pkg/worker/mount.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package worker

import (
"fmt"
"log/slog"
"os"
"path"

Expand All @@ -25,7 +26,7 @@ func NewContainerMountManager(config types.AppConfig) *ContainerMountManager {
}

// SetupContainerMounts initializes any external storage for a container
func (c *ContainerMountManager) SetupContainerMounts(request *types.ContainerRequest) error {
func (c *ContainerMountManager) SetupContainerMounts(request *types.ContainerRequest, outputLogger *slog.Logger) error {
// Create local workspace path so we can symlink volumes before the container starts
os.MkdirAll(defaultContainerDirectory, os.FileMode(0755))

Expand All @@ -42,12 +43,14 @@ func (c *ContainerMountManager) SetupContainerMounts(request *types.ContainerReq
}

if m.MountType == storage.StorageModeMountPoint && m.MountPointConfig != nil {
log.Info().Interface("mount", m).Interface("config", m.MountPointConfig).Msg("setting up container mounts")
// Add containerId to local mount path for mountpoint storage
m.LocalPath = path.Join(m.LocalPath, request.ContainerId)
m.LocalPath = path.Join(m.LocalPath, request.ContainerId, m.MountPointConfig.BucketName)
request.Mounts[i].LocalPath = m.LocalPath

err := c.setupMountPointS3(request.ContainerId, m)
if err != nil {
outputLogger.Info(fmt.Sprintf("failed to setup s3 mount, error: %v\n", err))
return err
}
}
Expand Down

0 comments on commit de5897c

Please sign in to comment.