From 923ec55b36f0d3ec0065dc525e6a579b7b048663 Mon Sep 17 00:00:00 2001 From: zvier Date: Wed, 18 Nov 2020 14:56:08 +0800 Subject: [PATCH] isula-build: support build Dockerfile only have FROM command Signed-off-by: liuzekun --- Makefile | 2 +- builder/dockerfile/builder_test.go | 16 ++++----- builder/dockerfile/cmd_builder.go | 1 + builder/dockerfile/cmd_builder_test.go | 41 ++++++++++++------------ builder/dockerfile/parser/parser.go | 8 ----- builder/dockerfile/parser/parser_test.go | 2 -- builder/dockerfile/stage_builder.go | 2 -- 7 files changed, 31 insertions(+), 41 deletions(-) diff --git a/Makefile b/Makefile index 11166cc2..7e268678 100644 --- a/Makefile +++ b/Makefile @@ -97,7 +97,7 @@ proto: install: install -D -m0551 bin/isula-build $(BINDIR) install -D -m0550 bin/isula-builder $(BINDIR) - @[ ! -d ${CONFIG_DIR}/${CONFIG_FILE} ] && install -dm0640 ${CONFIG_DIR} + @[ ! -d ${CONFIG_DIR}/${CONFIG_FILE} ] && install -dm0650 ${CONFIG_DIR} @( [ -f ${CONFIG_DIR}/${CONFIG_FILE} ] && printf "%-20s %s\n" "${CONFIG_FILE}" "already exist in ${CONFIG_DIR}, please replace it manually." ) || install -D -m0600 ${LOCAL_CONF_PREFIX}/${CONFIG_FILE} ${CONFIG_DIR}/${CONFIG_FILE} @( [ -f ${CONFIG_DIR}/${POLICY_FILE} ] && printf "%-20s %s\n" "${POLICY_FILE}" "already exist in ${CONFIG_DIR}, please replace it manually." ) || install -D -m0600 ${LOCAL_CONF_PREFIX}/${POLICY_FILE} ${CONFIG_DIR}/${POLICY_FILE} @( [ -f ${CONFIG_DIR}/${REGIST_FILE} ] && printf "%-20s %s\n" "${REGIST_FILE}" "already exist in ${CONFIG_DIR}, please replace it manually." ) || install -D -m0600 ${LOCAL_CONF_PREFIX}/${REGIST_FILE} ${CONFIG_DIR}/${REGIST_FILE} diff --git a/builder/dockerfile/builder_test.go b/builder/dockerfile/builder_test.go index 94842c71..b02768ea 100644 --- a/builder/dockerfile/builder_test.go +++ b/builder/dockerfile/builder_test.go @@ -248,13 +248,13 @@ RUN ls } // check the arg and env taken by the command: RUN ls - assert.DeepEqual(t, b.stageBuilders[0].commands[0].args, + assert.DeepEqual(t, b.stageBuilders[0].commands[1].args, map[string]string{"no_proxy": "10.0.0.0"}) - assert.DeepEqual(t, b.stageBuilders[1].commands[1].args, + assert.DeepEqual(t, b.stageBuilders[1].commands[2].args, map[string]string{"testArg": "0.1", "no_proxy": "10.0.0.0"}) - assert.DeepEqual(t, b.stageBuilders[2].commands[2].args, + assert.DeepEqual(t, b.stageBuilders[2].commands[3].args, map[string]string{"no_proxy": "10.0.0.0"}) - assert.DeepEqual(t, b.stageBuilders[2].commands[2].envs, + assert.DeepEqual(t, b.stageBuilders[2].commands[3].envs, map[string]string{"testArg": "1.0"}) } @@ -291,13 +291,13 @@ RUN ls } // check the arg and env taken by the command: RUN ls - assert.DeepEqual(t, b.stageBuilders[0].commands[0].args, + assert.DeepEqual(t, b.stageBuilders[0].commands[1].args, map[string]string{"HTTPS_PROXY": "127.0.0.1"}) - assert.DeepEqual(t, b.stageBuilders[1].commands[1].args, + assert.DeepEqual(t, b.stageBuilders[1].commands[2].args, map[string]string{"testArg": "0.1", "HTTPS_PROXY": "127.0.0.1"}) - assert.DeepEqual(t, b.stageBuilders[2].commands[2].args, + assert.DeepEqual(t, b.stageBuilders[2].commands[3].args, map[string]string{"HTTPS_PROXY": "127.0.0.1"}) - assert.DeepEqual(t, b.stageBuilders[2].commands[2].envs, + assert.DeepEqual(t, b.stageBuilders[2].commands[3].envs, map[string]string{"testArg": "1.0"}) } diff --git a/builder/dockerfile/cmd_builder.go b/builder/dockerfile/cmd_builder.go index 65ae364d..8b0d5ab9 100644 --- a/builder/dockerfile/cmd_builder.go +++ b/builder/dockerfile/cmd_builder.go @@ -39,6 +39,7 @@ var ( func init() { cmdExecutors = map[string]func(cb *cmdBuilder) error{ + dockerfile.From: executeNoop, dockerfile.Add: executeAdd, dockerfile.Arg: executeNoop, dockerfile.Copy: executeCopy, diff --git a/builder/dockerfile/cmd_builder_test.go b/builder/dockerfile/cmd_builder_test.go index 471314ba..df1c08f9 100644 --- a/builder/dockerfile/cmd_builder_test.go +++ b/builder/dockerfile/cmd_builder_test.go @@ -113,7 +113,7 @@ func TestExecuteHealthCheck(t *testing.T) { } err := s.analyzeStage(context.Background()) assert.NilError(t, err) - if err := s.commands[0].cmdExecutor(); (err != nil) != tt.wantErr { + if err := s.commands[1].cmdExecutor(); (err != nil) != tt.wantErr { t.Errorf("CmdExecutor() error: %v, wantErr: %v", err, tt.wantErr) } tt.funcCheck(t, s) @@ -195,7 +195,7 @@ CMD [""]`, } err := s.analyzeStage(context.Background()) assert.NilError(t, err) - if err := s.commands[0].cmdExecutor(); (err != nil) != tt.wantErr { + if err := s.commands[1].cmdExecutor(); (err != nil) != tt.wantErr { t.Errorf("cmdExecutor() error: %v, wantErr: %v", err, tt.wantErr) } tt.funcCheck(t, s) @@ -288,7 +288,7 @@ SHELL ["/bin/bash", "-c"]`, } err := s.analyzeStage(context.Background()) assert.NilError(t, err) - if err := s.commands[0].cmdExecutor(); (err != nil) != tt.wantErr { + if err := s.commands[1].cmdExecutor(); (err != nil) != tt.wantErr { t.Errorf("SHELL cmdExecutor() error: %v, wantErr: %v", err, tt.wantErr) } tt.funcCheck(t, s) @@ -316,15 +316,15 @@ CMD ls` err := s.analyzeStage(context.Background()) assert.NilError(t, err) - if err := s.commands[0].cmdExecutor(); err != nil { + if err := s.commands[1].cmdExecutor(); err != nil { t.Errorf("CMD cmdExecutor() error: %v", err) } assert.DeepEqual(t, s.docker.Config.Cmd, strslice.StrSlice{"/bin/sh", "-c", "ls"}) - if err := s.commands[1].cmdExecutor(); err != nil { + if err := s.commands[2].cmdExecutor(); err != nil { t.Errorf("SHELL cmdExecutor() error: %v", err) } - if err := s.commands[2].cmdExecutor(); err != nil { + if err := s.commands[3].cmdExecutor(); err != nil { t.Errorf("CMD cmdExecutor() error: %v", err) } assert.DeepEqual(t, s.shellForm, strslice.StrSlice{"/bin/bash", "-c"}) @@ -360,9 +360,10 @@ func TestExecuteNoop(t *testing.T) { // the "STEP 1: FROM alpine" in production is done at stageBuilder.prepare() // no cmdExecutor for FROM, so no print for FROM here - expectedString := `STEP 1: ARG testArg -STEP 2: ENV env1=env2 -STEP 3: ONBUILD CMD ls + expectedString := `STEP 1: FROM alpine +STEP 2: ARG testArg +STEP 3: ENV env1=env2 +STEP 4: ONBUILD CMD ls ` assert.Equal(t, stepPrints, expectedString) } @@ -441,7 +442,7 @@ ENTRYPOINT [""]`, } err := s.analyzeStage(context.Background()) assert.NilError(t, err) - if err := s.commands[0].cmdExecutor(); (err != nil) != tt.wantErr { + if err := s.commands[1].cmdExecutor(); (err != nil) != tt.wantErr { t.Errorf("cmdExecutor() error: %v, wantErr: %v", err, tt.wantErr) } tt.funcCheck(t, s) @@ -903,15 +904,15 @@ WORKDIR /c` } err := s.analyzeStage(context.Background()) assert.NilError(t, err) - if err := s.commands[0].cmdExecutor(); err != nil { + if err := s.commands[1].cmdExecutor(); err != nil { t.Errorf("WORKDIR cmdExecutor() error: %v", err) } assert.DeepEqual(t, s.docker.Config.WorkingDir, "/a") - if err := s.commands[1].cmdExecutor(); err != nil { + if err := s.commands[2].cmdExecutor(); err != nil { t.Errorf("WORKDIR cmdExecutor() error: %v", err) } assert.DeepEqual(t, s.docker.Config.WorkingDir, "/b") - if err := s.commands[2].cmdExecutor(); err != nil { + if err := s.commands[3].cmdExecutor(); err != nil { t.Errorf("WORKDIR cmdExecutor() error: %v", err) } assert.DeepEqual(t, s.docker.Config.WorkingDir, "/c") @@ -938,15 +939,15 @@ WORKDIR c` } err := s.analyzeStage(context.Background()) assert.NilError(t, err) - if err := s.commands[0].cmdExecutor(); err != nil { + if err := s.commands[1].cmdExecutor(); err != nil { t.Errorf("WORKDIR cmdExecutor() error: %v", err) } assert.DeepEqual(t, s.docker.Config.WorkingDir, "/a") - if err := s.commands[1].cmdExecutor(); err != nil { + if err := s.commands[2].cmdExecutor(); err != nil { t.Errorf("WORKDIR cmdExecutor() error: %v", err) } assert.DeepEqual(t, s.docker.Config.WorkingDir, "/a/b") - if err := s.commands[2].cmdExecutor(); err != nil { + if err := s.commands[3].cmdExecutor(); err != nil { t.Errorf("WORKDIR cmdExecutor() error: %v", err) } assert.DeepEqual(t, s.docker.Config.WorkingDir, "/a/b/c") @@ -973,15 +974,15 @@ WORKDIR $DIRPATH/$DIRNAME` } err := s.analyzeStage(context.Background()) assert.NilError(t, err) - if err = s.commands[0].cmdExecutor(); err != nil { + if err = s.commands[1].cmdExecutor(); err != nil { t.Errorf("WORKDIR cmdExecutor() error: %v", err) } assert.NilError(t, err) - if err = s.commands[1].cmdExecutor(); err != nil { + if err = s.commands[2].cmdExecutor(); err != nil { t.Errorf("WORKDIR cmdExecutor() error: %v", err) } assert.NilError(t, err) - if err = s.commands[2].cmdExecutor(); err != nil { + if err = s.commands[3].cmdExecutor(); err != nil { t.Errorf("WORKDIR cmdExecutor() error: %v", err) } assert.NilError(t, err) @@ -1025,7 +1026,7 @@ Maintainer iSula iSula@huawei.com`, } err := s.analyzeStage(context.Background()) assert.NilError(t, err) - if err := s.commands[0].cmdExecutor(); (err != nil) != tt.wantErr { + if err := s.commands[1].cmdExecutor(); (err != nil) != tt.wantErr { t.Errorf("cmdExecutor() error: %v, wantErr: %v", err, tt.wantErr) } tt.funcCheck(t, s) diff --git a/builder/dockerfile/parser/parser.go b/builder/dockerfile/parser/parser.go index e8711fe7..82835e67 100644 --- a/builder/dockerfile/parser/parser.go +++ b/builder/dockerfile/parser/parser.go @@ -196,8 +196,6 @@ func constructPages(lines []*parser.Line, onbuild bool) ([]*parser.Page, error) currentPage *parser.Page pageNum int ) - // a stage should have at least one FROM and one command - const minLinesPerPage = 2 for _, line := range lines { if line == nil { @@ -215,9 +213,6 @@ func constructPages(lines []*parser.Line, onbuild bool) ([]*parser.Page, error) return nil, errors.New("onbuild does not support the from command") } if currentPage != nil { - if !onbuild && len(currentPage.Lines) < minLinesPerPage { - return nil, errors.Errorf("stage %s should have at least one command", currentPage.Name) - } pages = append(pages, currentPage) } @@ -254,9 +249,6 @@ func constructPages(lines []*parser.Line, onbuild bool) ([]*parser.Page, error) currentPage.End = line.End currentPage.AddLine(line) } - if !onbuild && len(currentPage.Lines) < minLinesPerPage { - return nil, errors.Errorf("stage %s should have at least one command", currentPage.Name) - } // the last stage always need to commit currentPage.NeedCommit = true pages = append(pages, currentPage) diff --git a/builder/dockerfile/parser/parser_test.go b/builder/dockerfile/parser/parser_test.go index 34b1a61c..fe27dd95 100644 --- a/builder/dockerfile/parser/parser_test.go +++ b/builder/dockerfile/parser/parser_test.go @@ -180,8 +180,6 @@ func TestParse(t *testing.T) { }, { name: "busybox_no_command", - isErr: true, - errStr: "stage 1 should have at least one command", }, { name: "env_before_from", diff --git a/builder/dockerfile/stage_builder.go b/builder/dockerfile/stage_builder.go index 7c928e34..47f55bf1 100644 --- a/builder/dockerfile/stage_builder.go +++ b/builder/dockerfile/stage_builder.go @@ -172,9 +172,7 @@ func (s *stageBuilder) analyzeStage(ctx context.Context) error { cb := newCmdBuilder(ctx, line, s, stageArgs, stageEnvs) switch line.Command { - // From cmd is already pre-processed, we just pass it case dockerfile.From: - continue case dockerfile.Arg: if cb.args, err = analyzeArg(s.builder, line, stageArgs, stageEnvs); err != nil { return err -- 2.19.1