kind
kind copied to clipboard
kubernetes-sigs
feat(cmd/load): auto pull images if not exist
Fixes #2424
We had to move pkg/{build/nodeimage => }/internal/container/docker package. We couldn't access the Pull() function in the .../nodeimage/internal/container/docker package due to Go internal package access limitations. How should we proceed here?
Signed-off-by: Furkan [email protected] Co-authored-by: Batuhan [email protected]
Welcome @Dentrax!
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. :smiley:
Hi @Dentrax. 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.
/assign @BenTheElder IIRC Ben wanted the cmd to be as much independent as possible, so I think that you should implement the pull directly , as the other functions in the same file.
However, before doing anything let's wait for him to accept the feature, I can't remember know why this is this way, I vaguely remember to discuss this because I found this useful in some cases ...
We had to move pkg/{build/nodeimage => }/internal/container/docker package. We couldn't access the Pull() function in the .../nodeimage/internal/container/docker package due to Go internal package access limitations. How should we proceed here?
internal is intentional, these methods weren't designed for re-use.
This is such a simple thing to implement, we can just duplicate it for now.
Also I think probably pull in this case should only pull once. But further we should discuss on the issue first, please see our contributor guide https://kind.sigs.k8s.io/docs/contributing/getting-started/#4-reaching-out
Keywords which can automatically close issues and at(@) or hashtag(#) mentions are not allowed in commit messages.
The list of commits with invalid commit messages:
- ca13ea9 feat(cmd/load): auto pull images if not exist
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.
[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: Dentrax 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.
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
We had to move pkg/{build/nodeimage => }/internal/container/docker package. We couldn't access the Pull() function in the .../nodeimage/internal/container/docker package due to Go internal package access limitations. How should we proceed here?
internal is intentional, these methods weren't designed for re-use.
This is such a simple thing to implement, we can just duplicate it for now.
Also I think probably pull in this case should only pull once. But further we should discuss on the issue first, please see our contributor guide https://kind.sigs.k8s.io/docs/contributing/getting-started/#4-reaching-out
Thanks, I just updated the PR. Duplicated the pull function from internal/container/docker package. And add a new flag called --pull to not break as-is flow, as you said in the issue.
@Dentrax: 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.
I this is become stale, I'm interested in pursuing further.. lemme know if any objection? @Dentrax @BenTheElder @aojea
I this is become stale, I'm interested in pursuing further.. lemme know if any objection? @Dentrax @BenTheElder @aojea
See the related issue, is not clear the use case for this feature
I this is become stale, I'm interested in pursuing further.. lemme know if any objection? @Dentrax @BenTheElder @aojea
See the related issue, is not clear the use case for this feature
Lemme add what am I looking for..
This is stale because it was filed without following the contributor guide, we require feature design discussion up front, at this point we're highly focused on keeping kind reliable.
based on the issue comment, I don't think this change solves your request anyhow.
This feature needs an agreed design, let's continue on https://github.com/kubernetes-sigs/kind/issues/2424 first.
https://kind.sigs.k8s.io/docs/contributing/getting-started/
Thanks for the contribution anyhow.