Skip to content

vendor buildkit v0.9.1-0.20211025222436-33fb83eb7166#42968

Closed
crazy-max wants to merge 2 commits intomoby:masterfrom
crazy-max:vendor-buildkit
Closed

vendor buildkit v0.9.1-0.20211025222436-33fb83eb7166#42968
crazy-max wants to merge 2 commits intomoby:masterfrom
crazy-max:vendor-buildkit

Conversation

@crazy-max
Copy link
Copy Markdown
Member

@crazy-max crazy-max commented Oct 25, 2021 •
edited
Loading

vendor: github.com/moby/buildkit v0.9.1-0.20211025222436-33fb83eb7166

moby/buildkit@9f254e1...33fb83e

vendor: go.opentelemetry.io

Needs to also upgrade the following dependencies:

  • google.golang.org/grpc v1.40.0
  • google.golang.org/protobuf v1.26.0
  • google.golang.org/api v0.25.0
  • google.golang.org/genproto v0.0.0-20210602131652-f16073e35f0c
  • github.com/golang/protobuf v1.5.2

vendor: github.com/containerd/stargz-snapshotter estargz/v0.8.1-0.20210910092506-a3ecdc9366fb

For estargz compression type

vendor: github.com/containerd/containerd v1.5.1-0.20210721160646-ee27cde735e2

See https://github.com/moby/buildkit/blob/f352ee857b206a3cb81ac7fc88fdef7ae514a911/go.mod#L124

notes

cc @tonistiigi @thaJeztah @aaronlehmann

Signed-off-by: CrazyMax crazy-max@users.noreply.github.com

@crazy-max crazy-max changed the title vendor buildkit v0.9.1-0.20211021081802-f352ee857b20 vendor buildkit v0.9.1-0.20211025222436-33fb83eb7166 Oct 25, 2021
@crazy-max crazy-max force-pushed the vendor-buildkit branch 3 times, most recently from dfdf27d to 248ea14 Compare October 26, 2021 00:06
@thaJeztah
Copy link
Copy Markdown
Member

thaJeztah commented Oct 26, 2021

Seeing a bunch of these;

Error response from daemon: ENV is not a valid change command
Error response from daemon: ENTRYPOINT is not a valid change command
Error response from daemon: EXPOSE is not a valid change command
Error response from daemon: LABEL is not a valid change command

Also:

=== RUN   TestDockerSuite/TestBuildLineErrorOnBuild
    docker_cli_build_test.go:6074: assertion failed:
        Command:  /usr/local/cli/docker build -t test_build_line_error_onbuild -
        ExitCode: 1
        Error:    exit status 1
        Stdout:   Sending build context to Docker daemon  2.048kB


        Stderr:   Error response from daemon: dockerfile parse error on line 2: ONBUILD requires at least one argument


        Failures:
        Expected stderr to contain "parse error line 2: ONBUILD requires at least one argument"
    --- FAIL: TestDockerSuite/TestBuildLineErrorOnBuild (0.07s)

=== RUN   TestDockerSuite/TestBuildLineErrorUnknownInstruction
    docker_cli_build_test.go:6088: assertion failed:
        Command:  /usr/local/cli/docker build -t test_build_line_error_unknown_instruction -
        ExitCode: 1
        Error:    exit status 1
        Stdout:   Sending build context to Docker daemon  2.048kB


        Stderr:   Error response from daemon: dockerfile parse error on line 3: unknown instruction: NOINSTRUCTION


        Failures:
        Expected stderr to contain "parse error line 3: unknown instruction: NOINSTRUCTION"
    --- FAIL: TestDockerSuite/TestBuildLineErrorUnknownInstruction (0.07s)

=== RUN   TestDockerSuite/TestBuildLineErrorWithComments
    docker_cli_build_test.go:6119: assertion failed:
        Command:  /usr/local/cli/docker build -t test_build_line_error_with_comments -
        ExitCode: 1
        Error:    exit status 1
        Stdout:   Sending build context to Docker daemon  2.048kB


        Stderr:   Error response from daemon: dockerfile parse error on line 5: unknown instruction: NOINSTRUCTION


        Failures:
        Expected stderr to contain "parse error line 5: unknown instruction: NOINSTRUCTION"
    --- FAIL: TestDockerSuite/TestBuildLineErrorWithComments (0.08s)

=== RUN   TestDockerSuite/TestBuildLineErrorWithEmptyLines
    docker_cli_build_test.go:6105: assertion failed:
        Command:  /usr/local/cli/docker build -t test_build_line_error_with_empty_lines -
        ExitCode: 1
        Error:    exit status 1
        Stdout:   Sending build context to Docker daemon  2.048kB


        Stderr:   Error response from daemon: dockerfile parse error on line 6: unknown instruction: NOINSTRUCTION


        Failures:
        Expected stderr to contain "parse error line 6: unknown instruction: NOINSTRUCTION"
    --- FAIL: TestDockerSuite/TestBuildLineErrorWithEmptyLines (0.07s)

Looks like these are because the error output changed (now has on before line 2);

parse error line 2: ONBUILD requires at least one argument
parse error on line 2: ONBUILD requires at least one argument

