fix: expand cache probe support to more directives

coder · Jul 12, 2024 · 48a5932 · 48a5932
1 parent dc26a92
commit 48a5932
Show file tree

Hide file tree

Showing 16 changed files with 105 additions and 8 deletions.
diff --git a/pkg/commands/arg.go b/pkg/commands/arg.go
@@ -41,6 +41,10 @@ func (r *ArgCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bui
 	return nil
 }
 
+func (r *ArgCommand) CachedExecuteCommand(config *v1.Config, buildArgs *dockerfile.BuildArgs) error {
+	return r.ExecuteCommand(config, buildArgs)
+}
+
 func ParseArg(key string, val *string, env []string, ba *dockerfile.BuildArgs) (string, *string, error) {
 	replacementEnvs := ba.ReplacementEnvs(env)
 	resolvedKey, err := util.ResolveEnvironmentReplacement(key, replacementEnvs, false)

diff --git a/pkg/commands/cmd.go b/pkg/commands/cmd.go
@@ -54,6 +54,10 @@ func (c *CmdCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bui
 	return nil
 }
 
+func (c *CmdCommand) CachedExecuteCommand(config *v1.Config, buildArgs *dockerfile.BuildArgs) error {
+	return c.ExecuteCommand(config, buildArgs)
+}
+
 // String returns some information about the command for the image config history
 func (c *CmdCommand) String() string {
 	return c.cmd.String()

diff --git a/pkg/commands/entrypoint.go b/pkg/commands/entrypoint.go
@@ -51,6 +51,10 @@ func (e *EntrypointCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerf
 	return nil
 }
 
+func (e *EntrypointCommand) CachedExecuteCommand(config *v1.Config, buildArgs *dockerfile.BuildArgs) error {
+	return e.ExecuteCommand(config, buildArgs)
+}
+
 // String returns some information about the command for the image config history
 func (e *EntrypointCommand) String() string {
 	return e.cmd.String()

diff --git a/pkg/commands/env.go b/pkg/commands/env.go
@@ -35,6 +35,10 @@ func (e *EnvCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bui
 	return util.UpdateConfigEnv(newEnvs, config, replacementEnvs)
 }
 
+func (e *EnvCommand) CachedExecuteCommand(config *v1.Config, buildArgs *dockerfile.BuildArgs) error {
+	return e.ExecuteCommand(config, buildArgs)
+}
+
 // String returns some information about the command for the image config history
 func (e *EnvCommand) String() string {
 	return e.cmd.String()

diff --git a/pkg/commands/expose.go b/pkg/commands/expose.go
@@ -63,6 +63,10 @@ func (r *ExposeCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.
 	return nil
 }
 
+func (r *ExposeCommand) CachedExecuteCommand(config *v1.Config, buildArgs *dockerfile.BuildArgs) error {
+	return r.ExecuteCommand(config, buildArgs)
+}
+
 func validProtocol(protocol string) bool {
 	validProtocols := [2]string{"tcp", "udp"}
 	for _, p := range validProtocols {

diff --git a/pkg/commands/healthcheck.go b/pkg/commands/healthcheck.go
@@ -46,6 +46,10 @@ func (h *HealthCheckCommand) ExecuteCommand(config *v1.Config, buildArgs *docker
 	return nil
 }
 
+func (h *HealthCheckCommand) CachedExecuteCommand(config *v1.Config, buildArgs *dockerfile.BuildArgs) error {
+	return h.ExecuteCommand(config, buildArgs)
+}
+
 // String returns some information about the command for the image config history
 func (h *HealthCheckCommand) String() string {
 	return h.cmd.String()

diff --git a/pkg/commands/label.go b/pkg/commands/label.go
@@ -34,6 +34,10 @@ func (r *LabelCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.B
 	return updateLabels(r.cmd.Labels, config, buildArgs)
 }
 
+func (r *LabelCommand) CachedExecuteCommand(config *v1.Config, buildArgs *dockerfile.BuildArgs) error {
+	return r.ExecuteCommand(config, buildArgs)
+}
+
 func updateLabels(labels []instructions.KeyValuePair, config *v1.Config, buildArgs *dockerfile.BuildArgs) error {
 	existingLabels := config.Labels
 	if existingLabels == nil {

diff --git a/pkg/commands/onbuild.go b/pkg/commands/onbuild.go
@@ -40,6 +40,10 @@ func (o *OnBuildCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile
 	return nil
 }
 
+func (o *OnBuildCommand) CachedExecuteCommand(config *v1.Config, buildArgs *dockerfile.BuildArgs) error {
+	return o.ExecuteCommand(config, buildArgs)
+}
+
 // String returns some information about the command for the image config history
 func (o *OnBuildCommand) String() string {
 	return o.cmd.String()

diff --git a/pkg/commands/run_marker.go b/pkg/commands/run_marker.go
@@ -47,6 +47,11 @@ func (r *RunMarkerCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfi
 	return nil
 }
 
+func (r *RunMarkerCommand) CachedExecuteCommand(config *v1.Config, buildArgs *dockerfile.BuildArgs) error {
+	r.Files = []string{}
+	return nil
+}
+
 // String returns some information about the command for the image config
 func (r *RunMarkerCommand) String() string {
 	return r.cmd.String()

diff --git a/pkg/commands/shell.go b/pkg/commands/shell.go
@@ -33,6 +33,10 @@ func (s *ShellCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.B
 	return nil
 }
 
+func (s *ShellCommand) CachedExecuteCommand(config *v1.Config, buildArgs *dockerfile.BuildArgs) error {
+	return s.ExecuteCommand(config, buildArgs)
+}
+
 // String returns some information about the command for the image config history
 func (s *ShellCommand) String() string {
 	return s.cmd.String()

diff --git a/pkg/commands/stopsignal.go b/pkg/commands/stopsignal.go
@@ -53,6 +53,10 @@ func (s *StopSignalCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerf
 	return nil
 }
 
+func (s *StopSignalCommand) CachedExecuteCommand(config *v1.Config, buildArgs *dockerfile.BuildArgs) error {
+	return s.ExecuteCommand(config, buildArgs)
+}
+
 // String returns some information about the command for the image config history
 func (s *StopSignalCommand) String() string {
 	return s.cmd.String()

diff --git a/pkg/commands/user.go b/pkg/commands/user.go
@@ -55,6 +55,10 @@ func (r *UserCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bu
 	return nil
 }
 
+func (r *UserCommand) CachedExecuteCommand(config *v1.Config, buildArgs *dockerfile.BuildArgs) error {
+	return r.ExecuteCommand(config, buildArgs)
+}
+
 func (r *UserCommand) String() string {
 	return r.cmd.String()
 }
diff --git a/pkg/commands/volume.go b/pkg/commands/volume.go
@@ -63,6 +63,27 @@ func (v *VolumeCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.
 	return nil
 }
 
+func (v *VolumeCommand) CachedExecuteCommand(config *v1.Config, buildArgs *dockerfile.BuildArgs) error {
+	volumes := v.cmd.Volumes
+	replacementEnvs := buildArgs.ReplacementEnvs(config.Env)
+	resolvedVolumes, err := util.ResolveEnvironmentReplacementList(volumes, replacementEnvs, true)
+	if err != nil {
+		return err
+	}
+	existingVolumes := config.Volumes
+	if existingVolumes == nil {
+		existingVolumes = map[string]struct{}{}
+	}
+	for _, volume := range resolvedVolumes {
+		var x struct{}
+		existingVolumes[volume] = x
+		util.AddVolumePathToIgnoreList(volume)
+	}
+	config.Volumes = existingVolumes
+
+	return nil
+}
+
 func (v *VolumeCommand) FilesToSnapshot() []string {
 	return []string{}
 }

diff --git a/pkg/commands/workdir.go b/pkg/commands/workdir.go
@@ -72,13 +72,36 @@ func (w *WorkdirCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile
 
 		logrus.Infof("Creating directory %s with uid %d and gid %d", config.WorkingDir, uid, gid)
 		w.snapshotFiles = append(w.snapshotFiles, config.WorkingDir)
-		if err := mkdirAllWithPermissions(config.WorkingDir, 0755, uid, gid); err != nil {
+		if err := mkdirAllWithPermissions(config.WorkingDir, 0o755, uid, gid); err != nil {
 			return errors.Wrapf(err, "creating workdir %s", config.WorkingDir)
 		}
 	}
 	return nil
 }
 
+func (w *WorkdirCommand) CachedExecuteCommand(config *v1.Config, buildArgs *dockerfile.BuildArgs) error {
+	logrus.Info("Cmd: workdir")
+	workdirPath := w.cmd.Path
+	replacementEnvs := buildArgs.ReplacementEnvs(config.Env)
+	resolvedWorkingDir, err := util.ResolveEnvironmentReplacement(workdirPath, replacementEnvs, true)
+	if err != nil {
+		return err
+	}
+	if filepath.IsAbs(resolvedWorkingDir) {
+		config.WorkingDir = resolvedWorkingDir
+	} else {
+		if config.WorkingDir != "" {
+			config.WorkingDir = filepath.Join(config.WorkingDir, resolvedWorkingDir)
+		} else {
+			config.WorkingDir = filepath.Join("/", resolvedWorkingDir)
+		}
+	}
+	logrus.Infof("Changed working directory to %s", config.WorkingDir)
+
+	w.snapshotFiles = []string{}
+	return nil
+}
+
 // FilesToSnapshot returns the workingdir, which should have been created if it didn't already exist
 func (w *WorkdirCommand) FilesToSnapshot() []string {
 	return w.snapshotFiles

diff --git a/pkg/executor/build.go b/pkg/executor/build.go
@@ -451,7 +451,6 @@ func (s *stageBuilder) build() error {
 
 // probeCache builds a stage entirely from the build cache.
 // All COPY and RUN commands are faked.
-// Note: USER and ENV commands are not supported.
 func (s *stageBuilder) probeCache() error {
 	// Set the initial cache key to be the base image digest, the build args and the SrcContext.
 	var compositeKey *CompositeCache
@@ -501,12 +500,17 @@ func (s *stageBuilder) probeCache() error {
 			}
 		} else {
 			switch command.(type) {
-			case *commands.UserCommand:
+			case *commands.AddCommand:
+				// The ADD directive does not support caching.
+				return errors.Errorf("ADD is not supported in cache probe mode, use COPY instead")
+			case *commands.CopyCommand:
+				// If the cache is valid, we expect CachingCopyCommand.
+				return errors.Errorf("uncached COPY command is not supported in cache probe mode")
+			case *commands.RunCommand:
+				// If the cache is valid, we expect CachingRunCommand.
+				return errors.Errorf("uncached RUN command is not supported in cache probe mode")
 			default:
-				return errors.Errorf("uncached command %T encountered when probing cache", command)
-			}
-			if err := command.ExecuteCommand(&s.cf.Config, s.args); err != nil {
-				return errors.Wrap(err, "failed to execute command")
+				return errors.Errorf("unsupported command %T encountered in cache probe mode, missing CachedExecuteCommand", command)
 			}
 		}
 		files = command.FilesToSnapshot()

diff --git a/pkg/executor/cache_probe_test.go b/pkg/executor/cache_probe_test.go
@@ -55,7 +55,7 @@ COPY foo/bar.txt copied/
 			CacheRepo:       regCache + "/test",
 		}
 		_, err := DoCacheProbe(opts)
-		if err == nil || !strings.Contains(err.Error(), "uncached command") {
+		if err == nil || !strings.Contains(err.Error(), "uncached COPY command") {
 			t.Errorf("unexpected error, got %v", err)
 		}
 	})