Support dependencies between build artifacts - 1

cmd/skaffold/app/cmd/build.go

@@ -19,6 +19,7 @@ package cmd
 import (
 	"bytes"
 	"context"
+	"errors"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -67,7 +68,13 @@ func doBuild(ctx context.Context, out io.Writer) error {
 	}
 
 	return withRunner(ctx, func(r runner.Runner, config *latest.SkaffoldConfig) error {
-		bRes, err := r.BuildAndTest(ctx, buildOut, targetArtifacts(opts, config))
+		ar := targetArtifacts(opts, config)
+
+		// TODO: Remove this block after fixing artifact cache logic for artifacts with dependencies
+		if err := failForArtifactDependenciesWithCacheEnabled(ar, opts.CacheArtifacts); err != nil {
+			return err
+		}
+		bRes, err := r.BuildAndTest(ctx, buildOut, ar)
 
 		if quietFlag || buildOutputFlag != "" {
 			cmdOut := flags.BuildOutput{Builds: bRes}
@@ -104,3 +111,15 @@ func targetArtifacts(opts config.SkaffoldOptions, cfg *latest.SkaffoldConfig) []
 
 	return targetArtifacts
 }
+
+func failForArtifactDependenciesWithCacheEnabled(artifacts []*latest.Artifact, cacheEnabled bool) error {
+	if !cacheEnabled {
+		return nil
+	}
+	for _, a := range artifacts {
+		if len(a.Dependencies) > 0 {
+			return errors.New("defining dependencies between artifacts is not yet supported for `skaffold build` with cache enabled. Run with `--cache-artifacts=false` flag")
+		}
+	}
+	return nil
+}

cmd/skaffold/app/cmd/dev.go

@@ -61,6 +61,10 @@ func runDev(ctx context.Context, out io.Writer) error {
 			return nil
 		default:
 			err := withRunner(ctx, func(r runner.Runner, config *latest.SkaffoldConfig) error {
+				// TODO: Remove this block after fixing devloop rebuild logic for artifacts with dependencies
+				if err := checkForArtifactDependencies(config.Build.Artifacts); err != nil {
+					return err
+				}
 				err := r.Dev(ctx, out, config.Build.Artifacts)
 
 				if r.HasDeployed() {
@@ -91,3 +95,12 @@ func runDev(ctx context.Context, out io.Writer) error {
 		}
 	}
 }
+
+func checkForArtifactDependencies(artifacts []*latest.Artifact) error {
+	for _, a := range artifacts {
+		if len(a.Dependencies) > 0 {
+			return errors.New("defining dependencies between artifacts is not yet supported for `skaffold dev` and `skaffold debug`")
+		}
+	}
+	return nil
+}

docs/content/en/schemas/v2beta9.json

@@ -64,6 +64,14 @@
                 "gcr.io/k8s-skaffold/example"
               ]
             },
+            "requires": {
+              "items": {
+                "$ref": "#/definitions/ArtifactDependency"
+              },
+              "type": "array",
+              "description": "describes build artifacts that this artifact depends on.",
+              "x-intellij-html-description": "describes build artifacts that this artifact depends on."
+            },
             "sync": {
               "$ref": "#/definitions/Sync",
               "description": "*beta* local files synced to pods instead of triggering an image build when modified. If no files are listed, sync all the files and infer the destination.",
@@ -74,7 +82,8 @@
           "preferredOrder": [
             "image",
             "context",
-            "sync"
+            "sync",
+            "requires"
           ],
           "additionalProperties": false
         },
@@ -99,6 +108,14 @@
                 "gcr.io/k8s-skaffold/example"
               ]
             },
+            "requires": {
+              "items": {
+                "$ref": "#/definitions/ArtifactDependency"
+              },
+              "type": "array",
+              "description": "describes build artifacts that this artifact depends on.",
+              "x-intellij-html-description": "describes build artifacts that this artifact depends on."
+            },
             "sync": {
               "$ref": "#/definitions/Sync",
               "description": "*beta* local files synced to pods instead of triggering an image build when modified. If no files are listed, sync all the files and infer the destination.",
@@ -110,6 +127,7 @@
             "image",
             "context",
             "sync",
+            "requires",
             "docker"
           ],
           "additionalProperties": false
@@ -135,6 +153,14 @@
                 "gcr.io/k8s-skaffold/example"
               ]
             },
+            "requires": {
+              "items": {
+                "$ref": "#/definitions/ArtifactDependency"
+              },
+              "type": "array",
+              "description": "describes build artifacts that this artifact depends on.",
+              "x-intellij-html-description": "describes build artifacts that this artifact depends on."
+            },
             "sync": {
               "$ref": "#/definitions/Sync",
               "description": "*beta* local files synced to pods instead of triggering an image build when modified. If no files are listed, sync all the files and infer the destination.",
@@ -146,6 +172,7 @@
             "image",
             "context",
             "sync",
+            "requires",
             "bazel"
           ],
           "additionalProperties": false
