New command: `build` to build images using minikube #11164

afbjorklund · 2021-04-21T18:31:38Z

This is a rebase of PR #10742

Currently only handles tarballs, not directories

Will create a temporary tarball, if given a dir Some code from github.com/fsouza/go-dockerclient (but not exported as a library, for some reason) Upgrade docker client and cherry-pick windows From v18.09.0 to v19.03.15, plus c3a0a37446 Update go-dockerclient and remove internal deps

Use the long name also for examples and usage

medyagh

since this is a new feature @afbjorklund would you please add an integration test ? maybe in FunctionalTest Next To docker-env test?

that way it could serve as both Example of using and also we will ensure we never break it

afbjorklund · 2021-04-21T20:40:13Z

It seems that there were no tests for any of the previous minikube image commands ?

afbjorklund · 2021-04-22T06:56:01Z

/ok-to-test

minikube-pr-bot · 2021-04-22T07:46:03Z

timing minikube failed, please try again

afbjorklund · 2021-04-23T15:55:56Z

It seems that there were no tests for any of the previous minikube image commands ?

Obviously I only looked for image ls, there were tests for image load and image rm.

minikube-pr-bot · 2021-04-23T17:03:09Z

timing minikube failed, please try again

medyagh

windows integration test failing

2021-04-23T16:06:32.9069971Z === CONT  TestFunctional/parallel/BuildImage
2021-04-23T16:06:32.9074954Z     functional_test.go:291: (dbg) Non-zero exit: ./minikube-windows-amd64.exe ssh -p functional-20210423160039-16840 -- docker image inspect my-image: exit status 1 (1.591562s)
2021-04-23T16:06:32.9093489Z         
2021-04-23T16:06:32.9095921Z         -- stdout --
2021-04-23T16:06:32.9103442Z         	[]
2021-04-23T16:06:32.9105356Z         	Error: No such image: my-image
2021-04-23T16:06:32.9106462Z         
2021-04-23T16:06:32.9110386Z         -- /stdout --
2021-04-23T16:06:32.9112516Z         ** stderr ** 
2021-04-23T16:06:32.9116138Z         	ssh: Process exited with status 1
2021-04-23T16:06:32.9122414Z         
2021-04-23T16:06:32.9124982Z         ** /stderr **
2021-04-23T16:06:32.9131411Z     functional_test.go:329: listing images: exit status 1
2021-04-23T16:06:32.9133450Z         
2021-04-23T16:06:32.9138610Z         -- stdout --
2021-04-23T16:06:32.9142331Z         	[]
2021-04-23T16:06:32.9146494Z         	Error: No such image: my-image
2021-04-23T16:06:32.9149756Z         
2021-04-23T16:06:32.9160974Z         -- /stdout --
2021-04-23T16:06:32.9165037Z         ** stderr ** 
2021-04-23T16:06:32.9166630Z         	ssh: Process exited with status 1
2021-04-23T16:06:32.9168890Z         
2021-04-23T16:06:32.9172849Z         ** /stderr **
2021-04-23T16:06:32.9177731Z     helpers_test.go:218: -----------------------post-mortem--------------------------------

medyagh · 2021-04-23T18:26:14Z

go.mod

