Prevent minikube from crashing if namespace or service doesn't exist #5844

rajula96reddy · 2019-11-06T11:30:55Z

Fixes issue #5836

Old behavior

$ minikube service newservice -n unknown --url

💣 Error opening service: Service newservice was not found in "unknown" namespace. You may select another namespace by using 'minikube service newservice -n : Temporary Error: Error getting service newservice: services "newservice" not found

😿 Sorry that minikube crashed. If this was unexpected, we would love to hear from you:
👉 https://github.com/kubernetes/minikube/issues/new/choose

New behaviour

$ minikube service newservice -n unknown --url

💣  Error opening service: Service newservice was not found in "unknown" namespace. You may select another namespace by using 'minikube service newservice -n <namespace>'. Or list out all the services using 'minikube service list': Temporary Error: Error getting service newservice: services "newservice" not found

PS: Hey! This is my first contribution to K8s 🎉. Pardon me if there are any blunders 😅

k8s-ci-robot · 2019-11-06T11:31:02Z

Welcome @rajula96reddy!

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 · 2019-11-06T11:31:03Z

Hi @rajula96reddy. 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 · 2019-11-06T11:31:04Z

Can one of the admins verify this patch?

codecov-io · 2019-11-06T11:36:59Z

Codecov Report

Merging #5844 into master will decrease coverage by 0.02%.
The diff coverage is 30%.

@@            Coverage Diff             @@
##           master    #5844      +/-   ##
==========================================
- Coverage   37.01%   36.98%   -0.03%     
==========================================
  Files         144      144              
  Lines        9010     9019       +9     
==========================================
+ Hits         3335     3336       +1     
- Misses       5253     5260       +7     
- Partials      422      423       +1

Impacted Files	Coverage Δ
cmd/minikube/cmd/service.go	`8.86% <0%> (-0.48%)`	⬇️
pkg/minikube/service/service.go	`76.31% <50%> (-1.53%)`	⬇️

rajula96reddy · 2019-11-06T12:48:35Z

/assign @medyagh

medyagh · 2019-11-06T18:45:15Z

@rajula96reddy welcome to contributing to minikube ! thank you for making such a quick PR !

the PR looks good just small request.

💣  Error opening service: Service newservice was not found in "unknown" namespace. You may select another namespace by using 'minikube service newservice -n <namespace>'. Or list out all the services using 'minikube service list': Temporary Error: Error getting service newservice: services "newservice" not found

how about making the format a bit nicer ? and get rid of the temporary error to the end user ? they don't need to see the temp error.

here is an example of using a template with exit

minikube/cmd/minikube/cmd/start.go

Line 1035 in 134182f

