Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Podman load docker images #1766

Closed
wants to merge 1 commit into from
Closed

Podman load docker images #1766

wants to merge 1 commit into from

Conversation

aojea
Copy link
Contributor

@aojea aojea commented Aug 6, 2020 

podman: save and load images

the kind load docker-image was not working in podman because it
was using docker commands directly.

This moves the helper functions used by the kind command to methods
in the Provider interface, so it works both for podman and docker.

podman: save and load images
bcceba9 
the kind load docker-image was not working in podman because it
was using docker commands directly.

This moves the helper functions used by the kind command to methods
in the Provider interface, so it works both for podman and docker.
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Aug 6, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: aojea
To complete the pull request process, please assign munnerz
You can assign the PR to them by writing /assign @munnerz in a comment when ready.

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

Needs approval from an approver in each of these files:

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

@k8s-ci-robot k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Aug 6, 2020
@aojea
Copy link
Contributor Author

aojea commented Aug 6, 2020

/assign @BenTheElder @amwat

amwat
amwat reviewed Aug 6, 2020
View reviewed changes
Copy link
Contributor

@amwat amwat left a comment

Choose a reason for hiding this comment

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

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 6, 2020
@BenTheElder
Copy link
Member

/hold
I think this breaks the meaning of the command, I have a better idea, one sec.

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 6, 2020
BenTheElder
BenTheElder reviewed Aug 6, 2020
View reviewed changes
pkg/cluster/internal/providers/provider/provider.go
@@ -45,4 +45,8 @@ type Provider interface {
GetAPIServerInternalEndpoint(cluster string) (string, error)
// CollectLogs will populate dir with cluster logs and other debug files
CollectLogs(dir string, nodes []nodes.Node) error
// SaveImage saves image to dest, as in `docker save`
SaveImage(image, dest string) error
Copy link
Member

Choose a reason for hiding this comment

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

Node providers cannot all provide this anyhow, if I have a kubectl + pods based provider, then I want the images to come from the local machine.

Instead we need to cover all the local machine when obtaining images. We can provide a unified UX for that.

@aojea
Copy link
Contributor Author

aojea commented Aug 6, 2020

test
/area provider/podman

@k8s-ci-robot k8s-ci-robot added the area/provider/podman Issues or PRs related to podman label Aug 6, 2020
@BenTheElder BenTheElder mentioned this pull request Aug 7, 2020
Closed
@aojea aojea mentioned this pull request Aug 12, 2020
Open
4 tasks
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 27, 2020
@k8s-ci-robot
Copy link
Contributor

@aojea: PR needs rebase.

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.

@aojea
Copy link
Contributor Author

aojea commented Sep 30, 2020

@BenTheElder how do you suggest to move on with this?
load images is important for CIs

@aojea aojea closed this Oct 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@amwat amwat amwat left review comments

@BenTheElder BenTheElder BenTheElder left review comments

Assignees

@BenTheElder BenTheElder

@amwat amwat

Labels
area/provider/podman Issues or PRs related to podman cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@aojea @k8s-ci-robot @BenTheElder @amwat