Skip to content
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.

Sign up for GitHub

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

Improving error message for service not in default namespace #5312

Closed
wants to merge 1 commit into from
Closed

Improving error message for service not in default namespace #5312

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkg/minikube/service/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ func WaitAndMaybeOpenService(api libmachine.API, namespace string, service strin
chkSVC := func() error { return CheckService(namespace, service) }

if err := retry.Expo(chkSVC, time.Duration(interval)*time.Second, time.Duration(wait)*time.Second); err != nil {
return errors.Wrapf(err, "Could not find finalized endpoint being pointed to by %s", service)
return errors.Wrapf(err, "Service %s was not found in default namespace, please try again with 'minikube service %s -n %s'", service, service, namespace)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,
I'm not the core reviewer, but I believe it's not the best solution.
function CheckService
looks like:

  212 // CheckService checks if a service is listening on a port.                                                             
  213 func CheckService(namespace string, service string) error {                                                             
  214         client, err := K8s.GetCoreClient()                                                                              
  215         if err != nil {                                                                                                 
  216                 return errors.Wrap(err, "Error getting kubernetes client")                                              
  217         }                                                                                                               
  218                                                                                                                         
  219         svc, err := client.Services(namespace).Get(service, meta.GetOptions{})                                          
  220         if err != nil {                                                                                                 
  221                 return &retry.RetriableError{                                                                           
  222                         Err: errors.Wrapf(err, "Error getting service %s", service),                                    
  223                 }                                                                                                       
  224         }                                                                                                               
  225         if len(svc.Spec.Ports) == 0 {                                                                                   
  226                 return fmt.Errorf("%s:%s has no ports", namespace, service)                                             
  227         }                                                                                                               
  228         glog.Infof("Found service: %+v", svc)                                                                           
  229         return nil                                                                                                      
  230 }

what if error was returned due to no ports?
maybe before printing error about default namespace we should check if namespace == "default" and embed error msg from CheckService

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @n0npax ! Thanks, I'll take another look at this today!

}

urls, err := GetServiceURLsForService(api, namespace, service, urlTemplate)
Expand Down