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

Docker app build #655

Merged
merged 28 commits into from Oct 7, 2019
Merged

Docker app build #655

merged 28 commits into from Oct 7, 2019

Conversation

ndeloof
Copy link
Contributor

@ndeloof ndeloof commented Oct 1, 2019

- What I did
Introduce support for "build" in docker app relying on buildx

- How I did it
Mostly by vendoring buildx bake command.
bundle command has been removed as obsolete
push command has been updated

- How to verify it
includes a dedicated e2e test

- Description for the changelog
Introduce docker app build command

- A picture of a cute animal (not mandatory but encouraged)
image

internal/commands/build.go Outdated Show resolved Hide resolved
internal/commands/bundle.go Show resolved Hide resolved
internal/commands/build.go Outdated Show resolved Hide resolved
internal/commands/build.go Outdated Show resolved Hide resolved
internal/commands/build.go Outdated Show resolved Hide resolved
internal/commands/build.go Outdated Show resolved Hide resolved
Gopkg.toml Outdated Show resolved Hide resolved
Gopkg.toml Outdated Show resolved Hide resolved
e2e/build_test.go Outdated Show resolved Hide resolved
e2e/build_test.go Outdated Show resolved Hide resolved
@ndeloof ndeloof force-pushed the build branch 2 times, most recently from b1a1885 to 6f3e3fe Compare October 2, 2019 15:34
@ndeloof ndeloof changed the title [WiP] Docker app build Docker app build Oct 2, 2019
@ndeloof ndeloof marked this pull request as ready for review October 2, 2019 16:18
@ndeloof ndeloof force-pushed the build branch 2 times, most recently from 096ad10 to 2218cc1 Compare October 2, 2019 18:33
@codecov
Copy link

codecov bot commented Oct 3, 2019

Codecov Report

Merging #655 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #655   +/-   ##
=======================================
  Coverage   71.21%   71.21%           
=======================================
  Files          57       57           
  Lines        3088     3088           
=======================================
  Hits         2199     2199           
  Misses        599      599           
  Partials      290      290

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e0122a7...3281831. Read the comment docs.

@ndeloof ndeloof force-pushed the build branch 5 times, most recently from 3c708e3 to b714187 Compare October 3, 2019 13:00
@ndeloof ndeloof requested review from rumpl and eunomie October 3, 2019 13:39
Copy link
Member

@eunomie eunomie left a comment

Choose a reason for hiding this comment

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

Some code in test must be removed, I think it's due to some conflict resolution after a rebase

e2e/images_test.go Outdated Show resolved Hide resolved
e2e/images_test.go Outdated Show resolved Hide resolved
e2e/images_test.go Outdated Show resolved Hide resolved
e2e/images_test.go Outdated Show resolved Hide resolved
ndeloof and others added 18 commits October 7, 2019 11:49
Code moved to packager

Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
bake do offer utility functions to convert a compose file into Targets,
then Targets into build.Options, but doing so it also implement compose
environment variable replacement, which doesnt apply to docker app build

Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
If we don't, parsing into cli/compose/types can be broken by
parameters that we don't want replaced by env

Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
otherwise we get a cryptic failure about 404 on grpc endpoint

Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
so we don't depend on docker version installed on CI nodes

Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Co-Authored-By: Yves Brissaud <yves.brissaud@gmail.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Also apply recommendations from code review

Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Copy link
Member

@eunomie eunomie left a comment

Choose a reason for hiding this comment

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

LGTM

Comment on lines +92 to +99
namedRef, err := reference.ParseNormalizedNamed(tag)
if err != nil {
return nil, err
}
ref, ok := reference.TagNameOnly(namedRef).(reference.NamedTagged)
if !ok {
return nil, fmt.Errorf("tag %q must be name with a tag in the 'name:tag' format", tag)
}
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this can be replaced by reference.ParseDockerRef. It does a little more but handle the ParseNormalizedNamed + TagNameOnly

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This code actually has been moved, not created (unfortunately GH doesn't mark the full file as "new" in the diff view.
This whole thing anyway has to be revisited once we start working on "ID" as we will rely on more abstract reference.Reference an not just Named.

Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
@ndeloof ndeloof merged commit 90d8144 into docker:master Oct 7, 2019
@ndeloof ndeloof deleted the build branch October 7, 2019 13:41
@ndeloof ndeloof restored the build branch October 10, 2019 09:01
@ndeloof ndeloof deleted the build branch October 10, 2019 09:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants