minikube start --image-repository will now accept URLs with port #11585

dmpe · 2021-06-05T19:01:42Z

fixes:

misspellings -> rename variables and functions
--image-repository shall now accept URLs with ports

Notes:

my first attempt at go
my first (potential) contribution to kubernetes/minikube project (k8s for that matter either) 🌴

…URLs with ports, fixing fix validation logic

k8s-ci-robot · 2021-06-05T19:01:50Z

Welcome @dmpe!

It looks like this is your first PR to kubernetes/minikube 🎉. 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/minikube 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 · 2021-06-05T19:01:50Z

Hi @dmpe. Thanks for your PR.

I'm waiting for a kubernetes 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.

minikube-bot · 2021-06-05T19:03:03Z

Can one of the admins verify this patch?

dmpe · 2021-06-11T09:51:01Z

@afbjorklund can you please ok'it to test - so that test can be run.

thanks.

dmpe · 2021-06-19T08:45:11Z

@RA489 maybe you can also take a look? Thanks

dmpe · 2021-07-01T17:33:55Z

@medyagh Hi, can you please take a look on this. I would like to get it merged :) Thanks.

medyagh · 2021-07-05T14:55:08Z

/ok-to-test

k8s-ci-robot · 2021-07-05T14:55:19Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dmpe, 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~~ [medyagh]

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

minikube-pr-bot · 2021-07-05T16:34:58Z

kvm2 driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 11585) |
+----------------+----------+---------------------+
| minikube start | 48.9s    | 49.8s               |
| enable ingress | 34.7s    | 34.4s               |
+----------------+----------+---------------------+

Times for minikube (PR 11585) ingress: 33.8s 33.7s 36.8s 33.8s 33.8s
Times for minikube ingress: 33.8s 34.7s 34.4s 36.3s 34.3s

Times for minikube start: 51.1s 50.4s 48.0s 47.1s 47.9s
Times for minikube (PR 11585) start: 52.3s 49.2s 52.5s 47.9s 47.2s

docker driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 11585) |
+----------------+----------+---------------------+
| minikube start | 22.1s    | 22.4s               |
| enable ingress | 31.2s    | 29.7s               |
+----------------+----------+---------------------+

Times for minikube start: 22.4s 22.0s 22.5s 22.7s 21.2s
Times for minikube (PR 11585) start: 21.7s 23.2s 22.5s 22.5s 21.9s

Times for minikube ingress: 29.0s 28.0s 29.5s 33.0s 36.5s
Times for minikube (PR 11585) ingress: 29.0s 32.6s 26.5s 30.5s 30.0s

docker driver with containerd runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 11585) |
+----------------+----------+---------------------+
| minikube start | 44.9s    | 45.4s               |
| enable ingress |          |                     |
+----------------+----------+---------------------+

Times for minikube start: 43.5s 44.3s 43.9s 43.4s 49.3s
Times for minikube (PR 11585) start: 44.0s 43.5s 43.8s 47.9s 48.0s

minikube-pr-bot · 2021-07-06T11:03:34Z

kvm2 driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 11585) |
+----------------+----------+---------------------+
| minikube start | 47.4s    | 49.3s               |
| enable ingress | 36.3s    | 40.3s               |
+----------------+----------+---------------------+

Times for minikube start: 49.2s 46.5s 47.6s 46.4s 47.3s
Times for minikube (PR 11585) start: 47.5s 49.5s 46.7s 52.6s 50.0s

Times for minikube ingress: 33.7s 42.2s 34.7s 36.8s 34.2s
Times for minikube (PR 11585) ingress: 42.7s 37.2s 36.2s 42.2s 43.1s

docker driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 11585) |
+----------------+----------+---------------------+
| minikube start | 21.8s    | 21.7s               |
| enable ingress | 35.2s    | 34.4s               |
+----------------+----------+---------------------+

Times for minikube start: 22.9s 21.3s 21.2s 22.1s 21.4s
Times for minikube (PR 11585) start: 21.4s 21.5s 23.1s 21.9s 20.7s

