-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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.
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
Support Ingress on MacOS, driver docker #12089
Conversation
Hi @zhan9san. 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 Once the patch is verified, the new status will be reflected by the 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. |
Can one of the admins verify this patch? |
return | ||
} | ||
|
||
resourcePorts := []int32{80} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
80, 443
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi,
Thanks for your attention. I add port 443 as well.
But you may know the certificate problem below
$ curl https://hello-world.info
curl: (60) SSL certificate problem: unable to get local issuer certificate
More details here: https://curl.haxx.se/docs/sslcerts.html
curl failed to verify the legitimacy of the server and therefore could not
establish a secure connection to it. To learn more about this situation and
how to fix it, please visit the web page mentioned above.
curl -k https://hello-world.info
Hello, world!
Version: 1.0.0
Hostname: web-79d88c97d6-xl5gv
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi,
Thanks for taking my comment into consideration
The way I tested
# start new minikube with ingress addon
minikube start --nodes=2 --addons=ingress
# create workload
kubectl create deployment web --replicas=2 \
--image=gcr.io/google-samples/hello-app:1.0
# create service
kubectl expose deployment web \
--type=NodePort --port=8080
generate self-singed certificates for hello-world.info using this doc
# create tls secret
kubectl create secret tls hello-world-tls \
--cert hello-world.info.pem \
--key hello-world.info.key
# create ingress
kubectl create ingress hello-world-ingress \
--rule=hello-world.info/=web:8080,tls=hello-world-tls
add 127.0.0.1 hello-world.info
to /etc/hosts
start tunnel
minikube tunnel
check
curl https://hello-world.info/
and this worked
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm glad to know it worked for you.
Besides, it tried your openssl-script and it worked great as well.
Hi @zhan9san, thank you so much for this PR! This fixes a long standing issue in our codebase and it's greatly appreciated. Could you change the ingress test at https://github.com/kubernetes/minikube/blob/master/test/integration/addons_test.go#L146 so that it doesn't skip the test for darwin docker driver anymore? |
Currently, I don't find integration test in Github workflow. Is it always running in local laptop?
As for the change in Error log is from this file, box-cli-maker, detect_unix.go and box-cli-maker, box.go
The file, This issue results from the wrong value of minikube/pkg/minikube/out/out.go Line 121 in a85e155
minikube/pkg/minikube/out/out.go Line 349 in a85e155
Here is an example to re-produce the issue, GoTerminal The default value of That's why the Minikube binary file works well but integration/unit test fail. |
yeah, not of all of our integration tests pass currently, it's something we're actively working on. the integration tests don't run in github actions, they run in jenkins after one of our maintainers mark the PR as ok to test. |
/ok-to-test |
kvm2 driver with docker runtime
Times for minikube (PR 12089) start: 47.9s 47.3s 47.5s 47.1s 47.1s Times for minikube ingress: 32.4s 31.8s 31.8s 30.8s 30.8s docker driver with docker runtime
Times for minikube start: 23.4s 21.7s 22.4s 22.8s 23.1s Times for minikube ingress: 27.0s 27.5s 27.0s 27.0s 27.0s docker driver with containerd runtime
Times for minikube start: 27.5s 43.3s 43.2s 43.4s 43.7s |
Hey @zhan9san, the tests look good! Once you rebase to fix the merge conflict, I'll go ahead and merge this PR. Thanks again for the contribution! |
These are the flake rates of all failed tests.
Too many tests failed - See test logs for more details. To see the flake rates of all tests by environment, click here. |
Hi @shahiddev |
kvm2 driver with docker runtime
Times for minikube start: 49.5s 48.8s 46.5s 48.2s 51.8s Times for minikube ingress: 31.3s 30.3s 30.8s 30.8s 30.8s docker driver with docker runtime
Times for minikube (PR 12089) ingress: 27.0s 28.0s 25.5s 26.5s 34.6s Times for minikube start: 23.0s 21.2s 21.3s 23.4s 23.0s docker driver with containerd runtime
Times for minikube start: 28.1s 44.8s 43.5s 44.4s 44.0s |
These are the flake rates of all failed tests.
Too many tests failed - See test logs for more details. To see the flake rates of all tests by environment, click here. |
Sorry, it seems the conflicts are not fixed completely. I'll fix it these days |
da5c26e
to
22bf286
Compare
kvm2 driver with docker runtime
Times for minikube start: 48.8s 46.5s 48.0s 46.3s 51.5s Times for minikube ingress: 33.3s 33.7s 32.7s 33.8s 32.2s docker driver with docker runtime
Times for minikube ingress: 38.0s 32.5s 32.0s 35.5s 37.0s Times for minikube start: 23.5s 21.2s 22.0s 21.8s 21.1s docker driver with containerd runtime
Times for minikube (PR 12089) start: 43.3s 43.0s 43.3s 44.2s 41.6s |
These are the flake rates of all failed tests.
Too many tests failed - See test logs for more details. To see the flake rates of all tests by environment, click here. |
These are the flake rates of all failed tests.
Too many tests failed - See test logs for more details. To see the flake rates of all tests by environment, click here. |
These are the flake rates of all failed tests.
Too many tests failed - See test logs for more details. To see the flake rates of all tests by environment, click here. |
22bf286
to
def3f06
Compare
I performed another rebase operation. After that,
Concerns:
Because I don't have much experience in Golang, please correct me if I say something wrong. |
kvm2 driver with docker runtime
Times for minikube start: 53.6s 52.6s 49.7s 51.4s 51.7s Times for minikube (PR 12089) ingress: 40.5s 41.9s 33.4s 40.4s 41.5s docker driver with docker runtime
Times for minikube start: 24.5s 23.1s 23.8s 23.0s 22.9s Times for minikube (PR 12089) ingress: 35.6s 36.1s 36.1s 35.6s 35.6s docker driver with containerd runtime
Times for minikube start: 31.5s 27.1s 44.6s 44.6s 43.6s |
These are the flake rates of all failed tests.
Too many tests failed - See test logs for more details. To see the flake rates of all tests by environment, click here. |
Hi @sharifelgamal |
It seems ok to me, and the tests pass, I just want another pair of eyes before we merge it. cc @medyagh |
thank you @zhan9san looks good to me |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: medyagh, zhan9san 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:
Approvers can indicate their approval by writing |
The referenced issue (kubernetes/minikube#7332) appears to have been resolved by kubernetes/minikube#12089
The referenced issue (kubernetes/minikube#7332) appears to have been resolved by kubernetes/minikube#12089 Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
@zhan9san Are you required to add the IP at the bottom of the /etc/hosts file? or can you also skip that part and just curl 127.0.0.1? |
Hello. I think issue is still present. I followed 13788 discussion.
is not anyhow responding and is stuck until quit. I was looking into 13806 and as that is a fresh PR wanted to confirm if that is patchable at that moment. Also, I noticed this comment about alternate addresses.
I tried grepping cluster ip:
Looks like ps is missing here for cluster ip, grep of cluster ip returns a record but without port. Is that a possible reason and if so what would be the fix? I was able to access bitnami postgres by both other service and by dbeaver using minikube tunneling so I think that is an ingress issue. Also addressing already mentioned comment, this neither does help:
|
It seems your are not using |
@zhan9san
Here i run minikube tunnel on other tab, provide pass and confirm with enter.
|
Hi @Findaa Thanks for your detailed explanation. Could you try to kill all other obsolete ssh process manually? and try again? No need to recreate the service and ingress. Based on the log above,
sudo kill -9 34334 34653 60910 42128 42129
Press minikube tunnel --cleanup
$ curl hello-world.info
Hello, world!
Version: 1.0.0
Hostname: web-746c8679d4-9rt96 |
Amazing, that helped. Thank you a lot @zhan9san that is working solution. |
The root cause is that the 80 port is already bound which will result in unexpected behavior of For your case, it's obsolete ssh process, maybe it's Nginx, Apache or other process for others.
I can't give you an exact answer. But I believe you know how to debug and fix it. |
Could you please kindly review this PR?
fixes #7332
How to use
sudo permission will be asked for it. It needs password