Service describe: information about revision status, tags, latest [created, ready] revisions, and latest traffic (#379)

* Change symbols, add latest and latest ready

* More info on what is latest and tags. Filter annotations and labels.

* Make current tests pass

* More tests

* No longer require Annotations in test

* Add error

* Adjust tests to match adjusted output

* Respond to review comments

* one more

* Fix double printing of revisions + test for it

* Differentiate between latest traffic and latest ready w/o that being reason for traffic

* Re-build with go 1.12

* Rebased

* Limit non-verbose output to error and image

* Fix tests for new format

docs/cmd/kn_plugin.md

@@ -4,10 +4,9 @@ Plugin command group
 
 ### Synopsis
 
-Provides utilities for interacting and managing with `kn` plugins.
+Provides utilities for interacting and managing with kn plugins.
-
-Plugins provide extended functionality that is not part of the core `kn` command-line distribution.
 
+Plugins provide extended functionality that is not part of the core kn command-line distribution.
 Please refer to the documentation and examples for more information about how write your own plugins.
 
 ```
@@ -34,3 +33,4 @@ kn plugin [flags]
 
 * [kn](kn.md)	 - Knative client
 * [kn plugin list](kn_plugin_list.md)	 - List plugins
+

pkg/kn/commands/service/describe.go

@@ -24,6 +24,7 @@ import (
 	"strings"
 	"time"
 
+	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/cli-runtime/pkg/genericclioptions"
 	"knative.dev/serving/pkg/apis/autoscaling"
 	"knative.dev/serving/pkg/apis/serving"
@@ -66,9 +67,12 @@ type revisionDesc struct {
 	configurationGeneration int
 	creationTimestamp       time.Time
 
-	percent int
+	// traffic stuff
-	latest  *bool
+	percent       int
+	tag           string
+	latestTraffic *bool
 
+	// basic revision stuff
 	logURL         string
 	timeoutSeconds *int64
 
@@ -89,6 +93,12 @@ type revisionDesc struct {
 	requestsCPU    string
 	limitsMemory   string
 	limitsCPU      string
+
+	// status info
+	ready         corev1.ConditionStatus
+	reason        string
+	latestCreated bool
+	latestReady   bool
 }
 
 // [REMOVE COMMENT WHEN MOVING TO 0.7.0]
@@ -151,7 +161,7 @@ func NewServiceDescribeCommand(p *commands.KnParams) *cobra.Command {
 				return err
 			}
 
-			return describe(cmd.OutOrStdout(), service, revisionDescs)
+			return describe(cmd.OutOrStdout(), service, revisionDescs, printDetails)
 		},
 	}
 	flags := command.Flags()
@@ -162,7 +172,7 @@ func NewServiceDescribeCommand(p *commands.KnParams) *cobra.Command {
 }
 
 // Main action describing the service
-func describe(w io.Writer, service *v1alpha1.Service, revisions []*revisionDesc) error {
+func describe(w io.Writer, service *v1alpha1.Service, revisions []*revisionDesc, printDetails bool) error {
 	dw := printers.NewPrefixWriter(w)
 
 	// Service info
@@ -173,7 +183,7 @@ func describe(w io.Writer, service *v1alpha1.Service, revisions []*revisionDesc)
 	}
 
 	// Revisions summary info
-	writeRevisions(dw, revisions)
+	writeRevisions(dw, revisions, printDetails)
 	dw.WriteLine()
 	if err := dw.Flush(); err != nil {
 		return err
@@ -205,29 +215,34 @@ func writeService(dw printers.PrefixWriter, service *v1alpha1.Service) {
 // Write out revisions associated with this service. By default only active
 // target revisions are printed, but with --all also inactive revisions
 // created by this services are shown
-func writeRevisions(dw printers.PrefixWriter, revisions []*revisionDesc) {
+func writeRevisions(dw printers.PrefixWriter, revisions []*revisionDesc, printDetails bool) {
 	dw.WriteColsLn(printers.Level0, l("Revisions"))
 	for _, revisionDesc := range revisions {
-		dw.WriteColsLn(printers.Level1, formatPercentage(revisionDesc.percent), l("Name"), getRevisionNameWithGenerationAndAge(revisionDesc))
+		dw.WriteColsLn(printers.Level1, formatBullet(revisionDesc.percent, revisionDesc.ready), revisionHeader(revisionDesc))
+		if revisionDesc.ready == v1.ConditionFalse {
+			dw.WriteColsLn(printers.Level1, "", l("Error"), revisionDesc.reason)
+		}
 		dw.WriteColsLn(printers.Level1, "", l("Image"), getImageDesc(revisionDesc))
-		if revisionDesc.port != nil {
+		if printDetails {
-			dw.WriteColsLn(printers.Level1, "", l("Port"), strconv.FormatInt(int64(*revisionDesc.port), 10))
+			if revisionDesc.port != nil {
-		}
+				dw.WriteColsLn(printers.Level1, "", l("Port"), strconv.FormatInt(int64(*revisionDesc.port), 10))
-		writeSliceDesc(dw, printers.Level1, revisionDesc.env, l("Env"), "\t")
+			}
+			writeSliceDesc(dw, printers.Level1, revisionDesc.env, l("Env"), "\t")
 
-		// Scale spec if given
+			// Scale spec if given
-		if revisionDesc.maxScale != nil || revisionDesc.minScale != nil {
+			if revisionDesc.maxScale != nil || revisionDesc.minScale != nil {
-			dw.WriteColsLn(printers.Level1, "", l("Scale"), formatScale(revisionDesc.minScale, revisionDesc.maxScale))
+				dw.WriteColsLn(printers.Level1, "", l("Scale"), formatScale(revisionDesc.minScale, revisionDesc.maxScale))
-		}
+			}
 
-		// Concurrency specs if given
+			// Concurrency specs if given
-		if revisionDesc.concurrencyLimit != nil || revisionDesc.concurrencyTarget != nil {
+			if revisionDesc.concurrencyLimit != nil || revisionDesc.concurrencyTarget != nil {
-			writeConcurrencyOptions(dw, revisionDesc)
+				writeConcurrencyOptions(dw, revisionDesc)
-		}
+			}
 
-		// Resources if given
+			// Resources if given
-		writeResources(dw, "Memory", revisionDesc.requestsMemory, revisionDesc.limitsMemory)
+			writeResources(dw, "Memory", revisionDesc.requestsMemory, revisionDesc.limitsMemory)
-		writeResources(dw, "CPU", revisionDesc.requestsCPU, revisionDesc.limitsCPU)
+			writeResources(dw, "CPU", revisionDesc.requestsCPU, revisionDesc.limitsCPU)
+		}
 	}
 }
 
@@ -249,12 +264,12 @@ func writeConditions(dw printers.PrefixWriter, service *v1alpha1.Service) {
 }
 
 func writeConcurrencyOptions(dw printers.PrefixWriter, desc *revisionDesc) {
-	dw.WriteColsLn(printers.Level1, "", l("Concurrency"))
+	dw.WriteColsLn(printers.Level2, "", l("Concurrency"))
 	if desc.concurrencyLimit != nil {
-		dw.WriteColsLn(printers.Level2, "", "", l("Limit"), strconv.FormatInt(*desc.concurrencyLimit, 10))
+		dw.WriteColsLn(printers.Level3, "", "", l("Limit"), strconv.FormatInt(*desc.concurrencyLimit, 10))
 	}
 	if desc.concurrencyTarget != nil {
-		dw.WriteColsLn(printers.Level2, "", "", l("Target"), strconv.Itoa(*desc.concurrencyTarget))
+		dw.WriteColsLn(printers.Level3, "", "", l("Target"), strconv.Itoa(*desc.concurrencyTarget))
 	}
 }
 
@@ -284,8 +299,19 @@ func formatScale(minScale *int, maxScale *int) string {
 }
 
 // Format the revision name along with its generation. Use colors if enabled.
-func getRevisionNameWithGenerationAndAge(desc *revisionDesc) string {
+func revisionHeader(desc *revisionDesc) string {
-	return desc.name + " " +
+	header := desc.name
+	if desc.latestTraffic != nil && *desc.latestTraffic {
+		header = fmt.Sprintf("@latest (%s)", desc.name)
+	} else if desc.latestReady {
+		header = desc.name + " (current @latest)"
+	} else if desc.latestCreated {
+		header = desc.name + " (latest created)"
+	}
+	if desc.tag != "" {
+		header = fmt.Sprintf("%s #%s", header, desc.tag)
+	}
+	return header + " " +
 		"[" + strconv.Itoa(desc.configurationGeneration) + "]" +
 		" " +
 		"(" + age(desc.creationTimestamp) + ")"
@@ -314,9 +340,9 @@ func formatStatus(status corev1.ConditionStatus) string {
 	case v1.ConditionTrue:
 		return "++"
 	case v1.ConditionFalse:
-		return "--"
+		return "!!"
 	default:
-		return ""
+		return "??"
 	}
 }
 
@@ -325,7 +351,7 @@ func getImageDesc(desc *revisionDesc) string {
 	image := desc.image
 	// Check if the user image is likely a more user-friendly description
 	pinnedDesc := "at"
-	if desc.userImage != "" && strings.Contains(image, "@") && desc.imageDigest != "" {
+	if desc.userImage != "" && desc.imageDigest != "" {
 		parts := strings.Split(image, "@")
 		// Check if the user image refers to the same thing.
 		if strings.HasPrefix(desc.userImage, parts[0]) {
@@ -345,11 +371,17 @@ func getImageDesc(desc *revisionDesc) string {
 func shortenDigest(digest string) string {
 	match := imageDigestRegexp.FindStringSubmatch(digest)
 	if len(match) > 1 {
-		return string(match[1][:12])
+		return string(match[1][:6])
 	}
 	return digest
 }
 
+var boringDomains = map[string]bool{
+	"serving.knative.dev":   true,
+	"client.knative.dev":    true,
+	"kubectl.kubernetes.io": true,
+}
+
 // Write a map either compact in a single line (possibly truncated) or, if printDetails is set,
 // over multiple line, one line per key-value pair. The output is sorted by keys.
 func writeMapDesc(dw printers.PrefixWriter, indent int, m map[string]string, label string, labelPrefix string) {
@@ -359,7 +391,13 @@ func writeMapDesc(dw printers.PrefixWriter, indent int, m map[string]string, lab
 
 	var keys []string
 	for k := range m {
-		keys = append(keys, k)
+		parts := strings.Split(k, "/")
+		if printDetails || len(parts) <= 1 || !boringDomains[parts[0]] {
+			keys = append(keys, k)
+		}
+	}
+	if len(keys) == 0 {
+		return
 	}
 	sort.Strings(keys)
 
@@ -415,7 +453,7 @@ func writeResources(dw printers.PrefixWriter, label string, request string, limi
 		return
 	}
 
-	dw.WriteColsLn(printers.Level1, "", l(label), value)
+	dw.WriteColsLn(printers.Level2, "", l(label), value)
 }
 
 // Join to key=value pair, comma separated, and truncate if longer than a limit
@@ -436,11 +474,22 @@ func joinAndTruncate(sortedKeys []string, m map[string]string) string {
 }
 
 // Format target percentage that it fits in the revision table
-func formatPercentage(percentage int) string {
+func formatBullet(percentage int, status corev1.ConditionStatus) string {
-	if percentage == 0 {
+	symbol := "+"
-		return "   -"
+	switch status {
+	case v1.ConditionTrue:
+		if percentage > 0 {
+			symbol = "%"
+		}
+	case v1.ConditionFalse:
+		symbol = "!"
+	default:
+		symbol = "?"
 	}
-	return fmt.Sprintf("%3d%%", percentage)
+	if percentage == 0 {
+		return fmt.Sprintf("   %s", symbol)
+	}
+	return fmt.Sprintf("%3d%s", percentage, symbol)
 }
 
 func age(t time.Time) string {
@@ -453,22 +502,28 @@ func age(t time.Time) string {
 // Call the backend to query revisions for the given service and build up
 // the view objects used for output
 func getRevisionDescriptions(client serving_kn_v1alpha1.KnServingClient, service *v1alpha1.Service, withDetails bool) ([]*revisionDesc, error) {
-	revisionDescs := make(map[string]*revisionDesc)
+	revisionsSeen := sets.NewString()
+	revisionDescs := []*revisionDesc{}
 
 	trafficTargets := service.Status.Traffic
-
+	var err error
 	for _, target := range trafficTargets {
 		revision, err := extractRevisionFromTarget(client, target)
 		if err != nil {
 			return nil, fmt.Errorf("cannot extract revision from service %s: %v", service.Name, err)
 		}
-		revisionDescs[revision.Name], err = newRevisionDesc(revision, &target)
+		revisionsSeen.Insert(revision.Name)
+		desc, err := newRevisionDesc(revision, &target, service)
 		if err != nil {
 			return nil, err
 		}
+		revisionDescs = append(revisionDescs, desc)
+	}
+	if revisionDescs, err = completeWithLatestRevisions(client, service, revisionsSeen, revisionDescs); err != nil {
+		return nil, err
 	}
 	if withDetails {
-		if err := completeWithUntargetedRevisions(client, service, revisionDescs); err != nil {
+		if revisionDescs, err = completeWithUntargetedRevisions(client, service, revisionsSeen, revisionDescs); err != nil {
 			return nil, err
 		}
 	}
@@ -476,36 +531,53 @@ func getRevisionDescriptions(client serving_kn_v1alpha1.KnServingClient, service
 }
 
 // Order the list of revisions so that the newest revisions are at the top
-func orderByConfigurationGeneration(descs map[string]*revisionDesc) []*revisionDesc {
+func orderByConfigurationGeneration(descs []*revisionDesc) []*revisionDesc {
-	descsList := make([]*revisionDesc, len(descs))
+	sort.SliceStable(descs, func(i, j int) bool {
-	idx := 0
+		return descs[i].configurationGeneration > descs[j].configurationGeneration
-	for _, desc := range descs {
-		descsList[idx] = desc
-		idx++
-	}
-	sort.SliceStable(descsList, func(i, j int) bool {
-		return descsList[i].configurationGeneration > descsList[j].configurationGeneration
 	})
-	return descsList
+	return descs
 }
 
-func completeWithUntargetedRevisions(client serving_kn_v1alpha1.KnServingClient, service *v1alpha1.Service, descs map[string]*revisionDesc) error {
+func completeWithLatestRevisions(client serving_kn_v1alpha1.KnServingClient, service *v1alpha1.Service, revisionsSeen sets.String, descs []*revisionDesc) ([]*revisionDesc, error) {
+	for _, revisionName := range []string{service.Status.LatestCreatedRevisionName, service.Status.LatestReadyRevisionName} {
+		if revisionsSeen.Has(revisionName) {
+			continue
+		}
+		revisionsSeen.Insert(revisionName)
+		rev, err := client.GetRevision(revisionName)
+		if err != nil {
+			return nil, err
+		}
+		newDesc, err := newRevisionDesc(rev, nil, service)
+		if err != nil {
+			return nil, err
+		}
+		descs = append(descs, newDesc)
+	}
+	return descs, nil
+}
+
+func completeWithUntargetedRevisions(client serving_kn_v1alpha1.KnServingClient, service *v1alpha1.Service, revisionsSeen sets.String, descs []*revisionDesc) ([]*revisionDesc, error) {
 	revisions, err := client.ListRevisions(serving_kn_v1alpha1.WithService(service.Name))
 	if err != nil {
-		return err
+		return nil, err
 	}
 	for _, revision := range revisions.Items {
-		if _, ok := descs[revision.Name]; !ok {
+		if revisionsSeen.Has(revision.Name) {
-			descs[revision.Name], err = newRevisionDesc(&revision, nil)
+			continue
-			if err != nil {
-				return err
-			}
 		}
+		revisionsSeen.Insert(revision.Name)
+		newDesc, err := newRevisionDesc(&revision, nil, service)
+		if err != nil {
+			return nil, err
+		}
+		descs = append(descs, newDesc)
+
 	}
-	return nil
+	return descs, nil
 }
 
-func newRevisionDesc(revision *v1alpha1.Revision, target *v1alpha1.TrafficTarget) (*revisionDesc, error) {
+func newRevisionDesc(revision *v1alpha1.Revision, target *v1alpha1.TrafficTarget, service *v1alpha1.Service) (*revisionDesc, error) {
 	container := extractContainer(revision)
 	generation, err := strconv.ParseInt(revision.Labels[serving.ConfigurationGenerationLabelKey], 0, 0)
 	if err != nil {
@@ -521,8 +593,12 @@ func newRevisionDesc(revision *v1alpha1.Revision, target *v1alpha1.TrafficTarget
 
 		configurationGeneration: int(generation),
 		configuration:           revision.Labels[serving.ConfigurationLabelKey],
+
+		latestCreated: revision.Name == service.Status.LatestCreatedRevisionName,
+		latestReady:   revision.Name == service.Status.LatestReadyRevisionName,
 	}
 
+	addStatusInfo(&revisionDesc, revision)
 	addTargetInfo(&revisionDesc, target)
 	addContainerInfo(&revisionDesc, container)
 	addResourcesInfo(&revisionDesc, container)
@@ -533,10 +609,20 @@ func newRevisionDesc(revision *v1alpha1.Revision, target *v1alpha1.TrafficTarget
 	return &revisionDesc, nil
 }
 
+func addStatusInfo(desc *revisionDesc, revision *v1alpha1.Revision) {
+	for _, condition := range revision.Status.Conditions {
+		if condition.Type == "Ready" {
+			desc.reason = condition.Reason
+			desc.ready = condition.Status
+		}
+	}
+}
+
 func addTargetInfo(desc *revisionDesc, target *v1alpha1.TrafficTarget) {
 	if target != nil {
 		desc.percent = target.Percent
-		desc.latest = target.LatestRevision
+		desc.latestTraffic = target.LatestRevision
+		desc.tag = target.Tag
 	}
 }
 

pkg/kn/commands/service/describe_test.go

@@ -37,6 +37,7 @@ import (
 	client_serving "knative.dev/client/pkg/serving"
 	knclient "knative.dev/client/pkg/serving/v1alpha1"
 	"knative.dev/client/pkg/util"
+	"knative.dev/pkg/ptr"
 )
 
 const (
@@ -63,14 +64,177 @@ func TestServiceDescribeBasic(t *testing.T) {
 	assert.NilError(t, err)
 
 	validateServiceOutput(t, "foo", output)
-	assert.Assert(t, util.ContainsAll(output, "Env:", "label1=lval1, label2=lval2\n"))
+	assert.Assert(t, util.ContainsAll(output, "123456"))
-	assert.Assert(t, util.ContainsAll(output, "1234567"))
 	assert.Assert(t, util.ContainsAll(output, "Annotations:", "anno1=aval1, anno2=aval2, anno3="))
 	assert.Assert(t, cmp.Regexp(`(?m)\s*Annotations:.*\.\.\.$`, output))
 	assert.Assert(t, util.ContainsAll(output, "Labels:", "label1=lval1, label2=lval2\n"))
 	assert.Assert(t, util.ContainsAll(output, "[1]"))
-	// no digest added (added only for details)
+
-	assert.Assert(t, !strings.Contains(output, "(123456789012)"))
+	assert.Equal(t, strings.Count(output, "rev1"), 1)
+
+	// Validate that all recorded API methods have been called
+	r.Validate()
+}
+
+func TestServiceDescribeSad(t *testing.T) {
+	client := knclient.NewMockKnClient(t)
+	r := client.Recorder()
+
+	expectedService := createTestService("foo", []string{"rev1"}, goodConditions())
+	expectedService.Status.Conditions[0].Status = v1.ConditionFalse
+	r.GetService("foo", &expectedService, nil)
+	rev1 := createTestRevision("rev1", 1)
+	r.GetRevision("rev1", &rev1, nil)
+
+	output, err := executeServiceCommand(client, "describe", "foo")
+	assert.NilError(t, err)
+	validateServiceOutput(t, "foo", output)
+	assert.Assert(t, util.ContainsAll(output, "!!", "Ready"))
+
+	r.Validate()
+}
+
+func TestServiceDescribeLatest(t *testing.T) {
+
+	// New mock client
+	client := knclient.NewMockKnClient(t)
+	r := client.Recorder()
+
+	expectedService := createTestService("foo", []string{"rev1"}, goodConditions())
+	expectedService.Status.Traffic[0].LatestRevision = ptr.Bool(true)
+
+	// Get service & revision
+	r.GetService("foo", &expectedService, nil)
+	rev1 := createTestRevision("rev1", 1)
+	r.GetRevision("rev1", &rev1, nil)
+
+	output, err := executeServiceCommand(client, "describe", "foo")
+	assert.NilError(t, err)
+	validateServiceOutput(t, "foo", output)
+	assert.Assert(t, util.ContainsAll(output, "@latest (rev1)"))
+
+	// Validate that all recorded API methods have been called
+	r.Validate()
+}
+
+func TestServiceDescribeLatestNotInTraffic(t *testing.T) {
+
+	// New mock client
+	client := knclient.NewMockKnClient(t)
+
+	// Recording:
+	r := client.Recorder()
+	// Prepare service
+	expectedService := createTestService("foo", []string{"rev1", "rev2"}, goodConditions())
+	expectedService.Status.Traffic = expectedService.Status.Traffic[:1]
+	expectedService.Status.Traffic[0].LatestRevision = ptr.Bool(false)
+	expectedService.Status.Traffic[0].Percent = 100
+
+	// Get service & revision
+	r.GetService("foo", &expectedService, nil)
+	rev1 := createTestRevision("rev1", 1)
+	rev2 := createTestRevision("rev2", 2)
+	r.GetRevision("rev1", &rev1, nil)
+	r.GetRevision("rev2", &rev2, nil)
+
+	// Testing:
+	output, err := executeServiceCommand(client, "describe", "foo")
+	assert.NilError(t, err)
+
+	validateServiceOutput(t, "foo", output)
+	assert.Assert(t, util.ContainsAll(output, "rev2 (current @latest)"))
+
+	// Validate that all recorded API methods have been called
+	r.Validate()
+}
+
+func TestServiceDescribeEachNamedOnce(t *testing.T) {
+
+	// New mock client
+	client := knclient.NewMockKnClient(t)
+
+	// Recording:
+	r := client.Recorder()
+	// Prepare service
+	expectedService := createTestService("foo", []string{"rev1", "rev2"}, goodConditions())
+	expectedService.Status.Traffic = expectedService.Status.Traffic[:1]
+	expectedService.Status.Traffic[0].LatestRevision = ptr.Bool(false)
+	expectedService.Status.Traffic[0].Percent = 100
+
+	// Get service & revision
+	r.GetService("foo", &expectedService, nil)
+	rev1 := createTestRevision("rev1", 1)
+	rev2 := createTestRevision("rev2", 2)
+	r.GetRevision("rev1", &rev1, nil)
+	r.GetRevision("rev2", &rev2, nil)
+
+	// Testing:
+	output, err := executeServiceCommand(client, "describe", "foo")
+	assert.NilError(t, err)
+
+	validateServiceOutput(t, "foo", output)
+	assert.Assert(t, util.ContainsAll(output, "rev1", "rev2"))
+	assert.Equal(t, strings.Count(output, "rev2"), 1)
+	assert.Equal(t, strings.Count(output, "rev1"), 1)
+
+	// Validate that all recorded API methods have been called
+	r.Validate()
+}
+
+func TestServiceDescribeLatestAndCurrentBothHaveTrafficEntries(t *testing.T) {
+	// New mock client
+	client := knclient.NewMockKnClient(t)
+
+	// Recording:
+	r := client.Recorder()
+	// Prepare service
+	expectedService := createTestService("foo", []string{"rev1", "rev1"}, goodConditions())
+	expectedService.Status.Traffic[0].LatestRevision = ptr.Bool(true)
+	expectedService.Status.Traffic[0].Tag = "latest"
+	expectedService.Status.Traffic[1].Tag = "current"
+
+	// Get service & revision
+	r.GetService("foo", &expectedService, nil)
+	rev1 := createTestRevision("rev1", 1)
+	r.GetRevision("rev1", &rev1, nil)
+	r.GetRevision("rev1", &rev1, nil)
+
+	// Testing:
+	output, err := executeServiceCommand(client, "describe", "foo")
+	assert.NilError(t, err)
+
+	validateServiceOutput(t, "foo", output)
+	assert.Assert(t, util.ContainsAll(output, "@latest (rev1) #latest", "rev1 (current @latest) #current", "50%"))
+
+	// Validate that all recorded API methods have been called
+	r.Validate()
+}
+
+func TestServiceDescribeLatestCreatedIsBroken(t *testing.T) {
+	// New mock client
+	client := knclient.NewMockKnClient(t)
+
+	// Recording:
+	r := client.Recorder()
+	// Prepare service
+	expectedService := createTestService("foo", []string{"rev1"}, goodConditions())
+	expectedService.Status.Traffic[0].LatestRevision = ptr.Bool(true)
+	expectedService.Status.LatestCreatedRevisionName = "rev2"
+
+	// Get service & revision
+	r.GetService("foo", &expectedService, nil)
+	rev1 := createTestRevision("rev1", 1)
+	rev2 := createTestRevision("rev2", 2)
+	rev2.Status.Conditions[0].Status = v1.ConditionFalse
+	r.GetRevision("rev1", &rev1, nil)
+	r.GetRevision("rev2", &rev2, nil)
+
+	// Testing:
+	output, err := executeServiceCommand(client, "describe", "foo")
+	assert.NilError(t, err)
+
+	validateServiceOutput(t, "foo", output)
+	assert.Assert(t, util.ContainsAll(output, "!", "rev2", "100%", "@latest (rev1)"))
 
 	// Validate that all recorded API methods have been called
 	r.Validate()
@@ -238,8 +402,8 @@ func TestServiceDescribeUserImageVsImage(t *testing.T) {
 
 	validateServiceOutput(t, "foo", output)
 
-	assert.Assert(t, util.ContainsAll(output, "Image", "Name", "gcr.io/test/image:latest (at 123456789012)",
+	assert.Assert(t, util.ContainsAll(output, "Image", "Name",
-		"gcr.io/test/image:latest (pinned to 123456789012)", "gcr.io/a/b (at 123456789012)", "gcr.io/x/y"))
+		"gcr.io/test/image:latest (pinned to 123456)", "gcr.io/a/b (at 123456)", "gcr.io/x/y"))
 	assert.Assert(t, util.ContainsAll(output, "[1]", "[2]"))
 
 	// Validate that all recorded API methods have been called
@@ -285,7 +449,7 @@ func TestServiceDescribeVerbose(t *testing.T) {
 
 	validateServiceOutput(t, "foo", output)
 
-	assert.Assert(t, util.ContainsAll(output, "Image", "Name", "gcr.io/test/image (at 123456789012)", "50%", "(0s)"))
+	assert.Assert(t, util.ContainsAll(output, "Image", "Name", "gcr.io/test/image (at 123456)", "50%", "(0s)"))
 	assert.Assert(t, util.ContainsAll(output, "Env:", "label1=lval1\n", "label2=lval2\n"))
 	assert.Assert(t, util.ContainsAll(output, "Annotations:", "anno1=aval1\n", "anno2=aval2\n"))
 	assert.Assert(t, util.ContainsAll(output, "Labels:", "label1=lval1\n", "label2=lval2\n"))
@@ -327,7 +491,7 @@ func validateServiceOutput(t *testing.T, service string, output string) {
 	assert.Assert(t, cmp.Regexp("Address:\\s+http://"+service+".default.svc.cluster.local", output))
 	assert.Assert(t, cmp.Regexp("URL:\\s+"+service+".default.example.com", output))
 
-	assert.Assert(t, util.ContainsAll(output, "Age:", "Revisions:", "Conditions:", "Labels:", "Annotations:", "Port:", "8080"))
+	assert.Assert(t, util.ContainsAll(output, "Age:", "Revisions:", "Conditions:", "Labels:", "Annotations:"))
 	assert.Assert(t, util.ContainsAll(output, "Ready", "RoutesReady", "OK", "TYPE", "AGE", "REASON"))
 }
 
@@ -363,6 +527,9 @@ func createTestService(name string, revisionNames []string, conditions duckv1bet
 			},
 		},
 	}
+	service.Status.LatestCreatedRevisionName = revisionNames[len(revisionNames)-1]
+	service.Status.LatestReadyRevisionName = revisionNames[len(revisionNames)-1]
+
 	if len(revisionNames) > 0 {
 		trafficTargets := make([]v1alpha1.TrafficTarget, 0)
 		for _, rname := range revisionNames {
@@ -455,6 +622,9 @@ func createTestRevision(revision string, gen int64) v1alpha1.Revision {
 		},
 		Status: v1alpha1.RevisionStatus{
 			ImageDigest: "gcr.io/test/image@" + imageDigest,
+			Status: duckv1beta1.Status{
+				Conditions: goodConditions(),
+			},
 		},
 	}
 }

test/e2e/basic_workflow_test.go

@@ -99,7 +99,7 @@ func (test *e2eTest) serviceDescribe(t *testing.T, serviceName string) {
 
 	assert.Assert(t, util.ContainsAll(out, serviceName, test.kn.namespace, KnDefaultTestImage))
 	assert.Assert(t, util.ContainsAll(out, "Conditions", "ConfigurationsReady", "Ready", "RoutesReady"))
-	assert.Assert(t, util.ContainsAll(out, "Name", "Namespace", "URL", "Address", "Annotations", "Age", "Revisions"))
+	assert.Assert(t, util.ContainsAll(out, "Name", "Namespace", "URL", "Address", "Age", "Revisions"))
 }
 
 func (test *e2eTest) serviceUpdate(t *testing.T, serviceName string, args []string) {

vendor/modules.txt

@@ -186,9 +186,9 @@ k8s.io/apimachinery/pkg/util/duration
 k8s.io/apimachinery/pkg/apis/meta/v1beta1
 k8s.io/apimachinery/pkg/runtime
 k8s.io/apimachinery/pkg/api/resource
+k8s.io/apimachinery/pkg/util/sets
 k8s.io/apimachinery/pkg/api/meta
 k8s.io/apimachinery/pkg/util/runtime
-k8s.io/apimachinery/pkg/util/sets
 k8s.io/apimachinery/pkg/fields
 k8s.io/apimachinery/pkg/labels
 k8s.io/apimachinery/pkg/runtime/schema