Times for minikube ingress: 31.9s 34.9s 38.0s 37.5s 33.5s
Times for minikube (PR 11585) ingress: 32.0s 36.5s 33.0s 32.0s 38.5s

docker driver with containerd runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 11585) |
+----------------+----------+---------------------+
| minikube start | 42.2s    | 45.4s               |
| enable ingress |          |                     |
+----------------+----------+---------------------+

Times for minikube (PR 11585) start: 47.0s 42.8s 47.5s 42.5s 47.3s
Times for minikube start: 31.2s 44.1s 43.4s 48.8s 43.3s

dmpe · 2021-07-06T12:56:13Z

Hi @medyagh, @RA489 @afbjorklund

Thanks for running tests on this. Besides them I have also just run my own testing with sonatype/nexus3 container.
I have started empty nexus3, configured 2 repos: k8s.gcr.io and gcr.io in order to proxy images locally (as we do at work) and then run:

sudo ./minikube-linux-amd64 start --driver=none --image-repository http://localhost:6668 (proxy to k8s.gcr.io)

Result:

😄  minikube v1.22.0-beta.0 on Ubuntu 21.04
✨  Using the none driver based on user configuration
    ▪ The --image-repository flag your provided contains Scheme: http, which will be removed automatically
✅  Using image repository localhost:6668
👍  Starting control plane node minikube in cluster minikube
🤹  Running on localhost (CPUs=4, Memory=4178MB, Disk=79614MB) ...
ℹ️  OS release is Ubuntu 21.04
❗  This bare metal machine is having trouble accessing https://localhost:6668
💡  To pull new external images, you may need to configure a proxy: https://minikube.sigs.k8s.io/docs/reference/networking/proxy/
🐳  Preparing Kubernetes v1.20.8 on Docker 20.10.7 ...
    ▪ kubelet.resolv-conf=/run/systemd/resolve/resolv.conf
    ▪ Generating certificates and keys ...
    ▪ Booting up control plane ...
    ▪ Configuring RBAC rules ...
🤹  Configuring local host environment ...

❗  The 'none' driver is designed for experts who need to integrate with an existing VM
💡  Most users should use the newer 'docker' driver instead, which does not require root!
📘  For more information, see: https://minikube.sigs.k8s.io/docs/reference/drivers/none/

❗  kubectl and minikube configuration will be stored in /root
❗  To use kubectl or minikube commands as your own user, you may need to relocate them. For example, to overwrite your own settings, run:

    ▪ sudo mv /root/.kube /root/.minikube $HOME
    ▪ sudo chown -R $USER $HOME/.kube $HOME/.minikube

💡  This can also be done automatically by setting the env var CHANGE_MINIKUBE_NONE_USER=true
🔎  Verifying Kubernetes components...
    ▪ Using image localhost:6668/k8s-minikube/storage-provisioner:v5 (global image repository)
🌟  Enabled addons: default-storageclass, storage-provisioner
🏄  Done! kubectl is now configured to use "minikube" cluster and "default" namespace by default

I.e. it is now working and using nexus3 docker images, instead of going outside. This is also validated by the fact, that nexus3 contains all those docker images.

2 Points:

The --image-repository flag your provided contains Scheme: http, which will be removed automatically

In order to use minikube with nexus3 ports, you will need to specify scheme, even though I am not sure it is necessary. Certainly docker pull localhost:6669/k8s-minikube/storage-provisioner:v5 is working, while docker pull http://localhost:6669/k8s-minikube/storage-provisioner:v5 is correctly not working. So the message applies only for certain contexts - and with ports it shall not apply at all.

This bare metal machine is having trouble accessing https://localhost:6668

This message is also confusing, because port 6668 is not https, but rather (as already said above) http. It seems to be hardcoded https://github.com/kubernetes/minikube/blob/master/pkg/minikube/node/start.go#L653 here. Probably a new issue and new PR, later.