@@ -21,7 +21,7 @@ require (
 	github.com/cloudfoundry-attic/jibber_jabber v0.0.0-20151120183258-bcc4c8345a21
 	github.com/cloudfoundry/jibber_jabber v0.0.0-20151120183258-bcc4c8345a21 // indirect
 	github.com/docker/cli v0.0.0-20200303162255-7d407207c304 // indirect
-	github.com/docker/docker v17.12.0-ce-rc1.0.20200916142827-bd33bbf0497b+incompatible


is there a PR upstream that we could comment here to remove the replace after the PR gets merged ?

I had to backport a build fix, since the older Docker 19.03 doesn't build with the newer Windows versions.

See moby/moby@19.03...afbjorklund:v19.03.15-windows

Alternatively one could upgrade everything to the Docker 20.10 API, but that is much more intrusive and big

The patch was: moby/moby@c3a0a37

afbjorklund · 2021-04-23T18:29:12Z

The integration test is failing with containerd/buildkitd it seems, due to the naming of the image:
If you build something with a short name ("myimage"), it gets tagged as "docker.io/library/myimage"

        #8 exporting to image
        #8 sha256:e8c613e07b0b7ff33893b694f7759a10d42e180f2b4dc349fb57dc6b71dcab00
        #8 exporting layers
        #8 exporting layers 0.2s done
        #8 exporting manifest sha256:d7a819d73f2ab7530f6008982101303db287581998611fc3986ad9ace3ff9089 done
        #8 exporting config sha256:a8fdd9c45e461cee2130ba01b0007d5c08c9058b429fd26d8d0bdaa42d3eb2c8 done
        #8 naming to my-image:latest done
        #8 DONE 0.2s

$ sudo crictl images my-image
IMAGE                                     TAG                  IMAGE ID            SIZE
docker.io/library/my-image                latest               a8fdd9c45e461       767kB

We need to build "localhost/my-image:latest", then it would be fully qualifed and not get Docker Hub:ed

IMAGE                                     TAG                  IMAGE ID            SIZE
localhost/my-image                        latest               d6b1c27da1a42       767kB

It would also work with crio/podman, that will prepend this "localhost" prefix to all unqualified images...

        STEP 4: COMMIT my-image
        --> 2d032ceb437

REPOSITORY          TAG     IMAGE ID      CREATED         SIZE
localhost/my-image  latest  2d032ceb437c  45 seconds ago  1.46 MB

medyagh · 2021-04-23T18:28:35Z

pkg/minikube/cruntime/containerd.go

+	return tmp, nil
+}
+
+func downloadRemote(cr CommandRunner, src string) (string, error) {


add unit test for this ? or maybe break down the logic of detecting remote and add unit tests, so we know exactly what user would input

Right, but then we need to start a web server or something to host the files to build (the ones in testdata/build) ?

I think the detection part of the (the scheme detection) could be put in a separate func with unit test ?

We only implement it for buildkit, both docker and podman have their own (internal) implementation...

test/integration/functional_test.go

afbjorklund · 2021-04-23T18:50:27Z

windows integration test failing

It's related to the DOS paths from the host:

=== CONT  TestFunctional/parallel/BuildImage
    functional_test.go:315: (dbg) Done: ./minikube-windows-amd64.exe -p functional-20210423160130-2736 image build -t my-image testdata\build: (10.8477205s)
    functional_test.go:323: (dbg) Stderr: ./minikube-windows-amd64.exe -p functional-20210423160130-2736 image build -t my-image testdata\build:
        unable to prepare context: path "C:\\Users\\jenkins\\AppData\\Local\\Temp\\build.211045959.tar" not found
    functional_test.go:291: (dbg) Run:  ./minikube-windows-amd64.exe ssh -p functional-20210423160130-2736 -- docker image inspect my-image

The code needs to be more careful about which paths are from the host (possibly weird), and which are on machine (and sane)

And add some cleanup and improved logging as well

minikube-pr-bot · 2021-04-23T21:04:09Z

kvm2 driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 11164) |
+----------------+----------+---------------------+
| minikube start | 51.3s    | 48.8s               |
| enable ingress | 35.9s    | 36.1s               |
+----------------+----------+---------------------+

Times for minikube ingress: 35.9s 34.8s 36.8s 36.3s 35.7s
Times for minikube (PR 11164) ingress: 33.8s 35.3s 34.3s 34.7s 42.3s

Times for minikube start: 52.8s 52.9s 51.8s 51.1s 47.7s
Times for minikube (PR 11164) start: 51.7s 47.1s 47.8s 46.9s 50.4s

docker driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 11164) |
+----------------+----------+---------------------+
| minikube start | 22.0s    | 21.4s               |
| enable ingress | 33.6s    | 32.4s               |
+----------------+----------+---------------------+

Times for minikube start: 23.8s 21.4s 21.5s 22.0s 21.2s
Times for minikube (PR 11164) start: 21.9s 21.5s 21.0s 21.3s 21.2s

Times for minikube ingress: 31.5s 33.0s 32.5s 34.5s 36.5s
Times for minikube (PR 11164) ingress: 29.0s 30.0s 31.5s 35.0s 36.5s

docker driver with containerd runtime
error collecting results for docker driver: timing run 0 with minikube: timing cmd: [out/minikube addons enable ingress]: waiting for minikube: exit status 10

So need to be on the lookout for "C:\\path"

Using file: scheme means build local context

It hides the docker.io and the "library" parts

minikube-pr-bot · 2021-04-23T21:47:47Z

kvm2 driver with docker runtime
error collecting results for kvm2 driver: timing run 0 with minikube: timing cmd: [out/minikube start --driver=kvm2 --container-runtime=docker]: waiting for minikube: exit status 63
docker driver with docker runtime
error downloading artifacts: downloading artifacts: exit status 81docker driver with containerd runtime
error downloading artifacts: downloading artifacts: exit status 81

