Fix port validation error on specifying tcp/udp or range of ports. #13812

tyabu12 · 2022-03-19T02:03:49Z

Tests

$ ./out/minikube start --driver=docker --ports=127.0.0.1:8080:8080/tcp 
😄  minikube v1.25.2 on Darwin 12.2.1 (arm64)
✨  Using the docker driver based on user configuration
👍  Starting control plane node minikube in cluster minikube
🚜  Pulling base image ...
🔥  Creating docker container (CPUs=2, Memory=4000MB) ...
🐳  Preparing Kubernetes v1.23.3 on Docker 20.10.12 ...
    ▪ Generating certificates and keys ...
    ▪ Booting up control plane ...
    ▪ Configuring RBAC rules ...
🔎  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

$ docker ps 
CONTAINER ID   IMAGE                                 COMMAND                  CREATED          STATUS          PORTS                                                                                                                                                            NAMES
71a90cb64130   gcr.io/k8s-minikube/kicbase:v0.0.30   "/usr/local/bin/entr…"   23 seconds ago   Up 22 seconds   127.0.0.1:8080->8080/tcp, 127.0.0.1:51870->22/tcp, 127.0.0.1:51871->2376/tcp, 127.0.0.1:51873->5000/tcp, 127.0.0.1:51869->8443/tcp, 127.0.0.1:51872->32443/tcp   minikube

$ ./out/minikube start --driver=docker --ports=127.0.0.1:8080:8080/udp
😄  minikube v1.25.2 on Darwin 12.2.1 (arm64)
✨  Using the docker driver based on user configuration
👍  Starting control plane node minikube in cluster minikube
🚜  Pulling base image ...
🔥  Creating docker container (CPUs=2, Memory=4000MB) ...
🐳  Preparing Kubernetes v1.23.3 on Docker 20.10.12 ...
    ▪ Generating certificates and keys ...
    ▪ Booting up control plane ...
    ▪ Configuring RBAC rules ...
🔎  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

$ docker ps
CONTAINER ID   IMAGE                                 COMMAND                  CREATED          STATUS          PORTS                                                                                                                                                            NAMES
d5de1e011cb3   gcr.io/k8s-minikube/kicbase:v0.0.30   "/usr/local/bin/entr…"   18 seconds ago   Up 17 seconds   127.0.0.1:8080->8080/udp, 127.0.0.1:52360->22/tcp, 127.0.0.1:52356->2376/tcp, 127.0.0.1:52358->5000/tcp, 127.0.0.1:52359->8443/tcp, 127.0.0.1:52357->32443/tcp   minikube

$ ./out/minikube start --driver=docker --ports=127.0.0.1:8080-8081:8080-8081
😄  minikube v1.25.2 on Darwin 12.2.1 (arm64)
✨  Using the docker driver based on user configuration
👍  Starting control plane node minikube in cluster minikube
🚜  Pulling base image ...
🔥  Creating docker container (CPUs=2, Memory=4000MB) ...
🐳  Preparing Kubernetes v1.23.3 on Docker 20.10.12 ...
    ▪ Generating certificates and keys ...
    ▪ Booting up control plane ...
    ▪ Configuring RBAC rules ...
🔎  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

$ docker ps
CONTAINER ID   IMAGE                                 COMMAND                  CREATED          STATUS          PORTS                                                                                                                                                                      NAMES
be4e51639805   gcr.io/k8s-minikube/kicbase:v0.0.30   "/usr/local/bin/entr…"   18 seconds ago   Up 18 seconds   127.0.0.1:8080-8081->8080-8081/tcp, 127.0.0.1:53079->22/tcp, 127.0.0.1:53080->2376/tcp, 127.0.0.1:53077->5000/tcp, 127.0.0.1:53078->8443/tcp, 127.0.0.1:53076->32443/tcp   minikube

$ ./out/minikube start --driver=docker --ports=127.0.0.1:8080-8081:8080-8081/udp
😄  minikube v1.25.2 on Darwin 12.2.1 (arm64)
✨  Using the docker driver based on user configuration
👍  Starting control plane node minikube in cluster minikube
🚜  Pulling base image ...
🔥  Creating docker container (CPUs=2, Memory=4000MB) ...
🐳  Preparing Kubernetes v1.23.3 on Docker 20.10.12 ...
    ▪ Generating certificates and keys ...
    ▪ Booting up control plane ...
    ▪ Configuring RBAC rules ...
🔎  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

