contenthash: add test using counter metric in scanPath

While we now have tests ensuring that needsScan does not regress and
indicate that a scan is neccessary, it seems prudent to also include
checks that scanPath is definitely not running on any new paths when we
don't expect it.

Suggested-by: Tonis Tiigi <[email protected]>
Signed-off-by: Aleksa Sarai <[email protected]>

Author: cyphar

cache/contenthash/checksum.go

@@ -10,6 +10,7 @@ import (
 	"path/filepath"
 	"strings"
 	"sync"
+	"sync/atomic"
 
 	iradix "github.com/hashicorp/go-immutable-radix/v2"
 	simplelru "github.com/hashicorp/golang-lru/v2/simplelru"
@@ -995,6 +996,13 @@ func (cc *cacheContext) needsScan(root *iradix.Node[*CacheRecord], path string,
 	return cr == nil && !hasParentInTree, nil
 }
 
+// Only used by TestNeedScanChecksumRegression to make sure scanPath is not
+// called for paths we have already scanned.
+var (
+	scanCounterEnable bool
+	scanCounter       atomic.Uint64
+)
+
 func (cc *cacheContext) scanPath(ctx context.Context, m *mount, p string, followTrailing bool) (retErr error) {
 	p = path.Join("/", p)
 
@@ -1027,6 +1035,9 @@ func (cc *cacheContext) scanPath(ctx context.Context, m *mount, p string, follow
 	}
 
 	err = filepath.Walk(scanPath, func(itemPath string, fi os.FileInfo, err error) error {
+		if scanCounterEnable {
+			scanCounter.Add(1)
+		}
 		if err != nil {
 			// If the root doesn't exist, ignore the error.
 			if itemPath == scanPath && errors.Is(err, os.ErrNotExist) {

cache/contenthash/checksum_test.go

@@ -112,6 +112,12 @@ func TestChecksumSymlinkNoParentScan(t *testing.T) {
 
 // https://github.com/moby/buildkit/issues/5042
 func TestNeedScanChecksumRegression(t *testing.T) {
+	// This test cannot be run in parallel because we use scanCounter.
+	scanCounterEnable = true
+	defer func() {
+		scanCounterEnable = false
+	}()
+
 	tmpdir := t.TempDir()
 
 	snapshotter, err := native.NewSnapshotter(filepath.Join(tmpdir, "snapshots"))
@@ -166,11 +172,22 @@ func TestNeedScanChecksumRegression(t *testing.T) {
 		require.Equalf(t, test.expectNeedsScan, needs, "needsScan(%q, followTrailing=%v)", test.path, test.followTrailing)
 	}
 
+	// Make sure trying to checksum a subpath results in no further scans.
+	initialScanCounter := scanCounter.Load()
+	_, err = cc.Checksum(context.TODO(), ref, "/bb/cc", ChecksumOpts{FollowLinks: true}, nil)
+	require.NoError(t, err)
+	require.Equal(t, initialScanCounter, scanCounter.Load())
+	_, err = cc.Checksum(context.TODO(), ref, "/bb/non-existent", ChecksumOpts{FollowLinks: true}, nil)
+	require.Error(t, err)
+	require.Equal(t, initialScanCounter, scanCounter.Load())
+
 	// Looking up a non-existent path in / will checksum the whole tree. See
 	// <https://github.com/moby/buildkit/issues/5042> for more information.
 	// This means that needsScan will return true for any path.
 	_, err = cc.Checksum(context.TODO(), ref, "/non-existent", ChecksumOpts{FollowLinks: true}, nil)
 	require.Error(t, err)
+	fullScanCounter := scanCounter.Load()
+	require.NotEqual(t, fullScanCounter, initialScanCounter)
 
 	root = cc.tree.Root()
 	for _, path := range []string{
@@ -184,6 +201,21 @@ func TestNeedScanChecksumRegression(t *testing.T) {
 		require.NoErrorf(t, err, "needsScan(%q, followTrailing=true)", path)
 		require.Falsef(t, needs2, "needsScan(%q, followTrailing=true)", path)
 	}
+
+	// Looking up any more paths should not result in any more scans because we
+	// already know / was scanned.
+	_, err = cc.Checksum(context.TODO(), ref, "/non-existent", ChecksumOpts{FollowLinks: true}, nil)
+	require.Error(t, err)
+	require.Equal(t, fullScanCounter, scanCounter.Load())
+	_, err = cc.Checksum(context.TODO(), ref, "/different/non/existent", ChecksumOpts{FollowLinks: true}, nil)
+	require.Error(t, err)
+	require.Equal(t, fullScanCounter, scanCounter.Load())
+	_, err = cc.Checksum(context.TODO(), ref, "/aa/root/aa/non-exist", ChecksumOpts{FollowLinks: true}, nil)
+	require.Error(t, err)
+	require.Equal(t, fullScanCounter, scanCounter.Load())
+	_, err = cc.Checksum(context.TODO(), ref, "/aa/root/bb/cc", ChecksumOpts{FollowLinks: true}, nil)
+	require.NoError(t, err)
+	require.Equal(t, fullScanCounter, scanCounter.Load())
 }
 
 func TestChecksumNonLexicalSymlinks(t *testing.T) {