From de4c36f9371e71fcbc1c5987c21bab8a9216840b Mon Sep 17 00:00:00 2001 From: Lv Jiawei Date: Tue, 14 Apr 2020 17:00:16 +0800 Subject: [PATCH] Show all revisions when run `service describe -v` (#790) * The `kn service describe -v` command shows repetitive revisions, because the revision would be covered by next one. --- CHANGELOG.adoc | 4 ++++ pkg/kn/commands/service/describe.go | 10 +++++----- pkg/kn/commands/service/describe_test.go | 17 ++++++++++++----- 3 files changed, 21 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index 9fa0ad9695..4e873babbe 100644 --- a/CHANGELOG.adoc +++ b/CHANGELOG.adoc @@ -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] diff --git a/pkg/kn/commands/service/describe.go b/pkg/kn/commands/service/describe.go index d304124842..7741d34323 100644 --- a/pkg/kn/commands/service/describe.go +++ b/pkg/kn/commands/service/describe.go @@ -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 } @@ -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 } @@ -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 } @@ -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, diff --git a/pkg/kn/commands/service/describe_test.go b/pkg/kn/commands/service/describe_test.go index 0c7513038a..dc2543e257 100644 --- a/pkg/kn/commands/service/describe_test.go +++ b/pkg/kn/commands/service/describe_test.go @@ -432,11 +432,15 @@ 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{ @@ -444,7 +448,7 @@ func TestServiceDescribeVerbose(t *testing.T) { APIVersion: "serving.knative.dev/v1", }, Items: []servingv1.Revision{ - rev1, rev2, + rev1, rev2, rev3, }, } @@ -452,8 +456,7 @@ func TestServiceDescribeVerbose(t *testing.T) { 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") @@ -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()