Skip to content

Commit

Permalink
Show all revisions when run service describe -v (#790)
Browse files Browse the repository at this point in the history
* The `kn service describe -v` command shows repetitive revisions, because
  the revision would be covered by next one.
  • Loading branch information
MIBc authored Apr 14, 2020
1 parent b7a2a09 commit de4c36f
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 10 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@
|===
| | Description | PR

| 🐛
| Service describe command shows repetitive revisions
| https://github.com/knative/client/pull/790[#790]

| 🎁
| Add concurrency-utilization option for `service create` and `service update` (`--concurrency-utilization`)
| https://github.com/knative/client/pull/788[#788]
Expand Down
10 changes: 5 additions & 5 deletions pkg/kn/commands/service/describe.go
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ func getRevisionDescriptions(client clientservingv1.KnServingClient, service *se
return nil, fmt.Errorf("cannot extract revision from service %s: %v", service.Name, err)
}
revisionsSeen.Insert(revision.Name)
desc, err := newRevisionDesc(revision, &target, service)
desc, err := newRevisionDesc(*revision, &target, service)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -302,7 +302,7 @@ func completeWithLatestRevisions(client clientservingv1.KnServingClient, service
if err != nil {
return nil, err
}
newDesc, err := newRevisionDesc(rev, nil, service)
newDesc, err := newRevisionDesc(*rev, nil, service)
if err != nil {
return nil, err
}
Expand All @@ -321,7 +321,7 @@ func completeWithUntargetedRevisions(client clientservingv1.KnServingClient, ser
continue
}
revisionsSeen.Insert(revision.Name)
newDesc, err := newRevisionDesc(&revision, nil, service)
newDesc, err := newRevisionDesc(revision, nil, service)
if err != nil {
return nil, err
}
Expand All @@ -331,13 +331,13 @@ func completeWithUntargetedRevisions(client clientservingv1.KnServingClient, ser
return descs, nil
}

func newRevisionDesc(revision *servingv1.Revision, target *servingv1.TrafficTarget, service *servingv1.Service) (*revisionDesc, error) {
func newRevisionDesc(revision servingv1.Revision, target *servingv1.TrafficTarget, service *servingv1.Service) (*revisionDesc, error) {
generation, err := strconv.ParseInt(revision.Labels[serving.ConfigurationGenerationLabelKey], 0, 0)
if err != nil {
return nil, fmt.Errorf("cannot extract configuration generation for revision %s: %v", revision.Name, err)
}
revisionDesc := revisionDesc{
revision: revision,
revision: &revision,
configurationGeneration: int(generation),
latestCreated: revision.Name == service.Status.LatestCreatedRevisionName,
latestReady: revision.Name == service.Status.LatestReadyRevisionName,
Expand Down
17 changes: 12 additions & 5 deletions pkg/kn/commands/service/describe_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -432,28 +432,31 @@ func TestServiceDescribeVerbose(t *testing.T) {
r := client.Recorder()

// Prepare service
expectedService := createTestService("foo", []string{"rev1", "rev2"}, goodConditions())
expectedService := createTestService("foo", []string{"rev1", "rev2", "rev3"}, goodConditions())
expectedService.Status.Traffic = expectedService.Status.Traffic[2:]
expectedService.Status.Traffic[0].LatestRevision = ptr.Bool(true)
expectedService.Status.Traffic[0].Percent = ptr.Int64(int64(100))
r.GetService("foo", &expectedService, nil)

rev1 := createTestRevision("rev1", 1)
rev2 := createTestRevision("rev2", 2)
rev3 := createTestRevision("rev3", 3)

revList := servingv1.RevisionList{
TypeMeta: metav1.TypeMeta{
Kind: "RevisionList",
APIVersion: "serving.knative.dev/v1",
},
Items: []servingv1.Revision{
rev1, rev2,
rev1, rev2, rev3,
},
}

// Return the list of all revisions
r.ListRevisions(knclient.HasLabelSelector(api_serving.ServiceLabelKey, "foo"), &revList, nil)

// Fetch the revisions
r.GetRevision("rev1", &rev1, nil)
r.GetRevision("rev2", &rev2, nil)
r.GetRevision("rev3", &rev3, nil)

// Testing:
output, err := executeServiceCommand(client, "describe", "foo", "--verbose")
Expand All @@ -462,12 +465,16 @@ func TestServiceDescribeVerbose(t *testing.T) {
validateServiceOutput(t, "foo", output)

assert.Assert(t, cmp.Regexp("Cluster:\\s+https://foo.default.svc.cluster.local", output))
assert.Assert(t, util.ContainsAll(output, "Image", "Name", "gcr.io/test/image (at 123456)", "50%", "(0s)"))
assert.Assert(t, util.ContainsAll(output, "Image", "Name", "gcr.io/test/image (at 123456)", "100%", "(0s)"))
assert.Assert(t, util.ContainsAll(output, "Env:", "env1=eval1\n", "env2=eval2\n"))
assert.Assert(t, util.ContainsAll(output, "EnvFrom:", "cm:test1\n", "cm:test2\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"))
assert.Assert(t, util.ContainsAll(output, "[1]", "[2]"))
assert.Assert(t, util.ContainsAll(output, "rev1", "rev2", "rev3"))
assert.Equal(t, strings.Count(output, "rev3"), 1)
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()
Expand Down

0 comments on commit de4c36f

Please sign in to comment.