add --prune & --prune-all options to delete all the unreferenced revisions (#1217)

CHANGELOG.adoc

@@ -17,6 +17,10 @@
 |===
 | | Description | PR
 
+| 🎁
+| Add `--prune` & `--prune-all` options to delete the unreferenced revisions
+| https://github.com/knative/client/pull/1217[#1217]
+
 | ✨
 | Update CRDs API version to `v1`
 | https://github.com/knative/client/issues/1248[#1248]

docs/cmd/kn_revision_delete.md

@@ -12,6 +12,12 @@ kn revision delete NAME [NAME ...]
 
   # Delete a revision 'svc1-abcde' in default namespace
   kn revision delete svc1-abcde
+
+  # Delete all unreferenced revisions
+  kn revision delete --prune-all
+
+  # Delete all unreferenced revisions for a given service 'mysvc'
+  kn revision delete --prune mysvc
 ```
 
 ### Options
@@ -20,6 +26,8 @@ kn revision delete NAME [NAME ...]
   -h, --help               help for delete
   -n, --namespace string   Specify the namespace to operate in.
       --no-wait            Do not wait for 'revision delete' operation to be completed. (default true)
+      --prune string       Remove unreferenced revisions for a given service in a namespace.
+      --prune-all          Remove all unreferenced revisions in a namespace.
       --wait               Wait for 'revision delete' operation to be completed.
       --wait-timeout int   Seconds to wait before giving up on waiting for revision to be deleted. (default 600)
 ```

lib/test/revision.go

@@ -139,3 +139,17 @@ func RevisionListWithService(r *KnRunResultCollector, serviceNames ...string) {
 		}
 	}
 }
+
+// RevisionDeleteWithPruneOption verifies removeing all unreferenced revisions for a given service in sync mode
+func RevisionDeleteWithPruneOption(r *KnRunResultCollector, serviceName, revName string) {
+	out := r.KnTest().Kn().Run("revision", "delete", "--prune", serviceName)
+	assert.Check(r.T(), util.ContainsAll(out.Stdout, "Revision", "deleted", revName, "namespace", r.KnTest().Kn().Namespace()))
+	r.AssertNoError(out)
+}
+
+// RevisionDeleteWithPruneAllOption verifies removeing all unreferenced revision in sync mode
+func RevisionDeleteWithPruneAllOption(r *KnRunResultCollector, revName1, revName2 string) {
+	out := r.KnTest().Kn().Run("revision", "delete", "--prune-all")
+	assert.Check(r.T(), util.ContainsAll(out.Stdout, "Revision", "deleted", revName1, revName1, "namespace", r.KnTest().Kn().Namespace()))
+	r.AssertNoError(out)
+}

pkg/kn/commands/revision/delete.go