@@ -171,6 +198,14 @@
               "description": "builds images using the [Jib plugins for Maven or Gradle](https://github.com/GoogleContainerTools/jib/).",
               "x-intellij-html-description": "builds images using the <a href=\"https://github.com/GoogleContainerTools/jib/\">Jib plugins for Maven or Gradle</a>."
             },
+            "requires": {
+              "items": {
+                "$ref": "#/definitions/ArtifactDependency"
+              },
+              "type": "array",
+              "description": "describes build artifacts that this artifact depends on.",
+              "x-intellij-html-description": "describes build artifacts that this artifact depends on."
+            },
             "sync": {
               "$ref": "#/definitions/Sync",
               "description": "*beta* local files synced to pods instead of triggering an image build when modified. If no files are listed, sync all the files and infer the destination.",
@@ -182,6 +217,7 @@
             "image",
             "context",
             "sync",
+            "requires",
             "jib"
           ],
           "additionalProperties": false
@@ -207,6 +243,14 @@
               "description": "builds images using [kaniko](https://github.com/GoogleContainerTools/kaniko).",
               "x-intellij-html-description": "builds images using <a href=\"https://github.com/GoogleContainerTools/kaniko\">kaniko</a>."
             },
+            "requires": {
+              "items": {
+                "$ref": "#/definitions/ArtifactDependency"
+              },
+              "type": "array",
+              "description": "describes build artifacts that this artifact depends on.",
+              "x-intellij-html-description": "describes build artifacts that this artifact depends on."
+            },
             "sync": {
               "$ref": "#/definitions/Sync",
               "description": "*beta* local files synced to pods instead of triggering an image build when modified. If no files are listed, sync all the files and infer the destination.",
@@ -218,6 +262,7 @@
             "image",
             "context",
             "sync",
+            "requires",
             "kaniko"
           ],
           "additionalProperties": false
@@ -243,6 +288,14 @@
                 "gcr.io/k8s-skaffold/example"
               ]
             },
+            "requires": {
+              "items": {
+                "$ref": "#/definitions/ArtifactDependency"
+              },
+              "type": "array",
+              "description": "describes build artifacts that this artifact depends on.",
+              "x-intellij-html-description": "describes build artifacts that this artifact depends on."
+            },
             "sync": {
               "$ref": "#/definitions/Sync",
               "description": "*beta* local files synced to pods instead of triggering an image build when modified. If no files are listed, sync all the files and infer the destination.",
@@ -254,6 +307,7 @@
             "image",
             "context",
             "sync",
+            "requires",
             "buildpacks"
           ],
           "additionalProperties": false
@@ -279,6 +333,14 @@
                 "gcr.io/k8s-skaffold/example"
               ]
             },
+            "requires": {
+              "items": {
+                "$ref": "#/definitions/ArtifactDependency"
+              },
+              "type": "array",
+              "description": "describes build artifacts that this artifact depends on.",
+              "x-intellij-html-description": "describes build artifacts that this artifact depends on."
+            },
             "sync": {
               "$ref": "#/definitions/Sync",
               "description": "*beta* local files synced to pods instead of triggering an image build when modified. If no files are listed, sync all the files and infer the destination.",
@@ -290,6 +352,7 @@
             "image",
             "context",
             "sync",
+            "requires",
             "custom"
           ],
           "additionalProperties": false
@@ -298,6 +361,30 @@
       "description": "items that need to be built, along with the context in which they should be built.",
       "x-intellij-html-description": "items that need to be built, along with the context in which they should be built."
     },
