Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support build artifact dependencies #4752

Closed
wants to merge 20 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions docs/content/en/schemas/v2beta6.json
Original file line number Diff line number Diff line change
Expand Up @@ -1279,8 +1279,8 @@
],
"properties": {
"artifactOverrides": {
"description": "key value pairs where the key represents the parameter used in the `--set-string` Helm CLI flag to define a container image and the value corresponds to artifact i.e. `ImageName` defined in `Build.Artifacts` section. The resulting command-line is controlled by `ImageStrategy`.",
"x-intellij-html-description": "key value pairs where the key represents the parameter used in the <code>--set-string</code> Helm CLI flag to define a container image and the value corresponds to artifact i.e. <code>ImageName</code> defined in <code>Build.Artifacts</code> section. The resulting command-line is controlled by <code>ImageStrategy</code>."
"description": "key value pairs. If present, Skaffold will send `--set-string` flag to Helm CLI and append all pairs after the flag.",
"x-intellij-html-description": "key value pairs. If present, Skaffold will send <code>--set-string</code> flag to Helm CLI and append all pairs after the flag."
},
"chartPath": {
"type": "string",
Expand All @@ -1289,8 +1289,8 @@
},
"imageStrategy": {
"$ref": "#/definitions/HelmImageStrategy",
"description": "controls how an `ArtifactOverrides` entry is turned into `--set-string` Helm CLI flag or flags.",
"x-intellij-html-description": "controls how an <code>ArtifactOverrides</code> entry is turned into <code>--set-string</code> Helm CLI flag or flags."
"description": "adds image configurations to the Helm `values` file.",
"x-intellij-html-description": "adds image configurations to the Helm <code>values</code> file."
},
"name": {
"type": "string",
Expand Down
97 changes: 92 additions & 5 deletions docs/content/en/schemas/v2beta8.json
Original file line number Diff line number Diff line change
Expand Up @@ -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.",
Expand All @@ -74,7 +82,8 @@
"preferredOrder": [
"image",
"context",
"sync"
"sync",
"requires"
],
"additionalProperties": false
},
Expand All @@ -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.",
Expand All @@ -110,6 +127,7 @@
"image",
"context",
"sync",
"requires",
"docker"
],
"additionalProperties": false
Expand All @@ -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.",
Expand All @@ -146,6 +172,7 @@
"image",
"context",
"sync",
"requires",
"bazel"
],
"additionalProperties": false
Expand All @@ -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.",
Expand All @@ -182,6 +217,7 @@
"image",
"context",
"sync",
"requires",
"jib"
],
"additionalProperties": false
Expand All @@ -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.",
Expand All @@ -218,6 +262,7 @@
"image",
"context",
"sync",
"requires",
"kaniko"
],
"additionalProperties": false
Expand All @@ -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.",
Expand All @@ -254,6 +307,7 @@
"image",
"context",
"sync",
"requires",
"buildpacks"
],
"additionalProperties": false
Expand All @@ -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.",
Expand All @@ -290,6 +352,7 @@
"image",
"context",
"sync",
"requires",
"custom"
],
"additionalProperties": false
Expand All @@ -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 will be replaced with the image reference in the builder definition files. For example, the `docker` builder will use the alias as a build-time variable. Defaults to the value of `image`.",
"x-intellij-html-description": "a token that will be replaced with the image reference in the builder definition files. For example, the <code>docker</code> builder will use the alias as a build-time variable. Defaults to the value of <code>image</code>."
},
"image": {
"type": "string",
"description": "artifact image name.",
"x-intellij-html-description": "artifact 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."
Expand Down Expand Up @@ -1285,8 +1372,8 @@
],
"properties": {
"artifactOverrides": {
"description": "key value pairs. If present, Skaffold will send `--set-string` flag to Helm CLI and append all pairs after the flag.",
"x-intellij-html-description": "key value pairs. If present, Skaffold will send <code>--set-string</code> flag to Helm CLI and append all pairs after the flag."
"description": "key value pairs where the key represents the parameter used in the `--set-string` Helm CLI flag to define a container image and the value corresponds to artifact i.e. `ImageName` defined in `Build.Artifacts` section. The resulting command-line is controlled by `ImageStrategy`.",
"x-intellij-html-description": "key value pairs where the key represents the parameter used in the <code>--set-string</code> Helm CLI flag to define a container image and the value corresponds to artifact i.e. <code>ImageName</code> defined in <code>Build.Artifacts</code> section. The resulting command-line is controlled by <code>ImageStrategy</code>."
},
"chartPath": {
"type": "string",
Expand All @@ -1295,8 +1382,8 @@
},
"imageStrategy": {
"$ref": "#/definitions/HelmImageStrategy",
"description": "adds image configurations to the Helm `values` file.",
"x-intellij-html-description": "adds image configurations to the Helm <code>values</code> file."
"description": "controls how an `ArtifactOverrides` entry is turned into `--set-string` Helm CLI flag or flags.",
"x-intellij-html-description": "controls how an <code>ArtifactOverrides</code> entry is turned into <code>--set-string</code> Helm CLI flag or flags."
},
"name": {
"type": "string",
Expand Down
2 changes: 1 addition & 1 deletion pkg/skaffold/build/cluster/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func (b *Builder) Build(ctx context.Context, out io.Writer, tags tag.ImageTags,
}

builder := build.WithLogFile(b.buildArtifact, b.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) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/skaffold/build/gcb/cloud_build.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/skaffold/build/local/local.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func (b *Builder) Build(ctx context.Context, out io.Writer, tags tag.ImageTags,
defer b.localDocker.Close()

builder := build.WithLogFile(b.buildArtifact, b.muted)
return build.InParallel(ctx, out, tags, artifacts, builder, *b.cfg.Concurrency)
return build.InOrder(ctx, out, tags, artifacts, builder, *b.cfg.Concurrency)
}

func (b *Builder) buildArtifact(ctx context.Context, out io.Writer, a *latest.Artifact, tag string) (string, error) {
Expand Down
81 changes: 60 additions & 21 deletions pkg/skaffold/build/parallel.go → pkg/skaffold/build/scheduler.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,49 +35,46 @@ type ArtifactBuilder func(ctx context.Context, out io.Writer, artifact *latest.A

// For testing
var (
buffSize = bufferedLinesPerArtifact
runInSequence = InSequence
buffSize = bufferedLinesPerArtifact
)

// 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)
}

// InOrder builds a list of artifacts in dependency order.
// `concurrency` specifies the max number of builds that can run at any one time. If concurrency is 0, then it's set to the length of the `artifacts` slice.
// Each artifact build runs in its own goroutine. It limits the max concurrency using a buffered channel like a semaphore.
// At the same time, it uses the `artifactChanModel` to model the artifacts dependency graph and to make each artifact build wait for its required artifacts' builds.
func InOrder(ctx context.Context, out io.Writer, tags tag.ImageTags, artifacts []*latest.Artifact, buildArtifact ArtifactBuilder, concurrency int) ([]Artifact, error) {
gsquared94 marked this conversation as resolved.
Show resolved Hide resolved
acmSlice := makeArtifactChanModel(artifacts)
var wg sync.WaitGroup
defer wg.Wait()

ctx, cancel := context.WithCancel(ctx)
defer cancel()

results := new(sync.Map)
outputs := make([]chan string, len(artifacts))
outputs := make([]chan string, len(acmSlice))

if concurrency == 0 {
concurrency = len(artifacts)
concurrency = len(acmSlice)
}
sem := make(chan bool, concurrency)

// Run builds in //
wg.Add(len(artifacts))
for i := range artifacts {
for i := range acmSlice {
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) {
// Create a goroutine for each element in acmSlice. 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.
go func(a *artifactChanModel) {
a.waitForDependencies(ctx)
sem <- true
runBuild(ctx, w, tags, artifacts[i], results, buildArtifact)
// Run build and write output/logs to piped writer and store build result in sync.Map
runBuild(ctx, w, tags, a.artifact, results, buildArtifact)
a.markComplete()
<-sem

wg.Done()
}(i)
}(acmSlice[i])

// Read build output/logs and write to buffered channel
go readOutputAndWriteToChannel(r, outputs[i])
Expand All @@ -87,6 +84,48 @@ func InParallel(ctx context.Context, out io.Writer, tags tag.ImageTags, artifact
return collectResults(out, artifacts, results, outputs)
}

// artifactChanModel models the artifact dependency graph using a set of channels.
// Each artifact has a channel that it closes once it completes building (either success or failure) by calling `markComplete`. This notifies *all* listeners waiting for this artifact.
// Additionally it has a list of channels for each of its dependencies.
// Calling `waitForDependencies` ensures that all required artifacts' channels have already been closed and as such have finished building.
// This model allows for composing any arbitrary function with dependency ordering.
type artifactChanModel struct {
artifact *latest.Artifact
artifactChan chan interface{}
requiredArtifactChans []chan interface{}
}

func (a *artifactChanModel) markComplete() {
// closing channel notifies all listeners waiting for this build to complete
close(a.artifactChan)
}
func (a *artifactChanModel) waitForDependencies(ctx context.Context) {
for _, dep := range a.requiredArtifactChans {
// wait for dependency to complete build
select {
case <-ctx.Done():
case <-dep:
}
}
}

func makeArtifactChanModel(artifacts []*latest.Artifact) []*artifactChanModel {
chanMap := make(map[string]chan interface{})
for _, a := range artifacts {
chanMap[a.ImageName] = make(chan interface{})
}

var acmSlice []*artifactChanModel
for _, a := range artifacts {
acm := &artifactChanModel{artifact: a, artifactChan: chanMap[a.ImageName]}
for _, d := range a.Dependencies {
acm.requiredArtifactChans = append(acm.requiredArtifactChans, chanMap[d.ImageName])
}
acmSlice = append(acmSlice, acm)
}
return acmSlice
}

func runBuild(ctx context.Context, cw io.WriteCloser, tags tag.ImageTags, artifact *latest.Artifact, results *sync.Map, build ArtifactBuilder) {
event.BuildInProgress(artifact.ImageName)

Expand Down
Loading