$ docker ps
CONTAINER ID   IMAGE                                 COMMAND                  CREATED          STATUS          PORTS                                                                                                                                                                      NAMES
355278e5ca5a   gcr.io/k8s-minikube/kicbase:v0.0.30   "/usr/local/bin/entr…"   17 seconds ago   Up 16 seconds   127.0.0.1:8080-8081->8080-8081/udp, 127.0.0.1:53570->22/tcp, 127.0.0.1:53571->2376/tcp, 127.0.0.1:53568->5000/tcp, 127.0.0.1:53569->8443/tcp, 127.0.0.1:53567->32443/tcp   minikube

linux-foundation-easycla · 2022-03-19T02:03:52Z

The committers listed above are authorized under a signed CLA.

✅ login: tyabu12 / name: Booyah (513a757)

k8s-ci-robot · 2022-03-19T02:03:56Z

Welcome @tyabu12!

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 · 2022-03-19T02:03:57Z

Hi @tyabu12. 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 · 2022-03-19T02:05:04Z

Can one of the admins verify this patch?

klaases · 2022-03-22T17:43:10Z

cmd/minikube/cmd/start.go

@@ -1246,24 +1247,20 @@ func validateFlags(cmd *cobra.Command, drvName string) {

 // This function validates that the --ports are not below 1024 for the host and not outside range


Please update function comment for goDoc.

// validatePorts validates that the --ports are not below 1024 for the host and not outside range

@klaases
Thank you for reviewing.
I updated the function comment at [1832ed6] .

cmd/minikube/cmd/start_test.go

cmd/minikube/cmd/start.go

medyagh · 2022-03-24T21:51:12Z

/ok-to-test

minikube-pr-bot · 2022-03-25T02:11:39Z

kvm2 driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 13812) |
+----------------+----------+---------------------+
| minikube start | 52.5s    | 53.7s               |
| enable ingress | 28.9s    | 30.6s               |
+----------------+----------+---------------------+

Times for minikube start: 54.4s 52.2s 51.7s 51.3s 53.1s
Times for minikube (PR 13812) start: 52.5s 55.9s 53.1s 55.0s 52.1s

Times for minikube ingress: 26.6s 30.1s 30.6s 27.6s 29.6s
Times for minikube (PR 13812) ingress: 30.6s 30.1s 30.1s 32.1s 30.1s

docker driver with docker 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
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 · 2022-03-25T03:28:12Z

These are the flake rates of all failed tests.

Environment	Failed Tests	Flake Rate (%)
Docker_macOS	TestDockerFlags (gopogh)	6.80 (chart)
Docker_macOS	TestForceSystemdEnv (gopogh)	6.80 (chart)
Docker_macOS	TestForceSystemdFlag (gopogh)	6.80 (chart)
Docker_macOS	TestCertExpiration (gopogh)	7.48 (chart)
Docker_macOS	TestCertOptions (gopogh)	7.48 (chart)
Docker_macOS	TestRunningBinaryUpgrade (gopogh)	9.52 (chart)
Docker_macOS	TestKubernetesUpgrade (gopogh)	12.24 (chart)
Docker_Linux_containerd	TestNetworkPlugins/group/custom-weave/Start (gopogh)	13.91 (chart)
Docker_macOS	TestPause/serial/Start (gopogh)	14.97 (chart)
Docker_macOS	TestNoKubernetes/serial/StartWithK8s (gopogh)	15.65 (chart)
Hyper-V_Windows	TestSkaffold (gopogh)	15.94 (chart)
Docker_macOS	TestNetworkPlugins/group/auto/Start (gopogh)	16.54 (chart)
Docker_macOS	TestNetworkPlugins/group/cilium/Start (gopogh)	16.54 (chart)
Docker_macOS	TestNetworkPlugins/group/false/Start (gopogh)	16.54 (chart)
Docker_macOS	TestNoKubernetes/serial/ProfileList (gopogh)	16.67 (chart)
Docker_macOS	TestNoKubernetes/serial/StartNoArgs (gopogh)	16.67 (chart)
Docker_macOS	TestNoKubernetes/serial/Stop (gopogh)	16.67 (chart)
Docker_macOS	TestNetworkPlugins/group/enable-default-cni/Start (gopogh)	17.29 (chart)
Docker_macOS	TestNoKubernetes/serial/Start (gopogh)	19.71 (chart)
Docker_macOS	TestNoKubernetes/serial/StartWithStopK8s (gopogh)	21.23 (chart)
Docker_macOS	TestNetworkPlugins/group/bridge/Start (gopogh)	26.32 (chart)
Docker_Linux_containerd	TestStartStop/group/old-k8s-version/serial/FirstStart (gopogh)	28.57 (chart)
Docker_Linux_containerd	TestStartStop/group/old-k8s-version/serial/DeployApp (gopogh)	29.17 (chart)
Docker_Linux_containerd	TestStartStop/group/old-k8s-version/serial/SecondStart (gopogh)	29.17 (chart)
Docker_Linux_containerd	TestStartStop/group/old-k8s-version/serial/UserAppExistsAfterStop (gopogh)	30.33 (chart)
Docker_macOS	TestStartStop/group/default-k8s-different-port/serial/AddonExistsAfterStop (gopogh)	30.56 (chart)
Docker_macOS	TestStartStop/group/default-k8s-different-port/serial/EnableAddonAfterStop (gopogh)	30.56 (chart)
Docker_macOS	TestStartStop/group/default-k8s-different-port/serial/EnableAddonWhileActive (gopogh)	30.56 (chart)
Docker_macOS	TestStartStop/group/default-k8s-different-port/serial/FirstStart (gopogh)	30.56 (chart)
Docker_macOS	TestStartStop/group/default-k8s-different-port/serial/UserAppExistsAfterStop (gopogh)	30.56 (chart)
More tests...	Continued...

