feat(mock): expose NoPreClean option for mock roots (#324)

* Extend mock runner to be able to request skipping pre-clean of a preexisting root
* Fix bug where Clone() hadn't been copying all fields of the runner object
* Add further clarification to progress strings

Author: reubeno

internal/rpm/mock/mock.go

@@ -18,6 +18,7 @@ import (
 	"github.com/kballard/go-shellquote"
 	"github.com/microsoft/azldev/internal/global/opctx"
 	"github.com/microsoft/azldev/internal/utils/hostinfo"
+	"github.com/samber/lo"
 )
 
 const (
@@ -46,6 +47,10 @@ type Runner struct {
 	bindMounts     []bindMountRequest
 	enableNetwork  bool
 
+	// noPreClean requests that the root *not* be cleaned on command execution, even if mock deems
+	// it ought to be.
+	noPreClean bool
+
 	// baseDir is the base directory where mock will create all of its root directories. If not set,
 	// it defaults to the host system default (typically, /var/lib/mock).
 	baseDir string // optional
@@ -83,6 +88,9 @@ func (r *Runner) Clone() *Runner {
 		mockConfigPath: r.mockConfigPath,
 		bindMounts:     deep.MustCopy(r.bindMounts),
 		enableNetwork:  r.enableNetwork,
+		noPreClean:     r.noPreClean,
+		baseDir:        r.baseDir,
+		rootDir:        r.rootDir,
 	}
 }
 
@@ -97,13 +105,39 @@ func (r *Runner) AddBindMount(hostPath, mockRootPath string) *Runner {
 	return r
 }
 
+// BindMounts retrieves the set of bind mounts configured for this [Runner], expressed as
+// a map from host path to mock root path.
+func (r *Runner) BindMounts() map[string]string {
+	return lo.SliceToMap(r.bindMounts, func(item bindMountRequest) (string, string) {
+		return item.hostPath, item.mockRootPath
+	})
+}
+
 // Updates the [Runner]'s configuration to enable external network access from within the mock root.
 func (r *Runner) EnableNetwork() *Runner {
 	r.enableNetwork = true
 
 	return r
 }
 
+// HasNetworkEnabled indicates whether the [Runner] is configured to enable network access.
+func (r *Runner) HasNetworkEnabled() bool {
+	return r.enableNetwork
+}
+
+// WithNoPreClean updates the [Runner]'s configuration to ensure that mock does *not* pre-clean
+// the root when invoking an operation.
+func (r *Runner) WithNoPreClean() *Runner {
+	r.noPreClean = true
+
+	return r
+}
+
+// HasNoPreClean indicates whether the [Runner] is configured to avoid pre-cleaning the root.
+func (r *Runner) HasNoPreClean() bool {
+	return r.noPreClean
+}
+
 // WithBaseDir updates the [Runner]'s configuration to set which directory mock roots are created
 // under by default. If not set, mock will write under its default base (/var/lib/mock).
 func (r *Runner) WithBaseDir(baseDir string) *Runner {
@@ -112,6 +146,11 @@ func (r *Runner) WithBaseDir(baseDir string) *Runner {
 	return r
 }
 
+// BaseDir retrieves the path to the base directory used by this [Runner] for mock roots.
+func (r *Runner) BaseDir() string {
+	return r.baseDir
+}
+
 // WithRootDir update's the [Runner]'s configuration to set which directory the root is created
 // under.
 func (r *Runner) WithRootDir(rootDir string) *Runner {
@@ -120,6 +159,11 @@ func (r *Runner) WithRootDir(rootDir string) *Runner {
 	return r
 }
 
+// RootDir retrieves the path to the mock root used by this [Runner].
+func (r *Runner) RootDir() string {
+	return r.rootDir
+}
+
 // Retrieves the path to the mock .cfg file used by this [Runner].
 func (r *Runner) ConfigPath() string {
 	return r.mockConfigPath
@@ -147,7 +191,7 @@ func (r *Runner) InitRoot(ctx context.Context) (err error) {
 	}
 
 	if !r.verbose {
-		extcmd.SetLongRunning("Waiting for mock...")
+		extcmd.SetLongRunning("Waiting for mock (initializing build root)...")
 	}
 
 	err = extcmd.Run(ctx)
@@ -226,7 +270,7 @@ func (r *Runner) BuildSRPM(
 	}
 
 	if !r.verbose {
-		extcmd.SetLongRunning("Waiting for mock...")
+		extcmd.SetLongRunning("Waiting for mock (building SRPM)...")
 	}
 
 	// Watch output logs in real-time so we can asynchronously synthesize progress updates.
@@ -314,7 +358,7 @@ func (r *Runner) BuildRPM(ctx context.Context, srpmPath, outputDirPath string, o
 		return fmt.Errorf("failed to create external command for mock: %w", err)
 	}
 
-	extcmd = extcmd.SetLongRunning("Waiting for mock...")
+	extcmd = extcmd.SetLongRunning("Waiting for mock (building RPM)...")
 
 	// Watch output logs in real-time so we can asynchronously synthesize progress updates.
 	err = addMockLogListeners(r.eventListener, extcmd, outputDirPath)
@@ -457,6 +501,8 @@ func (r *Runner) ScrubRoot(ctx context.Context) error {
 		return fmt.Errorf("failed to create external command for mock: %w", err)
 	}
 
+	extcmd = extcmd.SetLongRunning("Waiting for mock (cleaning build root)...")
+
 	err = extcmd.Run(ctx)
 	if err != nil {
 		return fmt.Errorf("mock failed to scrub root: %w", err)
@@ -493,6 +539,10 @@ func (r *Runner) getBaseArgs() (args []string) {
 		args = append(args, "--enable-network")
 	}
 
+	if r.noPreClean {
+		args = append(args, "--no-clean")
+	}
+
 	if len(r.bindMounts) > 0 {
 		pairs := []string{}
 

internal/rpm/mock/mock_test.go

@@ -53,6 +53,48 @@ func TestMockNotPresent(t *testing.T) {
 	require.Error(t, err)
 }
 
+func TestRunnerDefaults(t *testing.T) {
+	ctx := newTestCtxWithMockPrereqsPresent()
+	runner := mock.NewRunner(ctx, testMockConfigPath)
+
+	// Pin defaults.
+	assert.Equal(t, testMockConfigPath, runner.ConfigPath())
+	assert.Empty(t, runner.BindMounts())
+	assert.False(t, runner.HasNetworkEnabled())
+	assert.False(t, runner.HasNoPreClean())
+	assert.Empty(t, runner.BaseDir())
+	assert.Empty(t, runner.RootDir())
+}
+
+func TestClone(t *testing.T) {
+	const (
+		testBaseDir   = "/base"
+		testRootDir   = "/root"
+		testHostPath  = "/host"
+		testGuestPath = "/guest"
+	)
+
+	ctx := newTestCtxWithMockPrereqsPresent()
+	runner := mock.NewRunner(ctx, testMockConfigPath)
+
+	// Set a number of non-default options so we can confirm they propagate to the clone.
+	runner.AddBindMount(testHostPath, testGuestPath)
+	runner.WithBaseDir(testBaseDir)
+	runner.WithRootDir(testRootDir)
+	runner.WithNoPreClean()
+	runner.EnableNetwork()
+
+	clone := runner.Clone()
+
+	// Confirm that the clone matches the original runner.
+	assert.Equal(t, testMockConfigPath, clone.ConfigPath())
+	assert.Equal(t, map[string]string{testHostPath: testGuestPath}, clone.BindMounts())
+	assert.True(t, clone.HasNetworkEnabled())
+	assert.True(t, clone.HasNoPreClean())
+	assert.Equal(t, testBaseDir, clone.BaseDir())
+	assert.Equal(t, testRootDir, clone.RootDir())
+}
+
 func TestGetRootPath(t *testing.T) {
 	ctx := newTestCtxWithMockPrereqsPresent()
 
@@ -187,13 +229,18 @@ func TestCmdInChroot_Success(t *testing.T) {
 
 	// Make sure our args end up in the command line (with quoting as needed).
 	assert.Contains(t, cmd.GetArgs(), "arg1 'arg2 with spaces'")
+
+	// Ensure we *don't* see unexpected options.
+	assert.NotContains(t, cmd.GetArgs(), "--no-clean")
+	assert.NotContains(t, cmd.GetArgs(), "--plugin-option=bind_mount")
 }
 
 func TestCmdInChroot_BindMount(t *testing.T) {
 	ctx := newTestCtxWithMockPrereqsPresent()
 
 	runner := mock.NewRunner(ctx, testMockConfigPath)
 	runner.AddBindMount("/host-path", "/mock-path")
+	assert.Equal(t, map[string]string{"/host-path": "/mock-path"}, runner.BindMounts())
 
 	cmd, err := runner.CmdInChroot(ctx, []string{"arg"}, false /*interactive*/)
 	require.NoError(t, err)
@@ -202,3 +249,33 @@ func TestCmdInChroot_BindMount(t *testing.T) {
 	// Look for bind-mount arg.
 	assert.Contains(t, cmd.GetArgs(), `--plugin-option=bind_mount:dirs=[("/host-path", "/mock-path")]`)
 }
+
+func TestCmdInChroot_NoPreClean(t *testing.T) {
+	ctx := newTestCtxWithMockPrereqsPresent()
+
+	runner := mock.NewRunner(ctx, testMockConfigPath)
+	runner.WithNoPreClean()
+	assert.True(t, runner.HasNoPreClean())
+
+	cmd, err := runner.CmdInChroot(ctx, []string{"arg"}, false /*interactive*/)
+	require.NoError(t, err)
+	require.NotNil(t, cmd)
+
+	// Look for no-clean arg.
+	assert.Contains(t, cmd.GetArgs(), "--no-clean")
+}
+
+func TestCmdInChroot_EnableNetworking(t *testing.T) {
+	ctx := newTestCtxWithMockPrereqsPresent()
+
+	runner := mock.NewRunner(ctx, testMockConfigPath)
+	runner.EnableNetwork()
+	assert.True(t, runner.HasNetworkEnabled())
+
+	cmd, err := runner.CmdInChroot(ctx, []string{"arg"}, false /*interactive*/)
+	require.NoError(t, err)
+	require.NotNil(t, cmd)
+
+	// Look for enable-network arg.
+	assert.Contains(t, cmd.GetArgs(), "--enable-network")
+}