Refactor cache path and temporary file handling

- Move common path and file creation logic to utils.go
- Add `BuildCachePath` function for consistent cache path generation
- Add `WipFile` function for temporary file creation with directory handling
- Update cache and hardlink managers to use the new utility functions

Signed-off-by: ChengyuZhu6 <[email protected]>

Author: ChengyuZhu6

cache/cache.go

@@ -228,20 +228,7 @@ func (dc *directoryCache) Get(key string, opts ...Option) (Reader, error) {
 		opt = o(opt)
 	}
 
-	// 1. Try to get from hardlink first
-	if dc.hlManager != nil {
-		if linkPath, exists := dc.hlManager.GetLink(key); exists {
-			if r, err := os.Open(linkPath); err == nil {
-				return &reader{
-					ReaderAt:  r,
-					closeFunc: r.Close,
-				}, nil
-			}
-			// If failed to open, continue with normal flow
-		}
-	}
-
-	// 2. Try to get from memory cache
+	// Try to get from memory cache
 	if !dc.direct && !opt.direct {
 		if b, done, ok := dc.cache.Get(key); ok {
 			return &reader{
@@ -265,15 +252,23 @@ func (dc *directoryCache) Get(key string, opts ...Option) (Reader, error) {
 		}
 	}
 
-	// 3. Open the cache file and read the target region
+	// Try to get from hardlink first
+	filepath := BuildCachePath(dc.directory, key)
+	if dc.hlManager != nil {
+		if linkPath, exists := dc.hlManager.GetLink(key); exists {
+			filepath = linkPath
+		}
+	}
+
+	// Open the cache file and read the target region
 	// TODO: If the target cache is write-in-progress, should we wait for the completion
 	//       or simply report the cache miss?
-	file, err := os.Open(dc.cachePath(key))
+	file, err := os.Open(filepath)
 	if err != nil {
 		return nil, fmt.Errorf("failed to open blob file for %q: %w", key, err)
 	}
 
-	// 4. If in direct mode, don't cache file descriptor
+	// If in direct mode, don't cache file descriptor
 	// This option is useful for preventing memory cache from being polluted by data
 	// that won't be accessed immediately.
 	if dc.direct || opt.direct {
@@ -288,7 +283,7 @@ func (dc *directoryCache) Get(key string, opts ...Option) (Reader, error) {
 		}, nil
 	}
 
-	// 5. Cache file descriptor
+	// Cache file descriptor
 	// TODO: should we cache the entire file data on memory?
 	//       but making I/O (possibly huge) on every fetching
 	//       might be costly.
@@ -330,7 +325,7 @@ func (dc *directoryCache) Add(key string, opts ...Option) (Writer, error) {
 	}
 
 	// Create temporary file
-	w, err := dc.wipFile(key)
+	w, err := WipFile(dc.wipDirectory, key)
 	if err != nil {
 		return nil, err
 	}
@@ -344,7 +339,7 @@ func (dc *directoryCache) Add(key string, opts ...Option) (Writer, error) {
 			}
 
 			// Commit file
-			targetPath := dc.cachePath(key)
+			targetPath := BuildCachePath(dc.directory, key)
 			if err := os.MkdirAll(filepath.Dir(targetPath), 0700); err != nil {
 				return fmt.Errorf("failed to create cache directory: %w", err)
 			}
@@ -400,14 +395,6 @@ func (dc *directoryCache) isClosed() bool {
 	return closed
 }
 
-func (dc *directoryCache) cachePath(key string) string {
-	return filepath.Join(dc.directory, key[:2], key)
-}
-
-func (dc *directoryCache) wipFile(key string) (*os.File, error) {
-	return os.CreateTemp(dc.wipDirectory, key+"-*")
-}
-
 func NewMemoryCache() BlobCache {
 	return &MemoryCache{
 		Membuf: map[string]*bytes.Buffer{},
@@ -498,7 +485,7 @@ func (dc *directoryCache) CreateHardlink(key string) error {
 		return nil
 	}
 	log.L.Debugf("Creating hardlink for key %q", key)
-	return dc.hlManager.CreateLink(key, dc.cachePath(key))
+	return dc.hlManager.CreateLink(key, BuildCachePath(dc.directory, key))
 }
 
 func (dc *directoryCache) HasHardlink(key string) bool {

cache/hardlink.go

@@ -130,14 +130,13 @@ func (hm *HardlinkManager) CreateLink(key string, sourcePath string) error {
 	}
 
 	// Create hardlink in hardlinks directory
-	linkPath := filepath.Join(hm.hlDir, key[:2], key)
+	linkPath := BuildCachePath(hm.hlDir, key)
 	if err := os.MkdirAll(filepath.Dir(linkPath), 0700); err != nil {
 		return fmt.Errorf("failed to create link dir: %w", err)
 	}
 
 	// Create temporary link path
-	tmpLinkPath := linkPath + ".tmp"
-
+	tmpLinkPath := BuildCachePath(hm.hlDir, key+".wip")
 	// Remove temporary link if it exists
 	os.Remove(tmpLinkPath)
 

cache/utils.go

@@ -0,0 +1,40 @@
+/*
+   Copyright The containerd Authors.
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package cache
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+)
+
+// BuildCachePath returns the path for a cache entry with the given key
+//
+//revive:disable:exported
+func BuildCachePath(directory string, key string) string {
+	return filepath.Join(directory, key[:2], key)
+}
+
+//revive:enable:exported
+
+// WipFile creates a temporary file in the given directory with the given key pattern
+func WipFile(wipDirectory string, key string) (*os.File, error) {
+	if err := os.MkdirAll(wipDirectory, 0700); err != nil {
+		return nil, fmt.Errorf("failed to create wip directory: %w", err)
+	}
+	return os.CreateTemp(wipDirectory, key+"-*")
+}