@@ -21,24 +21,39 @@ import (
 	"time"
 
 	"github.com/spf13/cobra"
-
 	"knative.dev/client/pkg/kn/commands"
+	v1 "knative.dev/client/pkg/serving/v1"
+	servingv1 "knative.dev/serving/pkg/apis/serving/v1"
 )
 
 // NewRevisionDeleteCommand represent 'revision delete' command
 func NewRevisionDeleteCommand(p *commands.KnParams) *cobra.Command {
 	var waitFlags commands.WaitFlags
-
+	// prune filter, used with "-p"
+	var pruneFilter string
+	var pruneAll bool
 	RevisionDeleteCommand := &cobra.Command{
 		Use:   "delete NAME [NAME ...]",
 		Short: "Delete revisions",
 		Example: `
   # Delete a revision 'svc1-abcde' in default namespace
-  kn revision delete svc1-abcde`,
+  kn revision delete svc1-abcde
+
+  # Delete all unreferenced revisions
+  kn revision delete --prune-all
+
+  # Delete all unreferenced revisions for a given service 'mysvc'
+  kn revision delete --prune mysvc`,
 		RunE: func(cmd *cobra.Command, args []string) error {
-			if len(args) < 1 {
+			prune := cmd.Flags().Changed("prune")
+			argsLen := len(args)
+			if argsLen < 1 && !pruneAll && !prune {
 				return errors.New("'kn revision delete' requires one or more revision name")
 			}
+			if argsLen > 0 && pruneAll {
+				return errors.New("'kn revision delete' with --prune-all flag requires no arguments")
+			}
+
 			namespace, err := p.GetNamespace(cmd)
 			if err != nil {
 				return err
@@ -47,6 +62,21 @@ func NewRevisionDeleteCommand(p *commands.KnParams) *cobra.Command {
 			if err != nil {
 				return err
 			}
+			// Create list filters
+			var params []v1.ListConfig
+			if prune {
+				params = append(params, v1.WithService(pruneFilter))
+			}
+			if prune || pruneAll {
+				args, err = getUnreferencedRevisionNames(params, client)
+				if err != nil {
+					return err
+				}
+				if len(args) == 0 {
+					fmt.Fprintf(cmd.OutOrStdout(), "No unreferenced revisions found.\n")
+					return nil
+				}
+			}
 
 			errs := []string{}
 			for _, name := range args {
@@ -67,7 +97,27 @@ func NewRevisionDeleteCommand(p *commands.KnParams) *cobra.Command {
 			return nil
 		},
 	}
+	flags := RevisionDeleteCommand.Flags()
+	flags.StringVar(&pruneFilter, "prune", "", "Remove unreferenced revisions for a given service in a namespace.")
+	flags.BoolVar(&pruneAll, "prune-all", false, "Remove all unreferenced revisions in a namespace.")
 	commands.AddNamespaceFlags(RevisionDeleteCommand.Flags(), false)
 	waitFlags.AddConditionWaitFlags(RevisionDeleteCommand, commands.WaitDefaultTimeout, "delete", "revision", "deleted")
 	return RevisionDeleteCommand
 }
+
+// Return unreferenced revision names
+func getUnreferencedRevisionNames(lConfig []v1.ListConfig, client v1.KnServingClient) ([]string, error) {
+	revisionList, err := client.ListRevisions(lConfig...)
+	if err != nil {
+		return []string{}, err
+	}
+	// Sort revisions by namespace, service, generation (in this order)
+	sortRevisions(revisionList)
+	revisionNames := []string{}
+	for _, revision := range revisionList.Items {
+		if revision.GetRoutingState() != servingv1.RoutingStateActive {
+			revisionNames = append(revisionNames, revision.Name)
+		}
+	}
+	return revisionNames, nil
+}

pkg/kn/commands/revision/delete_mock_test.go

@@ -0,0 +1,172 @@
+/*
+Copyright 2020 The Knative Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+package revision
+
+import (
+	"errors"
+	"fmt"
+	"testing"
+
+	"gotest.tools/v3/assert"
+	clientservingv1 "knative.dev/client/pkg/serving/v1"
+	"knative.dev/client/pkg/util"
+	"knative.dev/client/pkg/util/mock"
+	"knative.dev/serving/pkg/apis/serving"
+	servingv1 "knative.dev/serving/pkg/apis/serving/v1"
+)
+
+func TestRevisionDeletePruneAllMock(t *testing.T) {
+	// New mock client
+	client := clientservingv1.NewMockKnServiceClient(t)
+
+	// Recording:
+	r := client.Recorder()
+
+	// Wait for delete event
+	r.DeleteRevision("foo", mock.Any(), nil)
+	r.DeleteRevision("bar", mock.Any(), nil)
+	r.DeleteRevision("baz", mock.Any(), nil)
+
+	revision1 := createMockRevisionWithParams("foo", "svc1", "1", "50", "")
+	revision1.Labels[serving.RoutingStateLabelKey] = string(servingv1.RoutingStateReserve)
+	revision2 := createMockRevisionWithParams("bar", "svc2", "1", "50", "")
+	revision2.Labels[serving.RoutingStateLabelKey] = string(servingv1.RoutingStateReserve)
+	revision3 := createMockRevisionWithParams("baz", "svc3", "1", "50", "")
+	revision3.Labels[serving.RoutingStateLabelKey] = string(servingv1.RoutingStateReserve)
+	revisionList := &servingv1.RevisionList{Items: []servingv1.Revision{*revision1, *revision2, *revision3}}
+	r.ListRevisions(mock.Any(), revisionList, nil)
+
+	output, err := executeRevisionCommand(client, "delete", "--prune-all")
+	fmt.Println(output)
+	assert.NilError(t, err)
+	assert.Assert(t, util.ContainsAll(output, "Revision", "deleted", "foo", "bar", "baz", "default"))
+	r.Validate()
+}
+
+func TestRevisionDeleteCheckErrorForNotFoundRevisionsMock(t *testing.T) {
+	// New mock client
+	client := clientservingv1.NewMockKnServiceClient(t)
+
+	// Recording:
+	r := client.Recorder()
+
+	r.DeleteRevision("foo", mock.Any(), nil)
+	r.DeleteRevision("bar", mock.Any(), errors.New("revisions.serving.knative.dev \"bar\" not found"))
+	r.DeleteRevision("baz", mock.Any(), errors.New("revisions.serving.knative.dev \"baz\" not found"))
+
+	output, err := executeRevisionCommand(client, "delete", "foo", "bar", "baz")
+	if err == nil {
+		t.Fatal("Expected revision not found error, returned nil")
+	}
+	assert.Assert(t, util.ContainsAll(output, "'foo' deleted", "\"bar\" not found", "\"baz\" not found"))
+
+	r.Validate()
+}
+
+func TestRevisionDeletePruneWithArgMock(t *testing.T) {
+	// New mock client
+	client := clientservingv1.NewMockKnServiceClient(t)
+	// Recording:
+	r := client.Recorder()
+	r.DeleteRevision("foo", mock.Any(), nil)
+	r.DeleteRevision("baz", mock.Any(), nil)
+	r.DeleteRevision("bar", mock.Any(), nil)
+
+	revision1 := createMockRevisionWithParams("foo", "svc1", "1", "50", "")
+	revision1.Labels[serving.RoutingStateLabelKey] = string(servingv1.RoutingStateReserve)
+	revision2 := createMockRevisionWithParams("bar", "svc1", "1", "50", "")
+	revision2.Labels[serving.RoutingStateLabelKey] = string(servingv1.RoutingStateActive)
+	revision3 := createMockRevisionWithParams("baz", "svc1", "1", "50", "")
+	revision3.Labels[serving.RoutingStateLabelKey] = string(servingv1.RoutingStateActive)
+	revisionList := &servingv1.RevisionList{Items: []servingv1.Revision{*revision1, *revision2, *revision3}}
+	r.ListRevisions(mock.Any(), revisionList, nil)
+
+	output, err := executeRevisionCommand(client, "delete", "--prune", "svc1")
+	assert.NilError(t, err)
+	assert.Assert(t, util.ContainsAll(output, "Revision", "deleted", "foo", "default"))
+	assert.Assert(t, util.ContainsNone(output, "bar", "baz"))
+
+}
+
+func TestRevisionDeletePruneErrorFromArgMock(t *testing.T) {
+	// New mock client
+	client := clientservingv1.NewMockKnServiceClient(t)
+	// Recording:
+	r := client.Recorder()
+	revisionList := &servingv1.RevisionList{Items: []servingv1.Revision{}}
+	r.ListRevisions(mock.Any(), revisionList, nil)
+
+	_, err := executeRevisionCommand(client, "delete", "--prune")
+	assert.Error(t, err, "flag needs an argument: --prune")
+}
+
+func TestRevisionDeletePruneNoRevisionsMock(t *testing.T) {
+	// New mock client
+	client := clientservingv1.NewMockKnServiceClient(t)
+
+	// Recording:
+	r := client.Recorder()
+	revisionList := &servingv1.RevisionList{Items: []servingv1.Revision{}}
+	r.ListRevisions(mock.Any(), revisionList, nil)
+
+	output, err := executeRevisionCommand(client, "delete", "--prune", "mysvc")
+	assert.NilError(t, err)
+	assert.Assert(t, util.ContainsAll(output, "No", "unreferenced", "revisions", "found"))
+
+	r.Validate()
+}
+
+func TestRevisionDeleteNoNameMock(t *testing.T) {
+	// New mock client
+	client := clientservingv1.NewMockKnServiceClient(t)
+
+	// Recording:
+	r := client.Recorder()
+
+	_, err := executeRevisionCommand(client, "delete")
+	assert.ErrorContains(t, err, "'kn revision delete' requires one or more revision name")
+
+	r.Validate()
+
+}
+
+func TestRevisionDeletePruneAllNoRevisionsMock(t *testing.T) {
+	// New mock client
+	client := clientservingv1.NewMockKnServiceClient(t)
+
+	// Recording:
+	r := client.Recorder()
+	revisionList := &servingv1.RevisionList{Items: []servingv1.Revision{}}
+	r.ListRevisions(mock.Any(), revisionList, nil)
+
+	output, err := executeRevisionCommand(client, "delete", "--prune-all")
+	assert.NilError(t, err)
+	assert.Assert(t, util.ContainsAll(output, "No", "unreferenced", "revisions", "found"))
+
+	r.Validate()
+}
+
+func TestRevisionDeletePruneAllErrorFromArgMock(t *testing.T) {
+	// New mock client
+	client := clientservingv1.NewMockKnServiceClient(t)
+	// Recording:
+	r := client.Recorder()
+	revisionList := &servingv1.RevisionList{Items: []servingv1.Revision{}}
+	r.ListRevisions(mock.Any(), revisionList, nil)
+
+	_, err := executeRevisionCommand(client, "delete", "--prune-all", "mysvc")
+	assert.Error(t, err, "'kn revision delete' with --prune-all flag requires no arguments")
+}

pkg/kn/commands/revision/delete_test.go

@@ -25,9 +25,7 @@ import (
 	clienttesting "k8s.io/client-go/testing"
 
 	"knative.dev/client/pkg/kn/commands"
-	clientservingv1 "knative.dev/client/pkg/serving/v1"
 	"knative.dev/client/pkg/util"
-	"knative.dev/client/pkg/util/mock"
 	"knative.dev/client/pkg/wait"
 	servingv1 "knative.dev/serving/pkg/apis/serving/v1"
 )
@@ -104,23 +102,3 @@ func getRevisionDeleteEvents(name string) []watch.Event {
 		{Type: watch.Deleted, Object: &servingv1.Revision{ObjectMeta: metav1.ObjectMeta{Name: name}}},
 	}
 }
-
-func TestRevisionDeleteCheckErrorForNotFoundRevisionsMock(t *testing.T) {
-	// New mock client
-	client := clientservingv1.NewMockKnServiceClient(t)
-
-	// Recording:
-	r := client.Recorder()
-
-	r.DeleteRevision("foo", mock.Any(), nil)
-	r.DeleteRevision("bar", mock.Any(), errors.New("revisions.serving.knative.dev \"bar\" not found."))
-	r.DeleteRevision("baz", mock.Any(), errors.New("revisions.serving.knative.dev \"baz\" not found."))
-
-	output, err := executeRevisionCommand(client, "delete", "foo", "bar", "baz")
-	if err == nil {
-		t.Fatal("Expected revision not found error, returned nil")
-	}
-	assert.Assert(t, util.ContainsAll(output, "'foo' deleted", "\"bar\" not found", "\"baz\" not found"))
-
-	r.Validate()
-}

test/e2e/revision_test.go

@@ -59,6 +59,25 @@ func TestRevision(t *testing.T) {
 	nonexistRevision := "hello-nonexist"
 	test.RevisionMultipleDelete(r, existRevision1, existRevision2, nonexistRevision)
 
+	t.Log("update hello service and increase revision count to 4")
+	test.ServiceUpdate(r, "hello", "--env", "TARGET=kn", "--port", "8888")
+	t.Log("delete all unreferenced revision from hello service and return no error")
+	unRefRevision := test.FindRevisionByGeneration(r, "hello", 3)
+	test.RevisionDeleteWithPruneOption(r, "hello", unRefRevision)
+
+	t.Log("update hello service and increase revision count to 5")
+	test.ServiceUpdate(r, "hello", "--env", "TARGET=kn", "--port", "9000")
+	t.Log("create hello service and return no error")
+	test.ServiceCreate(r, "hello1")
+	t.Log("update hello1 service and increase revision count to 2")
+	test.ServiceUpdate(r, "hello1", "--env", "TARGET=kn", "--port", "8888")
+	t.Log("delete all unreferenced revisions return no error")
+	unRefRevision1 := test.FindRevisionByGeneration(r, "hello", 4)
+	unRefRevision2 := test.FindRevisionByGeneration(r, "hello1", 1)
+	test.RevisionDeleteWithPruneAllOption(r, unRefRevision1, unRefRevision2)
+	t.Log("delete hello1 service and return no error")
+	test.ServiceDelete(r, "hello1")
+
 	t.Log("delete latest revision from hello service and return no error")
 	revName = test.FindRevision(r, "hello")
 	test.RevisionDelete(r, revName)