From 4762724a94e66d3ce685fd08172ec028601c5700 Mon Sep 17 00:00:00 2001 From: DCCooper <1866858@gmail.com> Date: Sat, 19 Sep 2020 15:28:16 +0800 Subject: [PATCH] isula-build:sync patch from upstream changes include: - parse all stages and mark it wheather need to commit - fix build hang problem when error happened before pipe open - change default healthcheck timeout to 20s - add -t shortname for --tag and remove it from --timeout Signed-off-by: DCCooper <1866858@gmail.com> --- VERSION-openeuler | 2 +- git-commit | 2 +- isula-build.spec | 5 +- ...-and-mark-it-wheather-need-to-commit.patch | 118 +++++++++++++ ...-hang-problem-when-error-happened-be.patch | 118 +++++++++++++ ...ge-default-healthcheck-timeout-to-20.patch | 158 ++++++++++++++++++ ...t-shortname-for-tag-and-remove-it-fr.patch | 41 +++++ series.conf | 4 + 8 files changed, 445 insertions(+), 3 deletions(-) create mode 100644 patch/0038-parse-all-stages-and-mark-it-wheather-need-to-commit.patch create mode 100644 patch/0039-bugfix-fix-build-hang-problem-when-error-happened-be.patch create mode 100644 patch/0040-isula-build-change-default-healthcheck-timeout-to-20.patch create mode 100644 patch/0041-isula-build-add-t-shortname-for-tag-and-remove-it-fr.patch diff --git a/VERSION-openeuler b/VERSION-openeuler index e50d6d7..40fc689 100644 --- a/VERSION-openeuler +++ b/VERSION-openeuler @@ -1 +1 @@ -0.9.3-1 +0.9.3-2 diff --git a/git-commit b/git-commit index 8122565..8ade3c1 100644 --- a/git-commit +++ b/git-commit @@ -1 +1 @@ -fd832e9c4d84b42249d267ce922c3444f20c260b +0f7339c3932370ff275a870b0959f25e870750fb diff --git a/isula-build.spec b/isula-build.spec index a182eb7..1ece90c 100644 --- a/isula-build.spec +++ b/isula-build.spec @@ -2,7 +2,7 @@ Name: isula-build Version: 0.9.3 -Release: 1 +Release: 2 Summary: A tool to build container images License: Mulan PSL V2 URL: https://gitee.com/openeuler/isula-build @@ -76,6 +76,9 @@ rm -rf %{buildroot} /usr/share/bash-completion/completions/isula-build %changelog +* Thu Sep 10 2020 lixiang - 0.9.3-2 +- Sync patch from upstream + * Thu Sep 10 2020 lixiang - 0.9.3-1 - Bump version to 0.9.3 diff --git a/patch/0038-parse-all-stages-and-mark-it-wheather-need-to-commit.patch b/patch/0038-parse-all-stages-and-mark-it-wheather-need-to-commit.patch new file mode 100644 index 0000000..7485bae --- /dev/null +++ b/patch/0038-parse-all-stages-and-mark-it-wheather-need-to-commit.patch @@ -0,0 +1,118 @@ +From f585c435806bbb927553b5b956274d4c80469696 Mon Sep 17 00:00:00 2001 +From: Jeff Zvier +Date: Wed, 9 Sep 2020 14:30:10 +0800 +Subject: [PATCH] parse all stages and mark it wheather need to commit + +Signed-off-by: Jeff Zvier +--- + builder/dockerfile/builder_test.go | 7 ++++--- + builder/dockerfile/parser/parser.go | 9 +++++++++ + builder/dockerfile/stage_builder.go | 9 ++++----- + pkg/parser/page.go | 9 +++++---- + 4 files changed, 22 insertions(+), 12 deletions(-) + +diff --git a/builder/dockerfile/builder_test.go b/builder/dockerfile/builder_test.go +index cc1904d9..61172748 100644 +--- a/builder/dockerfile/builder_test.go ++++ b/builder/dockerfile/builder_test.go +@@ -34,8 +34,8 @@ import ( + "isula.org/isula-build/pkg/logger" + "isula.org/isula-build/pkg/parser" + "isula.org/isula-build/store" +- testutil "isula.org/isula-build/util" + "isula.org/isula-build/util" ++ testutil "isula.org/isula-build/util" + ) + + func TestParseFiles(t *testing.T) { +@@ -168,8 +168,9 @@ temp?` + Flags: map[string]string{"from": "date"}, + }, + }, +- Begin: 9, +- End: 11, ++ Begin: 9, ++ NeedCommit: true, ++ End: 11, + }, + }, + Warnings: nil, +diff --git a/builder/dockerfile/parser/parser.go b/builder/dockerfile/parser/parser.go +index 3041f9ba..a5b079d8 100644 +--- a/builder/dockerfile/parser/parser.go ++++ b/builder/dockerfile/parser/parser.go +@@ -188,6 +188,7 @@ func constructPages(lines []*parser.Line, onbuild bool) ([]*parser.Page, error) + } + + var ( ++ pageMap = make(map[string]*parser.Page) + pages = make([]*parser.Page, 0, 0) + currentPage *parser.Page + pageNum int +@@ -237,6 +238,12 @@ func constructPages(lines []*parser.Line, onbuild bool) ([]*parser.Page, error) + } else { + page.Name = strconv.Itoa(len(pages)) + } ++ pageMap[page.Name] = page ++ // if the base image for current stage is from the previous stage, ++ // mark the previous stage need to commit ++ if from, ok := pageMap[line.Cells[0].Value]; ok { ++ from.NeedCommit = true ++ } + currentPage = page + } + // because a valid dockerfile is always start with 'FROM' command here, so no need +@@ -247,6 +254,8 @@ func constructPages(lines []*parser.Line, onbuild bool) ([]*parser.Page, error) + 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) + + if len(pages) == 0 { +diff --git a/builder/dockerfile/stage_builder.go b/builder/dockerfile/stage_builder.go +index ad77ecea..32397bd7 100644 +--- a/builder/dockerfile/stage_builder.go ++++ b/builder/dockerfile/stage_builder.go +@@ -209,13 +209,12 @@ func (s *stageBuilder) stageBuild(ctx context.Context) (string, error) { + } + } + +- // 3. commit for new image +- s.imageID, err = s.commit(ctx) +- if err != nil { +- return "", errors.Wrapf(err, "commit image for stage %s failed", s.name) ++ // 3. commit for new image if needed ++ if s.rawStage.NeedCommit { ++ s.imageID, err = s.commit(ctx) + } + +- return s.imageID, nil ++ return s.imageID, errors.Wrapf(err, "commit image for stage %s failed", s.name) + } + + func prepareImage(opt *image.PrepareImageOptions) (*image.Describe, error) { +diff --git a/pkg/parser/page.go b/pkg/parser/page.go +index b7f746cd..320c9daf 100644 +--- a/pkg/parser/page.go ++++ b/pkg/parser/page.go +@@ -21,10 +21,11 @@ import ( + + // Page is a Dockerfile stage + type Page struct { +- Lines []*Line // all lines which the page contains +- Name string // page name +- Begin int // the begin line number of the page in the physical Dockerfile +- End int // the end line number of the page in the physical Dockerfile ++ Lines []*Line // all lines which the page contains ++ Name string // page name ++ Begin int // the begin line number of the page in the physical Dockerfile ++ End int // the end line number of the page in the physical Dockerfile ++ NeedCommit bool // mark the page whether need to commit + } + + func (p *Page) dump() string { +-- +2.19.1 + diff --git a/patch/0039-bugfix-fix-build-hang-problem-when-error-happened-be.patch b/patch/0039-bugfix-fix-build-hang-problem-when-error-happened-be.patch new file mode 100644 index 0000000..1d45e43 --- /dev/null +++ b/patch/0039-bugfix-fix-build-hang-problem-when-error-happened-be.patch @@ -0,0 +1,118 @@ +From a275947784ed0772b08bbd43114c5cc0a1ba8feb Mon Sep 17 00:00:00 2001 +From: DCCooper <1866858@gmail.com> +Date: Tue, 15 Sep 2020 19:42:14 +0800 +Subject: [PATCH] bugfix: fix build hang problem when error happened before + pipe open + +Signed-off-by: DCCooper <1866858@gmail.com> +--- + builder/builder.go | 2 +- + builder/dockerfile/builder.go | 3 ++- + cmd/cli/build.go | 4 +++- + daemon/build.go | 8 +++++++- + daemon/status.go | 6 ++++++ + 5 files changed, 19 insertions(+), 4 deletions(-) + +diff --git a/builder/builder.go b/builder/builder.go +index 02c40ea7..1f650a5c 100644 +--- a/builder/builder.go ++++ b/builder/builder.go +@@ -29,7 +29,7 @@ import ( + + // Builder is an interface for building an image + type Builder interface { +- Build() (imageID string, err error) ++ Build(chan struct{}) (imageID string, err error) + StatusChan() <-chan string + CleanResources() error + OutputPipeWrapper() *exporter.PipeWrapper +diff --git a/builder/dockerfile/builder.go b/builder/dockerfile/builder.go +index 27c688f9..fd0bd695 100644 +--- a/builder/dockerfile/builder.go ++++ b/builder/dockerfile/builder.go +@@ -407,7 +407,7 @@ func getFlagsAndArgs(line *parser.Line, allowFlags map[string]bool) (map[string] + } + + // Build makes the image +-func (b *Builder) Build() (string, error) { ++func (b *Builder) Build(syncChan chan struct{}) (string, error) { + var ( + executeTimer = b.cliLog.StartTimer("\nTotal") + err error +@@ -446,6 +446,7 @@ func (b *Builder) Build() (string, error) { + } + } + ++ close(syncChan) + // 4. export images + if err = b.export(imageID); err != nil { + return "", errors.Wrapf(err, "exporting images failed") +diff --git a/cmd/cli/build.go b/cmd/cli/build.go +index 08c591b5..5c94bedd 100644 +--- a/cmd/cli/build.go ++++ b/cmd/cli/build.go +@@ -150,7 +150,9 @@ func buildCommand(c *cobra.Command, args []string) error { + logrus.Debugf("Status get failed: %v", err2) + cancel() + } +- return errors.Wrap(err2, "error runStatus") ++ // user should not pay attention on runStatus error ++ // the errors were already printed to daemon log if any ++ return nil + }) + + return eg.Wait() +diff --git a/daemon/build.go b/daemon/build.go +index b66a9058..6c5e0acb 100644 +--- a/daemon/build.go ++++ b/daemon/build.go +@@ -59,11 +59,12 @@ func (b *Backend) Build(req *pb.BuildRequest, stream pb.Control_BuildServer) err + + pipeWrapper := builder.OutputPipeWrapper() + eg, ctx := errgroup.WithContext(ctx) ++ syncPipeChan := make(chan struct{}) + eg.Go(func() error { + b.syncBuildStatus(req.BuildID) <- struct{}{} + b.closeStatusChan(req.BuildID) + var berr error +- imageID, berr = builder.Build() ++ imageID, berr = builder.Build(syncPipeChan) + + if berr != nil && pipeWrapper != nil { + // in case there is error during Build stage, the backend will always waiting for content write into +@@ -85,6 +86,11 @@ func (b *Backend) Build(req *pb.BuildRequest, stream pb.Control_BuildServer) err + if pipeWrapper == nil { + return nil + } ++ select { ++ case <-syncPipeChan: ++ case <-ctx.Done(): ++ return nil ++ } + f, perr := exporter.PipeArchiveStream(pipeWrapper) + if perr != nil { + return perr +diff --git a/daemon/status.go b/daemon/status.go +index c08a6e77..d8b666ee 100644 +--- a/daemon/status.go ++++ b/daemon/status.go +@@ -41,10 +41,16 @@ func (b *Backend) Status(req *pb.StatusRequest, stream pb.Control_StatusServer) + + builder, err := b.daemon.Builder(req.BuildID) + if err != nil { ++ logrus.WithFields(logrus.Fields{ ++ "BuildID": req.GetBuildID(), ++ }).Error(err) + return err + } + for value := range builder.StatusChan() { + if err := stream.Send(&pb.StatusResponse{Content: value}); err != nil { ++ logrus.WithFields(logrus.Fields{ ++ "BuildID": req.GetBuildID(), ++ }).Error(err) + return err + } + } +-- +2.19.1 + diff --git a/patch/0040-isula-build-change-default-healthcheck-timeout-to-20.patch b/patch/0040-isula-build-change-default-healthcheck-timeout-to-20.patch new file mode 100644 index 0000000..a2f3224 --- /dev/null +++ b/patch/0040-isula-build-change-default-healthcheck-timeout-to-20.patch @@ -0,0 +1,158 @@ +From 855fb35fe16a2a5177e6978da0c896ea96621078 Mon Sep 17 00:00:00 2001 +From: Lu Jingxiao +Date: Thu, 17 Sep 2020 08:51:39 +0800 +Subject: [PATCH] isula-build: change default healthcheck timeout to 20s + +Currently we have a healthcheck before GRPC requesting +from Client to Server, and the default timeout is 500ms, +which could be very sensitive to running environment. +After discussion, 20s as a default timeout value should be much +more reasonable. + +Fixes: #I1VK61 + +Signed-off-by: Lu Jingxiao +--- + cmd/cli/grpc_client.go | 6 +++--- + cmd/cli/grpc_client_test.go | 4 ++-- + cmd/cli/main.go | 2 +- + tests/lib/common.sh | 18 +++++++++--------- + 4 files changed, 15 insertions(+), 15 deletions(-) + +diff --git a/cmd/cli/grpc_client.go b/cmd/cli/grpc_client.go +index 581e7f89..5f10fb01 100644 +--- a/cmd/cli/grpc_client.go ++++ b/cmd/cli/grpc_client.go +@@ -30,9 +30,9 @@ import ( + ) + + const ( +- defaultStartTimeout = 500 * time.Millisecond +- minStartTimeout = 30 * time.Millisecond +- maxStartTimeout = 20 * time.Second ++ defaultStartTimeout = 20 * time.Second ++ minStartTimeout = 100 * time.Millisecond ++ maxStartTimeout = 120 * time.Second + defaultGrpcMaxDelay = 3 * time.Second + ) + +diff --git a/cmd/cli/grpc_client_test.go b/cmd/cli/grpc_client_test.go +index e1db130c..e1e14935 100644 +--- a/cmd/cli/grpc_client_test.go ++++ b/cmd/cli/grpc_client_test.go +@@ -42,13 +42,13 @@ func TestGetStartTimeout(t *testing.T) { + }, + { + name: "TC3 - abnormal case with larger than max start timeout", +- args: args{timeout: "21s"}, ++ args: args{timeout: "121s"}, + want: -1, + wantErr: true, + }, + { + name: "TC4 - abnormal case with less than min start timeout", +- args: args{timeout: "19ms"}, ++ args: args{timeout: "99ms"}, + want: -1, + wantErr: true, + }, +diff --git a/cmd/cli/main.go b/cmd/cli/main.go +index 53e7af99..18bbb965 100644 +--- a/cmd/cli/main.go ++++ b/cmd/cli/main.go +@@ -95,7 +95,7 @@ func setupRootCmd(rootCmd *cobra.Command) { + rootCmd.SetFlagErrorFunc(util.FlagErrorFunc) + rootCmd.PersistentFlags().StringVar(&cliOpts.LogLevel, "log-level", "error", "Log level to be used. Either \"debug\", \"info\", \"warn\" or \"error\"") + rootCmd.PersistentFlags().BoolVarP(&cliOpts.Debug, "debug", "D", false, "Open debug mode") +- rootCmd.PersistentFlags().StringVarP(&cliOpts.Timeout, "timeout", "t", "500ms", "Timeout for connecting to daemon") ++ rootCmd.PersistentFlags().StringVarP(&cliOpts.Timeout, "timeout", "t", "", "Timeout for connecting to daemon") + rootCmd.PersistentFlags().BoolP("help", "h", false, "Print usage") + rootCmd.PersistentFlags().BoolP("version", "v", false, "Version for isula-build client") + } +diff --git a/tests/lib/common.sh b/tests/lib/common.sh +index 4d2d9141..7c4052bf 100755 +--- a/tests/lib/common.sh ++++ b/tests/lib/common.sh +@@ -45,7 +45,7 @@ function start_isula_builder() { + + function cleanup() { + isula-build ctr-img rm -p > /dev/null 2>&1 +- kill -9 "${pidofbuilder}" > /dev/null 2>&1 ++ kill -15 "${pidofbuilder}" > /dev/null 2>&1 + rm -f /tmp/buildlog-* + } + +@@ -54,14 +54,14 @@ function test_build_without_output() { + if ! isula-build ctr-img build --tag "$1":latest "$2" > /tmp/buildlog-client 2>&1; then + echo "FAIL" + echo "LOG DIR:/tmp/buildlog-client and /tmp/buildlog-daemon (build without output)" +- kill -9 "${pidofbuilder}" ++ kill -15 "${pidofbuilder}" + exit 1 + fi + + if ! isula-build ctr-img rm "$1":latest > /tmp/buildlog-client 2>&1; then + echo "FAIL" + echo "LOG DIR:/tmp/buildlog-client and /tmp/buildlog-daemon (build without output)" +- kill -9 "${pidofbuilder}" ++ kill -15 "${pidofbuilder}" + exit 1 + fi + } +@@ -71,7 +71,7 @@ function test_build_with_docker_archive_output() { + if ! isula-build ctr-img build --output=docker-archive:/tmp/"$1".tar:"$1":latest "$2" > /tmp/buildlog-client 2>&1; then + echo "FAIL" + echo "LOG DIR:/tmp/buildlog-client and /tmp/buildlog-daemon (build with docker-archive output)" +- kill -9 "${pidofbuilder}" ++ kill -15 "${pidofbuilder}" + exit 1 + else + rm -f /tmp/"$1".tar +@@ -80,7 +80,7 @@ function test_build_with_docker_archive_output() { + if ! isula-build ctr-img rm "$1":latest > /tmp/buildlog-client 2>&1; then + echo "FAIL" + echo "LOG DIR:/tmp/buildlog-client and /tmp/buildlog-daemon (build with docker-archive output)" +- kill -9 "${pidofbuilder}" ++ kill -15 "${pidofbuilder}" + exit 1 + fi + } +@@ -95,7 +95,7 @@ function test_build_with_docker_daemon_output() { + if ! isula-build ctr-img build --output=docker-daemon:isula/"$1":latest "$2" > /tmp/buildlog-client 2>&1; then + echo "FAIL" + echo "LOG DIR:/tmp/buildlog-client and /tmp/buildlog-daemon (build with docker-daemon output)" +- kill -9 "${pidofbuilder}" ++ kill -15 "${pidofbuilder}" + exit 1 + else + docker rmi isula/"$1" > /dev/null 2>&1 +@@ -104,7 +104,7 @@ function test_build_with_docker_daemon_output() { + if ! isula-build ctr-img rm isula/"$1":latest > /tmp/buildlog-client 2>&1; then + echo "FAIL" + echo "LOG DIR:/tmp/buildlog-client and /tmp/buildlog-daemon (build with docker-daemon output)" +- kill -9 "${pidofbuilder}" ++ kill -15 "${pidofbuilder}" + exit 1 + fi + } +@@ -119,7 +119,7 @@ function test_build_with_isulad_output() { + if ! isula-build ctr-img build --output=isulad:isula/"$1":latest "$2" > /tmp/buildlog-client 2>&1; then + echo "FAIL" + echo "LOG DIR:/tmp/buildlog-client and /tmp/buildlog-daemon (build with isulad output)" +- kill -9 "${pidofbuilder}" ++ kill -15 "${pidofbuilder}" + exit 1 + else + isula rmi isula/"$1" > /dev/null 2>&1 +@@ -128,7 +128,7 @@ function test_build_with_isulad_output() { + if ! isula-build ctr-img rm isula/"$1":latest > /tmp/buildlog-client 2>&1; then + echo "FAIL" + echo "LOG DIR:/tmp/buildlog-client and /tmp/buildlog-daemon (build with isulad output)" +- kill -9 "${pidofbuilder}" ++ kill -15 "${pidofbuilder}" + exit 1 + fi + } +-- +2.19.1 + diff --git a/patch/0041-isula-build-add-t-shortname-for-tag-and-remove-it-fr.patch b/patch/0041-isula-build-add-t-shortname-for-tag-and-remove-it-fr.patch new file mode 100644 index 0000000..3d37600 --- /dev/null +++ b/patch/0041-isula-build-add-t-shortname-for-tag-and-remove-it-fr.patch @@ -0,0 +1,41 @@ +From 708c72d0db42096806bf7d2bdc147e6a1ffc0f19 Mon Sep 17 00:00:00 2001 +From: DCCooper <1866858@gmail.com> +Date: Sat, 19 Sep 2020 10:29:56 +0800 +Subject: [PATCH] isula-build: add -t shortname for --tag and remove it from + --timeout + +Signed-off-by: DCCooper <1866858@gmail.com> +--- + cmd/cli/build.go | 2 +- + cmd/cli/main.go | 2 +- + 2 files changed, 2 insertions(+), 2 deletions(-) + +diff --git a/cmd/cli/build.go b/cmd/cli/build.go +index 5c94bedd..a0245b1d 100644 +--- a/cmd/cli/build.go ++++ b/cmd/cli/build.go +@@ -114,7 +114,7 @@ func NewBuildCmd() *cobra.Command { + buildCmd.PersistentFlags().StringArrayVar(&buildOpts.buildArgs, "build-arg", []string{}, "Arguments used during build time") + buildCmd.PersistentFlags().StringArrayVar(&buildOpts.capAddList, "cap-add", []string{}, "Add Linux capabilities for RUN command") + buildCmd.PersistentFlags().StringVar(&buildOpts.imageIDFile, "iidfile", "", "Write image ID to the file") +- buildCmd.PersistentFlags().StringVarP(&buildOpts.additionalTag, "tag", "", "", "Add tag to the built image") ++ buildCmd.PersistentFlags().StringVarP(&buildOpts.additionalTag, "tag", "t", "", "Add tag to the built image") + + return buildCmd + } +diff --git a/cmd/cli/main.go b/cmd/cli/main.go +index 18bbb965..47206e72 100644 +--- a/cmd/cli/main.go ++++ b/cmd/cli/main.go +@@ -95,7 +95,7 @@ func setupRootCmd(rootCmd *cobra.Command) { + rootCmd.SetFlagErrorFunc(util.FlagErrorFunc) + rootCmd.PersistentFlags().StringVar(&cliOpts.LogLevel, "log-level", "error", "Log level to be used. Either \"debug\", \"info\", \"warn\" or \"error\"") + rootCmd.PersistentFlags().BoolVarP(&cliOpts.Debug, "debug", "D", false, "Open debug mode") +- rootCmd.PersistentFlags().StringVarP(&cliOpts.Timeout, "timeout", "t", "", "Timeout for connecting to daemon") ++ rootCmd.PersistentFlags().StringVar(&cliOpts.Timeout, "timeout", "", "Timeout for connecting to daemon") + rootCmd.PersistentFlags().BoolP("help", "h", false, "Print usage") + rootCmd.PersistentFlags().BoolP("version", "v", false, "Version for isula-build client") + } +-- +2.19.1 + diff --git a/series.conf b/series.conf index a82657b..64830f1 100644 --- a/series.conf +++ b/series.conf @@ -4,3 +4,7 @@ patch/0027-fix-goroutine-leak-with-close-tarLogger-in-a-defer-c.patch patch/0030-xattr-support-ima-and-evm.patch patch/0033-isula-build-remove-docker-releated-path-for-authenti.patch patch/0037-isula-build-fix-goroutine-leak-problem.patch +patch/0038-parse-all-stages-and-mark-it-wheather-need-to-commit.patch +patch/0039-bugfix-fix-build-hang-problem-when-error-happened-be.patch +patch/0040-isula-build-change-default-healthcheck-timeout-to-20.patch +patch/0041-isula-build-add-t-shortname-for-tag-and-remove-it-fr.patch