Repository: knative-sandbox/kn-plugin-func
Pull request #1063 (2022-06-14 13:48:15)

fix: s2i build as-dockerfile mode
# Changes
* Use `"as dockerfile"` mode of `s2i` builds:
  Rationale: `podman` < 4.0 has faulty `/commit` endpoint so `"daemon mode"` is not working


/kind bug


```release-note
fix: s2i+podman<4.0 build error "CommitFailure: invalid change "LABEL" - must be formatted as KEY VALU"
```



matejvasek: This version inspects appropriate label and sets s2i script url. This fixes the issue with openjdk builds.
codecov[bot]: # [Codecov](https://codecov.io/gh/knative-sandbox/kn-plugin-func/pull/1063?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox) Report
> Merging [#1063](https://codecov.io/gh/knative-sandbox/kn-plugin-func/pull/1063?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox) (b152c29) into [main](https://codecov.io/gh/knative-sandbox/kn-plugin-func/commit/5f6d11c49180440fcf81046dd475c99857cdb701?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox) (5f6d11c) will **increase** coverage by `0.17%`.
> The diff coverage is `40.94%`.

```diff
@@            Coverage Diff             @@
##             main    #1063      +/-   ##
==========================================
+ Coverage   46.79%   46.97%   +0.17%     
==========================================
  Files          59       58       -1     
  Lines        7761     7841      +80     
==========================================
+ Hits         3632     3683      +51     
- Misses       3783     3799      +16     
- Partials      346      359      +13     
```


| [Impacted Files](https://codecov.io/gh/knative-sandbox/kn-plugin-func/pull/1063?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox) | Coverage Δ | |
|---|---|---|
| [s2i/builder.go](https://codecov.io/gh/knative-sandbox/kn-plugin-func/pull/1063/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox#diff-czJpL2J1aWxkZXIuZ28=) | `56.15% <40.94%> (-14.63%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/knative-sandbox/kn-plugin-func/pull/1063?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/knative-sandbox/kn-plugin-func/pull/1063?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox). Last update [5f6d11c...b152c29](https://codecov.io/gh/knative-sandbox/kn-plugin-func/pull/1063?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=knative-sandbox).

matejvasek: @lance @lkingland @zroubalik  PTAL
matejvasek: Too bad this won't make it to midstream release. Maybe we could do at least upstream release?
knative-prow[bot]: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/knative-sandbox/kn-plugin-func/pull/1063#" title="Author self-approved">matejvasek</a>*, *<a href="https://github.com/knative-sandbox/kn-plugin-func/pull/1063#pullrequestreview-1007138073" title="LGTM">zroubalik</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=knative-sandbox%2Fkn-plugin-func).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/knative-sandbox/kn-plugin-func/blob/main/OWNERS)~~ [matejvasek,zroubalik]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->

Patch:
diff --git a/client.go b/client.go
index 4dac5512..fbec4dbc 100644
--- a/client.go
+++ b/client.go
@@ -2,9 +2,11 @@ package function
 
 import (
 	"context"
+	"crypto/sha256"
 	"errors"
 	"fmt"
 	"io"
+	"io/fs"
 	"net/http"
 	"os"
 	"path/filepath"
@@ -36,6 +38,15 @@ const (
 
 	// DefaultBuildType is the default build type for a Function
 	DefaultBuildType = BuildTypeLocal
+
+	// RunDataDir holds transient runtime metadata
+	// By default it is excluded from source control.
+	RunDataDir = ".func"
+
+	// buildstamp is the name of the file within the run data directory whose
+	// existence indicates the Function has been built, and whose content is
+	// a fingerprint of the filesystem at the time of the build.
+	buildstamp = "built"
 )
 
 // Client for managing Function instances.
@@ -538,7 +549,7 @@ func (c *Client) Create(cfg Function) (err error) {
 	f := NewFunctionWith(cfg)
 
 	// Create a .func diretory which is also added to a .gitignore
-	if err = createRuntimeDir(f); err != nil {
+	if err = ensureRuntimeDir(f); err != nil {
 		return
 	}
 
@@ -560,12 +571,31 @@ func (c *Client) Create(cfg Function) (err error) {
 	return
 }
 
-// createRuntimeDir creates a .func directory in the root of the given
+// Tag the Function as having been built
+// This is locally-scoped data, only indicating there presumably exists
+// a container image in the cache of the the configured builder, thus this info
+// is placed in a .func (non-source controlled) local metadata directory, which
+// is not stritly required to exist, so it is created if needed.
+func updateBuildStamp(f Function) (err error) {
+	if err = ensureRuntimeDir(f); err != nil {
+		return err
+	}
+	hash, err := fingerprint(f)
+	if err != nil {
+		return err
+	}
+	if err = os.WriteFile(filepath.Join(f.Root, RunDataDir, buildstamp), []byte(hash), os.ModePerm); err != nil {
+		return err
+	}
+	return
+}
+
+// ensureRuntimeDir creates a .func directory in the root of the given
 // Function which is also registered as ignored in .gitignore
 // TODO: Mutate extant .gitignore file if it exists rather than failing
 // if present (see contentious files in function.go), such that a user
 // can `git init` a directory prior to `func init` in the same directory).
-func createRuntimeDir(f Function) error {
+func ensureRuntimeDir(f Function) error {
 	if err := os.MkdirAll(filepath.Join(f.Root, RunDataDir), os.ModePerm); err != nil {
 		return err
 	}
@@ -611,6 +641,11 @@ func (c *Client) Build(ctx context.Context, path string) (err error) {
 		return
 	}
 
+	// Tag the function as having been built
+	if err = updateBuildStamp(f); err != nil {
+		return
+	}
+
 	// TODO: create a status structure and return it here for optional
 	// use by the cli for user echo (rather than rely on verbose mode here)
 	message := fmt.Sprintf("🙌 Function image built: %v", f.Image)
@@ -663,7 +698,7 @@ func (c *Client) Deploy(ctx context.Context, path string) (err error) {
 
 	// Functions must be built (have an associated image) before being deployed.
 	// Note that externally built images may be specified in the func.yaml
-	if !f.Built() {
+	if !f.HasImage() {
 		return ErrNotBuilt
 	}
 
@@ -859,7 +894,7 @@ func (c *Client) Push(ctx context.Context, path string) (err error) {
 		return
 	}
 
-	if !f.Built() {
+	if !f.HasImage() {
 		return ErrNotBuilt
 	}
 
@@ -873,6 +908,79 @@ func (c *Client) Push(ctx context.Context, path string) (err error) {
 	return f.Write()
 }
 
+// Built returns true if the given path contains a Function which has been
+// built without any filesystem modificaitons since (is not stale).
+func (c *Client) Built(path string) bool {
+	f, err := NewFunction(path)
+	if err != nil {
+		return false
+	}
+
+	// Missing a build image always means !Built (but does not satisfy staleness
+	// checks).
+	// NOTE: This will be updated in the future such that a build does not
+	// automatically update the Function's serialized, source-controlled state,
+	// because merely building does not indicate the Function has changed, but
+	// rather that field should be populated on deploy.  I.e. the Image name
+	// and image stamp should reside as transient data in .func until such time
+	// as the given image has been deployed.
+	// An example of how this bug manifests is that every rebuild of a Function
+	// registers the func.yaml as being dirty for source-control purposes, when
+	// this should only happen on deploy.
+	if !f.HasImage() {
+		return false
+	}
+
+	buildstampPath := filepath.Join(path, RunDataDir, buildstamp)
+
+	// If there is no build stamp, it is also not built.
+	// This case should be redundant with the above check for an image, but is
+	// temporarily necessary (see the long-winded caviat note above).
+	if _, err := os.Stat(buildstampPath); err != nil {
+		return false
+	}
+
+	// Calculate the Function's Filesystem hash and see if it has changed.
+	hash, err := fingerprint(f)
+	if err != nil {
+		fmt.Fprintf(os.Stderr, "error calculating Function's fingerprint: %v\n", err)
+		return false
+	}
+	b, err := os.ReadFile(buildstampPath)
+	if err != nil {
+		fmt.Fprintf(os.Stderr, "error reading Function's fingerprint: %v\n", err)
+		return false
+	}
+	if string(b) != hash {
+		return false // changes detected
+	}
+
+	// Function has a populated image, existing buildstamp, and the calculated
+	// fingerprint has not changed.
+	// It's a pretty good chance the thing doesn't need to be rebuilt, though
+	// of course filesystem racing conditions do exist, including both direct
+	// source code modifications or changes to the image cache.
+	return true
+}
+
+// fingerprint returns a hash of the filenames and modification timestamps of
+// the files within a Function's root.
+func fingerprint(f Function) (string, error) {
+	h := sha256.New()
+	err := filepath.Walk(f.Root, func(path string, info fs.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
+		// Always ignore .func, .git (TODO: .funcignore)
+		if info.IsDir() && (info.Name() == RunDataDir || info.Name() == ".git") {
+			return filepath.SkipDir
+		}
+		fmt.Fprintf(h, "%v:%v:", path, info.ModTime().UnixNano())
+		return nil
+	})
+	return fmt.Sprintf("%x", h.Sum(nil)), err
+}
+
 // DEFAULTS
 // ---------
 
diff --git a/cmd/deploy.go b/cmd/deploy.go
index d25e373b..5b6a726c 100644
--- a/cmd/deploy.go
+++ b/cmd/deploy.go
@@ -95,7 +95,7 @@ func runDeploy(cmd *cobra.Command, _ []string, newClient ClientFactory) (err err
 		return
 	}
 
-	function.Envs, err = mergeEnvs(function.Envs, config.EnvToUpdate, config.EnvToRemove)
+	function.Envs, _, err = mergeEnvs(function.Envs, config.EnvToUpdate, config.EnvToRemove)
 	if err != nil {
 		return
 	}
diff --git a/cmd/root.go b/cmd/root.go
index 4a2ee8dd..6984b238 100644
--- a/cmd/root.go
+++ b/cmd/root.go
@@ -287,15 +287,18 @@ func envFromCmd(cmd *cobra.Command) (*util.OrderedMap, []string, error) {
 	return util.NewOrderedMap(), []string{}, nil
 }
 
-func mergeEnvs(envs []fn.Env, envToUpdate *util.OrderedMap, envToRemove []string) ([]fn.Env, error) {
+func mergeEnvs(envs []fn.Env, envToUpdate *util.OrderedMap, envToRemove []string) ([]fn.Env, int, error) {
 	updated := sets.NewString()
 
+	var counter int
+
 	for i := range envs {
 		if envs[i].Name != nil {
 			value, present := envToUpdate.GetString(*envs[i].Name)
 			if present {
 				envs[i].Value = &value
 				updated.Insert(*envs[i].Name)
+				counter++
 			}
 		}
 	}
@@ -306,6 +309,7 @@ func mergeEnvs(envs []fn.Env, envToUpdate *util.OrderedMap, envToRemove []string
 			n := name
 			v := value
 			envs = append(envs, fn.Env{Name: &n, Value: &v})
+			counter++
 		}
 	}
 
@@ -313,6 +317,7 @@ func mergeEnvs(envs []fn.Env, envToUpdate *util.OrderedMap, envToRemove []string
 		for i, envVar := range envs {
 			if *envVar.Name == name {
 				envs = append(envs[:i], envs[i+1:]...)
+				counter++
 				break
 			}
 		}
@@ -320,10 +325,10 @@ func mergeEnvs(envs []fn.Env, envToUpdate *util.OrderedMap, envToRemove []string
 
 	errMsg := fn.ValidateEnvs(envs)
 	if len(errMsg) > 0 {
-		return []fn.Env{}, fmt.Errorf(strings.Join(errMsg, "\n"))
+		return []fn.Env{}, 0, fmt.Errorf(strings.Join(errMsg, "\n"))
 	}
 
-	return envs, nil
+	return envs, counter, nil
 }
 
 // setPathFlag ensures common text/wording when the --path flag is used
diff --git a/cmd/run.go b/cmd/run.go
index ed7589c0..ab3df730 100644
--- a/cmd/run.go
+++ b/cmd/run.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"strconv"
 
 	"github.com/ory/viper"
 	"github.com/spf13/cobra"
@@ -19,14 +20,27 @@ func NewRunCmd(newClient ClientFactory) *cobra.Command {
 		Long: `Run the function locally
 
 Runs the function locally in the current directory or in the directory
-specified by --path flag. The function must already have been built with the 'build' command.
+specified by --path flag.
+
+Building
+By default the Function will be built if never built, or if changes are detected
+to the Function's source.  Use --build to override this behavior.
+
 `,
 		Example: `
-# Build function's image first
-{{.Name}} build
-
-# Run it locally as a container
+# Run the Function locally, building if necessary
 {{.Name}} run
+
+# Run the Function, forcing a rebuild of the image.
+#   This is useful when the Function's image was manually deleted, necessitating
+#   A rebuild even when no changes have been made the Function's source.
+{{.Name}} run --build
+
+# Run the Function's existing image, disabling auto-build.
+#   This is useful when filesystem changes have been made, but one wishes to
+#   run the previously built image without rebuilding.
+{{.Name}} run --build=false
+
 `,
 		SuggestFor: []string{"rnu"},
 		PreRunE:    bindEnv("build", "path", "registry"),
@@ -36,7 +50,8 @@ specified by --path flag. The function must already have been built with the 'bu
 		"Environment variable to set in the form NAME=VALUE. "+
 			"You may provide this flag multiple times for setting multiple environment variables. "+
 			"To unset, specify the environment variable name followed by a \"-\" (e.g., NAME-).")
-	cmd.Flags().BoolP("build", "b", false, "Build the function only if the function has not been built before")
+	cmd.Flags().StringP("build", "b", "auto", "Build the function. [auto|true|false].")
+	cmd.Flags().Lookup("build").NoOptDefVal = "true" // --build is equivalient to --build=true
 	cmd.Flags().StringP("registry", "r", GetDefaultRegistry(), "Registry + namespace part of the image if building, ex 'quay.io/myuser' (Env: $FUNC_REGISTRY)")
 	setPathFlag(cmd)
 
@@ -60,14 +75,16 @@ func runRun(cmd *cobra.Command, args []string, newClient ClientFactory) (err err
 		return
 	}
 
-	function.Envs, err = mergeEnvs(function.Envs, config.EnvToUpdate, config.EnvToRemove)
+	var updated int
+	function.Envs, updated, err = mergeEnvs(function.Envs, config.EnvToUpdate, config.EnvToRemove)
 	if err != nil {
 		return
 	}
-
-	err = function.Write()
-	if err != nil {
-		return
+	if updated > 0 {
+		err = function.Write()
+		if err != nil {
+			return
+		}
 	}
 
 	// Check if the Function has been initialized
@@ -81,11 +98,32 @@ func runRun(cmd *cobra.Command, args []string, newClient ClientFactory) (err err
 	client, done := newClient(ClientConfig{Verbose: config.Verbose}, fn.WithRegistry(config.Registry))
 	defer done()
 
-	// Build if not built and --build
-	if config.Build && !function.Built() {
-		if err = client.Build(cmd.Context(), config.Path); err != nil {
-			return
+	// Build?
+	// If --build was set to 'auto', only build if client detects the Function
+	// is stale (has either never been built or has had filesystem modifications
+	// since the last build).
+	if config.Build == "auto" {
+		if !client.Built(function.Root) {
+			if err = client.Build(cmd.Context(), config.Path); err != nil {
+				return
+			}
 		}
+		fmt.Println("Detected Function was already built.  Use --build to override this behavior.")
+		// Otherwise, --build should parse to a truthy value which indicates an explicit
+		// override.
+	} else {
+		build, err := strconv.ParseBool(config.Build)
+		if err != nil {
+			return fmt.Errorf("invalid value for --build '%v'.  accepts 'auto', 'true' or 'false' (or similarly truthy value)", build)
+		}
+		if build {
+			if err = client.Build(cmd.Context(), config.Path); err != nil {
+				return err
+			}
+		} else {
+			fmt.Println("Function build disabled.  Skipping build.")
+		}
+
 	}
 
 	// Run the Function at path
@@ -122,8 +160,9 @@ type runConfig struct {
 	// Envs passed via cmd to removed
 	EnvToRemove []string
 
-	// Perform build if function hasn't been built yet
-	Build bool
+	// Perform build.  Acceptable values are the keyword 'auto', or a truthy
+	// value such as 'true', 'false, '1' or '0'.
+	Build string
 
 	// Registry for the build tag if building
 	Registry string
@@ -136,7 +175,7 @@ func newRunConfig(cmd *cobra.Command) (c runConfig, err error) {
 	}
 
 	return runConfig{
-		Build:       viper.GetBool("build"),
+		Build:       viper.GetString("build"),
 		Path:        viper.GetString("path"),
 		Verbose:     viper.GetBool("verbose"), // defined on root
 		Registry:    viper.GetString("registry"),
diff --git a/function.go b/function.go
index a7c4276b..a3f94a66 100644
--- a/function.go
+++ b/function.go
@@ -313,7 +313,7 @@ func (f Function) Initialized() bool {
 // image indicated actually exists, just that it _should_ exist based off
 // the current state of the Function object, in particular the value of
 // the Image and ImageDiget fields.
-func (f Function) Built() bool {
+func (f Function) HasImage() bool {
 	// If Image (the override) and ImageDigest (the most recent build stamp) are
 	// both empty, the Function is considered unbuilt.
 	// TODO: upgrade to a "build complete" timestamp.
diff --git a/job.go b/job.go
index 62bf51b6..c3ba3aa6 100644
--- a/job.go
+++ b/job.go
@@ -6,12 +6,6 @@ import (
 	"path/filepath"
 )
 
-const (
-	// RunDataDir holds transient runtime metadata
-	// By default it is excluded from source control.
-	RunDataDir = ".func"
-)
-
 // Job represents a running Function job (presumably started by this process'
 // Runner instance.
 type Job struct {
diff --git a/s2i/builder.go b/s2i/builder.go
index 940db898..90d96d54 100644
--- a/s2i/builder.go
+++ b/s2i/builder.go
@@ -1,14 +1,22 @@
 package s2i
 
 import (
+	"archive/tar"
 	"context"
+	"encoding/json"
 	"errors"
 	"fmt"
+	"io"
+	"io/fs"
 	"net/url"
 	"os"
+	"path/filepath"
 
+	"github.com/docker/docker/api/types"
 	dockerClient "github.com/docker/docker/client"
-
+	"github.com/google/go-containerregistry/pkg/name"
+	v1 "github.com/google/go-containerregistry/pkg/v1"
+	"github.com/google/go-containerregistry/pkg/v1/remote"
 	"github.com/openshift/source-to-image/pkg/api"
 	"github.com/openshift/source-to-image/pkg/api/validation"
 	"github.com/openshift/source-to-image/pkg/build"
@@ -17,7 +25,7 @@ import (
 	"github.com/openshift/source-to-image/pkg/scm/git"
 
 	fn "knative.dev/kn-plugin-func"
-	docker "knative.dev/kn-plugin-func/docker"
+	"knative.dev/kn-plugin-func/docker"
 )
 
 var (
@@ -36,10 +44,17 @@ var DefaultBuilderImages = map[string]string{
 	"quarkus":    "registry.access.redhat.com/ubi8/openjdk-17",
 }
 
+// DockerClient is subset of dockerClient.CommonAPIClient required by this package
+type DockerClient interface {
+	ImageBuild(ctx context.Context, context io.Reader, options types.ImageBuildOptions) (types.ImageBuildResponse, error)
+	ImageInspectWithRaw(ctx context.Context, image string) (types.ImageInspect, []byte, error)
+}
+
 // Builder of Functions using the s2i subsystem.
 type Builder struct {
 	verbose bool
 	impl    build.Builder // S2I builder implementation (aka "Strategy")
+	cli     DockerClient
 }
 
 type Option func(*Builder)
@@ -60,6 +75,12 @@ func WithImpl(s build.Builder) Option {
 	}
 }
 
+func WithDockerClient(cli DockerClient) Option {
+	return func(b *Builder) {
+		b.cli = cli
+	}
+}
+
 // NewBuilder creates a new instance of a Builder with static defaults.
 func NewBuilder(options ...Option) *Builder {
 	b := &Builder{}
@@ -70,6 +91,8 @@ func NewBuilder(options ...Option) *Builder {
 }
 
 func (b *Builder) Build(ctx context.Context, f fn.Function) (err error) {
+	// TODO this function currently doesn't support private s2i builder images since credentials are not set
+
 	// Builder image from the Function  if defined, default otherwise.
 	builderImage, err := builderImage(f)
 	if err != nil {
@@ -87,6 +110,27 @@ func (b *Builder) Build(ctx context.Context, f fn.Function) (err error) {
 	cfg.RuntimeImagePullPolicy = api.DefaultRuntimeImagePullPolicy
 	cfg.DockerConfig = s2idocker.GetDefaultDockerConfig()
 
+	tmp, err := os.MkdirTemp("", "s2i-build")
+	if err != nil {
+		return fmt.Errorf("cannot create temporary dir for s2i build: %w", err)
+	}
+	defer os.RemoveAll(tmp)
+
+	cfg.AsDockerfile = filepath.Join(tmp, "Dockerfile")
+
+	if b.cli == nil {
+		b.cli, _, err = docker.NewClient(dockerClient.DefaultDockerHost)
+		if err != nil {
+			return fmt.Errorf("cannot create docker client: %w", err)
+		}
+	}
+
+	scriptURL, err := s2iScriptURL(ctx, b.cli, cfg.BuilderImage)
+	if err != nil {
+		return fmt.Errorf("cannot get s2i script url: %w", err)
+	}
+	cfg.ScriptsURL = scriptURL
+
 	// Excludes
 	// Do not include .git, .env, .func or any language-specific cache directories
 	// (node_modules, etc) in the tar file sent to the builder, as this both
@@ -115,20 +159,9 @@ func (b *Builder) Build(ctx context.Context, f fn.Function) (err error) {
 
 	// Create the S2I builder instance if not overridden
 	if b.impl == nil {
-		var client dockerClient.CommonAPIClient
-		client, _, err = docker.NewClient(dockerClient.DefaultDockerHost)
-		if err != nil {
-			return
-		}
-		defer client.Close()
-
-		if isPodman(ctx, client) {
-			client = podmanDockerClient{client}
-		}
-
-		b.impl, _, err = strategies.Strategy(client, cfg, build.Overrides{})
+		b.impl, _, err = strategies.Strategy(nil, cfg, build.Overrides{})
 		if err != nil {
-			return
+			return fmt.Errorf("cannot create s2i builder: %w", err)
 		}
 	}
 
@@ -143,7 +176,150 @@ func (b *Builder) Build(ctx context.Context, f fn.Function) (err error) {
 			fmt.Println(message)
 		}
 	}
-	return
+
+	pr, pw := io.Pipe()
+
+	go func() {
+		tw := tar.NewWriter(pw)
+		err := filepath.Walk(tmp, func(path string, fi fs.FileInfo, err error) error {
+			if err != nil {
+				return err
+			}
+
+			p, err := filepath.Rel(tmp, path)
+			if err != nil {
+				return fmt.Errorf("cannot get relative path: %w", err)
+			}
+
+			hdr, err := tar.FileInfoHeader(fi, p)
+			if err != nil {
+				return fmt.Errorf("cannot create tar header: %w", err)
+			}
+			hdr.Name = p
+
+			err = tw.WriteHeader(hdr)
+			if err != nil {
+				return fmt.Errorf("cannot write header to thar stream: %w", err)
+			}
+			if fi.Mode().IsRegular() {
+				var r io.ReadCloser
+				r, err = os.Open(path)
+				if err != nil {
+					return fmt.Errorf("cannot open source file: %w", err)
+				}
+				_, err = io.Copy(tw, r)
+				if err != nil {
+					return fmt.Errorf("cannot copy file to tar stream :%w", err)
+				}
+			}
+
+			return nil
+		})
+		_ = tw.Close()
+		_ = pw.CloseWithError(err)
+	}()
+
+	opts := types.ImageBuildOptions{
+		Tags: []string{f.Image},
+	}
+
+	resp, err := b.cli.ImageBuild(ctx, pr, opts)
+	if err != nil {
+		return fmt.Errorf("cannot build the app image: %w", err)
+	}
+	defer resp.Body.Close()
+
+	var out io.Writer = io.Discard
+	if b.verbose {
+		out = os.Stderr
+	}
+
+	errMsg, err := parseBuildResponse(resp.Body, out)
+	if err != nil {
+		return fmt.Errorf("cannot parse response body: %w", err)
+	}
+	if errMsg != "" {
+		return fmt.Errorf("cannot build the app: %s", errMsg)
+	}
+
+	return nil
+}
+
+func parseBuildResponse(r io.Reader, w io.Writer) (errorMessage string, err error) {
+	obj := struct {
+		ErrorDetail struct {
+			Message string `json:"message"`
+		} `json:"errorDetail"`
+		Stream string `json:"stream"`
+	}{}
+	d := json.NewDecoder(r)
+	for {
+		err = d.Decode(&obj)
+		if err != nil {
+			if errors.Is(err, io.EOF) {
+				break
+			}
+			return "", err
+		}
+		if obj.ErrorDetail.Message != "" {
+			errorMessage = obj.ErrorDetail.Message
+			return errorMessage, nil
+		}
+		if obj.Stream != "" {
+			_, err = w.Write([]byte(obj.Stream))
+			if err != nil {
+				return "", err
+			}
+		}
+	}
+	return "", nil
+}
+
+func s2iScriptURL(ctx context.Context, cli DockerClient, image string) (string, error) {
+	img, _, err := cli.ImageInspectWithRaw(ctx, image)
+	if err != nil {
+		if dockerClient.IsErrNotFound(err) { // image is not in the daemon, get info directly from registry
+			var (
+				ref name.Reference
+				img v1.Image
+				cfg *v1.ConfigFile
+			)
+
+			ref, err = name.ParseReference(image)
+			if err != nil {
+				return "", fmt.Errorf("cannot parse image name: %w", err)
+			}
+			img, err = remote.Image(ref)
+			if err != nil {
+				return "", fmt.Errorf("cannot get image from registry: %w", err)
+			}
+			cfg, err = img.ConfigFile()
+			if err != nil {
+				return "", fmt.Errorf("cannot get config for image: %w", err)
+			}
+
+			if cfg.Config.Labels != nil {
+				if u, ok := cfg.Config.Labels["io.openshift.s2i.scripts-url"]; ok {
+					return u, nil
+				}
+			}
+		}
+		return "", err
+	}
+
+	if img.Config != nil && img.Config.Labels != nil {
+		if u, ok := img.Config.Labels["io.openshift.s2i.scripts-url"]; ok {
+			return u, nil
+		}
+	}
+
+	if img.ContainerConfig != nil && img.ContainerConfig.Labels != nil {
+		if u, ok := img.ContainerConfig.Labels["io.openshift.s2i.scripts-url"]; ok {
+			return u, nil
+		}
+	}
+
+	return "", nil
 }
 
 // builderImage for Function
diff --git a/s2i/docker_client_wrapper.go b/s2i/docker_client_wrapper.go
deleted file mode 100644
index e67f3725..00000000
--- a/s2i/docker_client_wrapper.go
+++ /dev/null
@@ -1,62 +0,0 @@
-package s2i
-
-import (
-	"context"
-	"encoding/json"
-	"fmt"
-	"strings"
-
-	"github.com/docker/docker/api/types"
-	"github.com/docker/docker/client"
-)
-
-// Wrapper to workaround https://github.com/containers/podman/issues/13770
-type podmanDockerClient struct {
-	client.CommonAPIClient
-}
-
-func (p podmanDockerClient) ContainerCommit(ctx context.Context, nameOrID string, opts types.ContainerCommitOptions) (types.IDResponse, error) {
-	if len(opts.Config.Cmd) > 0 {
-		bs, err := json.Marshal(opts.Config.Cmd)
-		if err != nil {
-			return types.IDResponse{}, err
-		}
-		opts.Changes = append(opts.Changes, "CMD "+string(bs))
-	}
-
-	if len(opts.Config.Entrypoint) > 0 {
-		bs, err := json.Marshal(opts.Config.Entrypoint)
-		if err != nil {
-			return types.IDResponse{}, err
-		}
-		opts.Changes = append(opts.Changes, "ENTRYPOINT "+string(bs))
-	}
-
-	if opts.Config.User != "" {
-		opts.Changes = append(opts.Changes, "USER "+opts.Config.User)
-	}
-
-	for _, e := range opts.Config.Env {
-		parts := strings.SplitN(e, "=", 2)
-		opts.Changes = append(opts.Changes, fmt.Sprintf("ENV %s=%q", parts[0], parts[1]))
-	}
-
-	for k, v := range opts.Config.Labels {
-		opts.Changes = append(opts.Changes, fmt.Sprintf("LABEL %q=%q", k, v))
-	}
-
-	return p.CommonAPIClient.ContainerCommit(ctx, nameOrID, opts)
-}
-
-func isPodman(ctx context.Context, cli client.CommonAPIClient) bool {
-	v, err := cli.ServerVersion(ctx)
-	if err != nil {
-		return false
-	}
-	for _, comp := range v.Components {
-		if comp.Name == "Podman Engine" {
-			return true
-		}
-	}
-	return false
-}


Test patch:
diff --git a/client_test.go b/client_test.go
index 7e1ba384..b7531b0b 100644
--- a/client_test.go
+++ b/client_test.go
@@ -362,7 +362,6 @@ func TestClient_New_RegistryRequired(t *testing.T) {
 	if err = client.New(context.Background(), fn.Function{Root: root}); err == nil {
 		t.Fatal("did not receive expected error creating a Function without specifying Registry")
 	}
-	fmt.Println(err)
 }
 
 // TestClient_New_ImageNameDerived ensures that the full image (tag) of the resultant OCI
@@ -1288,3 +1287,106 @@ func TestClient_Instances(t *testing.T) {
 		t.Fatalf("Expected endpoint '%v', got '%v'", expectedEndpoint, instance.Route)
 	}
 }
+
+// TestClient_BuiltStamps ensures that the client creates and considers a
+// buildstamp on build which reports whether or not a given path contains a built
+// Function.
+func TestClient_BuiltStamps(t *testing.T) {
+	root, rm := Mktemp(t)
+	defer rm()
+	builder := mock.NewBuilder()
+	client := fn.New(fn.WithBuilder(builder), fn.WithRegistry(TestRegistry))
+
+	// paths that do not contain a Function are !Built - Degenerate case
+	if client.Built(root) {
+		t.Fatal("path not containing a Function returned as being built")
+	}
+
+	// a freshly-created Function should be !Built
+	if err := client.Create(fn.Function{Runtime: TestRuntime, Root: root}); err != nil {
+		t.Fatal(err)
+	}
+	if client.Built(root) {
+		t.Fatal("newly created Function returned Built==true")
+	}
+
+	// a Function which was successfully built should return as being Built
+	if err := client.Build(context.Background(), root); err != nil {
+		t.Fatal(err)
+	}
+	if !client.Built(root) {
+		t.Fatal("freshly built Function should return Built==true")
+	}
+}
+
+// TestClient_BuiltDetects ensures that the client's Built command detects
+// filesystem changes as indicating the Function is no longer Built (aka stale)
+// This includes modifying timestamps, removing or adding files.
+func TestClient_BuiltDetects(t *testing.T) {
+	var (
+		ctx      = context.Background()
+		builder  = mock.NewBuilder()
+		client   = fn.New(fn.WithBuilder(builder), fn.WithRegistry(TestRegistry))
+		testfile = "example.go"
+		root, rm = Mktemp(t)
+	)
+	defer rm()
+
+	// Create and build a Function
+	if err := client.Create(fn.Function{Runtime: TestRuntime, Root: root}); err != nil {
+		t.Fatal(err)
+	}
+	if err := client.Build(ctx, root); err != nil {
+		t.Fatal(err)
+	}
+
+	// Prior to a filesystem edit, it will be Built.
+	if !client.Built(root) {
+		t.Fatal("feshly built Function reported Built==false (1)")
+	}
+
+	// Edit the filesystem by touching a file (updating modified timestamp)
+	if err := os.Chtimes(filepath.Join(root, "func.yaml"), time.Now(), time.Now()); err != nil {
+		fmt.Println(err)
+	}
+	if client.Built(root) {
+		t.Fatal("client did not detect file timestamp change as indicating build staleness")
+	}
+
+	// Build and double-check Built has been reset
+	if err := client.Build(ctx, root); err != nil {
+		t.Fatal(err)
+	}
+	if !client.Built(root) {
+		t.Fatal("freshly built Function reported Built==false (2)")
+	}
+
+	// Edit the Function's filesystem by adding a file.
+	f, err := os.Create(filepath.Join(root, testfile))
+	if err != nil {
+		t.Fatal(err)
+	}
+	f.Close()
+
+	// The system should now detect the Function is stale
+	if client.Built(root) {
+		t.Fatal("client did not detect an added file as indicating build staleness")
+	}
+
+	// Build and double-check Built has been reset
+	if err := client.Build(ctx, root); err != nil {
+		t.Fatal(err)
+	}
+	if !client.Built(root) {
+		t.Fatal("freshly built Function reported Built==false (3)")
+	}
+
+	// Remove the testfile, which should result in the client reporting that
+	// the Function is no longer Built (stale)
+	if err := os.Remove(filepath.Join(root, testfile)); err != nil {
+		t.Fatal(err)
+	}
+	if client.Built(root) {
+		t.Fatal("client did not detect a removed file as indicating build staleness")
+	}
+}
diff --git a/cmd/root_test.go b/cmd/root_test.go
index 198b372a..05d31560 100644
--- a/cmd/root_test.go
+++ b/cmd/root_test.go
@@ -146,7 +146,7 @@ func TestRoot_mergeEnvMaps(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			got, err := mergeEnvs(tt.args.envs, tt.args.toUpdate, tt.args.toRemove)
+			got, _, err := mergeEnvs(tt.args.envs, tt.args.toUpdate, tt.args.toRemove)
 			if err != nil {
 				t.Errorf("mergeEnvs() for initial vars %v and toUpdate %v and toRemove %v got error %v",
 					tt.args.envs, tt.args.toUpdate, tt.args.toRemove, err)
diff --git a/cmd/run_test.go b/cmd/run_test.go
index 5028dcd0..3c8ce6e3 100644
--- a/cmd/run_test.go
+++ b/cmd/run_test.go
@@ -6,7 +6,6 @@ import (
 	"os"
 	"testing"
 
-	"github.com/ory/viper"
 	fn "knative.dev/kn-plugin-func"
 	"knative.dev/kn-plugin-func/mock"
 	. "knative.dev/kn-plugin-func/testing"
@@ -14,57 +13,85 @@ import (
 
 func TestRun_Run(t *testing.T) {
 	tests := []struct {
-		name         string // name of the test
-		desc         string // description of the test
-		funcState    string // Function state, as described in func.yaml
-		buildFlag    bool   // value to which the --build flag should be set
-		buildError   error  // Set the builder to yield this error
-		runError     error  // Set the runner to yield this error
-		buildInvoked bool   // should Builder.Build be invoked?
-		runInvoked   bool   // should Runner.Run be invoked?
+		name         string   // name of the test
+		desc         string   // description of the test
+		funcState    string   // Function state, as described in func.yaml
+		args         []string // args for the test case
+		buildError   error    // Set the builder to yield this error
+		runError     error    // Set the runner to yield this error
+		buildInvoked bool     // should Builder.Build be invoked?
+		runInvoked   bool     // should Runner.Run be invoked?
 	}{
 		{
-			name: "run when not building",
-			desc: "Should run when build is not enabled",
+			name: "run and build by default",
+			desc: "Should run and build when build flag is not specified",
 			funcState: `name: test-func
 runtime: go
 created: 2009-11-10 23:00:00`,
-			buildFlag:    false,
-			buildInvoked: false,
+			args:         []string{},
+			buildInvoked: true,
+			runInvoked:   true,
+		},
+		{
+			name: "run and build flag",
+			desc: "Should run and build when build is merely provided (defaults to true on presence)",
+			funcState: `name: test-func
+runtime: go
+created: 2009-11-10 23:00:00`,
+			args:         []string{"--build"},
+			buildInvoked: true,
 			runInvoked:   true,
 		},
 		{
 			name: "run and build",
-			desc: "Should run and build when build is enabled and there is no image",
+			desc: "Should run and build when build is specifically requested",
 			funcState: `name: test-func
 runtime: go
 created: 2009-11-10 23:00:00`,
-			buildFlag:    true,
+			args:         []string{"--build=true"},
 			buildInvoked: true,
 			runInvoked:   true,
 		},
 		{
-			name: "skip rebuild",
-			desc: "Built image doesn't get built again",
-			// TODO: this might be improved by checking if the user provided
-			// the --build=true flag, allowing an override to force rebuild.
-			// This could be accomplished by adding a 'provideBuildFlag' struct
-			// member.
+			name: "run without build when disabled",
+			desc: "Should run but not build when build is expressly disabled",
 			funcState: `name: test-func
 runtime: go
-image: exampleimage
 created: 2009-11-10 23:00:00`,
-			buildFlag:    true,
+			args:         []string{"--build=false"}, // can be any truthy value: 0, 'false' etc.
 			buildInvoked: false,
 			runInvoked:   true,
 		},
 		{
-			name: "Build errors return",
-			desc: "Errors building cause an immediate return with error",
+			name: "run and build on auto",
+			desc: "Should run and buil when build flag set to auto",
+			funcState: `name: test-func
+runtime: go
+created: 2009-11-10 23:00:00`,
+			args:         []string{"--build=auto"}, // can be any truthy value: 0, 'false' etc.
+			buildInvoked: true,
+			runInvoked:   true,
+		},
+		{
+			name: "image existence builds",
+			desc: "Should build when image tag exists",
+			// The existence of an image tag value does not mean the Function
+			// is built; that is the purvew of the buld stamp staleness check.
 			funcState: `name: test-func
 runtime: go
+image: exampleimage
 created: 2009-11-10 23:00:00`,
-			buildFlag:    true,
+			args:         []string{},
+			buildInvoked: true,
+			runInvoked:   true,
+		},
+		{
+			name: "Build errors return",
+			desc: "Errors building cause an immediate return with error",
+			funcState: `name: test-func
+			runtime: go
+			created: 2009-11-10 23:00:00`,
+			args:         []string{},
 			buildError:   fmt.Errorf("generic build error"),
 			buildInvoked: true,
 			runInvoked:   false,
@@ -95,10 +122,7 @@ created: 2009-11-10 23:00:00`,
 					fn.WithRegistry("ghcr.com/reg"),
 				)
 			}))
-			cmd.SetArgs([]string{}) // Do not use test command args
-
-			// set test case's build
-			viper.SetDefault("build", tt.buildFlag)
+			cmd.SetArgs(tt.args) // Do not use test command args
 
 			// set test case's func.yaml
 			if err := os.WriteFile("func.yaml", []byte(tt.funcState), os.ModePerm); err != nil {
diff --git a/s2i/builder_test.go b/s2i/builder_test.go
index 81061d25..9c4ee974 100644
--- a/s2i/builder_test.go
+++ b/s2i/builder_test.go
@@ -1,10 +1,29 @@
 package s2i_test
 
 import (
+	"archive/tar"
+	"bytes"
 	"context"
 	"errors"
+	"fmt"
+	"io"
+	"io/ioutil"
+	"log"
+	"net"
+	"net/http"
+	"os"
+	"path/filepath"
+	"strings"
 	"testing"
 
+	"github.com/google/go-containerregistry/pkg/name"
+	"github.com/google/go-containerregistry/pkg/registry"
+	"github.com/google/go-containerregistry/pkg/v1/remote"
+	"github.com/google/go-containerregistry/pkg/v1/tarball"
+
+	"github.com/docker/docker/api/types"
+	"github.com/docker/docker/api/types/container"
+
 	"github.com/openshift/source-to-image/pkg/api"
 	fn "knative.dev/kn-plugin-func"
 	"knative.dev/kn-plugin-func/s2i"
@@ -37,9 +56,10 @@ func Test_ErrRuntimeNotSupported(t *testing.T) {
 // define a Builder Image will default.
 func Test_ImageDefault(t *testing.T) {
 	var (
-		i = &mockImpl{}                     // mock underlying s2i implementation
-		b = s2i.NewBuilder(s2i.WithImpl(i)) // Func S2I Builder logic
-		f = fn.Function{Runtime: "node"}    // Function with no builder image set
+		i = &mockImpl{}                                              // mock underlying s2i implementation
+		c = mockDocker{}                                             // mock docker client
+		b = s2i.NewBuilder(s2i.WithImpl(i), s2i.WithDockerClient(c)) // Func S2I Builder logic
+		f = fn.Function{Runtime: "node"}                             // Function with no builder image set
 	)
 
 	// An implementation of the underlying S2I implementation which verifies
@@ -63,9 +83,10 @@ func Test_ImageDefault(t *testing.T) {
 // image defined on the given Function if provided.
 func Test_BuilderImageConfigurable(t *testing.T) {
 	var (
-		i = &mockImpl{}                     // mock underlying s2i implementation
-		b = s2i.NewBuilder(s2i.WithImpl(i)) // Func S2I Builder logic
-		f = fn.Function{                    // Function with a builder image set
+		i = &mockImpl{}                                              // mock underlying s2i implementation
+		c = mockDocker{}                                             // mock docker client
+		b = s2i.NewBuilder(s2i.WithImpl(i), s2i.WithDockerClient(c)) // Func S2I Builder logic
+		f = fn.Function{                                             // Function with a builder image set
 			Runtime: "node",
 			BuilderImages: map[string]string{
 				"s2i": "example.com/user/builder-image",
@@ -93,6 +114,7 @@ func Test_BuilderImageConfigurable(t *testing.T) {
 // Test_Verbose ensures that the verbosity flag is propagated to the
 // S2I builder implementation.
 func Test_BuilderVerbose(t *testing.T) {
+	c := mockDocker{} // mock docker client
 	assert := func(verbose bool) {
 		i := &mockImpl{
 			BuildFn: func(cfg *api.Config) (r *api.Result, err error) {
@@ -101,7 +123,7 @@ func Test_BuilderVerbose(t *testing.T) {
 				}
 				return &api.Result{Messages: []string{"message"}}, nil
 			}}
-		if err := s2i.NewBuilder(s2i.WithVerbose(verbose), s2i.WithImpl(i)).Build(context.Background(), fn.Function{Runtime: "node"}); err != nil {
+		if err := s2i.NewBuilder(s2i.WithVerbose(verbose), s2i.WithImpl(i), s2i.WithDockerClient(c)).Build(context.Background(), fn.Function{Runtime: "node"}); err != nil {
 			t.Fatal(err)
 		}
 	}
@@ -122,7 +144,8 @@ func Test_BuildEnvs(t *testing.T) {
 			BuildEnvs: []fn.Env{{Name: &envName, Value: &envValue}},
 		}
 		i = &mockImpl{}
-		b = s2i.NewBuilder(s2i.WithImpl(i))
+		c = mockDocker{}
+		b = s2i.NewBuilder(s2i.WithImpl(i), s2i.WithDockerClient(c))
 	)
 	i.BuildFn = func(cfg *api.Config) (r *api.Result, err error) {
 		for _, v := range cfg.Environment {
@@ -140,6 +163,190 @@ func Test_BuildEnvs(t *testing.T) {
 	}
 }
 
+func TestS2IScriptURL(t *testing.T) {
+	testRegistry := startRegistry(t)
+
+	// builder that is only in registry not in daemon
+	remoteBuilder := testRegistry + "/default/builder:remote"
+	// builder that is in daemon
+	localBuilder := "example.com/default/builder:local"
+
+	// begin push testing builder to registry
+	tag, err := name.NewTag(remoteBuilder)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	img, err := tarball.ImageFromPath(filepath.Join("testData", "builder.tar"), nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	err = remote.Write(&tag, img)
+	if err != nil {
+		t.Fatal(err)
+	}
+	// end push testing builder to registry
+
+	scriptURL := "image:///usr/local/s2i"
+	cli := mockDocker{
+		inspect: func(ctx context.Context, image string) (types.ImageInspect, []byte, error) {
+			if image != localBuilder {
+				return types.ImageInspect{}, nil, notFoundErr{}
+			}
+			return types.ImageInspect{
+				Config: &container.Config{Labels: map[string]string{"io.openshift.s2i.scripts-url": scriptURL}},
+			}, nil, nil
+		},
+	}
+	impl := &mockImpl{
+		BuildFn: func(config *api.Config) (*api.Result, error) {
+			if config.ScriptsURL != scriptURL {
+				return nil, fmt.Errorf("unexepeted ScriptURL: %q", config.ScriptsURL)
+			}
+			return nil, nil
+		},
+	}
+
+	tests := []struct {
+		name         string
+		builderImage string
+	}{
+		{name: "builder in daemon", builderImage: localBuilder},
+		{name: "builder not in daemon", builderImage: remoteBuilder},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			f := fn.Function{
+				Runtime: "node",
+				BuilderImages: map[string]string{
+					"s2i": tt.builderImage,
+				},
+			}
+
+			b := s2i.NewBuilder(s2i.WithImpl(impl), s2i.WithDockerClient(cli))
+			err = b.Build(context.Background(), f)
+			if err != nil {
+				t.Error(err)
+			}
+		})
+	}
+
+}
+
+func startRegistry(t *testing.T) (addr string) {
+	s := http.Server{
+		Handler: registry.New(registry.Logger(log.New(io.Discard, "", 0))),
+	}
+	t.Cleanup(func() { s.Close() })
+
+	l, err := net.Listen("tcp", "localhost:0")
+	if err != nil {
+		t.Fatal(err)
+	}
+	addr = l.Addr().String()
+
+	go func() {
+		err = s.Serve(l)
+		if err != nil && !errors.Is(err, net.ErrClosed) {
+			fmt.Fprintln(os.Stderr, "ERROR: ", err)
+		}
+	}()
+
+	return addr
+}
+
+func TestBuildContextUpload(t *testing.T) {
+
+	dockerfileContent := []byte("FROM scratch\nLABEL A=42")
+	atxtContent := []byte("hello world!\n")
+
+	cli := mockDocker{
+		build: func(ctx context.Context, context io.Reader, options types.ImageBuildOptions) (types.ImageBuildResponse, error) {
+			tr := tar.NewReader(context)
+			for {
+				hdr, err := tr.Next()
+				if err != nil {
+					if errors.Is(err, io.EOF) {
+						break
+					}
+					return types.ImageBuildResponse{}, err
+				}
+				switch hdr.Name {
+				case ".":
+				case "Dockerfile":
+					bs, err := ioutil.ReadAll(tr)
+					if err != nil {
+						return types.ImageBuildResponse{}, err
+					}
+					if !bytes.Equal(bs, dockerfileContent) {
+						return types.ImageBuildResponse{}, errors.New("bad content for Dockerfile")
+					}
+				case "a.txt":
+					bs, err := ioutil.ReadAll(tr)
+					if err != nil {
+						return types.ImageBuildResponse{}, err
+					}
+					if !bytes.Equal(bs, atxtContent) {
+						return types.ImageBuildResponse{}, errors.New("bad content for a.txt")
+					}
+				default:
+					return types.ImageBuildResponse{}, errors.New("unexpected file")
+				}
+			}
+			return types.ImageBuildResponse{
+				Body:   io.NopCloser(strings.NewReader(`{"stream": "OK!"}`)),
+				OSType: "linux",
+			}, nil
+		},
+	}
+
+	impl := &mockImpl{
+		BuildFn: func(config *api.Config) (*api.Result, error) {
+			err := ioutil.WriteFile(config.AsDockerfile, dockerfileContent, 0644)
+			if err != nil {
+				return nil, err
+			}
+			err = ioutil.WriteFile(filepath.Join(filepath.Dir(config.AsDockerfile), "a.txt"), atxtContent, 0644)
+			if err != nil {
+				return nil, err
+			}
+
+			return nil, nil
+		},
+	}
+
+	f := fn.Function{
+		Runtime: "node",
+	}
+	b := s2i.NewBuilder(s2i.WithImpl(impl), s2i.WithDockerClient(cli))
+	err := b.Build(context.Background(), f)
+	if err != nil {
+		t.Error(err)
+	}
+}
+
+func TestBuildFail(t *testing.T) {
+	cli := mockDocker{
+		build: func(ctx context.Context, context io.Reader, options types.ImageBuildOptions) (types.ImageBuildResponse, error) {
+			return types.ImageBuildResponse{
+				Body:   io.NopCloser(strings.NewReader(`{"errorDetail": {"message": "Error: this is expected"}}`)),
+				OSType: "linux",
+			}, nil
+		},
+	}
+	impl := &mockImpl{
+		BuildFn: func(config *api.Config) (*api.Result, error) {
+			return &api.Result{Success: true}, nil
+		},
+	}
+	b := s2i.NewBuilder(s2i.WithImpl(impl), s2i.WithDockerClient(cli))
+	err := b.Build(context.Background(), fn.Function{Runtime: "node"})
+	if err == nil || !strings.Contains(err.Error(), "Error: this is expected") {
+		t.Error("didn't get expected error")
+	}
+}
+
 // mockImpl is a mock implementation of an S2I builder.
 type mockImpl struct {
 	BuildFn func(*api.Config) (*api.Result, error)
@@ -148,3 +355,39 @@ type mockImpl struct {
 func (i *mockImpl) Build(cfg *api.Config) (*api.Result, error) {
 	return i.BuildFn(cfg)
 }
+
+type mockDocker struct {
+	inspect func(ctx context.Context, image string) (types.ImageInspect, []byte, error)
+	build   func(ctx context.Context, context io.Reader, options types.ImageBuildOptions) (types.ImageBuildResponse, error)
+}
+
+func (m mockDocker) ImageInspectWithRaw(ctx context.Context, image string) (types.ImageInspect, []byte, error) {
+	if m.inspect != nil {
+		return m.inspect(ctx, image)
+	}
+
+	return types.ImageInspect{}, nil, nil
+}
+
+func (m mockDocker) ImageBuild(ctx context.Context, context io.Reader, options types.ImageBuildOptions) (types.ImageBuildResponse, error) {
+	if m.build != nil {
+		return m.build(ctx, context, options)
+	}
+
+	_, _ = io.Copy(io.Discard, context)
+	return types.ImageBuildResponse{
+		Body:   io.NopCloser(strings.NewReader("")),
+		OSType: "linux",
+	}, nil
+}
+
+type notFoundErr struct {
+}
+
+func (n notFoundErr) Error() string {
+	return "not found"
+}
+
+func (n notFoundErr) NotFound() bool {
+	return true
+}