Skip to content
This repository has been archived by the owner on May 4, 2021. It is now read-only.

Add --target option to build command #331

Merged
merged 2 commits into from
May 3, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
4 changes: 3 additions & 1 deletion bin/makisu/cmd/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ type buildCmd struct {
registryConfig string
destination string

target string
buildArgs []string
allowModifyFS bool
commit string
Expand Down Expand Up @@ -103,6 +104,7 @@ func getBuildCmd() *buildCmd {
buildCmd.PersistentFlags().StringVar(&buildCmd.registryConfig, "registry-config", "", "Set build-time variables")
buildCmd.PersistentFlags().StringVar(&buildCmd.destination, "dest", "", "Destination of the image tar")

buildCmd.PersistentFlags().StringVar(&buildCmd.target, "target", "", "Set the target build stage to build.")
buildCmd.PersistentFlags().StringArrayVar(&buildCmd.buildArgs, "build-arg", nil, "Argument to the dockerfile as per the spec of ARG. Format is \"--build-arg <arg>=<value>\"")
buildCmd.PersistentFlags().BoolVar(&buildCmd.allowModifyFS, "modifyfs", false, "Allow makisu to modify files outside of its internal storage dir")
buildCmd.PersistentFlags().StringVar(&buildCmd.commit, "commit", "implicit", "Set to explicit to only commit at steps with '#!COMMIT' annotations; Set to implicit to commit at every ADD/COPY/RUN step")
Expand Down Expand Up @@ -207,7 +209,7 @@ func (cmd *buildCmd) newBuildPlan(

// Create BuildPlan and validate it.
return builder.NewBuildPlan(
buildContext, imageName, replicas, cacheMgr, dockerfile, cmd.allowModifyFS, forceCommit)
buildContext, imageName, replicas, cacheMgr, dockerfile, cmd.allowModifyFS, forceCommit, cmd.target)
}

// Build image from the specified dockerfile.
Expand Down
17 changes: 16 additions & 1 deletion lib/builder/build_plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ type BuildPlan struct {

// stages contains the build stages defined in dockerfile.
stages []*buildStage
// Which stage is the target for this plan
stageTarget string

// TODO: this is not used for now.
// Aliases of stages.
Expand All @@ -63,7 +65,7 @@ type BuildPlan struct {
// returns a new BuildPlan.
func NewBuildPlan(
ctx *context.BuildContext, target image.Name, replicas []image.Name, cacheMgr cache.Manager,
parsedStages []*dockerfile.Stage, allowModifyFS, forceCommit bool) (*BuildPlan, error) {
parsedStages []*dockerfile.Stage, allowModifyFS, forceCommit bool, stageTarget string) (*BuildPlan, error) {

plan := &BuildPlan{
baseCtx: ctx,
Expand All @@ -72,6 +74,7 @@ func NewBuildPlan(
replicas: replicas,
cacheMgr: cacheMgr,
stages: make([]*buildStage, 0),
stageTarget: stageTarget,
stageAliases: make(map[string]struct{}),
stageIndexAliases: make(map[string]*buildStage),
opts: &buildPlanOptions{
Expand Down Expand Up @@ -156,6 +159,13 @@ func (plan *BuildPlan) processStagesAndAliases(
// scratch before executing a stage.
seedCacheID = stage.nodes[len(stage.nodes)-1].CacheID()
}
plan.stageAliases = existingAliases

if plan.stageTarget != "" {
if _, ok := plan.stageAliases[plan.stageTarget]; !ok {
return fmt.Errorf("target stage not found in dockerfile")
Rowern marked this conversation as resolved.
Show resolved Hide resolved
}
}

return nil
}
Expand Down Expand Up @@ -188,6 +198,11 @@ func (plan *BuildPlan) Execute() (*image.DistributionManifest, error) {
for k, v := range orignalEnv {
os.Setenv(k, v)
}

if plan.stageTarget != "" && currStage.alias == plan.stageTarget {
log.Infof("Finished building target stage")
Rowern marked this conversation as resolved.
Show resolved Hide resolved
break
}
}

// Wait for cache layers to be pushed. This will make them available to
Expand Down
57 changes: 50 additions & 7 deletions lib/builder/build_plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func TestBuildPlanExecution(t *testing.T) {
}
stages := []*dockerfile.Stage{{from, directives}}

plan, err := NewBuildPlan(ctx, target, nil, cacheMgr, stages, true, false)
plan, err := NewBuildPlan(ctx, target, nil, cacheMgr, stages, true, false, "")
require.NoError(err)

manifest, err := plan.Execute()
Expand Down Expand Up @@ -93,7 +93,7 @@ func TestBuildPlanContextDirs(t *testing.T) {
// Here we need to set the allowModifyFS to true because we copy
// files across stages.
// TODO(pourchet): support copy --from without relying on FS.
plan, err := NewBuildPlan(ctx, target, nil, cacheMgr, stages, true, false)
plan, err := NewBuildPlan(ctx, target, nil, cacheMgr, stages, true, false, "")
require.NoError(err)
require.Contains(plan.copyFromDirs, "stage1")
require.Len(plan.copyFromDirs, 1)
Expand All @@ -108,7 +108,7 @@ func TestBuildPlanContextDirs(t *testing.T) {
}
stages = []*dockerfile.Stage{{from, directives}}

_, err = NewBuildPlan(ctx, target, nil, cacheMgr, stages, false, false)
_, err = NewBuildPlan(ctx, target, nil, cacheMgr, stages, false, false, "")
require.Error(err)

// Copy from subsequent stage.
Expand All @@ -119,7 +119,7 @@ func TestBuildPlanContextDirs(t *testing.T) {
from2 = dockerfile.FromDirectiveFixture("", envImage.String(), "stage2")
stages = []*dockerfile.Stage{{from1, directives1}, {from2, nil}}

_, err = NewBuildPlan(ctx, target, nil, cacheMgr, stages, false, false)
_, err = NewBuildPlan(ctx, target, nil, cacheMgr, stages, false, false, "")
require.Error(err)
}

Expand All @@ -142,7 +142,7 @@ func TestBuildPlanBadRun(t *testing.T) {
}
stages := []*dockerfile.Stage{{from, directives}}

plan, err := NewBuildPlan(ctx, target, nil, cacheMgr, stages, true, false)
plan, err := NewBuildPlan(ctx, target, nil, cacheMgr, stages, true, false, "")
require.NoError(err)

_, err = plan.Execute()
Expand All @@ -166,14 +166,57 @@ func TestDuplicateStageAlias(t *testing.T) {
from2 := dockerfile.FromDirectiveFixture("", envImage.String(), "alias")
stages := []*dockerfile.Stage{{from1, nil}, {from2, nil}}

_, err = NewBuildPlan(ctx, target, nil, cacheMgr, stages, false, false)
_, err = NewBuildPlan(ctx, target, nil, cacheMgr, stages, false, false, "")
require.Error(err)

// Same image different alias.
from1 = dockerfile.FromDirectiveFixture("", envImage.String(), "alias1")
from2 = dockerfile.FromDirectiveFixture("", envImage.String(), "alias2")
stages = []*dockerfile.Stage{{from1, nil}, {from2, nil}}

_, err = NewBuildPlan(ctx, target, nil, cacheMgr, stages, false, false)
_, err = NewBuildPlan(ctx, target, nil, cacheMgr, stages, false, false, "")
require.NoError(err)
}

func TestTargetStageMissing(t *testing.T) {
require := require.New(t)

ctx, cleanup := context.BuildContextFixture()
defer cleanup()

target := image.NewImageName("", "testrepo", "testtag")
envImage, err := image.ParseName("scratch")
require.NoError(err)

cacheMgr := cache.New(ctx.ImageStore, nil, registry.NoopClientFixture())

// Same image same alias.
from1 := dockerfile.FromDirectiveFixture("", envImage.String(), "alias1")
from2 := dockerfile.FromDirectiveFixture("", envImage.String(), "alias2")
stages := []*dockerfile.Stage{{from1, nil}, {from2, nil}}

_, err = NewBuildPlan(ctx, target, nil, cacheMgr, stages, false, false, "alias3")
require.Error(err)
}

func TestTargetStageOk(t *testing.T) {
require := require.New(t)

ctx, cleanup := context.BuildContextFixture()
defer cleanup()

target := image.NewImageName("", "testrepo", "testtag")
envImage, err := image.ParseName("scratch")
require.NoError(err)

cacheMgr := cache.New(ctx.ImageStore, nil, registry.NoopClientFixture())

// Same image same alias.
from1 := dockerfile.FromDirectiveFixture("", envImage.String(), "alias1")
from2 := dockerfile.FromDirectiveFixture("", envImage.String(), "alias2")
from3 := dockerfile.FromDirectiveFixture("", envImage.String(), "alias3")
stages := []*dockerfile.Stage{{from1, nil}, {from2, nil}, {from3, nil}}

_, err = NewBuildPlan(ctx, target, nil, cacheMgr, stages, false, false, "alias2")
require.NoError(err)
}
10 changes: 10 additions & 0 deletions test/python/test_build.py
Original file line number Diff line number Diff line change
Expand Up @@ -300,3 +300,13 @@ def test_build_global_arg(registry1, storage_dir):
registry=registry1.addr, docker_args=docker_build_args)
code, err = docker_run_image(registry1.addr, new_image)
assert code == 0, err

def test_build_target(registry1, storage_dir):
new_image = new_image_name()
context_dir = os.path.join(
os.getcwd(), 'testdata/build-context/target')
makisu_build_image(
new_image, context_dir, storage_dir,
registry=registry1.addr, target="second")
code, err = docker_run_image(registry1.addr, new_image)
assert code == 0, err
5 changes: 4 additions & 1 deletion test/python/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ def makisu_run_cmd(volumes, args):
def makisu_build_image(
new_image_tag, context_dir, storage_dir, cache_dir=None, volumes=None,
docker_args=None, load=False, registry=None, replicas=None,
registry_cfg=None):
registry_cfg=None, target=None):

volumes = volumes or {}
volumes[storage_dir] = storage_dir # Sandbox and file store
Expand Down Expand Up @@ -144,6 +144,9 @@ def makisu_build_image(
if not cache_dir:
args.extend(['--local-cache-ttl', '0s'])

if target:
args.extend(['--target', target])

args.append('/context')

exit_code = makisu_run_cmd(volumes, args)
Expand Down
11 changes: 11 additions & 0 deletions testdata/build-context/target/Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
FROM alpine

CMD echo "This is not the correct one, with target"; exit 1;

FROM alpine:latest as second

CMD echo "This is the correct one, with target"; exit 0;

FROM alpine:latest as third

CMD echo "This is not the correct one, with target"; exit 1;