wdyt? There are some tests which fail, but seem to be unrelated to my PR.

The executable, used for testing, was downloaded using

gsutil ls gs://minikube-builds/11585/minikube-linux-amd64

cmd/minikube/cmd/start.go

spowelljr · 2021-07-12T17:16:24Z

@dmpe The tests look fine, we have many flaky tests. I left a comment on this PR, it's not a requirement to address the comment, so I'll leave it up to you to decide. If you don't want to implement the comment lmk and I'll merge this PR now.

minikube-pr-bot · 2021-07-12T19:12:10Z

kvm2 driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 11585) |
+----------------+----------+---------------------+
| minikube start | 51.2s    | 50.8s               |
| enable ingress | 32.5s    | 32.7s               |
+----------------+----------+---------------------+

Times for minikube ingress: 31.3s 33.9s 34.3s 31.4s 31.5s
Times for minikube (PR 11585) ingress: 32.9s 32.8s 32.9s 32.4s 32.4s

Times for minikube (PR 11585) start: 49.4s 52.9s 50.4s 53.8s 47.4s
Times for minikube start: 54.5s 49.2s 49.2s 52.4s 50.9s

docker driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 11585) |
+----------------+----------+---------------------+
| minikube start | 22.9s    | 22.9s               |
| enable ingress | 31.9s    | 33.1s               |
+----------------+----------+---------------------+

Times for minikube start: 23.9s 23.2s 22.3s 23.1s 21.8s
Times for minikube (PR 11585) start: 22.7s 23.0s 23.0s 23.0s 22.6s

Times for minikube (PR 11585) ingress: 27.5s 35.5s 31.0s 35.5s 36.0s
Times for minikube ingress: 35.0s 31.0s 27.0s 30.6s 36.0s

docker driver with containerd runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 11585) |
+----------------+----------+---------------------+
| minikube start | 40.3s    | 43.9s               |
| enable ingress |          |                     |
+----------------+----------+---------------------+

Times for minikube start: 27.8s 42.9s 43.8s 43.7s 43.3s
Times for minikube (PR 11585) start: 45.0s 43.6s 43.6s 43.6s 43.9s

dmpe · 2021-07-12T19:31:42Z

Hi @spowelljr - thank you very much for looking into this and giving me a feedback.

I have rebased on latest master, run make tests and did again above testing (works) and it looks good to me. There is an issue with coredns image (coredns:v1.8.0 - neither from gcr.io nor from k8s.gcr.io - does not exist, but retagging an image from dockerhub coredns/coredns:1.8.0 did the trick) but otherwise it is working good. Feel free to merge :)

Thanks again

### This was tested multiple times, here output from without using nexus3
####
minikube v1.22.0 on Ubuntu 21.04
✨  Using the none driver based on user configuration
👍  Starting control plane node minikube in cluster minikube
🤹  Running on localhost (CPUs=4, Memory=4178MB, Disk=79614MB) ...
ℹ️  OS release is Ubuntu 21.04
🐳  Preparing Kubernetes v1.21.2 on Docker 20.10.7 ...
    ▪ kubelet.resolv-conf=/run/systemd/resolve/resolv.conf
    ▪ Generating certificates and keys ...
    ▪ Booting up control plane ...
    ▪ Configuring RBAC rules ...
🤹  Configuring local host environment ...

❗  The 'none' driver is designed for experts who need to integrate with an existing VM
💡  Most users should use the newer 'docker' driver instead, which does not require root!
📘  For more information, see: https://minikube.sigs.k8s.io/docs/reference/drivers/none/

❗  kubectl and minikube configuration will be stored in /root
❗  To use kubectl or minikube commands as your own user, you may need to relocate them. For example, to overwrite your own settings, run:

    ▪ sudo mv /root/.kube /root/.minikube $HOME
    ▪ sudo chown -R $USER $HOME/.kube $HOME/.minikube