Perhaps we need to check for line 2: and ONBUILD requires at least one argument, or just check on the last bit only.

Also wondering if that test should be rewritten to use a test-table (feels like they're quite similar), and/or to be an API test, but that's a different topic.

thaJeztah
thaJeztah reviewed Oct 26, 2021
View reviewed changes
vendor.conf
# buildkit
github.com/moby/buildkit 9f254e18360a24c2ae47b26f772c3c89533bcbb7 # master / v0.9.0-dev
github.com/tonistiigi/fsutil d72af97c0eaf93c1d20360e3cb9c63c223675b83
github.com/moby/buildkit 33fb83eb71666c2b0b5934e3f4e651f8cfa255c4 # master / v0.9.1-0.20211025222436-33fb83eb7166
Copy link
Copy Markdown
Member

@thaJeztah thaJeztah Oct 26, 2021

Choose a reason for hiding this comment

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

v0.9.1-0 means 0 commits since v0.9.1, correct? In that case, perhaps better to mention it's v0.9.1;

Suggested change
github.com/moby/buildkit 33fb83eb71666c2b0b5934e3f4e651f8cfa255c4 # master / v0.9.1-0.20211025222436-33fb83eb7166
github.com/moby/buildkit 33fb83eb71666c2b0b5934e3f4e651f8cfa255c4 # v0.9.1

Copy link
Copy Markdown
Member Author

@crazy-max crazy-max Oct 26, 2021

Choose a reason for hiding this comment

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

v0.9.1-0 means 0 commits since v0.9.1, correct?

Actually it means it's a base version derived from a semantic version tag that precedes the revision 33fb83eb7166: https://golang.org/ref/mod#pseudo-versions

Copy link
Copy Markdown
Member

@thaJeztah thaJeztah Oct 26, 2021

Choose a reason for hiding this comment

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

Ah! 🤦 I looked at it and thought it was from a git describe (which uses -<number of commits>

Should we use tagged versions (or will there be new tags soon?)

Copy link
Copy Markdown
Member Author

@crazy-max crazy-max Oct 26, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

(or will there be new tags soon?)

Wanted to have the last changes from BuildKit because moby/buildkit#2425 was needed. Next release is v0.9.2 so I think I can open another PR to vendor this release and keep this one as draft for now while waiting for v0.10.0 (cc @tonistiigi).

thaJeztah
thaJeztah reviewed Oct 26, 2021
View reviewed changes
thaJeztah
thaJeztah reviewed Oct 26, 2021
View reviewed changes
@crazy-max
Copy link
Copy Markdown
Member Author

crazy-max commented Oct 26, 2021

Perhaps we need to check for line 2: and ONBUILD requires at least one argument, or just check on the last bit only.

Thanks for the review, yes indeed I will take a look on failed tests.

@crazy-max
Copy link
Copy Markdown
Member Author

crazy-max commented Oct 26, 2021

@thaJeztah

Perhaps we need to check for line 2: and ONBUILD requires at least one argument, or just check on the last bit only.

Related to https://github.com/moby/buildkit/pull/2218/files#diff-ad3df5875e5e135e646046bd3c77fd5f1bb8b2d424d8e97d1ffad6c1c23a4b9aR136. Tests have been updated.

Seeing a bunch of these;

Error response from daemon: ENV is not a valid change command
Error response from daemon: ENTRYPOINT is not a valid change command
Error response from daemon: EXPOSE is not a valid change command
Error response from daemon: LABEL is not a valid change command

Also related to moby/buildkit#2218. Instructions were checked as case-sensitive which is wrong anyway.

@crazy-max crazy-max force-pushed the vendor-buildkit branch 7 times, most recently from 72f34b4 to ba1c019 Compare October 28, 2021 13:06
@tonistiigi
Copy link
Copy Markdown
Member

tonistiigi commented Oct 28, 2021

@crazy-max As there seem to be swarmkit errors in CI let's make a PR first for containerd-only vendor and see if it has the same issues to understand where the breaking point is.

@crazy-max crazy-max mentioned this pull request Nov 2, 2021
Closed
@crazy-max
vendor buildkit v0.9.1-0.20211025222436-33fb83eb7166
b9c369b 
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
@crazy-max
vendor: go-cmp and prometheus
e9bebab 
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
@tonistiigi tonistiigi mentioned this pull request Jan 4, 2022
Closed
@crazy-max crazy-max mentioned this pull request Jan 5, 2022
Merged
@crazy-max crazy-max mentioned this pull request Feb 15, 2022
Merged
@thaJeztah
Copy link
Copy Markdown
Member

thaJeztah commented Mar 16, 2022

superseded by #43239

@thaJeztah thaJeztah closed this Mar 16, 2022
@crazy-max crazy-max deleted the vendor-buildkit branch March 26, 2022 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@thaJeztah thaJeztah thaJeztah left review comments

@tonistiigi tonistiigi Awaiting requested review from tonistiigi tonistiigi will be requested when the pull request is marked ready for review tonistiigi is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@crazy-max @thaJeztah @tonistiigi