minikube-pr-bot · 2021-04-23T21:54:17Z

kvm2 driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 11164) |
+----------------+----------+---------------------+
| minikube start | 49.0s    | 52.1s               |
| enable ingress | 42.2s    | 41.2s               |
+----------------+----------+---------------------+

Times for minikube ingress: 42.8s 43.3s 44.3s 43.7s 36.9s
Times for minikube (PR 11164) ingress: 43.8s 43.2s 36.9s 44.3s 37.7s

Times for minikube start: 56.1s 47.0s 47.8s 47.7s 46.7s
Times for minikube (PR 11164) start: 47.4s 54.2s 54.5s 52.7s 51.7s

docker driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 11164) |
+----------------+----------+---------------------+
| minikube start | 22.2s    | 21.6s               |
| enable ingress | 33.5s    | 35.2s               |
+----------------+----------+---------------------+

Times for minikube ingress: 32.0s 32.5s 33.5s 32.0s 37.5s
Times for minikube (PR 11164) ingress: 34.0s 36.5s 34.5s 37.0s 34.0s

Times for minikube start: 22.8s 21.9s 22.4s 22.6s 21.3s
Times for minikube (PR 11164) start: 21.6s 21.1s 21.4s 21.8s 22.0s

docker driver with containerd runtime
error collecting results for docker driver: timing run 0 with minikube: timing cmd: [out/minikube addons enable ingress]: waiting for minikube: exit status 10

minikube-pr-bot · 2021-04-23T22:40:12Z

kvm2 driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 11164) |
+----------------+----------+---------------------+
| minikube start | 48.8s    | 50.4s               |
| enable ingress | 39.4s    | 39.3s               |
+----------------+----------+---------------------+

Times for minikube start: 49.3s 47.2s 48.2s 48.0s 51.4s
Times for minikube (PR 11164) start: 49.6s 48.4s 52.0s 51.8s 50.1s

Times for minikube ingress: 42.9s 34.8s 43.3s 41.9s 34.3s
Times for minikube (PR 11164) ingress: 35.9s 37.0s 42.3s 37.8s 43.3s

docker driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 11164) |
+----------------+----------+---------------------+
| minikube start | 21.8s    | 21.3s               |
| enable ingress | 35.6s    | 35.8s               |
+----------------+----------+---------------------+

Times for minikube start: 21.9s 22.4s 21.7s 22.0s 21.0s
Times for minikube (PR 11164) start: 21.4s 21.2s 21.3s 21.3s 21.4s

Times for minikube ingress: 36.5s 33.0s 37.0s 34.5s 37.0s
Times for minikube (PR 11164) ingress: 35.5s 37.0s 37.0s 35.5s 34.0s

docker driver with containerd runtime
error collecting results for docker driver: timing run 0 with minikube: timing cmd: [out/minikube addons enable ingress]: waiting for minikube: exit status 10

medyagh

Looks good to me @ , Thank you for this feature. I am excited to see how our users will use this

k8s-ci-robot · 2021-04-25T04:03:30Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: afbjorklund, medyagh

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [afbjorklund,medyagh]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

afbjorklund added 21 commits April 21, 2021 19:56

Add initial re-implementation of the build command

af5828d

Currently only handles tarballs, not directories

Add some basic support for setting the image tag

a274482

Generate documentation for the build command

6f26e4b

Move minikube build cmd to minikube image build

6cfbf2c

Use the long name also for examples and usage

Make sure to show output from build cmd

844c696

Make sure to pass any file param to build

c94a6d7

Move absolute file path handling to machine

f670368

Don't add build context to the default file

22150fb

Don't fail when the default file is passed

a8a8788

Add option for pushing image to registry

02b4267

Improve comments and remove docker daemon

40cbe65

Allow building image from an url as well

9f9958f

Allow building tarball from stdin stream

db203cd

Allow passing environ and options to build

c961190

Add git url support to containerd runtime

947e31a

Address lint append suggestions from gocritic

e230f02

Add example parameters to the usage help text

9516122

Regenerate documentation for the unit test

c715324

Clean up duplicated function etc after merge

f7e0777

Clean up the order of the image commands

943561b

k8s-ci-robot requested review from prezha and RA489 April 21, 2021 18:31

k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Apr 21, 2021

afbjorklund requested review from medyagh and tstromberg April 21, 2021 18:32

