Confusing Error Message from kn service describe When Revision Not Available

[danielhelfand]

Bug report

When using kn service describe on a Service where the Revision is not available, the following error message is returned:

resource name may not be empty

It would be helpful to display a more informative error message or display that the Revision is not ready as part of the describe command.

Debugging this locally, this error comes from completeWithLatestRevisions and is returned here:

client/pkg/kn/commands/service/describe.go

Line 119 in c41e9fd

return err

It appears an empty string is being passed to GetRevision causing the error.

Expected behavior

kn service describe should describe the Service without issue. Also, regardless of the root of this issue, there should be more information about why this error message is occurring.

Steps to reproduce the problem

Find a way to create a Service with a Revision stuck in a pending state:

$ kn service list
NAME            URL                                                                                     LATEST
  AGE     CONDITIONS   READY     REASON
helloworld-go   http://helloworld-go.deploy-your-first-application-using-knative-w01-s001.example.com
  6m49s   0 OK / 3     Unknown   RevisionMissing : Configuration "helloworld-go" is waiting for a Revision to b
ecome ready.

Run kn service describe on the service.

kn version

Version:      v0.14.0
Build Date:   2020-04-22 08:56:32
Git Revision: c41e9fd
Supported APIs:
* Serving
  - serving.knative.dev/v1 (knative-serving v0.14.0)
* Eventing
  - sources.knative.dev/v1alpha2 (knative-eventing v0.14.1)
  - eventing.knative.dev/v1alpha1 (knative-eventing v0.14.1)

Knative (serving/eventing) version

Serving: v0.14.0

[rhuss]

Thanks, for the report, good find. We should address this for the next release. Volunteers ?

[danielhelfand]

Would be happy to pick it up if no one is working on it. Not sure if keeping the empty string in any capacity would be worth it? It's coming from LatestReadyRevisionName. Would just skipping the empty string be appropriate?

if revisionsSeen.Has(revisionName) || revisionName == "" {
    continue
}

Edit:

Seems to work if the revisionName is skipped in this case:

$ ./kn service describe helloworld-go -n deploy-your-first-application-using-knative-w01-s001
Name:       helloworld-go
Namespace:  deploy-your-first-application-using-knative-w01-s001
Age:        16h
URL:        http://helloworld-go.deploy-your-first-application-using-knative-w01-s001.example.com

Revisions:  
     ?  helloworld-go-4wr5b (latest created) [1] (16h)
        Image:  gcr.io/knative-samples/helloworld-go (at 5ea96b)

Conditions:  
  OK TYPE                   AGE REASON
  ?? Ready                  16h RevisionMissing
  ?? ConfigurationsReady    16h 
  ?? RoutesReady            16h RevisionMissing