Incremental Dockerfile Build

Allow incremental --as-dockerfile builds.

Trello card: https://trello.com/c/P6TZg0wO/1582-5-builds-s2i-incremental-build-as-dockerfile

examples/nginx-centos7/s2i/bin/assemble

@@ -17,10 +17,10 @@ fi
 # We set them here just for show, but you will need to set this up with your logic
 # according to the application directory that you chose.
 
-#if [ "$(ls /tmp/artifacts/ 2>/dev/null)" ]; then
-#  echo "---> Restoring build artifacts..."
-#  mv /tmp/artifacts/* <your directory here>
-#fi
+if [ "$(ls /tmp/artifacts/ 2>/dev/null)" ]; then
+  echo "---> Restoring build artifacts..."
+  mv /tmp/artifacts/* /etc/nginx
+fi
 
 # Override the default nginx index.html file.
 # This is what we consider in this example 'installing the application'

examples/nginx-centos7/s2i/bin/save-artifacts

@@ -7,4 +7,6 @@
 # For more information see the documentation:
 #	https://github.com/openshift/source-to-image/blob/master/docs/builder_image.md
 #
-#tar cf - <your files here>
+touch /tmp/artifact.txt
+cd /tmp
+tar cf - artifact.txt

pkg/build/strategies/dockerfile/dockerfile.go

@@ -125,6 +125,51 @@ func (builder *Dockerfile) CreateDockerfile(config *api.Config) error {
 	// where files will land inside the new image.
 	scriptsDestDir := filepath.Join(getDestination(config), "scripts")
 	sourceDestDir := filepath.Join(getDestination(config), "src")
+	artifactsDestDir := filepath.Join(getDestination(config), "artifacts")
+	artifactsTar := sanitize(filepath.ToSlash(filepath.Join(defaultDestination, "artifacts.tar")))
+
+	// only COPY scripts dir if required scripts are present, i.e. the dir is not empty;
+	// even if the "scripts" dir exists, the COPY would fail if it was empty
+	scriptsProvided, fileNames := checkValidDirWithContents(filepath.Join(config.WorkingDir, builder.uploadScriptsDir))
+	assembleProvided := false
+	runProvided := false
+	saveArtifactsProvided := false
+	for _, f := range fileNames {
+		glog.V(2).Infof("found override script file %s", f.Name())
+		if f.Name() == "run" {
+			runProvided = true
+		} else if f.Name() == "assemble" {
+			assembleProvided = true
+		} else if f.Name() == "save-artifacts" {
+			saveArtifactsProvided = true
+		}
+		if runProvided && assembleProvided && saveArtifactsProvided {
+			break
+		}
+	}
+
+	if config.Incremental {
+		imageTag := util.FirstNonEmpty(config.IncrementalFromTag, config.Tag)
+		if len(imageTag) == 0 {
+			return errors.New("Image tag is missing for incremental build")
+		}
+		buffer.WriteString(fmt.Sprintf("FROM %s as cached\n", imageTag))
+		if len(config.AssembleUser) > 0 {
+			buffer.WriteString(fmt.Sprintf("USER %s\n", imageUser))
+		}
+		var artifactsScript string
+		if saveArtifactsProvided {
+			glog.V(2).Infof("Override save-artifacts script is included in directory %q", builder.uploadScriptsDir)
+			buffer.WriteString("# Copying in override save-artifacts script\n")
+			artifactsScript = sanitize(filepath.ToSlash(filepath.Join(scriptsDestDir, "save-artifacts")))
+			uploadScript := sanitize(filepath.ToSlash(filepath.Join(builder.uploadScriptsDir, "save-artifacts")))
+			buffer.WriteString(fmt.Sprintf("COPY --chown=%s:0 %s %s\n", sanitize(imageUser), uploadScript, artifactsScript))
+		} else {
+			buffer.WriteString(fmt.Sprintf("# Save-artifacts script sourced from builder image based on user input or image metadata.\n"))
+			artifactsScript = sanitize(filepath.ToSlash(filepath.Join(config.ImageScriptsDir, "save-artifacts")))
+		}
+		buffer.WriteString(fmt.Sprintf("RUN if [ -s %[1]s ]; then %[1]s > %[2]s; else touch %[2]s; fi\n", artifactsScript, artifactsTar))
+	}
 
 	buffer.WriteString(fmt.Sprintf("FROM %s\n", config.BuilderImage))
 
@@ -132,6 +177,12 @@ func (builder *Dockerfile) CreateDockerfile(config *api.Config) error {
 		buffer.WriteString(fmt.Sprintf("USER %s\n", imageUser))
 	}
 
+	if config.Incremental {
+		buffer.WriteString(fmt.Sprintf("COPY --from=cached --chown=%[1]s:0 %[2]s %[2]s\n", sanitize(imageUser), artifactsTar))
+		buffer.WriteString(fmt.Sprintf("RUN if [ -s %[1]s ]; then mkdir -p %[2]s; tar -xf %[1]s -C %[2]s; fi && \\\n", artifactsTar, sanitize(filepath.ToSlash(artifactsDestDir))))
+		buffer.WriteString(fmt.Sprintf("    rm %s\n", artifactsTar))
+	}
+
 	generatedLabels := util.GenerateOutputImageLabels(builder.sourceInfo, config)
 	if len(generatedLabels) > 0 || len(config.Labels) > 0 {
 		first := true
@@ -157,23 +208,6 @@ func (builder *Dockerfile) CreateDockerfile(config *api.Config) error {
 	env := createBuildEnvironment(config.WorkingDir, config.Environment)
 	buffer.WriteString(fmt.Sprintf("%s", env))
 
-	// only COPY scripts dir if required scripts are present, i.e. the dir is not empty;
-	// even if the "scripts" dir exists, the COPY would fail if it was empty
-	scriptsProvided, fileNames := checkValidDirWithContents(filepath.Join(config.WorkingDir, builder.uploadScriptsDir))
-	assembleProvided := false
-	runProvided := false
-	for _, f := range fileNames {
-		glog.V(2).Infof("found override script file %s", f.Name())
-		if f.Name() == "run" {
-			runProvided = true
-		} else if f.Name() == "assemble" {
-			assembleProvided = true
-		}
-		if runProvided && assembleProvided {
-			break
-		}
-	}
-
 	if scriptsProvided {
 		glog.V(2).Infof("Override scripts are included in directory %q", builder.uploadScriptsDir)
 		buffer.WriteString("# Copying in override assemble/run scripts\n")

pkg/build/strategies/sti/postexecutorstep.go

@@ -394,7 +394,7 @@ func (step *reportSuccessStep) execute(ctx *postExecutorStepContext) error {
 	step.builder.result.Success = true
 	step.builder.result.ImageID = ctx.imageID
 
-	glog.V(3).Infof("Successfully built %s", firstNonEmpty(step.builder.config.Tag, ctx.imageID))
+	glog.V(3).Infof("Successfully built %s", util.FirstNonEmpty(step.builder.config.Tag, ctx.imageID))
 
 	return nil
 }

pkg/build/strategies/sti/sti.go

@@ -202,7 +202,7 @@ func (builder *STI) Build(config *api.Config) (*api.Result, error) {
 	}
 
 	if builder.incremental = builder.artifacts.Exists(config); builder.incremental {
-		tag := firstNonEmpty(config.IncrementalFromTag, config.Tag)
+		tag := util.FirstNonEmpty(config.IncrementalFromTag, config.Tag)
 		glog.V(1).Infof("Existing image for tag %s detected for incremental build", tag)
 	} else {
 		glog.V(1).Info("Clean build will be performed")
@@ -384,7 +384,7 @@ func (builder *STI) Prepare(config *api.Config) error {
 	if len(config.ScriptsURL) > 0 {
 		failedCount := 0
 		for _, result := range requiredAndOptional {
-			if includes(result.FailedSources, scripts.ScriptURLHandler) {
+			if util.Includes(result.FailedSources, scripts.ScriptURLHandler) {
 				failedCount++
 			}
 		}
@@ -464,7 +464,7 @@ func (builder *STI) Exists(config *api.Config) bool {
 		policy = api.DefaultPreviousImagePullPolicy
 	}
 
-	tag := firstNonEmpty(config.IncrementalFromTag, config.Tag)
+	tag := util.FirstNonEmpty(config.IncrementalFromTag, config.Tag)
 
 	startTime := time.Now()
 	result, err := dockerpkg.PullImage(tag, builder.incrementalDocker, policy)
@@ -498,7 +498,7 @@ func (builder *STI) Save(config *api.Config) (err error) {
 		return err
 	}
 
-	image := firstNonEmpty(config.IncrementalFromTag, config.Tag)
+	image := util.FirstNonEmpty(config.IncrementalFromTag, config.Tag)
 
 	outReader, outWriter := io.Pipe()
 	errReader, errWriter := io.Pipe()
@@ -758,21 +758,3 @@ func isMissingRequirements(text string) bool {
 	shCommand, _ := regexp.MatchString(`.*/bin/sh.*no such file or directory`, text)
 	return tarCommand || shCommand
 }
-
-func includes(arr []string, str string) bool {
-	for _, s := range arr {
-		if s == str {
-			return true
-		}
-	}
-	return false
-}
-
-func firstNonEmpty(args ...string) string {
-	for _, value := range args {
-		if len(value) > 0 {
-			return value
-		}
-	}
-	return ""
-}

pkg/cmd/cli/cmd/build.go

@@ -70,10 +70,6 @@ $ s2i build . centos/ruby-22-centos7 hello-world-app
 			}
 
 			if len(cfg.AsDockerfile) > 0 {
-				if cfg.Incremental {
-					fmt.Fprintln(os.Stderr, "ERROR: --incremental cannot be used with --as-dockerfile")
-					return
-				}
 				if cfg.RunImage {
 					fmt.Fprintln(os.Stderr, "ERROR: --run cannot be used with --as-dockerfile")
 					return

pkg/util/strings.go

@@ -0,0 +1,21 @@
+package util
+
+// Includes determines if the given string is in the provided slice of strings.
+func Includes(arr []string, str string) bool {
+	for _, s := range arr {
+		if s == str {
+			return true
+		}
+	}
+	return false
+}
+
+// FirstNonEmpty returns the first non-empty string in the provided list of strings.
+func FirstNonEmpty(args ...string) string {
+	for _, value := range args {
+		if len(value) > 0 {
+			return value
+		}
+	}
+	return ""
+}

test/integration/integration_test.go

@@ -962,6 +962,223 @@ func TestDockerfileBuildSourceScriptsRun(t *testing.T) {
 	}
 	runDockerfileTest(t, config, expected, nil, expectedFiles)
 }
+
+func TestDockerfileIncrementalBuild(t *testing.T) {
+	tempdir, err := ioutil.TempDir("", "s2i-dockerfiletest-dir")
+	if err != nil {
+		t.Errorf("Unable to create temporary directory: %v", err)
+	}
+	defer os.RemoveAll(tempdir)
+
+	config := &api.Config{
+		BuilderImage:    "docker.io/centos/nodejs-8-centos7",
+		AssembleUser:    "",
+		ImageWorkDir:    "",
+		ImageScriptsDir: "/usr/libexec/s2i",
+		Incremental:     true,
+		Source:          git.MustParse("https://github.com/sclorg/nodejs-ex"),
+		ScriptsURL:      "",
+		Tag:             "test:tag",
+		Injections:      api.VolumeList{},
+		Destination:     "",
+
+		Environment: api.EnvironmentList{},
+		Labels:      map[string]string{},
+
+		AsDockerfile: filepath.Join(tempdir, "Dockerfile"),
+	}
+
+	expected := []string{
+		"FROM test:tag as cached",
+		"RUN if [ -s /usr/libexec/s2i/save-artifacts ]; then /usr/libexec/s2i/save-artifacts > /tmp/artifacts.tar; else touch /tmp/artifacts.tar; fi",
+		"FROM docker.io/centos/nodejs-8-centos7",
+		"COPY --from=cached --chown=1001:0 /tmp/artifacts.tar /tmp/artifacts.tar",
+		"RUN if [ -s /tmp/artifacts.tar ]; then mkdir -p /tmp/artifacts; tar -xf /tmp/artifacts.tar -C /tmp/artifacts; fi",
+		"rm /tmp/artifacts.tar",
+		"COPY --chown=1001:0 upload/src /tmp/src",
+		"RUN /usr/libexec/s2i/assemble",
+		"CMD /usr/libexec/s2i/run",
+	}
+
+	runDockerfileTest(t, config, expected, nil, nil)
+}
+
+func TestDockerfileIncrementalSourceSave(t *testing.T) {
+	tempdir, err := ioutil.TempDir("", "s2i-dockerfiletest-dir")
+	if err != nil {
+		t.Errorf("Unable to create temporary directory: %v", err)
+	}
+	defer os.RemoveAll(tempdir)
+
+	sourcecode := filepath.Join(tempdir, "sourcecode")
+	sourcescripts := filepath.Join(sourcecode, ".s2i", "bin")
+	err = os.MkdirAll(sourcescripts, 0777)
+	if err != nil {
+		t.Errorf("Unable to create injection dir: %v", err)
+	}
+
+	saveArtifacts := filepath.Join(sourcescripts, "save-artifacts")
+	_, err = os.OpenFile(saveArtifacts, os.O_RDONLY|os.O_CREATE, 0666)
+	if err != nil {
+		t.Errorf("Unable to create save-artifacts file: %v", err)
+	}
+
+	config := &api.Config{
+		BuilderImage:    "docker.io/centos/nodejs-8-centos7",
+		AssembleUser:    "",
+		ImageWorkDir:    "",
+		ImageScriptsDir: "/usr/libexec/s2i",
+		Incremental:     true,
+		Source:          git.MustParse("file:///" + filepath.ToSlash(sourcecode)),
+		ScriptsURL:      "",
+		Tag:             "test:tag",
+		Injections:      api.VolumeList{},
+		Destination:     "/destination",
+
+		Environment: api.EnvironmentList{},
+		Labels:      map[string]string{},
+
+		AsDockerfile: filepath.Join(tempdir, "Dockerfile"),
+	}
+
+	expected := []string{
+		"FROM test:tag as cached",
+		"COPY --chown=1001:0 upload/scripts/save-artifacts /destination/scripts/save-artifacts",
+		"RUN if [ -s /destination/scripts/save-artifacts ]; then /destination/scripts/save-artifacts > /tmp/artifacts.tar;",
+		"FROM docker.io/centos/nodejs-8-centos7",
+		"mkdir -p /destination/artifacts",
+		"tar -xf /tmp/artifacts.tar -C /destination/artifacts",
+		"RUN /usr/libexec/s2i/assemble",
+		"CMD /usr/libexec/s2i/run",
+	}
+	expectedFiles := []string{
+		filepath.Join(tempdir, "upload/scripts/save-artifacts"),
+	}
+
+	runDockerfileTest(t, config, expected, nil, expectedFiles)
+}
+
+func TestDockerfileIncrementalSaveURL(t *testing.T) {
+	tempdir, err := ioutil.TempDir("", "s2i-dockerfiletest-dir")
+	if err != nil {
+		t.Errorf("Unable to create temporary directory: %v", err)
+	}
+	defer os.RemoveAll(tempdir)
+
+	saveArtifacts := filepath.Join(tempdir, "save-artifacts")
+	_, err = os.OpenFile(saveArtifacts, os.O_RDONLY|os.O_CREATE, 0666)
+	if err != nil {
+		t.Errorf("Unable to create save-artifacts file: %v", err)
+	}
+
+	config := &api.Config{
+		BuilderImage:    "docker.io/centos/nodejs-8-centos7",
+		AssembleUser:    "",
+		ImageWorkDir:    "",
+		ImageScriptsDir: "/usr/libexec/s2i",
+		Incremental:     true,
+		Source:          git.MustParse("https://github.com/sclorg/nodejs-ex"),
+		ScriptsURL:      "file://" + filepath.ToSlash(tempdir),
+		Tag:             "test:tag",
+		Injections:      api.VolumeList{},
+		Destination:     "/destination",
+
+		Environment: api.EnvironmentList{},
+		Labels:      map[string]string{},
+
+		AsDockerfile: filepath.Join(tempdir, "Dockerfile"),
+	}
+
+	expected := []string{
+		"FROM test:tag as cached",
+		"COPY --chown=1001:0 upload/scripts/save-artifacts /destination/scripts/save-artifacts",
+		"RUN if [ -s /destination/scripts/save-artifacts ]; then /destination/scripts/save-artifacts > /tmp/artifacts.tar;",
+		"FROM docker.io/centos/nodejs-8-centos7",
+		"mkdir -p /destination/artifacts",
+		"tar -xf /tmp/artifacts.tar -C /destination/artifacts",
+		"RUN /usr/libexec/s2i/assemble",
+		"CMD /usr/libexec/s2i/run",
+	}
+	expectedFiles := []string{
+		filepath.Join(tempdir, "upload/scripts/save-artifacts"),
+	}
+
+	runDockerfileTest(t, config, expected, nil, expectedFiles)
+}
+
+func TestDockerfileIncrementalTag(t *testing.T) {
+	tempdir, err := ioutil.TempDir("", "s2i-dockerfiletest-dir")
+	if err != nil {
+		t.Errorf("Unable to create temporary directory: %v", err)
+	}
+	defer os.RemoveAll(tempdir)
+
+	config := &api.Config{
+		BuilderImage:       "docker.io/centos/nodejs-8-centos7",
+		AssembleUser:       "",
+		ImageWorkDir:       "",
+		ImageScriptsDir:    "/usr/libexec/s2i",
+		Incremental:        true,
+		Source:             git.MustParse("https://github.com/sclorg/nodejs-ex"),
+		Tag:                "test:tag",
+		IncrementalFromTag: "incremental:tag",
+
+		Environment: api.EnvironmentList{},
+		Labels:      map[string]string{},
+
+		AsDockerfile: filepath.Join(tempdir, "Dockerfile"),
+	}
+
+	expected := []string{
+		"FROM incremental:tag as cached",
+		"/usr/libexec/s2i/save-artifacts > /tmp/artifacts.tar",
+		"FROM docker.io/centos/nodejs-8-centos7",
+		"mkdir -p /tmp/artifacts",
+		"tar -xf /tmp/artifacts.tar -C /tmp/artifacts",
+		"rm /tmp/artifacts.tar",
+		"RUN /usr/libexec/s2i/assemble",
+		"CMD /usr/libexec/s2i/run",
+	}
+
+	runDockerfileTest(t, config, expected, nil, nil)
+}
+
+func TestDockerfileIncrementalAssembleUser(t *testing.T) {
+	tempdir, err := ioutil.TempDir("", "s2i-dockerfiletest-dir")
+	if err != nil {
+		t.Errorf("Unable to create temporary directory: %v", err)
+	}
+	defer os.RemoveAll(tempdir)
+
+	config := &api.Config{
+		BuilderImage:    "docker.io/centos/nodejs-8-centos7",
+		AssembleUser:    "2250",
+		ImageWorkDir:    "",
+		ImageScriptsDir: "/usr/libexec/s2i",
+		Incremental:     true,
+		Source:          git.MustParse("https://github.com/sclorg/nodejs-ex"),
+		Tag:             "test:tag",
+		Environment:     api.EnvironmentList{},
+		Labels:          map[string]string{},
+
+		AsDockerfile: filepath.Join(tempdir, "Dockerfile"),
+	}
+
+	expected := []string{
+		"FROM test:tag as cached\nUSER 2250",
+		"/usr/libexec/s2i/save-artifacts > /tmp/artifacts.tar",
+		"FROM docker.io/centos/nodejs-8-centos7",
+		"COPY --from=cached --chown=2250:0 /tmp/artifacts.tar /tmp/artifacts.tar",
+		"mkdir -p /tmp/artifacts",
+		"tar -xf /tmp/artifacts.tar -C /tmp/artifacts",
+		"rm /tmp/artifacts.tar",
+		"RUN /usr/libexec/s2i/assemble",
+		"CMD /usr/libexec/s2i/run",
+	}
+
+	runDockerfileTest(t, config, expected, nil, nil)
+}
+
 func runDockerfileTest(t *testing.T, config *api.Config, expected []string, notExpected []string, expectedFiles []string) {
 
 	b, _, err := strategies.GetStrategy(nil, config)