medyagh reviewed Apr 21, 2021

View reviewed changes

afbjorklund force-pushed the build-rebase branch from a267c60 to 541acb6 Compare April 21, 2021 20:46

Add skeleton for minikube image integration test

a5f835b

afbjorklund force-pushed the build-rebase branch from 541acb6 to 2d02aba Compare April 22, 2021 06:55

k8s-ci-robot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Apr 22, 2021

Make sure to start buildkit socket for containerd

93c40d1

afbjorklund force-pushed the build-rebase branch from 2d02aba to 93c40d1 Compare April 22, 2021 07:02

Move the BuildImage test over to functional test

c9a0a7a

medyagh requested changes Apr 23, 2021

View reviewed changes

test/integration/functional_test.go Show resolved Hide resolved

Use canonical names for the functional test images

5d8d3d3

And add some cleanup and improved logging as well

afbjorklund added 3 commits April 23, 2021 23:04

url.Parse treats DOS volumes as URL schemes

d508459

So need to be on the lookout for "C:\\path"

Support building a path local to the machine

96e199c

Using file: scheme means build local context

The docker container runtime doesn't show name

4d0c0c6

It hides the docker.io and the "library" parts

afbjorklund requested a review from medyagh April 24, 2021 06:30

medyagh approved these changes Apr 25, 2021

View reviewed changes

medyagh merged commit 41bbb4f into kubernetes:master Apr 25, 2021

medyagh changed the title ~~Add re-implementation of the build command~~ New command: build to build images using minikube Apr 25, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New command: `build` to build images using minikube #11164

New command: `build` to build images using minikube #11164

afbjorklund commented Apr 21, 2021

medyagh left a comment

afbjorklund commented Apr 21, 2021

afbjorklund commented Apr 22, 2021

minikube-pr-bot commented Apr 22, 2021

afbjorklund commented Apr 23, 2021

minikube-pr-bot commented Apr 23, 2021

medyagh left a comment

medyagh Apr 23, 2021

afbjorklund Apr 23, 2021 •

edited

Loading

afbjorklund Apr 23, 2021

afbjorklund Apr 23, 2021

afbjorklund commented Apr 23, 2021 •

edited

Loading

medyagh Apr 23, 2021

afbjorklund Apr 23, 2021

medyagh Apr 23, 2021

afbjorklund Apr 23, 2021 •

edited

Loading

afbjorklund commented Apr 23, 2021

minikube-pr-bot commented Apr 23, 2021

minikube-pr-bot commented Apr 23, 2021

minikube-pr-bot commented Apr 23, 2021

minikube-pr-bot commented Apr 23, 2021

medyagh left a comment

k8s-ci-robot commented Apr 25, 2021

New command: build to build images using minikube #11164

New command: build to build images using minikube #11164

Conversation

afbjorklund commented Apr 21, 2021

medyagh left a comment

Choose a reason for hiding this comment

afbjorklund commented Apr 21, 2021

afbjorklund commented Apr 22, 2021

minikube-pr-bot commented Apr 22, 2021

afbjorklund commented Apr 23, 2021

minikube-pr-bot commented Apr 23, 2021

medyagh left a comment

Choose a reason for hiding this comment

medyagh Apr 23, 2021

Choose a reason for hiding this comment

afbjorklund Apr 23, 2021 • edited Loading

Choose a reason for hiding this comment

afbjorklund Apr 23, 2021

Choose a reason for hiding this comment

afbjorklund Apr 23, 2021

Choose a reason for hiding this comment

afbjorklund commented Apr 23, 2021 • edited Loading

medyagh Apr 23, 2021

Choose a reason for hiding this comment

afbjorklund Apr 23, 2021

Choose a reason for hiding this comment

medyagh Apr 23, 2021

Choose a reason for hiding this comment

afbjorklund Apr 23, 2021 • edited Loading

Choose a reason for hiding this comment

afbjorklund commented Apr 23, 2021

minikube-pr-bot commented Apr 23, 2021

minikube-pr-bot commented Apr 23, 2021

minikube-pr-bot commented Apr 23, 2021

minikube-pr-bot commented Apr 23, 2021

medyagh left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Apr 25, 2021

New command: `build` to build images using minikube #11164

New command: `build` to build images using minikube #11164

afbjorklund Apr 23, 2021 •

edited

Loading

afbjorklund commented Apr 23, 2021 •

edited

Loading

afbjorklund Apr 23, 2021 •

edited

Loading