Too many tests failed - See test logs for more details.

To see the flake rates of all tests by environment, click here.

tyabu12 · 2022-04-03T15:37:33Z

@klaases
Thanks for your review.
I added the commits as you have mentioned. Can you review again?

tyabu12 · 2022-04-11T04:59:49Z

/assign @medyagh

sharifelgamal · 2022-04-12T18:44:17Z

/ok-to-test

Let's run the tests one more time, the first run was a bit wonky.

minikube-pr-bot · 2022-04-12T20:05:22Z

kvm2 driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 13812) |
+----------------+----------+---------------------+
| minikube start | 51.3s    | 51.0s               |
| enable ingress | 28.6s    | 27.6s               |
+----------------+----------+---------------------+

Times for minikube start: 51.8s 51.6s 51.7s 51.7s 49.9s
Times for minikube (PR 13812) start: 50.2s 50.9s 51.0s 51.2s 51.9s

Times for minikube ingress: 28.6s 26.0s 30.1s 29.0s 29.1s
Times for minikube (PR 13812) ingress: 29.1s 25.5s 28.6s 30.0s 25.0s

docker driver with docker 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
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 · 2022-04-12T21:54:49Z

These are the flake rates of all failed tests.

Environment	Failed Tests	Flake Rate (%)
Docker_macOS	TestJSONOutput/pause/Command (gopogh)	8.86 (chart)
Hyperkit_macOS	TestFunctional/parallel/DashboardCmd (gopogh)	14.55 (chart)
Docker_Linux_containerd	TestPause/serial/Start (gopogh)	22.42 (chart)
Docker_Linux_containerd	TestStartStop/group/old-k8s-version/serial/DeployApp (gopogh)	31.40 (chart)
Docker_Linux_containerd	TestStartStop/group/old-k8s-version/serial/FirstStart (gopogh)	31.40 (chart)
Docker_Linux_containerd	TestStartStop/group/old-k8s-version/serial/SecondStart (gopogh)	32.23 (chart)
Docker_Linux_containerd	TestStartStop/group/old-k8s-version/serial/UserAppExistsAfterStop (gopogh)	33.33 (chart)
Docker_Linux_containerd	TestStartStop/group/embed-certs/serial/DeployApp (gopogh)	38.52 (chart)
Docker_Linux_containerd	TestStartStop/group/embed-certs/serial/SecondStart (gopogh)	38.52 (chart)
Docker_Linux_containerd	TestStartStop/group/embed-certs/serial/FirstStart (gopogh)	39.02 (chart)
Docker_Linux_containerd	TestStartStop/group/embed-certs/serial/UserAppExistsAfterStop (gopogh)	40.00 (chart)
Docker_macOS	TestNetworkPlugins/group/custom-weave/Start (gopogh)	47.83 (chart)
Docker_Linux_containerd	TestStartStop/group/default-k8s-different-port/serial/DeployApp (gopogh)	50.41 (chart)
Docker_Linux_containerd	TestStartStop/group/default-k8s-different-port/serial/FirstStart (gopogh)	50.82 (chart)
Docker_Linux_containerd	TestStartStop/group/default-k8s-different-port/serial/SecondStart (gopogh)	50.82 (chart)
Docker_Linux_containerd	TestStartStop/group/default-k8s-different-port/serial/UserAppExistsAfterStop (gopogh)	50.82 (chart)
Docker_Linux_containerd	TestNetworkPlugins/group/kindnet/Start (gopogh)	53.97 (chart)
Docker_Linux	TestNetworkPlugins/group/custom-weave/Start (gopogh)	61.39 (chart)
Docker_Linux	TestNetworkPlugins/group/kindnet/DNS (gopogh)	70.83 (chart)
Docker_Linux	TestNetworkPlugins/group/calico/Start (gopogh)	77.85 (chart)
Docker_Linux_containerd	TestNetworkPlugins/group/calico/Start (gopogh)	80.95 (chart)
Docker_Linux_containerd	TestNetworkPlugins/group/enable-default-cni/DNS (gopogh)	82.44 (chart)
Docker_Linux_containerd	TestNetworkPlugins/group/bridge/DNS (gopogh)	83.33 (chart)
Docker_Linux	TestNetworkPlugins/group/enable-default-cni/DNS (gopogh)	86.08 (chart)
Docker_Linux	TestNetworkPlugins/group/bridge/DNS (gopogh)	86.71 (chart)
Docker_Linux	TestNetworkPlugins/group/kubenet/DNS (gopogh)	87.97 (chart)
Docker_macOS	TestDownloadOnly/v1.16.0/preload-exists (gopogh)	99.37 (chart)