exit.WithCodeT(exit.IO, `minikube is unable to connect to the VM: {{.error}}

rajula96reddy · 2019-11-06T18:59:43Z

Hey! Thank you for your feedback. I will try to improve the format as suggested.

medyagh · 2019-11-06T21:46:08Z

FYI I created a new issue, #5846
this would be the next thing to do on Service refactor (not for this PR but next one)

k8s-ci-robot · 2019-11-07T06:39:28Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: rajula96reddy
To complete the pull request process, please assign medyagh
You can assign the PR to them by writing /assign @medyagh 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:

OWNERS

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

rajula96reddy · 2019-11-07T06:51:34Z

Hey @medyagh! I formatted the error. Let me know what you think about it?

Also, I was trying to test my change and wanted to test the below case

minikube/pkg/minikube/service/service.go

Lines 282 to 285 in 134182f

    
           serviceURL, err := GetServiceURLsForService(api, namespace, service, urlTemplate) 
        
           if err != nil { 
        
           	return urlList, errors.Wrap(err, "Check that minikube is running and that you have specified the correct namespace") 
        
           }

But I wasn't able to reach this statement, as it was either getting caught in the earlier check or it is not throwing any error at all. Any idea, how to reach this statement?

I have also noticed that if minikube is not running, minikube service example-service command is not throwing out anything like


💣  command runner: getting ssh client for bootstrapper: Error dialing tcp via ssh client: dial tcp 127.0.0.1:54552: connect: connection refused

😿  Sorry that minikube crashed. If this was unexpected, we would love to hear from you:
👉  https://github.com/kubernetes/minikube/issues/new/choose

Is this behavior expected?

RA489 · 2019-11-12T09:01:54Z

/ok-to-test

minikube-bot · 2019-11-12T09:02:10Z

Error: building minikube at head: updating minikube master branch: running [git pull origin master] in /home/performance-monitor/minikube:
From https://github.com/kubernetes/minikube

branch master -> FETCH_HEAD
error: Your local changes to the following files would be overwritten by merge:
go.mod
Please commit your changes or stash them before you merge.
Aborting
Updating 1d0ca6c..e611b38
: exit status 1

medyagh · 2019-12-11T19:11:42Z

@rajula96reddy I just wanted to see if you still have any questions on how to handle this PR ?

minikube-bot · 2019-12-18T11:03:32Z

All Times Minikube (PR 5844): [ 123.180857 122.815112 118.334852]
All Times minikube: [ 120.558695 116.922259 109.509355]

Average minikube: 115.663436
Average Minikube (PR 5844): 121.443607

Averages Time Per Log

+----------------------+-----------+--------------------+
|         LOG          | MINIKUBE  | MINIKUBE (PR 5844) |
+----------------------+-----------+--------------------+
| minikube v           |  0.321316 |           0.241735 |
| Creating kvm2        | 45.740124 |          45.792395 |
| Preparing Kubernetes | 44.908187 |          50.137522 |
| Pulling images       |  2.758694 |           3.090232 |
| Launching Kubernetes | 18.974369 |          19.247018 |
| Waiting for cluster  |  2.913272 |           2.890865 |
+----------------------+-----------+--------------------+

rajula96reddy · 2019-12-19T17:44:36Z

Hey @medyagh. Can you review my latest commit?

Also, I wanted to ask if I should make changes for #5846 also in this PR only?

medyagh · 2019-12-21T05:10:26Z

@rajula96reddy yes you could include this in this pr

medyagh · 2019-12-21T05:16:41Z

Here is an example of using error types https://blog.golang.org/error-handling-and-go

medyagh · 2020-01-13T22:23:07Z

@rajula96reddy are you still working on this ?

medyagh

see comments

medyagh · 2019-12-21T05:11:59Z

cmd/minikube/cmd/service.go

@@ -77,6 +77,10 @@ var serviceCmd = &cobra.Command{

 		urls, err := service.WaitForService(api, namespace, svc, serviceURLTemplate, serviceURLMode, https, wait, interval)
 		if err != nil {
+			if err.Error() == "Service not found in namespace" {


Can you make this error a type ? Instead of string

And than have a functional called

IsErrorrSVCNotFound(err)
?

medyagh · 2020-01-13T22:24:23Z

cmd/minikube/cmd/service.go

@@ -77,6 +77,10 @@ var serviceCmd = &cobra.Command{

 		urls, err := service.WaitForService(api, namespace, svc, serviceURLTemplate, serviceURLMode, https, wait, interval)


because getting list of ALL services is a very quick call,
First, get a list of services and then check if the service is in that list , if there isnt return quick,
then Seconly if the service exist, then wait for the service to be up !

medyagh · 2020-01-22T22:01:53Z

@rajula96reddy are you still working on this ?

rajula96reddy · 2020-01-23T03:12:38Z

@medyagh Yes! I have made changes to my commit couple of days back (5ac578b) & I am awaiting your review on the same.

minikube-pr-bot · 2020-01-25T00:02:00Z

All Times minikube: [ 100.438152 95.368815 92.722641]
All Times Minikube (PR 5844): [ 124.317613 126.464113 115.632643]

Average minikube: 96.176536
Average Minikube (PR 5844): 122.138123

Averages Time Per Log

+----------------------+-----------+--------------------+
|         LOG          | MINIKUBE  | MINIKUBE (PR 5844) |
+----------------------+-----------+--------------------+
| minikube v           |  0.297793 |           0.324520 |
| Creating kvm2        | 19.779730 |          47.206725 |
| Preparing Kubernetes | 49.817024 |          47.181363 |
| Pulling images       |  3.624858 |           2.949342 |
| Launching Kubernetes | 18.988780 |          19.495113 |
| Waiting for cluster  |  2.395348 |           2.894473 |
+----------------------+-----------+--------------------+

medyagh · 2020-01-28T23:13:05Z

@rajula96reddy please check the unit test failures https://travis-ci.org/kubernetes/minikube/builds/640656004?utm_source=github_status&utm_medium=notification

rajula96reddy · 2020-01-29T16:51:12Z

@medyagh Sorry for that. Let me check the test failures. Thanks!

rajula96reddy · 2020-02-04T19:20:15Z

@medyagh I have corrected the issue. Sorry, it was because I was trying to #5846 by calling the wrong function. It's resolved now. Can you please take a look & give your feedback? Thank you!

minikube-pr-bot · 2020-02-04T19:36:49Z

All Times Minikube (PR 5844): [ 97.375983 95.940267 96.675498]
All Times minikube: [ 96.914335 100.220012 98.540137]

Average minikube: 98.558162
Average Minikube (PR 5844): 96.663916

Averages Time Per Log

+----------------------+-----------+--------------------+
|         LOG          | MINIKUBE  | MINIKUBE (PR 5844) |
+----------------------+-----------+--------------------+
| minikube v           |  0.311744 |           0.296857 |
| Creating kvm2        | 20.190853 |          19.897212 |
| Preparing Kubernetes | 50.718608 |          50.011045 |
| Pulling images       |  4.072735 |           4.002056 |
| Launching Kubernetes | 20.526796 |          19.766602 |
| Waiting for cluster  |  1.078027 |           1.081082 |
+----------------------+-----------+--------------------+

tstromberg

Looks ready to merge other than one nit.

cmd/minikube/cmd/service.go

minikube-pr-bot · 2020-02-07T04:03:09Z

All Times minikube: [ 96.702800 94.278517 90.986860]
All Times Minikube (PR 5844): [ 91.670269 95.138358 91.838445]

Average minikube: 93.989392
Average Minikube (PR 5844): 92.882357

Averages Time Per Log

+----------------------+-----------+--------------------+
|         LOG          | MINIKUBE  | MINIKUBE (PR 5844) |
+----------------------+-----------+--------------------+
| minikube v           |  0.191398 |           0.212668 |
| Creating kvm2        | 21.193625 |          19.914098 |
| Preparing Kubernetes | 49.221663 |          50.848176 |
| Pulling images       |           |                    |
| Launching Kubernetes | 21.345468 |          19.985958 |
| Waiting for cluster  |  0.417006 |           0.048281 |
+----------------------+-----------+--------------------+

medyagh · 2020-03-07T07:38:05Z

@rajula96reddy we could merge this if u could please rebase

rajula96reddy · 2020-03-07T18:21:34Z

@medyagh Thanks for the feedback. I rebased as requested. Can you please confirm the same & proceed accordingly? Thanks!

tstromberg · 2020-03-21T17:26:38Z

Thank you!

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Nov 6, 2019

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 Nov 6, 2019

k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Nov 6, 2019

k8s-ci-robot requested review from josedonizetti and tstromberg November 6, 2019 11:31

k8s-ci-robot assigned medyagh Nov 6, 2019

medyagh mentioned this pull request Nov 6, 2019

Improving error message for service not in default namespace #5312

Closed

nanikjava mentioned this pull request Nov 6, 2019

fix minikube service retry logic (takes 35s) #5846

Closed

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 Nov 12, 2019

medyagh requested changes Jan 13, 2020

View reviewed changes

k8s-ci-robot removed the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jan 14, 2020

medyagh reopened this Jan 22, 2020

rajula96reddy requested a review from medyagh January 27, 2020 06:50

tstromberg suggested changes Feb 5, 2020

View reviewed changes

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

medyagh closed this Mar 7, 2020

medyagh reopened this Mar 7, 2020

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 7, 2020

rajula96reddy added 6 commits March 7, 2020 23:47

Updated error handling when service or namespace doesn't exist

5a32eb7

Formatted the error output

061b159

Modified the previous commit using Erros.New() function

8b22898

Created new error type & made corresponding changes

40c1c80

Modified 'checking if service exists before retry' logic

5d8d3f0

Deleted unnecessary comments

3ffdaf3

rajula96reddy force-pushed the issue-5836 branch from 45fb308 to 3ffdaf3 Compare March 7, 2020 18:18

k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed 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. labels Mar 7, 2020

tstromberg approved these changes Mar 21, 2020

View reviewed changes

tstromberg merged commit 6954549 into kubernetes:master Mar 21, 2020

rajula96reddy mentioned this pull request Jun 1, 2020

REQUEST: New membership for rajula96reddy kubernetes/org#1908

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prevent minikube from crashing if namespace or service doesn't exist #5844

Prevent minikube from crashing if namespace or service doesn't exist #5844

rajula96reddy commented Nov 6, 2019 •

edited

Loading

k8s-ci-robot commented Nov 6, 2019

k8s-ci-robot commented Nov 6, 2019

minikube-bot commented Nov 6, 2019

codecov-io commented Nov 6, 2019 •

edited

Loading

rajula96reddy commented Nov 6, 2019

medyagh commented Nov 6, 2019

rajula96reddy commented Nov 6, 2019

medyagh commented Nov 6, 2019

k8s-ci-robot commented Nov 7, 2019

rajula96reddy commented Nov 7, 2019

RA489 commented Nov 12, 2019

minikube-bot commented Nov 12, 2019

medyagh commented Dec 11, 2019

minikube-bot commented Dec 18, 2019

rajula96reddy commented Dec 19, 2019

medyagh commented Dec 21, 2019

medyagh commented Dec 21, 2019

medyagh commented Jan 13, 2020

medyagh left a comment

medyagh Dec 21, 2019

medyagh Jan 13, 2020

medyagh commented Jan 22, 2020

rajula96reddy commented Jan 23, 2020

minikube-pr-bot commented Jan 25, 2020

medyagh commented Jan 28, 2020

rajula96reddy commented Jan 29, 2020

rajula96reddy commented Feb 4, 2020

minikube-pr-bot commented Feb 4, 2020

tstromberg left a comment

minikube-pr-bot commented Feb 7, 2020

medyagh commented Mar 7, 2020

rajula96reddy commented Mar 7, 2020

tstromberg commented Mar 21, 2020

		@@ -77,6 +77,10 @@ var serviceCmd = &cobra.Command{

		urls, err := service.WaitForService(api, namespace, svc, serviceURLTemplate, serviceURLMode, https, wait, interval)

Prevent minikube from crashing if namespace or service doesn't exist #5844

Prevent minikube from crashing if namespace or service doesn't exist #5844

Conversation

rajula96reddy commented Nov 6, 2019 • edited Loading

Old behavior

New behaviour

k8s-ci-robot commented Nov 6, 2019

k8s-ci-robot commented Nov 6, 2019

minikube-bot commented Nov 6, 2019

codecov-io commented Nov 6, 2019 • edited Loading

Codecov Report

rajula96reddy commented Nov 6, 2019

medyagh commented Nov 6, 2019

rajula96reddy commented Nov 6, 2019

medyagh commented Nov 6, 2019

k8s-ci-robot commented Nov 7, 2019

rajula96reddy commented Nov 7, 2019

RA489 commented Nov 12, 2019

minikube-bot commented Nov 12, 2019

medyagh commented Dec 11, 2019

minikube-bot commented Dec 18, 2019

rajula96reddy commented Dec 19, 2019

medyagh commented Dec 21, 2019

medyagh commented Dec 21, 2019

medyagh commented Jan 13, 2020

medyagh left a comment

Choose a reason for hiding this comment

medyagh Dec 21, 2019

Choose a reason for hiding this comment

medyagh Jan 13, 2020

Choose a reason for hiding this comment

medyagh commented Jan 22, 2020

rajula96reddy commented Jan 23, 2020

minikube-pr-bot commented Jan 25, 2020

medyagh commented Jan 28, 2020

rajula96reddy commented Jan 29, 2020

rajula96reddy commented Feb 4, 2020

minikube-pr-bot commented Feb 4, 2020

tstromberg left a comment

Choose a reason for hiding this comment

minikube-pr-bot commented Feb 7, 2020

medyagh commented Mar 7, 2020

rajula96reddy commented Mar 7, 2020

tstromberg commented Mar 21, 2020

rajula96reddy commented Nov 6, 2019 •

edited

Loading

codecov-io commented Nov 6, 2019 •

edited

Loading