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 multi stage builds #141

Merged
merged 12 commits into from
May 14, 2018
26 changes: 14 additions & 12 deletions cmd/executor/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,23 +23,21 @@ import (

"github.com/genuinetools/amicontained/container"

"github.com/GoogleContainerTools/kaniko/pkg/executor"

"github.com/GoogleContainerTools/kaniko/pkg/constants"
"github.com/GoogleContainerTools/kaniko/pkg/executor"
"github.com/GoogleContainerTools/kaniko/pkg/util"
"github.com/sirupsen/logrus"
"github.com/spf13/cobra"
)

var (
dockerfilePath string
destination string
srcContext string
snapshotMode string
bucket string
dockerInsecureSkipTLSVerify bool
logLevel string
force bool
dockerfilePath string
destination string
srcContext string
snapshotMode string
bucket string
logLevel string
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like you dropped the dockerinsecuretlsverify in a merge conflict.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yah, it didn't look like it was doing anything now that we switched to go-containerregistry

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(probably should have done that in a different commit)

force bool
)

func init() {
Expand All @@ -48,7 +46,6 @@ func init() {
RootCmd.PersistentFlags().StringVarP(&bucket, "bucket", "b", "", "Name of the GCS bucket from which to access build context as tarball.")
RootCmd.PersistentFlags().StringVarP(&destination, "destination", "d", "", "Registry the final image should be pushed to (ex: gcr.io/test/example:latest)")
RootCmd.PersistentFlags().StringVarP(&snapshotMode, "snapshotMode", "", "full", "Set this flag to change the file attributes inspected during snapshotting")
RootCmd.PersistentFlags().BoolVarP(&dockerInsecureSkipTLSVerify, "insecure-skip-tls-verify", "", false, "Push to insecure registry ignoring TLS verify")
RootCmd.PersistentFlags().StringVarP(&logLevel, "verbosity", "v", constants.DefaultLogLevel, "Log level (debug, info, warn, error, fatal, panic")
RootCmd.PersistentFlags().BoolVarP(&force, "force", "", false, "Force building outside of a container")
}
Expand Down Expand Up @@ -76,7 +73,12 @@ var RootCmd = &cobra.Command{
logrus.Error(err)
os.Exit(1)
}
if err := executor.DoBuild(dockerfilePath, srcContext, destination, snapshotMode, dockerInsecureSkipTLSVerify); err != nil {
ref, image, err := executor.DoBuild(dockerfilePath, srcContext, snapshotMode)
if err != nil {
logrus.Error(err)
os.Exit(1)
}
if err := executor.DoPush(ref, image, destination); err != nil {
logrus.Error(err)
os.Exit(1)
}
Expand Down
9 changes: 9 additions & 0 deletions integration_tests/dockerfiles/Dockerfile_test_multistage
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
FROM gcr.io/distroless/base:latest
COPY . .

FROM scratch as second
ENV foopath context/foo
COPY --from=0 $foopath context/b* /foo/

FROM gcr.io/distroless/base:latest
COPY --from=second /foo /foo2
4 changes: 0 additions & 4 deletions integration_tests/dockerfiles/Dockerfile_test_scratch

This file was deleted.

12 changes: 12 additions & 0 deletions integration_tests/dockerfiles/config_test_multistage.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
[
{
"Image1": "gcr.io/kaniko-test/docker-test-multistage:latest",
"Image2": "gcr.io/kaniko-test/kaniko-test-multistage:latest",
"DiffType": "File",
"Diff": {
"Adds": null,
"Dels": null,
"Mods": null
}
}
]
12 changes: 0 additions & 12 deletions integration_tests/dockerfiles/config_test_scratch.json

This file was deleted.

8 changes: 4 additions & 4 deletions integration_tests/integration_test_yaml.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,12 +140,12 @@ var fileTests = []struct {
repo: "test-onbuild",
},
{
description: "test scratch",
dockerfilePath: "/workspace/integration_tests/dockerfiles/Dockerfile_test_scratch",
configPath: "/workspace/integration_tests/dockerfiles/config_test_scratch.json",
description: "test multistage",
dockerfilePath: "/workspace/integration_tests/dockerfiles/Dockerfile_test_multistage",
configPath: "/workspace/integration_tests/dockerfiles/config_test_multistage.json",
dockerContext: buildcontextPath,
kanikoContext: buildcontextPath,
repo: "test-scratch",
repo: "test-multistage",
},
}

Expand Down
13 changes: 13 additions & 0 deletions pkg/commands/copy.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package commands

import (
"github.com/GoogleContainerTools/kaniko/pkg/constants"
"os"
"path/filepath"
"strings"
Expand All @@ -40,6 +41,10 @@ func (c *CopyCommand) ExecuteCommand(config *v1.Config) error {
logrus.Infof("cmd: copy %s", srcs)
logrus.Infof("dest: %s", dest)

// Resolve from
if c.cmd.From != "" {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if multiple steps have the same from line? Should we add any randomness here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, what do you mean by randomness?

Right now, if "from" is specified as a previous stage, we change the build context to be the path to the directory where files from that stage are saved. That directory should remain unchanged even if multiple copy commands refer back to it

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was worried about a case like this:

FROM base1
RUN something

FROM base1
RUN somethingelse

FROM base2
COPY --from=0

But I guess we can't have conflicts here, because the users have to refer to them by index (or alias)

c.buildcontext = filepath.Join(constants.BuildContextDir, c.cmd.From)
}
// First, resolve any environment replacement
resolvedEnvs, err := util.ResolveEnvironmentReplacementList(c.cmd.SourcesAndDest, config.Env, true)
if err != nil {
Expand All @@ -58,11 +63,19 @@ func (c *CopyCommand) ExecuteCommand(config *v1.Config) error {
if err != nil {
return err
}
cwd := config.WorkingDir
if cwd == "" {
cwd = constants.RootDir
}
destPath, err := util.DestinationFilepath(src, dest, config.WorkingDir)
if err != nil {
return err
}
if fi.IsDir() {
if !filepath.IsAbs(dest) {
// we need to add '/' to the end to indicate the destination is a directory
dest = filepath.Join(cwd, dest) + "/"
}
if err := util.CopyDir(fullPath, dest); err != nil {
return err
}
Expand Down
6 changes: 6 additions & 0 deletions pkg/commands/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,12 @@ type EnvCommand struct {
cmd *instructions.EnvCommand
}

func NewEnvCommand(cmd *instructions.EnvCommand) EnvCommand {
return EnvCommand{
cmd: cmd,
}
}

func (e *EnvCommand) ExecuteCommand(config *v1.Config) error {
logrus.Info("cmd: ENV")
newEnvs := e.cmd.Env
Expand Down
96 changes: 94 additions & 2 deletions pkg/dockerfile/dockerfile.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,20 @@ package dockerfile

import (
"bytes"
"strings"

"github.com/GoogleContainerTools/kaniko/pkg/commands"
"github.com/GoogleContainerTools/kaniko/pkg/constants"
"github.com/GoogleContainerTools/kaniko/pkg/util"
"github.com/docker/docker/builder/dockerfile/instructions"
"github.com/docker/docker/builder/dockerfile/parser"
"github.com/google/go-containerregistry/authn"
"github.com/google/go-containerregistry/name"
"github.com/google/go-containerregistry/v1"
"github.com/google/go-containerregistry/v1/empty"
"github.com/google/go-containerregistry/v1/remote"
"net/http"
"path/filepath"
"strconv"
"strings"
)

// Parse parses the contents of a Dockerfile and returns a list of commands
Expand All @@ -37,6 +47,25 @@ func Parse(b []byte) ([]instructions.Stage, error) {
return stages, err
}

// ResolveStages resolves any calls to previous stages with names to indices
// Ex. --from=second_stage should be --from=1 for easier processing later on
func ResolveStages(stages []instructions.Stage) {
nameToIndex := make(map[string]string)
for i, stage := range stages {
index := strconv.Itoa(i)
nameToIndex[stage.Name] = index
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it looks like the second one here overwrites the first one. Do we need both?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so if the stage has a name associated the second won't override the first, so I added both in just to make it easier to resolve '--from' from names to indexes later on

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah ok. Maybe it would be more clear to have two separate maps?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it would get confusing if someone tried to name a stage with an index...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I see, that makes sense -- I changed it a bit to be less confusing with one map

nameToIndex[index] = index
for _, cmd := range stage.Commands {
switch c := cmd.(type) {
case *instructions.CopyCommand:
if c.From != "" {
c.From = nameToIndex[c.From]
}
}
}
}
}

// ParseCommands parses an array of commands into an array of instructions.Command; used for onbuild
func ParseCommands(cmdArray []string) ([]instructions.Command, error) {
var cmds []instructions.Command
Expand All @@ -54,3 +83,66 @@ func ParseCommands(cmdArray []string) ([]instructions.Command, error) {
}
return cmds, nil
}

// Dependencies returns a list of files in this stage that will be needed in later stages
func Dependencies(index int, stages []instructions.Stage) ([]string, error) {
var dependencies []string
for stageIndex, stage := range stages {
if stageIndex <= index {
continue
}
var sourceImage v1.Image
if stage.BaseName == constants.NoBaseImage {
sourceImage = empty.Image
} else {
// Initialize source image
ref, err := name.ParseReference(stage.BaseName, name.WeakValidation)
if err != nil {
return nil, err

}
auth, err := authn.DefaultKeychain.Resolve(ref.Context().Registry)
if err != nil {
return nil, err
}
sourceImage, err = remote.Image(ref, auth, http.DefaultTransport)
if err != nil {
return nil, err
}
}
imageConfig, err := sourceImage.ConfigFile()
if err != nil {
return nil, err
}
for _, cmd := range stage.Commands {
switch c := cmd.(type) {
case *instructions.EnvCommand:
envCommand := commands.NewEnvCommand(c)
if err := envCommand.ExecuteCommand(&imageConfig.Config); err != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we executing ENV commands in here? It seems like this shouldn't have any side effects.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so we need to keep track of which ENVs were declared before a copy command for a case like this:

FROM base1
RUN something

FROM base2
ENV file file
COPY --from=0 $file ./

So here I'm executing any EnvCommands on a temporary config for that stage, and that updated config is used to evaluate dependencies in any subsequent COPY commands

return nil, err
}
case *instructions.CopyCommand:
if c.From != strconv.Itoa(index) {
continue
}
// First, resolve any environment replacement
resolvedEnvs, err := util.ResolveEnvironmentReplacementList(c.SourcesAndDest, imageConfig.Config.Env, true)
if err != nil {
return nil, err
}
// Resolve wildcards and get a list of resolved sources
srcs, err := util.ResolveSources(resolvedEnvs, constants.RootDir)
if err != nil {
return nil, err
}
for index, src := range srcs {
if !filepath.IsAbs(src) {
srcs[index] = filepath.Join(constants.RootDir, src)
}
}
dependencies = append(dependencies, srcs...)
}
}
}
return dependencies, nil
}
95 changes: 95 additions & 0 deletions pkg/dockerfile/dockerfile_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
/*
Copyright 2018 Google LLC

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 dockerfile

import (
"fmt"
"github.com/GoogleContainerTools/kaniko/testutil"
"github.com/docker/docker/builder/dockerfile/instructions"
"io/ioutil"
"os"
"path/filepath"
"strconv"
"testing"
)

func Test_ResolveStages(t *testing.T) {
dockerfile := `
FROM scratch
RUN echo hi > /hi

FROM scratch AS second
COPY --from=0 /hi /hi2

FROM scratch
COPY --from=second /hi2 /hi3
`
stages, err := Parse([]byte(dockerfile))
if err != nil {
t.Fatal(err)
}
ResolveStages(stages)
for index, stage := range stages {
if index == 0 {
continue
}
copyCmd := stage.Commands[0].(*instructions.CopyCommand)
expectedStage := strconv.Itoa(index - 1)
if copyCmd.From != expectedStage {
t.Fatalf("unexpected copy command: %s resolved to stage %s, expected %s", copyCmd.String(), copyCmd.From, expectedStage)
}
}
}

func Test_Dependencies(t *testing.T) {
testDir, err := ioutil.TempDir("", "")
if err != nil {
t.Fatal(err)
}
helloPath := filepath.Join(testDir, "hello")
if err := os.Mkdir(helloPath, 0755); err != nil {
t.Fatal(err)
}

dockerfile := fmt.Sprintf(`
FROM scratch
COPY %s %s

FROM scratch AS second
ENV hienv %s
COPY a b
COPY --from=0 /$hienv %s /hi2/
`, helloPath, helloPath, helloPath, testDir)

stages, err := Parse([]byte(dockerfile))
if err != nil {
t.Fatal(err)
}

expectedDependencies := [][]string{
{
helloPath,
testDir,
},
nil,
}

for index := range stages {
actualDeps, err := Dependencies(index, stages)
testutil.CheckErrorAndDeepEqual(t, false, err, expectedDependencies[index], actualDeps)
}
}
Loading