To see the flake rates of all tests by environment, click here.

sharifelgamal

Thank you for your contribution!

k8s-ci-robot · 2022-04-13T16:01:55Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: klaases, sharifelgamal, tyabu12

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~~ [sharifelgamal]

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

Fix port validation error on specifying tcp/udp or range of ports.

513a757

k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Mar 19, 2022

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 Mar 19, 2022

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Mar 19, 2022

k8s-ci-robot requested review from blueelvis and klaases March 19, 2022 02:04

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Mar 19, 2022

klaases reviewed Mar 22, 2022

View reviewed changes

klaases approved these changes Mar 22, 2022

View reviewed changes

Update validatePorts function comment for goDoc.

1832ed6

klaases reviewed Mar 23, 2022

View reviewed changes

cmd/minikube/cmd/start_test.go Show resolved Hide resolved

klaases reviewed Mar 23, 2022

View reviewed changes

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

Add comment to TestValidatePorts.

afb3956

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 Mar 24, 2022

k8s-ci-robot assigned medyagh Apr 11, 2022

sharifelgamal approved these changes Apr 13, 2022

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 13, 2022

sharifelgamal merged commit a9cf475 into kubernetes:master Apr 13, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix port validation error on specifying tcp/udp or range of ports. #13812

Fix port validation error on specifying tcp/udp or range of ports. #13812

tyabu12 commented Mar 19, 2022

linux-foundation-easycla bot commented Mar 19, 2022 •

edited

k8s-ci-robot commented Mar 19, 2022

k8s-ci-robot commented Mar 19, 2022

minikube-bot commented Mar 19, 2022

klaases Mar 22, 2022

tyabu12 Mar 23, 2022 •

edited

medyagh commented Mar 24, 2022

minikube-pr-bot commented Mar 25, 2022

minikube-pr-bot commented Mar 25, 2022

tyabu12 commented Apr 3, 2022

tyabu12 commented Apr 11, 2022

sharifelgamal commented Apr 12, 2022

minikube-pr-bot commented Apr 12, 2022

minikube-pr-bot commented Apr 12, 2022

sharifelgamal left a comment

k8s-ci-robot commented Apr 13, 2022

		@@ -1246,24 +1247,20 @@ func validateFlags(cmd *cobra.Command, drvName string) {

		// This function validates that the --ports are not below 1024 for the host and not outside range

Fix port validation error on specifying tcp/udp or range of ports. #13812

Fix port validation error on specifying tcp/udp or range of ports. #13812

Conversation

tyabu12 commented Mar 19, 2022

Tests

linux-foundation-easycla bot commented Mar 19, 2022 • edited

k8s-ci-robot commented Mar 19, 2022

k8s-ci-robot commented Mar 19, 2022

minikube-bot commented Mar 19, 2022

klaases Mar 22, 2022

Choose a reason for hiding this comment

tyabu12 Mar 23, 2022 • edited

Choose a reason for hiding this comment

medyagh commented Mar 24, 2022

minikube-pr-bot commented Mar 25, 2022

minikube-pr-bot commented Mar 25, 2022

tyabu12 commented Apr 3, 2022

tyabu12 commented Apr 11, 2022

sharifelgamal commented Apr 12, 2022

minikube-pr-bot commented Apr 12, 2022

minikube-pr-bot commented Apr 12, 2022

sharifelgamal left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Apr 13, 2022

linux-foundation-easycla bot commented Mar 19, 2022 •

edited

tyabu12 Mar 23, 2022 •

edited