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 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
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 %s", plan.stageTarget)
}
}

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.Info("Finished building target stage")
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;