💡  This can also be done automatically by setting the env var CHANGE_MINIKUBE_NONE_USER=true
🔎  Verifying Kubernetes components...
    ▪ Using image gcr.io/k8s-minikube/storage-provisioner:v5
🌟  Enabled addons: storage-provisioner, default-storageclass
🏄  Done! kubectl is now configured to use "minikube" cluster and "default" namespace by default

dmpe added 2 commits June 5, 2021 20:50

fix(start): minikube start with --image-repository: add new test for …

445684d

…URLs with ports, fixing fix validation logic

smaller documentation fix

8badb78

k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jun 5, 2021

k8s-ci-robot requested review from afbjorklund and RA489 June 5, 2021 19:01

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 5, 2021

dmpe changed the title ~~wip: fix(start): minikube start --image-repository now accepts URL with port~~ [WIP]: fix(start): minikube start --image-repository now accepts URL with port Jun 5, 2021

dmpe marked this pull request as draft June 5, 2021 19:03

dmpe changed the title ~~[WIP]: fix(start): minikube start --image-repository now accepts URL with port~~ [WIP]: minikube start --image-repository will now accept URLs with port Jun 5, 2021

dmpe changed the title ~~[WIP]: minikube start --image-repository will now accept URLs with port~~ minikube start --image-repository will now accept URLs with port Jun 8, 2021

dmpe marked this pull request as ready for review June 8, 2021 17:47

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 8, 2021

medyagh approved these changes Jul 5, 2021

View reviewed changes

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jul 5, 2021

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 5, 2021

Merge remote-tracking branch 'upstream' into mytests

12f2d69

spowelljr reviewed Jul 12, 2021

View reviewed changes

cmd/minikube/cmd/start.go Outdated Show resolved Hide resolved

dmpe added 3 commits July 12, 2021 19:57

Merge branch 'master' into mytests

6ccc6e7

fix an issue with hasPorts variable - further simplification

a60b337

remove whitespace

0c721cb

spowelljr merged commit 3a18901 into kubernetes:master Jul 12, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

minikube start --image-repository will now accept URLs with port #11585

minikube start --image-repository will now accept URLs with port #11585

dmpe commented Jun 5, 2021 •

edited

Loading

k8s-ci-robot commented Jun 5, 2021

k8s-ci-robot commented Jun 5, 2021

minikube-bot commented Jun 5, 2021

dmpe commented Jun 11, 2021

dmpe commented Jun 19, 2021

dmpe commented Jul 1, 2021 •

edited

Loading

medyagh commented Jul 5, 2021

k8s-ci-robot commented Jul 5, 2021

minikube-pr-bot commented Jul 5, 2021

minikube-pr-bot commented Jul 6, 2021

dmpe commented Jul 6, 2021 •

edited

Loading

spowelljr commented Jul 12, 2021

minikube-pr-bot commented Jul 12, 2021

dmpe commented Jul 12, 2021 •

edited

Loading

minikube start --image-repository will now accept URLs with port #11585

minikube start --image-repository will now accept URLs with port #11585

Conversation

dmpe commented Jun 5, 2021 • edited Loading

Notes:

k8s-ci-robot commented Jun 5, 2021

k8s-ci-robot commented Jun 5, 2021

minikube-bot commented Jun 5, 2021

dmpe commented Jun 11, 2021

dmpe commented Jun 19, 2021

dmpe commented Jul 1, 2021 • edited Loading

medyagh commented Jul 5, 2021

k8s-ci-robot commented Jul 5, 2021

minikube-pr-bot commented Jul 5, 2021

minikube-pr-bot commented Jul 6, 2021

dmpe commented Jul 6, 2021 • edited Loading

spowelljr commented Jul 12, 2021

minikube-pr-bot commented Jul 12, 2021

dmpe commented Jul 12, 2021 • edited Loading

dmpe commented Jun 5, 2021 •

edited

Loading

dmpe commented Jul 1, 2021 •

edited

Loading

dmpe commented Jul 6, 2021 •

edited

Loading

dmpe commented Jul 12, 2021 •

edited

Loading