diff --git a/cmd/skaffold/app/cmd/build.go b/cmd/skaffold/app/cmd/build.go
index e5bf0e086c7..611cf6234d5 100644
--- a/cmd/skaffold/app/cmd/build.go
+++ b/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: [#4891] Remove this block after implementing proper image cache invalidation 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
+}
diff --git a/cmd/skaffold/app/cmd/dev.go b/cmd/skaffold/app/cmd/dev.go
index 524a17ce4b0..b583408ff3b 100644
--- a/cmd/skaffold/app/cmd/dev.go
+++ b/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: [#4892] Remove this block after fixing devloop rebuild and redeploy 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
+}
diff --git a/docs/content/en/schemas/v2beta9.json b/docs/content/en/schemas/v2beta9.json
index 4674dc459e9..33695479db0 100755
--- a/docs/content/en/schemas/v2beta9.json
+++ b/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 Jib plugins for Maven or Gradle."
},
+ "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 kaniko."
},
+ "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 docker builder will use the alias as a build-arg key. Defaults to the value of image."
+ },
+ "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."
diff --git a/integration/build_dependencies_test.go b/integration/build_dependencies_test.go
new file mode 100644
index 00000000000..b49933ad423
--- /dev/null
+++ b/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")
+}
diff --git a/integration/testdata/build-dependencies/app1/Dockerfile b/integration/testdata/build-dependencies/app1/Dockerfile
new file mode 100644
index 00000000000..0930d698c85
--- /dev/null
+++ b/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
\ No newline at end of file
diff --git a/integration/testdata/build-dependencies/app1/foo b/integration/testdata/build-dependencies/app1/foo
new file mode 100644
index 00000000000..e69de29bb2d
diff --git a/integration/testdata/build-dependencies/app2/Dockerfile b/integration/testdata/build-dependencies/app2/Dockerfile
new file mode 100644
index 00000000000..0930d698c85
--- /dev/null
+++ b/integration/testdata/build-dependencies/app2/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
\ No newline at end of file
diff --git a/integration/testdata/build-dependencies/app2/foo b/integration/testdata/build-dependencies/app2/foo
new file mode 100644
index 00000000000..e69de29bb2d
diff --git a/integration/testdata/build-dependencies/app3/Dockerfile b/integration/testdata/build-dependencies/app3/Dockerfile
new file mode 100644
index 00000000000..0930d698c85
--- /dev/null
+++ b/integration/testdata/build-dependencies/app3/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
\ No newline at end of file
diff --git a/integration/testdata/build-dependencies/app3/foo b/integration/testdata/build-dependencies/app3/foo
new file mode 100644
index 00000000000..e69de29bb2d
diff --git a/integration/testdata/build-dependencies/app4/Dockerfile b/integration/testdata/build-dependencies/app4/Dockerfile
new file mode 100644
index 00000000000..0930d698c85
--- /dev/null
+++ b/integration/testdata/build-dependencies/app4/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
\ No newline at end of file
diff --git a/integration/testdata/build-dependencies/app4/foo b/integration/testdata/build-dependencies/app4/foo
new file mode 100644
index 00000000000..e69de29bb2d
diff --git a/integration/testdata/build-dependencies/skaffold.yaml b/integration/testdata/build-dependencies/skaffold.yaml
new file mode 100644
index 00000000000..50290de99ae
--- /dev/null
+++ b/integration/testdata/build-dependencies/skaffold.yaml
@@ -0,0 +1,67 @@
+apiVersion: skaffold/v2beta9
+kind: Config
+build:
+ tagPolicy:
+ sha256: {}
+
+ artifacts:
+ - image: image1
+ context: app1
+ docker:
+ noCache: true
+ buildArgs:
+ SLEEP: "1"
+ FAIL: "0"
+ requires:
+ - image: image2
+
+ - image: image2
+ context: app2
+ docker:
+ noCache: true
+ buildArgs:
+ SLEEP: "3"
+ FAIL: "0"
+ requires:
+ - image: image3
+
+ - image: image3
+ context: app3
+ docker:
+ noCache: true
+ buildArgs:
+ SLEEP: "6"
+ FAIL: "0"
+
+ - image: image4
+ context: app4
+ docker:
+ noCache: true
+ buildArgs:
+ SLEEP: "9"
+ FAIL: "0"
+profiles:
+- name: concurrency-0
+ build:
+ local:
+ concurrency: 0
+- name: concurrency-3
+ build:
+ local:
+ concurrency: 3
+- name: invalid-dependency
+ patches:
+ - op: add
+ path: /build/artifacts/0/requires/0/image
+ value: "image5"
+- name: circular-dependency
+ patches:
+ - op: add
+ path: /build/artifacts/1/requires/0/image
+ value: "image1"
+- name: failed-dependency
+ patches:
+ - path: /build/artifacts/1/docker/buildArgs
+ value:
+ SLEEP: "3"
+ FAIL: "1"
diff --git a/pkg/skaffold/build/cluster/cluster.go b/pkg/skaffold/build/cluster/cluster.go
index e049ce007d5..4b2b1da2068 100644
--- a/pkg/skaffold/build/cluster/cluster.go
+++ b/pkg/skaffold/build/cluster/cluster.go
@@ -46,7 +46,7 @@ func (b *Builder) Build(ctx context.Context, out io.Writer, tags tag.ImageTags,
}
builder := build.WithLogFile(b.buildArtifact, b.cfg.Muted())
- return build.InParallel(ctx, out, tags, artifacts, builder, b.ClusterDetails.Concurrency)
+ return build.InOrder(ctx, out, tags, artifacts, builder, b.ClusterDetails.Concurrency)
}
func (b *Builder) buildArtifact(ctx context.Context, out io.Writer, artifact *latest.Artifact, tag string) (string, error) {
diff --git a/pkg/skaffold/build/gcb/cloud_build.go b/pkg/skaffold/build/gcb/cloud_build.go
index 082d1b453c3..93b1ab74982 100644
--- a/pkg/skaffold/build/gcb/cloud_build.go
+++ b/pkg/skaffold/build/gcb/cloud_build.go
@@ -47,7 +47,7 @@ import (
// Build builds a list of artifacts with Google Cloud Build.
func (b *Builder) Build(ctx context.Context, out io.Writer, tags tag.ImageTags, artifacts []*latest.Artifact) ([]build.Artifact, error) {
builder := build.WithLogFile(b.buildArtifactWithCloudBuild, b.muted)
- return build.InParallel(ctx, out, tags, artifacts, builder, b.GoogleCloudBuild.Concurrency)
+ return build.InOrder(ctx, out, tags, artifacts, builder, b.GoogleCloudBuild.Concurrency)
}
func (b *Builder) buildArtifactWithCloudBuild(ctx context.Context, out io.Writer, artifact *latest.Artifact, tag string) (string, error) {
diff --git a/pkg/skaffold/build/local/local.go b/pkg/skaffold/build/local/local.go
index 33099092fc3..74230a9f393 100644
--- a/pkg/skaffold/build/local/local.go
+++ b/pkg/skaffold/build/local/local.go
@@ -48,7 +48,7 @@ func (b *Builder) Build(ctx context.Context, out io.Writer, tags tag.ImageTags,
}
builder := build.WithLogFile(b.buildArtifact, b.muted)
- rt, err := build.InParallel(ctx, out, tags, artifacts, builder, *b.local.Concurrency)
+ rt, err := build.InOrder(ctx, out, tags, artifacts, builder, *b.local.Concurrency)
if b.prune {
if b.mode == config.RunModes.Build {
diff --git a/pkg/skaffold/build/local/prune.go b/pkg/skaffold/build/local/prune.go
index 367fa961bdb..a01dfd507f0 100644
--- a/pkg/skaffold/build/local/prune.go
+++ b/pkg/skaffold/build/local/prune.go
@@ -158,6 +158,10 @@ func (p *pruner) collectImagesToPrune(ctx context.Context, artifacts []*latest.A
imgs, err := p.listImages(ctx, a.ImageName)
if err != nil {
+ switch err {
+ case context.Canceled, context.DeadlineExceeded:
+ return nil
+ }
logrus.Warnf("failed to list images: %v", err)
continue
}
diff --git a/pkg/skaffold/build/model.go b/pkg/skaffold/build/model.go
new file mode 100644
index 00000000000..0886962bf14
--- /dev/null
+++ b/pkg/skaffold/build/model.go
@@ -0,0 +1,94 @@
+/*
+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 build
+
+import (
+ "context"
+
+ "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
+)
+
+// node models the artifact dependency graph using a set of channels.
+// Each build node has a wait channel which it closes once it completes building by calling markComplete.
+// This notifies all listeners waiting for this node's build to complete.
+// Additionally it has a reference to the channels for each of its dependencies.
+// Calling `waitForDependencies` ensures that all required nodes' channels have already been closed and as such have finished building before the current artifact build starts.
+type node struct {
+ imageName string
+ wait chan interface{}
+ dependencies []node
+}
+
+// markComplete broadcasts that this node's build is complete.
+func (a *node) markComplete() {
+ // closing channel notifies all listeners
+ close(a.wait)
+}
+
+// waitForDependencies waits for all required builds to complete or returns an error if any build fails
+func (a *node) waitForDependencies(ctx context.Context) error {
+ for _, dep := range a.dependencies {
+ // wait for required builds to complete
+ select {
+ case <-ctx.Done():
+ return ctx.Err()
+ case <-dep.wait:
+ }
+ }
+ return nil
+}
+
+func createNodes(artifacts []*latest.Artifact) []node {
+ nodeMap := make(map[string]node)
+ for _, a := range artifacts {
+ nodeMap[a.ImageName] = node{
+ imageName: a.ImageName,
+ wait: make(chan interface{}),
+ }
+ }
+
+ var nodes []node
+ for _, a := range artifacts {
+ ar := nodeMap[a.ImageName]
+ for _, d := range a.Dependencies {
+ ch, found := nodeMap[d.ImageName]
+ if !found {
+ // if a dependency is not present in `artifacts` slice then we ignore it.
+ continue
+ }
+ ar.dependencies = append(ar.dependencies, ch)
+ }
+ nodes = append(nodes, ar)
+ }
+ return nodes
+}
+
+// countingSemaphore uses a buffered channel of size `n` that acts like a counting semaphore, allowing up to `n` concurrent operations
+type countingSemaphore struct {
+ sem chan bool
+}
+
+func newCountingSemaphore(count int) countingSemaphore {
+ return countingSemaphore{sem: make(chan bool, count)}
+}
+
+func (c countingSemaphore) acquire() (release func()) {
+ c.sem <- true
+ return func() {
+ <-c.sem
+ }
+}
diff --git a/pkg/skaffold/build/parallel.go b/pkg/skaffold/build/parallel.go
deleted file mode 100644
index 2d04b4c2533..00000000000
--- a/pkg/skaffold/build/parallel.go
+++ /dev/null
@@ -1,147 +0,0 @@
-/*
-Copyright 2019 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 build
-
-import (
- "bufio"
- "context"
- "fmt"
- "io"
- "sync"
-
- "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/tag"
- "github.com/GoogleContainerTools/skaffold/pkg/skaffold/color"
- "github.com/GoogleContainerTools/skaffold/pkg/skaffold/event"
- "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
-)
-
-const bufferedLinesPerArtifact = 10000
-
-type ArtifactBuilder func(ctx context.Context, out io.Writer, artifact *latest.Artifact, tag string) (string, error)
-
-// For testing
-var (
- buffSize = bufferedLinesPerArtifact
- runInSequence = InSequence
-)
-
-// InParallel builds a list of artifacts in parallel but prints the logs in sequential order.
-func InParallel(ctx context.Context, out io.Writer, tags tag.ImageTags, artifacts []*latest.Artifact, buildArtifact ArtifactBuilder, concurrency int) ([]Artifact, error) {
- if len(artifacts) == 0 {
- return nil, nil
- }
-
- if len(artifacts) == 1 || concurrency == 1 {
- return runInSequence(ctx, out, tags, artifacts, buildArtifact)
- }
-
- var wg sync.WaitGroup
- defer wg.Wait()
-
- ctx, cancel := context.WithCancel(ctx)
- defer cancel()
-
- results := new(sync.Map)
- outputs := make([]chan string, len(artifacts))
-
- if concurrency == 0 {
- concurrency = len(artifacts)
- }
- sem := make(chan bool, concurrency)
-
- // Run builds in //
- wg.Add(len(artifacts))
- for i := range artifacts {
- outputs[i] = make(chan string, buffSize)
- r, w := io.Pipe()
-
- // Run build and write output/logs to piped writer and store build result in
- // sync.Map
- go func(i int) {
- sem <- true
- runBuild(ctx, w, tags, artifacts[i], results, buildArtifact)
- <-sem
-
- wg.Done()
- }(i)
-
- // Read build output/logs and write to buffered channel
- go readOutputAndWriteToChannel(r, outputs[i])
- }
-
- // Print logs and collect results in order.
- return collectResults(out, artifacts, results, outputs)
-}
-
-func runBuild(ctx context.Context, cw io.WriteCloser, tags tag.ImageTags, artifact *latest.Artifact, results *sync.Map, build ArtifactBuilder) {
- event.BuildInProgress(artifact.ImageName)
-
- finalTag, err := getBuildResult(ctx, cw, tags, artifact, build)
- if err != nil {
- event.BuildFailed(artifact.ImageName, err)
- results.Store(artifact.ImageName, err)
- } else {
- event.BuildComplete(artifact.ImageName)
- artifact := Artifact{ImageName: artifact.ImageName, Tag: finalTag}
- results.Store(artifact.ImageName, artifact)
- }
- cw.Close()
-}
-
-func readOutputAndWriteToChannel(r io.Reader, lines chan string) {
- scanner := bufio.NewScanner(r)
- for scanner.Scan() {
- lines <- scanner.Text()
- }
- close(lines)
-}
-
-func getBuildResult(ctx context.Context, cw io.Writer, tags tag.ImageTags, artifact *latest.Artifact, build ArtifactBuilder) (string, error) {
- color.Default.Fprintf(cw, "Building [%s]...\n", artifact.ImageName)
- tag, present := tags[artifact.ImageName]
- if !present {
- return "", fmt.Errorf("unable to find tag for image %s", artifact.ImageName)
- }
- return build(ctx, cw, artifact, tag)
-}
-
-func collectResults(out io.Writer, artifacts []*latest.Artifact, results *sync.Map, outputs []chan string) ([]Artifact, error) {
- var built []Artifact
- for i, artifact := range artifacts {
- // Wait for build to complete.
- printResult(out, outputs[i])
- v, ok := results.Load(artifact.ImageName)
- if !ok {
- return nil, fmt.Errorf("could not find build result for image %s", artifact.ImageName)
- }
- switch t := v.(type) {
- case error:
- return nil, fmt.Errorf("couldn't build %q: %w", artifact.ImageName, t)
- case Artifact:
- built = append(built, t)
- default:
- return nil, fmt.Errorf("unknown type %T for %s", t, artifact.ImageName)
- }
- }
- return built, nil
-}
-
-func printResult(out io.Writer, output chan string) {
- for line := range output {
- fmt.Fprintln(out, line)
- }
-}
diff --git a/pkg/skaffold/build/result.go b/pkg/skaffold/build/result.go
new file mode 100644
index 00000000000..97bb870155d
--- /dev/null
+++ b/pkg/skaffold/build/result.go
@@ -0,0 +1,153 @@
+/*
+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 build
+
+import (
+ "bufio"
+ "context"
+ "fmt"
+ "io"
+ "sync"
+
+ "github.com/sirupsen/logrus"
+
+ "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
+)
+
+const bufferedLinesPerArtifact = 10000
+
+// For testing
+var (
+ buffSize = bufferedLinesPerArtifact
+)
+
+// logAggregator provides an interface to create an output writer for each artifact build and later aggregate the logs in build order.
+// The order of output is not guaranteed between multiple builds running concurrently.
+type logAggregator interface {
+ // GetWriter returns an output writer tracked by the logAggregator
+ GetWriter() (io.WriteCloser, error)
+ // PrintInOrder prints the output from each allotted writer in build order.
+ // It blocks until the instantiated capacity of io writers have been all allotted and closed, or the context is cancelled.
+ PrintInOrder(ctx context.Context, out io.Writer)
+}
+
+type logAggregatorImpl struct {
+ messages chan chan string
+ size int
+ capacity int
+ countMutex sync.Mutex
+}
+
+func (l *logAggregatorImpl) GetWriter() (io.WriteCloser, error) {
+ if err := l.checkCapacity(); err != nil {
+ return nil, err
+ }
+ r, w := io.Pipe()
+ ch := make(chan string, buffSize)
+ l.messages <- ch
+ // write the build output to a buffered channel.
+ go l.writeToChannel(r, ch)
+ return w, nil
+}
+
+func (l *logAggregatorImpl) PrintInOrder(ctx context.Context, out io.Writer) {
+ go func() {
+ <-ctx.Done()
+ // we handle cancellation by passing a nil struct instead of closing the channel.
+ // This makes it easier to flush all pending messages on the buffered channel before returning and avoid any race with pending requests for new writers.
+ l.messages <- nil
+ }()
+ for i := 0; i < l.capacity; i++ {
+ ch := <-l.messages
+ if ch == nil {
+ return
+ }
+ // read from each build's message channel and write to the given output.
+ printResult(out, ch)
+ }
+}
+
+func (l *logAggregatorImpl) checkCapacity() error {
+ l.countMutex.Lock()
+ defer l.countMutex.Unlock()
+ if l.size == l.capacity {
+ return fmt.Errorf("failed to create writer: capacity exceeded")
+ }
+ l.size++
+ return nil
+}
+
+func printResult(out io.Writer, output chan string) {
+ for line := range output {
+ fmt.Fprintln(out, line)
+ }
+}
+
+func (l *logAggregatorImpl) writeToChannel(r io.Reader, lines chan string) {
+ scanner := bufio.NewScanner(r)
+ for scanner.Scan() {
+ lines <- scanner.Text()
+ }
+ close(lines)
+}
+
+func newLogAggregator(capacity int) logAggregator {
+ return &logAggregatorImpl{capacity: capacity, messages: make(chan chan string, capacity)}
+}
+
+// builtArtifacts stores the results of each artifact build.
+type builtArtifacts interface {
+ Record(a *latest.Artifact, tag string)
+ GetTag(a *latest.Artifact) (string, error)
+ GetArtifacts(s []*latest.Artifact) ([]Artifact, error)
+}
+
+func newArtifactsStore() builtArtifacts {
+ return &builtArtifactsImpl{m: new(sync.Map)}
+}
+
+type builtArtifactsImpl struct {
+ m *sync.Map
+}
+
+func (ba *builtArtifactsImpl) Record(a *latest.Artifact, tag string) {
+ ba.m.Store(a.ImageName, tag)
+}
+
+func (ba *builtArtifactsImpl) GetTag(a *latest.Artifact) (string, error) {
+ v, ok := ba.m.Load(a.ImageName)
+ if !ok {
+ return "", fmt.Errorf("could not find build result for image %s", a.ImageName)
+ }
+ t, ok := v.(string)
+ if !ok {
+ logrus.Fatalf("invalid build output recorded for image %s", a.ImageName)
+ }
+ return t, nil
+}
+
+func (ba *builtArtifactsImpl) GetArtifacts(s []*latest.Artifact) ([]Artifact, error) {
+ var builds []Artifact
+ for _, a := range s {
+ t, err := ba.GetTag(a)
+ if err != nil {
+ return nil, err
+ }
+ builds = append(builds, Artifact{ImageName: a.ImageName, Tag: t})
+ }
+ return builds, nil
+}
diff --git a/pkg/skaffold/build/scheduler.go b/pkg/skaffold/build/scheduler.go
new file mode 100644
index 00000000000..27223485783
--- /dev/null
+++ b/pkg/skaffold/build/scheduler.go
@@ -0,0 +1,131 @@
+/*
+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 build
+
+import (
+ "context"
+ "fmt"
+ "io"
+
+ "golang.org/x/sync/errgroup"
+
+ "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/tag"
+ "github.com/GoogleContainerTools/skaffold/pkg/skaffold/color"
+ "github.com/GoogleContainerTools/skaffold/pkg/skaffold/event"
+ "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
+)
+
+type ArtifactBuilder func(ctx context.Context, out io.Writer, artifact *latest.Artifact, tag string) (string, error)
+
+type scheduler struct {
+ artifacts []*latest.Artifact
+ nodes []node // size len(artifacts)
+ artifactBuilder ArtifactBuilder
+ logger logAggregator
+ results builtArtifacts
+ concurrencySem countingSemaphore
+}
+
+func newScheduler(artifacts []*latest.Artifact, artifactBuilder ArtifactBuilder, concurrency int) *scheduler {
+ s := scheduler{
+ artifacts: artifacts,
+ nodes: createNodes(artifacts),
+ artifactBuilder: artifactBuilder,
+ logger: newLogAggregator(len(artifacts)),
+ results: newArtifactsStore(),
+ concurrencySem: newCountingSemaphore(concurrency),
+ }
+ return &s
+}
+
+func (s *scheduler) run(ctx context.Context, out io.Writer, tags tag.ImageTags) ([]Artifact, error) {
+ g, gCtx := errgroup.WithContext(ctx)
+
+ for i := range s.artifacts {
+ i := i
+
+ // Create a goroutine for each element in dag. Each goroutine waits on its dependencies to finish building.
+ // Because our artifacts form a DAG, at least one of the goroutines should be able to start building.
+ // Wrap in an error group so that all other builds are cancelled as soon as any one fails.
+ g.Go(func() error {
+ return s.build(gCtx, tags, i)
+ })
+ }
+ // print output for all artifact builds in order
+ s.logger.PrintInOrder(gCtx, out)
+ if err := g.Wait(); err != nil {
+ return nil, err
+ }
+ return s.results.GetArtifacts(s.artifacts)
+}
+
+func (s *scheduler) build(ctx context.Context, tags tag.ImageTags, i int) error {
+ n := s.nodes[i]
+ a := s.artifacts[i]
+ err := n.waitForDependencies(ctx)
+ if err != nil {
+ // `waitForDependencies` only returns `context.Canceled` error
+ event.BuildCanceled(a.ImageName)
+ return err
+ }
+ release := s.concurrencySem.acquire()
+ defer release()
+
+ event.BuildInProgress(a.ImageName)
+
+ w, err := s.logger.GetWriter()
+ if err != nil {
+ event.BuildFailed(a.ImageName, err)
+ return err
+ }
+ defer w.Close()
+
+ finalTag, err := performBuild(ctx, w, tags, a, s.artifactBuilder)
+ if err != nil {
+ event.BuildFailed(a.ImageName, err)
+ return err
+ }
+
+ s.results.Record(a, finalTag)
+ n.markComplete()
+ event.BuildComplete(a.ImageName)
+ return nil
+}
+
+// InOrder builds a list of artifacts in dependency order.
+func InOrder(ctx context.Context, out io.Writer, tags tag.ImageTags, artifacts []*latest.Artifact, artifactBuilder ArtifactBuilder, concurrency int) ([]Artifact, error) {
+ // `concurrency` specifies the max number of builds that can run at any one time. If concurrency is 0, then all builds can run in parallel.
+ if concurrency == 0 {
+ concurrency = len(artifacts)
+ }
+ if concurrency > 1 {
+ color.Default.Fprintf(out, "Building %d artifacts in parallel\n", concurrency)
+ }
+ s := newScheduler(artifacts, artifactBuilder, concurrency)
+ ctx, cancel := context.WithCancel(ctx)
+ defer cancel()
+ return s.run(ctx, out, tags)
+}
+
+func performBuild(ctx context.Context, cw io.Writer, tags tag.ImageTags, artifact *latest.Artifact, build ArtifactBuilder) (string, error) {
+ color.Default.Fprintf(cw, "Building [%s]...\n", artifact.ImageName)
+ tag, present := tags[artifact.ImageName]
+ if !present {
+ return "", fmt.Errorf("unable to find tag for image %s", artifact.ImageName)
+ }
+ return build(ctx, cw, artifact, tag)
+}
diff --git a/pkg/skaffold/build/parallel_test.go b/pkg/skaffold/build/scheduler_test.go
similarity index 59%
rename from pkg/skaffold/build/parallel_test.go
rename to pkg/skaffold/build/scheduler_test.go
index d73d8067e5e..715fea0422d 100644
--- a/pkg/skaffold/build/parallel_test.go
+++ b/pkg/skaffold/build/scheduler_test.go
@@ -27,7 +27,10 @@ import (
"testing"
"time"
+ "github.com/google/go-cmp/cmp"
+
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/tag"
+ "github.com/GoogleContainerTools/skaffold/pkg/skaffold/event"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
"github.com/GoogleContainerTools/skaffold/testutil"
)
@@ -77,7 +80,7 @@ func TestGetBuild(t *testing.T) {
out := new(bytes.Buffer)
artifact := &latest.Artifact{ImageName: "skaffold/image1"}
- got, err := getBuildResult(context.Background(), out, test.tags, artifact, test.buildArtifact)
+ got, err := performBuild(context.Background(), out, test.tags, artifact, test.buildArtifact)
t.CheckErrorAndDeepEqual(test.shouldErr, err, test.expectedTag, got)
t.CheckDeepEqual(test.expectedOut, out.String())
@@ -85,7 +88,7 @@ func TestGetBuild(t *testing.T) {
}
}
-func TestCollectResults(t *testing.T) {
+func TestFormatResults(t *testing.T) {
tests := []struct {
description string
artifacts []*latest.Artifact
@@ -104,52 +107,9 @@ func TestCollectResults(t *testing.T) {
{ImageName: "skaffold/image2", Tag: "skaffold/image2:v0.0.2@sha256:abac"},
},
results: map[string]interface{}{
- "skaffold/image1": Artifact{
- ImageName: "skaffold/image1",
- Tag: "skaffold/image1:v0.0.1@sha256:abac",
- },
- "skaffold/image2": Artifact{
- ImageName: "skaffold/image2",
- Tag: "skaffold/image2:v0.0.2@sha256:abac",
- },
- },
- },
- {
- description: "first build errors",
- artifacts: []*latest.Artifact{
- {ImageName: "skaffold/image1"},
- {ImageName: "skaffold/image2"},
- },
- expected: nil,
- results: map[string]interface{}{
- "skaffold/image1": fmt.Errorf("Could not build image skaffold/image1"),
- "skaffold/image2": Artifact{
- ImageName: "skaffold/image2",
- Tag: "skaffold/image2:v0.0.2@sha256:abac",
- },
- },
- shouldErr: true,
- },
- {
- description: "arbitrary image build failure",
- artifacts: []*latest.Artifact{
- {ImageName: "skaffold/image1"},
- {ImageName: "skaffold/image2"},
- {ImageName: "skaffold/image3"},
- },
- expected: nil,
- results: map[string]interface{}{
- "skaffold/image1": Artifact{
- ImageName: "skaffold/image1",
- Tag: "skaffold/image1:v0.0.1@sha256:abac",
- },
- "skaffold/image2": fmt.Errorf("Could not build image skaffold/image1"),
- "skaffold/image3": Artifact{
- ImageName: "skaffold/image3",
- Tag: "skaffold/image3:v0.0.1@sha256:abac",
- },
+ "skaffold/image1": "skaffold/image1:v0.0.1@sha256:abac",
+ "skaffold/image2": "skaffold/image2:v0.0.2@sha256:abac",
},
- shouldErr: true,
},
{
description: "no build result produced for a build",
@@ -159,42 +119,26 @@ func TestCollectResults(t *testing.T) {
},
expected: nil,
results: map[string]interface{}{
- "skaffold/image1": Artifact{
- ImageName: "skaffold/image1:v0.0.1@sha256:abac",
- Tag: "skaffold/image1:v0.0.1@sha256:abac",
- },
- },
- shouldErr: true,
- },
- {
- description: "build produced an incorrect value type",
- artifacts: []*latest.Artifact{
- {ImageName: "skaffold/image1"},
- {ImageName: "skaffold/image2"},
- },
- expected: nil,
- results: map[string]interface{}{
- "skaffold/image1": 1,
+ "skaffold/image1": "skaffold/image1:v0.0.1@sha256:abac",
},
shouldErr: true,
},
}
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
- outputs := setUpChannels(len(test.artifacts))
- resultMap := new(sync.Map)
+ m := new(sync.Map)
for k, v := range test.results {
- resultMap.Store(k, v)
+ m.Store(k, v)
}
-
- got, err := collectResults(ioutil.Discard, test.artifacts, resultMap, outputs)
+ results := &builtArtifactsImpl{m: m}
+ got, err := results.GetArtifacts(test.artifacts)
t.CheckErrorAndDeepEqual(test.shouldErr, err, test.expected, got)
})
}
}
-func TestInParallel(t *testing.T) {
+func TestInOrder(t *testing.T) {
tests := []struct {
description string
buildFunc ArtifactBuilder
@@ -202,7 +146,7 @@ func TestInParallel(t *testing.T) {
}{
{
description: "short and nice build log",
- expected: "Building [skaffold/image1]...\nshort\nBuilding [skaffold/image2]...\nshort\n",
+ expected: "Building 2 artifacts in parallel\nBuilding [skaffold/image1]...\nshort\nBuilding [skaffold/image2]...\nshort\n",
buildFunc: func(ctx context.Context, out io.Writer, artifact *latest.Artifact, tag string) (string, error) {
out.Write([]byte("short"))
return fmt.Sprintf("%s:tag", artifact.ImageName), nil
@@ -210,7 +154,8 @@ func TestInParallel(t *testing.T) {
},
{
description: "long build log gets printed correctly",
- expected: `Building [skaffold/image1]...
+ expected: `Building 2 artifacts in parallel
+Building [skaffold/image1]...
This is a long string more than 10 bytes.
And new lines
Building [skaffold/image2]...
@@ -228,7 +173,7 @@ And new lines
out := new(bytes.Buffer)
artifacts := []*latest.Artifact{
{ImageName: "skaffold/image1"},
- {ImageName: "skaffold/image2"},
+ {ImageName: "skaffold/image2", Dependencies: []*latest.ArtifactDependency{{ImageName: "skaffold/image1"}}},
}
tags := tag.ImageTags{
"skaffold/image1": "skaffold/image1:v0.0.1",
@@ -236,14 +181,14 @@ And new lines
}
initializeEvents()
- InParallel(context.Background(), out, tags, artifacts, test.buildFunc, 0)
+ InOrder(context.Background(), out, tags, artifacts, test.buildFunc, 0)
t.CheckDeepEqual(test.expected, out.String())
})
}
}
-func TestInParallelConcurrency(t *testing.T) {
+func TestInOrderConcurrency(t *testing.T) {
tests := []struct {
artifacts int
limit int
@@ -291,7 +236,7 @@ func TestInParallelConcurrency(t *testing.T) {
}
initializeEvents()
- results, err := InParallel(context.Background(), ioutil.Discard, tags, artifacts, builder, test.limit)
+ results, err := InOrder(context.Background(), ioutil.Discard, tags, artifacts, builder, test.limit)
t.CheckNoError(err)
t.CheckDeepEqual(test.artifacts, len(results))
@@ -299,31 +244,66 @@ func TestInParallelConcurrency(t *testing.T) {
}
}
-func TestInParallelForArgs(t *testing.T) {
+func TestInOrderForArgs(t *testing.T) {
tests := []struct {
description string
- inSeqFunc func(context.Context, io.Writer, tag.ImageTags, []*latest.Artifact, ArtifactBuilder) ([]Artifact, error)
buildArtifact ArtifactBuilder
artifactLen int
+ concurrency int
+ dependency map[int][]int
expected []Artifact
+ err error
}{
{
- description: "runs in sequence for 1 artifact",
- inSeqFunc: func(context.Context, io.Writer, tag.ImageTags, []*latest.Artifact, ArtifactBuilder) ([]Artifact, error) {
- return []Artifact{{ImageName: "singleArtifact", Tag: "one"}}, nil
+ description: "runs in parallel for 2 artifacts with no dependency",
+ buildArtifact: func(_ context.Context, _ io.Writer, _ *latest.Artifact, tag string) (string, error) {
+ return tag, nil
+ },
+ artifactLen: 2,
+ expected: []Artifact{
+ {ImageName: "artifact1", Tag: "artifact1@tag1"},
+ {ImageName: "artifact2", Tag: "artifact2@tag2"},
+ },
+ },
+ {
+ description: "runs in parallel for 5 artifacts with dependencies",
+ buildArtifact: func(_ context.Context, _ io.Writer, _ *latest.Artifact, tag string) (string, error) {
+ return tag, nil
+ },
+ dependency: map[int][]int{
+ 0: {2, 3},
+ 1: {3},
+ 2: {1},
+ 3: {4},
+ },
+ artifactLen: 5,
+ expected: []Artifact{
+ {ImageName: "artifact1", Tag: "artifact1@tag1"},
+ {ImageName: "artifact2", Tag: "artifact2@tag2"},
+ {ImageName: "artifact3", Tag: "artifact3@tag3"},
+ {ImageName: "artifact4", Tag: "artifact4@tag4"},
+ {ImageName: "artifact5", Tag: "artifact5@tag5"},
},
- artifactLen: 1,
- expected: []Artifact{{ImageName: "singleArtifact", Tag: "one"}},
},
{
- description: "runs in parallel for 2 artifacts",
+ description: "runs with max concurrency of 2 for 5 artifacts with dependencies",
buildArtifact: func(_ context.Context, _ io.Writer, _ *latest.Artifact, tag string) (string, error) {
return tag, nil
},
- artifactLen: 2,
+ dependency: map[int][]int{
+ 0: {2, 3},
+ 1: {3},
+ 2: {1},
+ 3: {4},
+ },
+ artifactLen: 5,
+ concurrency: 2,
expected: []Artifact{
{ImageName: "artifact1", Tag: "artifact1@tag1"},
{ImageName: "artifact2", Tag: "artifact2@tag2"},
+ {ImageName: "artifact3", Tag: "artifact3@tag3"},
+ {ImageName: "artifact4", Tag: "artifact4@tag4"},
+ {ImageName: "artifact5", Tag: "artifact5@tag5"},
},
},
{
@@ -331,6 +311,41 @@ func TestInParallelForArgs(t *testing.T) {
artifactLen: 0,
expected: nil,
},
+ {
+ description: "build fails for artifacts without dependencies",
+ buildArtifact: func(c context.Context, _ io.Writer, a *latest.Artifact, tag string) (string, error) {
+ if a.ImageName == "artifact2" {
+ return "", fmt.Errorf(`some error occurred while building "artifact2"`)
+ }
+ select {
+ case <-c.Done():
+ return "", c.Err()
+ case <-time.After(5 * time.Second):
+ return tag, nil
+ }
+ },
+ artifactLen: 5,
+ expected: nil,
+ err: fmt.Errorf(`some error occurred while building "artifact2"`),
+ },
+ {
+ description: "build fails for artifacts with dependencies",
+ buildArtifact: func(_ context.Context, _ io.Writer, a *latest.Artifact, tag string) (string, error) {
+ if a.ImageName == "artifact2" {
+ return "", fmt.Errorf(`some error occurred while building "artifact2"`)
+ }
+ return tag, nil
+ },
+ dependency: map[int][]int{
+ 0: {1},
+ 1: {2},
+ 2: {3},
+ 3: {4},
+ },
+ artifactLen: 5,
+ expected: nil,
+ err: fmt.Errorf(`some error occurred while building "artifact2"`),
+ },
}
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
@@ -341,22 +356,52 @@ func TestInParallelForArgs(t *testing.T) {
artifacts[i] = &latest.Artifact{ImageName: a}
tags[a] = fmt.Sprintf("%s@tag%d", a, i+1)
}
- if test.inSeqFunc != nil {
- t.Override(&runInSequence, test.inSeqFunc)
- }
+
+ setDependencies(artifacts, test.dependency)
initializeEvents()
- actual, _ := InParallel(context.Background(), ioutil.Discard, tags, artifacts, test.buildArtifact, 0)
+ actual, err := InOrder(context.Background(), ioutil.Discard, tags, artifacts, test.buildArtifact, test.concurrency)
t.CheckDeepEqual(test.expected, actual)
+ t.CheckDeepEqual(test.err, err, cmp.Comparer(errorsComparer))
})
}
}
-func setUpChannels(n int) []chan string {
- outputs := make([]chan string, n)
- for i := 0; i < n; i++ {
- outputs[i] = make(chan string, 10)
- close(outputs[i])
+// setDependencies constructs a graph of artifact dependencies using the map as an adjacency list representation of indices in the artifacts array.
+// For example:
+// m = {
+// 0 : {1, 2},
+// 2 : {3},
+//}
+// implies that a[0] artifact depends on a[1] and a[2]; and a[2] depends on a[3].
+func setDependencies(a []*latest.Artifact, d map[int][]int) {
+ for k, dep := range d {
+ for i := range dep {
+ a[k].Dependencies = append(a[k].Dependencies, &latest.ArtifactDependency{
+ ImageName: a[dep[i]].ImageName,
+ })
+ }
+ }
+}
+
+func initializeEvents() {
+ pipe := latest.Pipeline{
+ Deploy: latest.DeployConfig{},
+ Build: latest.BuildConfig{
+ BuildType: latest.BuildType{
+ LocalBuild: &latest.LocalBuild{},
+ },
+ },
+ }
+ event.InitializeState(pipe, "temp", true, true, true)
+}
+
+func errorsComparer(a, b error) bool {
+ if a == nil && b == nil {
+ return true
+ }
+ if a == nil || b == nil {
+ return false
}
- return outputs
+ return a.Error() == b.Error()
}
diff --git a/pkg/skaffold/build/sequence.go b/pkg/skaffold/build/sequence.go
deleted file mode 100644
index e63ed7b70af..00000000000
--- a/pkg/skaffold/build/sequence.go
+++ /dev/null
@@ -1,59 +0,0 @@
-/*
-Copyright 2019 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 build
-
-import (
- "context"
- "fmt"
- "io"
-
- "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/tag"
- "github.com/GoogleContainerTools/skaffold/pkg/skaffold/color"
- "github.com/GoogleContainerTools/skaffold/pkg/skaffold/event"
- "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
-)
-
-// InSequence builds a list of artifacts in sequence.
-func InSequence(ctx context.Context, out io.Writer, tags tag.ImageTags, artifacts []*latest.Artifact, buildArtifact ArtifactBuilder) ([]Artifact, error) {
- var builds []Artifact
-
- for _, artifact := range artifacts {
- color.Default.Fprintf(out, "Building [%s]...\n", artifact.ImageName)
-
- event.BuildInProgress(artifact.ImageName)
-
- tag, present := tags[artifact.ImageName]
- if !present {
- return nil, fmt.Errorf("unable to find tag for image %s", artifact.ImageName)
- }
-
- finalTag, err := buildArtifact(ctx, out, artifact, tag)
- if err != nil {
- event.BuildFailed(artifact.ImageName, err)
- return nil, fmt.Errorf("couldn't build %q: %w", artifact.ImageName, err)
- }
-
- event.BuildComplete(artifact.ImageName)
-
- builds = append(builds, Artifact{
- ImageName: artifact.ImageName,
- Tag: finalTag,
- })
- }
-
- return builds, nil
-}
diff --git a/pkg/skaffold/build/sequence_test.go b/pkg/skaffold/build/sequence_test.go
deleted file mode 100644
index d9dc3d19479..00000000000
--- a/pkg/skaffold/build/sequence_test.go
+++ /dev/null
@@ -1,156 +0,0 @@
-/*
-Copyright 2019 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 build
-
-import (
- "bytes"
- "context"
- "fmt"
- "io"
- "io/ioutil"
- "testing"
-
- "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/tag"
- "github.com/GoogleContainerTools/skaffold/pkg/skaffold/event"
- "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
- "github.com/GoogleContainerTools/skaffold/testutil"
-)
-
-func TestInSequence(t *testing.T) {
- tests := []struct {
- description string
- buildArtifact ArtifactBuilder
- tags tag.ImageTags
- expectedArtifacts []Artifact
- expectedOut string
- shouldErr bool
- }{
- {
- description: "build succeeds",
- buildArtifact: func(ctx context.Context, out io.Writer, artifact *latest.Artifact, tag string) (string, error) {
- return fmt.Sprintf("%s@sha256:abac", tag), nil
- },
- tags: tag.ImageTags{
- "skaffold/image1": "skaffold/image1:v0.0.1",
- "skaffold/image2": "skaffold/image2:v0.0.2",
- },
- expectedArtifacts: []Artifact{
- {ImageName: "skaffold/image1", Tag: "skaffold/image1:v0.0.1@sha256:abac"},
- {ImageName: "skaffold/image2", Tag: "skaffold/image2:v0.0.2@sha256:abac"},
- },
- expectedOut: "Building [skaffold/image1]...\nBuilding [skaffold/image2]...\n",
- },
- {
- description: "build fails",
- buildArtifact: func(ctx context.Context, out io.Writer, artifact *latest.Artifact, tag string) (string, error) {
- return "", fmt.Errorf("build fails")
- },
- tags: tag.ImageTags{
- "skaffold/image1": "",
- },
- expectedOut: "Building [skaffold/image1]...\n",
- shouldErr: true,
- },
- {
- description: "tag not found",
- tags: tag.ImageTags{},
- expectedOut: "Building [skaffold/image1]...\n",
- shouldErr: true,
- },
- }
- for _, test := range tests {
- testutil.Run(t, test.description, func(t *testutil.T) {
- out := new(bytes.Buffer)
- artifacts := []*latest.Artifact{
- {ImageName: "skaffold/image1"},
- {ImageName: "skaffold/image2"},
- }
-
- got, err := InSequence(context.Background(), out, test.tags, artifacts, test.buildArtifact)
-
- t.CheckErrorAndDeepEqual(test.shouldErr, err, test.expectedArtifacts, got)
- t.CheckDeepEqual(test.expectedOut, out.String())
- })
- }
-}
-
-func TestInSequenceResultsOrder(t *testing.T) {
- tests := []struct {
- description string
- images []string
- expected []Artifact
- shouldErr bool
- }{
- {
- description: "shd concatenate the tag",
- images: []string{"a", "b", "c", "d"},
- expected: []Artifact{
- {ImageName: "a", Tag: "a:a"},
- {ImageName: "b", Tag: "b:ab"},
- {ImageName: "c", Tag: "c:abc"},
- {ImageName: "d", Tag: "d:abcd"},
- },
- },
- }
- for _, test := range tests {
- testutil.Run(t, test.description, func(t *testutil.T) {
- out := ioutil.Discard
- initializeEvents()
- artifacts := make([]*latest.Artifact, len(test.images))
- tags := tag.ImageTags{}
- for i, image := range test.images {
- artifacts[i] = &latest.Artifact{
- ImageName: image,
- }
- tags[image] = image
- }
- builder := concatTagger{}
-
- got, err := InSequence(context.Background(), out, tags, artifacts, builder.doBuild)
-
- t.CheckErrorAndDeepEqual(test.shouldErr, err, test.expected, got)
- })
- }
-}
-
-// concatTagger builder sums all the numbers
-type concatTagger struct {
- tag string
-}
-
-// doBuild calculate the tag based by concatinating the tag values for artifact
-// builds seen so far. It mimics artifact dependency where the next build result
-// depends on the previous build result.
-func (t *concatTagger) doBuild(ctx context.Context, out io.Writer, artifact *latest.Artifact, tag string) (string, error) {
- t.tag += tag
- return fmt.Sprintf("%s:%s", artifact.ImageName, t.tag), nil
-}
-
-func initializeEvents() {
- event.InitializeState(latest.Pipeline{
- Deploy: latest.DeployConfig{},
- Build: latest.BuildConfig{
- BuildType: latest.BuildType{
- LocalBuild: &latest.LocalBuild{},
- },
- },
- },
- "temp",
- true,
- true,
- true)
-}
diff --git a/pkg/skaffold/event/event.go b/pkg/skaffold/event/event.go
index 511c4a400b1..3e6a8dcccf5 100644
--- a/pkg/skaffold/event/event.go
+++ b/pkg/skaffold/event/event.go
@@ -39,6 +39,7 @@ const (
Started = "Started"
Succeeded = "Succeeded"
Terminated = "Terminated"
+ Canceled = "Canceled"
)
var handler = newHandler()
@@ -300,6 +301,11 @@ func BuildInProgress(imageName string) {
handler.handleBuildEvent(&proto.BuildEvent{Artifact: imageName, Status: InProgress})
}
+// BuildCanceled notifies that a build has been canceled.
+func BuildCanceled(imageName string) {
+ handler.handleBuildEvent(&proto.BuildEvent{Artifact: imageName, Status: Canceled})
+}
+
// BuildFailed notifies that a build has failed.
func BuildFailed(imageName string, err error) {
aiErr := sErrors.ActionableErr(sErrors.Build, err)
diff --git a/pkg/skaffold/schema/defaults/defaults.go b/pkg/skaffold/schema/defaults/defaults.go
index 2ed376e0e7e..da0abf6cba1 100644
--- a/pkg/skaffold/schema/defaults/defaults.go
+++ b/pkg/skaffold/schema/defaults/defaults.go
@@ -68,6 +68,10 @@ func Set(c *latest.SkaffoldConfig) error {
case a.BuildpackArtifact != nil:
setBuildpackArtifactDefaults(a.BuildpackArtifact)
}
+
+ for _, d := range a.Dependencies {
+ setDefaultArtifactDependencyAlias(d)
+ }
}
withLocalBuild(c,
@@ -371,3 +375,9 @@ func setDefaultAddress(pf *latest.PortForwardResource) {
pf.Address = constants.DefaultPortForwardAddress
}
}
+
+func setDefaultArtifactDependencyAlias(d *latest.ArtifactDependency) {
+ if d.Alias == "" {
+ d.Alias = d.ImageName
+ }
+}
diff --git a/pkg/skaffold/schema/defaults/defaults_test.go b/pkg/skaffold/schema/defaults/defaults_test.go
index e341f4332df..453d32b6484 100644
--- a/pkg/skaffold/schema/defaults/defaults_test.go
+++ b/pkg/skaffold/schema/defaults/defaults_test.go
@@ -35,6 +35,10 @@ func TestSetDefaults(t *testing.T) {
Artifacts: []*latest.Artifact{
{
ImageName: "first",
+ Dependencies: []*latest.ArtifactDependency{
+ {ImageName: "second", Alias: "secondAlias"},
+ {ImageName: "third"},
+ },
},
{
ImageName: "second",
@@ -83,6 +87,8 @@ func TestSetDefaults(t *testing.T) {
testutil.CheckDeepEqual(t, "first", cfg.Build.Artifacts[0].ImageName)
testutil.CheckDeepEqual(t, ".", cfg.Build.Artifacts[0].Workspace)
testutil.CheckDeepEqual(t, "Dockerfile", cfg.Build.Artifacts[0].DockerArtifact.DockerfilePath)
+ testutil.CheckDeepEqual(t, "secondAlias", cfg.Build.Artifacts[0].Dependencies[0].Alias)
+ testutil.CheckDeepEqual(t, "third", cfg.Build.Artifacts[0].Dependencies[1].Alias)
testutil.CheckDeepEqual(t, "second", cfg.Build.Artifacts[1].ImageName)
testutil.CheckDeepEqual(t, "folder", cfg.Build.Artifacts[1].Workspace)
diff --git a/pkg/skaffold/schema/latest/config.go b/pkg/skaffold/schema/latest/config.go
index a592893db93..66c0447484c 100644
--- a/pkg/skaffold/schema/latest/config.go
+++ b/pkg/skaffold/schema/latest/config.go
@@ -762,6 +762,9 @@ type Artifact struct {
// ArtifactType describes how to build an artifact.
ArtifactType `yaml:",inline"`
+
+ // Dependencies describes build artifacts that this artifact depends on.
+ Dependencies []*ArtifactDependency `yaml:"requires,omitempty"`
}
// Sync *beta* specifies what files to sync into the container.
@@ -884,6 +887,16 @@ type ArtifactType struct {
CustomArtifact *CustomArtifact `yaml:"custom,omitempty" yamltags:"oneOf=artifact"`
}
+// ArtifactDependency describes a specific build dependency for an artifact.
+type ArtifactDependency struct {
+ // ImageName is a reference to an artifact's image name.
+ ImageName string `yaml:"image" yamltags:"required"`
+ // Alias is 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`.
+ Alias string `yaml:"alias,omitempty"`
+}
+
// BuildpackArtifact *alpha* describes an artifact built using [Cloud Native Buildpacks](https://buildpacks.io/).
// It can be used to build images out of project's sources without any additional configuration.
type BuildpackArtifact struct {
diff --git a/pkg/skaffold/schema/validation/validation.go b/pkg/skaffold/schema/validation/validation.go
index 6c3c1a6448c..08308749433 100644
--- a/pkg/skaffold/schema/validation/validation.go
+++ b/pkg/skaffold/schema/validation/validation.go
@@ -19,6 +19,7 @@ package validation
import (
"fmt"
"reflect"
+ "regexp"
"strings"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/misc"
@@ -30,13 +31,15 @@ import (
var (
// for testing
- validateYamltags = yamltags.ValidateStruct
+ validateYamltags = yamltags.ValidateStruct
+ dependencyAliasPattern = regexp.MustCompile(`^[a-zA-Z_][a-zA-Z0-9_]*$`)
)
// Process checks if the Skaffold pipeline is valid and returns all encountered errors as a concatenated string
func Process(config *latest.SkaffoldConfig) error {
errs := visitStructs(config, validateYamltags)
errs = append(errs, validateImageNames(config.Build.Artifacts)...)
+ errs = append(errs, validateArtifactDependencies(config.Build.Artifacts)...)
errs = append(errs, validateDockerNetworkMode(config.Build.Artifacts)...)
errs = append(errs, validateCustomDependencies(config.Build.Artifacts)...)
errs = append(errs, validateSyncRules(config.Build.Artifacts)...)
@@ -77,6 +80,93 @@ func validateImageNames(artifacts []*latest.Artifact) (errs []error) {
return
}
+func validateArtifactDependencies(artifacts []*latest.Artifact) (errs []error) {
+ errs = append(errs, validateUniqueDependencyAliases(artifacts)...)
+ errs = append(errs, validateAcyclicDependencies(artifacts)...)
+ errs = append(errs, validateValidDependencyAliases(artifacts)...)
+ return
+}
+
+// validateAcyclicDependencies makes sure all artifact dependencies are found and don't have cyclic references
+func validateAcyclicDependencies(artifacts []*latest.Artifact) (errs []error) {
+ m := make(map[string]*latest.Artifact)
+ for _, artifact := range artifacts {
+ m[artifact.ImageName] = artifact
+ }
+ visited := make(map[string]bool)
+ for _, artifact := range artifacts {
+ if err := dfs(artifact, visited, make(map[string]bool), m); err != nil {
+ errs = append(errs, err)
+ return
+ }
+ }
+ return
+}
+
+// dfs runs a Depth First Search algorithm for cycle detection in a directed graph
+func dfs(artifact *latest.Artifact, visited, marked map[string]bool, artifacts map[string]*latest.Artifact) error {
+ if marked[artifact.ImageName] {
+ return fmt.Errorf("cycle detected in build dependencies involving %q", artifact.ImageName)
+ }
+ marked[artifact.ImageName] = true
+ defer func() {
+ marked[artifact.ImageName] = false
+ }()
+ if visited[artifact.ImageName] {
+ return nil
+ }
+ visited[artifact.ImageName] = true
+
+ for _, dep := range artifact.Dependencies {
+ d, found := artifacts[dep.ImageName]
+ if !found {
+ return fmt.Errorf("unknown build dependency %q for artifact %q", dep.ImageName, artifact.ImageName)
+ }
+ if err := dfs(d, visited, marked, artifacts); err != nil {
+ return err
+ }
+ }
+ return nil
+}
+
+// validateValidDependencyAliases makes sure that artifact dependency aliases are valid.
+// docker and custom builders require aliases match [a-zA-Z_][a-zA-Z0-9_]* pattern
+func validateValidDependencyAliases(artifacts []*latest.Artifact) (errs []error) {
+ for _, a := range artifacts {
+ if a.DockerArtifact == nil && a.CustomArtifact == nil {
+ continue
+ }
+ for _, d := range a.Dependencies {
+ if !dependencyAliasPattern.MatchString(d.Alias) {
+ errs = append(errs, fmt.Errorf("invalid build dependency for artifact %q: alias %q doesn't match required pattern %q", a.ImageName, d.Alias, dependencyAliasPattern.String()))
+ }
+ }
+ }
+ return
+}
+
+// validateUniqueDependencyAliases makes sure that artifact dependency aliases are unique for each artifact
+func validateUniqueDependencyAliases(artifacts []*latest.Artifact) (errs []error) {
+ type State int
+ var (
+ unseen State = 0
+ seen State = 1
+ recorded State = 2
+ )
+ for _, a := range artifacts {
+ aliasMap := make(map[string]State)
+ for _, d := range a.Dependencies {
+ if aliasMap[d.Alias] == seen {
+ errs = append(errs, fmt.Errorf("invalid build dependency for artifact %q: alias %q repeated", a.ImageName, d.Alias))
+ aliasMap[d.Alias] = recorded
+ } else if aliasMap[d.Alias] == unseen {
+ aliasMap[d.Alias] = seen
+ }
+ }
+ }
+ return
+}
+
// validateDockerNetworkMode makes sure that networkMode is one of `bridge`, `none`, or `host` if set.
func validateDockerNetworkMode(artifacts []*latest.Artifact) (errs []error) {
for _, a := range artifacts {
diff --git a/pkg/skaffold/schema/validation/validation_test.go b/pkg/skaffold/schema/validation/validation_test.go
index 12cb2380f0b..9fff1af2c25 100644
--- a/pkg/skaffold/schema/validation/validation_test.go
+++ b/pkg/skaffold/schema/validation/validation_test.go
@@ -20,6 +20,8 @@ import (
"fmt"
"testing"
+ "github.com/google/go-cmp/cmp"
+
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
"github.com/GoogleContainerTools/skaffold/testutil"
)
@@ -750,3 +752,188 @@ func TestValidateLogsConfig(t *testing.T) {
})
}
}
+
+func TestValidateAcyclicDependencies(t *testing.T) {
+ tests := []struct {
+ description string
+ artifactLen int
+ dependency map[int][]int
+ shouldErr bool
+ }{
+ {
+ description: "artifacts with no dependency",
+ artifactLen: 5,
+ },
+ {
+ description: "artifacts with no circular dependencies 1",
+ dependency: map[int][]int{
+ 0: {2, 3},
+ 1: {3},
+ 2: {1},
+ 3: {4},
+ },
+ artifactLen: 5,
+ },
+ {
+ description: "artifacts with no circular dependencies 2",
+ dependency: map[int][]int{
+ 0: {4, 5},
+ 1: {4, 5},
+ 2: {4, 5},
+ 3: {4, 5},
+ },
+ artifactLen: 6,
+ },
+ {
+ description: "artifacts with circular dependencies",
+ dependency: map[int][]int{
+ 0: {2, 3},
+ 1: {0},
+ 2: {1},
+ 3: {4},
+ },
+ artifactLen: 5,
+ shouldErr: true,
+ },
+ {
+ description: "artifacts with circular dependencies (self)",
+ dependency: map[int][]int{
+ 0: {0},
+ 1: {},
+ },
+ artifactLen: 2,
+ shouldErr: true,
+ },
+ {
+ description: "0 artifacts",
+ artifactLen: 0,
+ },
+ }
+ for _, test := range tests {
+ testutil.Run(t, test.description, func(t *testutil.T) {
+ artifacts := make([]*latest.Artifact, test.artifactLen)
+ for i := 0; i < test.artifactLen; i++ {
+ a := fmt.Sprintf("artifact%d", i+1)
+ artifacts[i] = &latest.Artifact{ImageName: a}
+ }
+
+ setDependencies(artifacts, test.dependency)
+ errs := validateAcyclicDependencies(artifacts)
+ expected := []error{
+ fmt.Errorf(`cycle detected in build dependencies involving "artifact1"`),
+ }
+ if test.shouldErr {
+ t.CheckDeepEqual(expected, errs, cmp.Comparer(errorsComparer))
+ } else {
+ t.CheckDeepEqual(0, len(errs))
+ }
+ })
+ }
+}
+
+// setDependencies constructs a graph of artifact dependencies using the map as an adjacency list representation of indices in the artifacts array.
+// For example:
+// m = {
+// 0 : {1, 2},
+// 2 : {3},
+//}
+// implies that a[0] artifact depends on a[1] and a[2]; and a[2] depends on a[3].
+func setDependencies(a []*latest.Artifact, d map[int][]int) {
+ for k, dep := range d {
+ for i := range dep {
+ a[k].Dependencies = append(a[k].Dependencies, &latest.ArtifactDependency{
+ ImageName: a[dep[i]].ImageName,
+ })
+ }
+ }
+}
+
+func TestValidateUniqueDependencyAliases(t *testing.T) {
+ artifacts := []*latest.Artifact{
+ {
+ ImageName: "artifact1",
+ Dependencies: []*latest.ArtifactDependency{
+ {Alias: "alias2", ImageName: "artifact2a"},
+ {Alias: "alias2", ImageName: "artifact2b"},
+ },
+ },
+ {
+ ImageName: "artifact2",
+ Dependencies: []*latest.ArtifactDependency{
+ {Alias: "alias1", ImageName: "artifact1"},
+ {Alias: "alias2", ImageName: "artifact1"},
+ },
+ },
+ }
+ expected := []error{
+ fmt.Errorf(`invalid build dependency for artifact "artifact1": alias "alias2" repeated`),
+ fmt.Errorf(`unknown build dependency "artifact2a" for artifact "artifact1"`),
+ }
+ errs := validateArtifactDependencies(artifacts)
+ testutil.CheckDeepEqual(t, expected, errs, cmp.Comparer(errorsComparer))
+}
+
+func TestValidateValidDependencyAliases(t *testing.T) {
+ artifacts := []*latest.Artifact{
+ {
+ ImageName: "artifact1",
+ },
+ {
+ ImageName: "artifact2",
+ ArtifactType: latest.ArtifactType{
+ DockerArtifact: &latest.DockerArtifact{},
+ },
+ Dependencies: []*latest.ArtifactDependency{
+ {Alias: "ARTIFACT_1", ImageName: "artifact1"},
+ {Alias: "1_ARTIFACT", ImageName: "artifact1"},
+ },
+ },
+ {
+ ImageName: "artifact3",
+ ArtifactType: latest.ArtifactType{
+ DockerArtifact: &latest.DockerArtifact{},
+ },
+ Dependencies: []*latest.ArtifactDependency{
+ {Alias: "artifact!", ImageName: "artifact1"},
+ {Alias: "artifact#1", ImageName: "artifact1"},
+ },
+ },
+ {
+ ImageName: "artifact4",
+ ArtifactType: latest.ArtifactType{
+ CustomArtifact: &latest.CustomArtifact{},
+ },
+ Dependencies: []*latest.ArtifactDependency{
+ {Alias: "alias1", ImageName: "artifact1"},
+ {Alias: "alias2", ImageName: "artifact2"},
+ },
+ },
+ {
+ ImageName: "artifact5",
+ ArtifactType: latest.ArtifactType{
+ BuildpackArtifact: &latest.BuildpackArtifact{},
+ },
+ Dependencies: []*latest.ArtifactDependency{
+ {Alias: "artifact!", ImageName: "artifact1"},
+ {Alias: "artifact#1", ImageName: "artifact1"},
+ },
+ },
+ }
+ expected := []error{
+ fmt.Errorf(`invalid build dependency for artifact "artifact2": alias "1_ARTIFACT" doesn't match required pattern %q`, dependencyAliasPattern),
+ fmt.Errorf(`invalid build dependency for artifact "artifact3": alias "artifact!" doesn't match required pattern %q`, dependencyAliasPattern),
+ fmt.Errorf(`invalid build dependency for artifact "artifact3": alias "artifact#1" doesn't match required pattern %q`, dependencyAliasPattern),
+ }
+ errs := validateArtifactDependencies(artifacts)
+ testutil.CheckDeepEqual(t, expected, errs, cmp.Comparer(errorsComparer))
+}
+
+func errorsComparer(a, b error) bool {
+ if a == nil && b == nil {
+ return true
+ }
+ if a == nil || b == nil {
+ return false
+ }
+ return a.Error() == b.Error()
+}