+    "ArtifactDependency": {
+      "required": [
+        "image"
+      ],
+      "properties": {
+        "alias": {
+          "type": "string",
+          "description": "a token that is replaced with the image reference in the builder definition files. For example, the `docker` builder will use the alias as a build-arg key. Defaults to the value of `image`.",
+          "x-intellij-html-description": "a token that is replaced with the image reference in the builder definition files. For example, the <code>docker</code> builder will use the alias as a build-arg key. Defaults to the value of <code>image</code>."
+        },
+        "image": {
+          "type": "string",
+          "description": "a reference to an artifact's image name.",
+          "x-intellij-html-description": "a reference to an artifact's image name."
+        }
+      },
+      "preferredOrder": [
+        "image",
+        "alias"
+      ],
+      "additionalProperties": false,
+      "description": "describes a specific build dependency for an artifact.",
+      "x-intellij-html-description": "describes a specific build dependency for an artifact."
+    },
     "Auto": {
       "description": "cannot be customized.",
       "x-intellij-html-description": "cannot be customized."

integration/build_dependencies_test.go

@@ -0,0 +1,114 @@
+/*
+Copyright 2020 The Skaffold Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package integration
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/sirupsen/logrus"
+
+	"github.com/GoogleContainerTools/skaffold/integration/skaffold"
+)
+
+func TestBuild_WithDependencies(t *testing.T) {
+	MarkIntegrationTest(t, CanRunWithoutGcp)
+
+	tests := []struct {
+		description  string
+		args         []string
+		cacheEnabled bool
+		failure      string
+	}{
+		{
+			description: "default concurrency=1",
+		},
+		{
+			description: "concurrency=0",
+			args:        []string{"-p", "concurrency-0"},
+		},
+		{
+			description: "concurrency=3",
+			args:        []string{"-p", "concurrency-3"},
+		},
+		{
+			description: "invalid dependency",
+			args:        []string{"-p", "invalid-dependency"},
+			failure:     `invalid skaffold config: unknown build dependency "image5" for artifact "image1"`,
+		},
+		{
+			description: "circular dependency",
+			args:        []string{"-p", "circular-dependency"},
+			failure:     `invalid skaffold config: cycle detected in build dependencies involving "image1"`,
+		},
+		{
+			description: "build failure with concurrency=1",
+			args:        []string{"-p", "failed-dependency"},
+			failure:     `unable to stream build output: The command '/bin/sh -c [ "${FAIL}" == "0" ] || false' returned a non-zero code: 1`,
+		},
+		{
+			description: "build failure with concurrency=0",
+			args:        []string{"-p", "failed-dependency", "-p", "concurrency-0"},
+			failure:     `unable to stream build output: The command '/bin/sh -c [ "${FAIL}" == "0" ] || false' returned a non-zero code: 1`,
+		},
+		{
+			description:  "build failure with cache-artifacts=true",
+			cacheEnabled: true,
+			failure:      "defining dependencies between artifacts is not yet supported for `skaffold build` with cache enabled. Run with `--cache-artifacts=false` flag",
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.description, func(t *testing.T) {
+			if test.cacheEnabled {
+				test.args = append(test.args, "--cache-artifacts=true")
+			} else {
+				test.args = append(test.args, "--cache-artifacts=false")
+			}
+
+			if test.failure == "" {
+				// Run without artifact caching
+				skaffold.Build(test.args...).InDir("testdata/build-dependencies").RunOrFail(t)
+				checkImagesExist(t)
+			} else {
+				if out, err := skaffold.Build(test.args...).InDir("testdata/build-dependencies").RunWithCombinedOutput(t); err == nil {
+					t.Fatal("expected build to fail")
+				} else if !strings.Contains(string(out), test.failure) {
+					logrus.Info("build output: ", string(out))
+					t.Fatalf("build failed but for wrong reason")
+				}
+			}
+		})
+	}
+}
+
+func TestDev_WithDependencies(t *testing.T) {
+	MarkIntegrationTest(t, CanRunWithoutGcp)
+	if out, err := skaffold.Dev().InDir("testdata/build-dependencies").RunWithCombinedOutput(t); err == nil {
+		t.Fatal("expected build to fail")
+	} else if !strings.Contains(string(out), "defining dependencies between artifacts is not yet supported for `skaffold dev` and `skaffold debug`") {
+		logrus.Info("dev output: ", string(out))
+		t.Fatalf("dev failed but for wrong reason")
+	}
+}
+
+func checkImagesExist(t *testing.T) {
+	checkImageExists(t, "gcr.io/k8s-skaffold/image1:latest")
+	checkImageExists(t, "gcr.io/k8s-skaffold/image2:latest")
+	checkImageExists(t, "gcr.io/k8s-skaffold/image3:latest")
+	checkImageExists(t, "gcr.io/k8s-skaffold/image4:latest")
+}

integration/testdata/build-dependencies/app1/Dockerfile

@@ -0,0 +1,15 @@
+FROM busybox
+
+# SLEEP is to simulate build time
+ARG SLEEP=0
+# FAIL=1 will cause the build to fail
+ARG FAIL=0
+COPY foo /foo
+
+ENV SLEEP_TIMEOUT=${SLEEP}
+ENV FAIL=${FAIL}
+RUN echo "sleep ${SLEEP_TIMEOUT}"
+RUN sleep ${SLEEP_TIMEOUT}
+RUN [ "${FAIL}" == "0" ] || false
+
+CMD while true; do cat /foo; sleep 1; done