feat: support for multiple images to kind load #1920

yashvardhan-kukreja · 2020-11-09T13:02:58Z

closes #1905

Usage:

Multiple image names provided (all images present locally):

./kind load docker-image nginx,busybox --name master

Multiple image names provided (all images are not present locally):

./kind load docker-image nginx,busybox,python --name master
ERROR: image: "python" not present locally

One image name provided without commas (backwards compatibility):

./kind load docker-image nginx --name master

k8s-ci-robot · 2020-11-09T13:03:06Z

Welcome @yashvardhan-kukreja!

It looks like this is your first PR to kubernetes-sigs/kind 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/kind has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2020-11-09T13:03:06Z

Hi @yashvardhan-kukreja. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

go.sum

pkg/cmd/kind/load/docker-image/docker-image.go

neolit123 · 2020-11-09T13:44:43Z

pkg/cmd/kind/load/docker-image/docker-image.go

+			fns = append(fns, func() error {
+				return loadImage(imageTarPath, selectedNode)
+			})
+		}


potentially, this code (the saving / loading) can be made to run in parallel via UntilErrorConcurrent:

kind/pkg/errors/concurrent.go

Line 25 in a2282e9

func UntilErrorConcurrent(funcs []func() error) error {

alright. I made the changes and brought the code performing the "saving of image into tar" just along the code where loading of images is happening. (Just above return loadImage(imageTarPath, selectedNode))

saving + loading in the same fn "should" work as long as the docker engine is fine with that.

site/content/docs/user/quick-start.md

pkg/cmd/kind/load/docker-image/docker-image.go

neolit123 · 2020-11-09T14:30:09Z

please, keep the commits squashed to one.

/ok-to-test

neolit123

/lgtm
/assign @BenTheElder

k8s-ci-robot · 2020-11-09T14:31:39Z

New changes are detected. LGTM label has been removed.

yashvardhan-kukreja · 2020-11-09T14:32:35Z

please, keep the commits squashed to one.

/ok-to-test

squashed... the lgtm label got removed. Don't know if that's ok or not.

neolit123 · 2020-11-09T14:55:29Z

squashed... the lgtm label got removed. Don't know if that's ok or not.

it's fine. pending review from @BenTheElder

yashvardhan-kukreja · 2020-11-19T03:50:38Z

kind load docker-image image1 image2

oh, if this is about separate arguments then spaces are the way to go.
if these are in a single flag then commas are preferred:

kind load docker-image --images="image1,image2"

yes. And in the above conversation and in the issue's description too, the former approach of separate args was suggested and asked for. :)

yashvardhan-kukreja · 2020-11-19T06:50:35Z

kind load docker-image image1 image2

oh, if this is about separate arguments then spaces are the way to go.
if these are in a single flag then commas are preferred:

kind load docker-image --images="image1,image2"

yes. And in the above conversation and in the issue's description too, the former approach of separate args was suggested and asked for. :)

And that's the change I made in the latest commit. So, it's working as expected :)

yashvardhan-kukreja · 2020-12-03T01:20:18Z

@neolit123 @aojea , sorry for pestering but can u please review this? 🙈

neolit123

LGTM

yashvardhan-kukreja · 2020-12-11T15:42:39Z

/retest

yashvardhan-kukreja · 2020-12-11T21:54:59Z

Please have a look at this PR @BenTheElder . It has already received one LGTM status.

BenTheElder · 2020-12-14T22:30:41Z

#1926 (comment)

yashvardhan-kukreja · 2020-12-17T21:16:26Z

/joke

Please ignore this. I just came to know about this k8s-bot's command and wanted to try it. 🙈

k8s-ci-robot · 2020-12-17T21:16:31Z

@yashvardhan-kukreja: Remember, the best angle to approach a problem from is the "try" angle.

In response to this:

/joke

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

jaredallard · 2020-12-17T23:34:11Z

Also ignore this, if we're demoing prow features, this is always a must

/meow

k8s-ci-robot · 2020-12-17T23:34:12Z

@jaredallard:

In response to this:

Also ignore this, if we're demoing prow features, this is always a must

/meow

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

BenTheElder

(See comment on your other PR) And thank you!

two nits we can address in follow-up given this lengthy delay 😞

pkg/cmd/kind/load/docker-image/docker-image.go

site/content/docs/user/quick-start.md

yashvardhan-kukreja · 2021-03-15T19:56:39Z

@BenTheElder I have the changes ready as per my above comments. If these sound good, let me know, I ll push them under this PR : )
Thanks!

k8s-ci-robot · 2021-03-15T20:11:47Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: yashvardhan-kukreja
To complete the pull request process, please ask for approval from bentheelder after the PR has been reviewed.

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

Needs approval from an approver in each of these files:

OWNERS

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

yashvardhan-kukreja · 2021-03-16T04:51:12Z

/retest

BenTheElder · 2021-03-16T04:57:13Z

test flakes are known / unrelated, don't worry about that.

site/content/docs/user/quick-start.md

…ments to 'kind load docker-image ' command

k8s-ci-robot · 2021-03-16T17:15:29Z

@yashvardhan-kukreja: The following tests failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
pull-kind-e2e-kubernetes	`d3327a3`	link	`/test pull-kind-e2e-kubernetes`
pull-kind-e2e-kubernetes-1-20	`d3327a3`	link	`/test pull-kind-e2e-kubernetes-1-20`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Nov 9, 2020

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Nov 9, 2020

k8s-ci-robot requested review from aojea and neolit123 November 9, 2020 13:03

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Nov 9, 2020