kn export defect to honor mode (#1212)

* kn export defect to honor mode

* add test for export mode without revisions

* change default to export

* add export as default

* change tests for export mode change

pkg/kn/commands/service/export.go

@@ -24,6 +24,7 @@ import (
 	"github.com/spf13/cobra"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/cli-runtime/pkg/genericclioptions"
 	"k8s.io/cli-runtime/pkg/printers"
 
@@ -64,6 +65,11 @@ var IgnoredRevisionLabels = []string{
 	"serving.knative.dev/serviceUID",
 }
 
+const (
+	ModeReplay = "replay"
+	ModeExport = "export"
+)
+
 // NewServiceExportCommand returns a new command for exporting a service.
 func NewServiceExportCommand(p *commands.KnParams) *cobra.Command {
 
@@ -129,35 +135,25 @@ func exportService(cmd *cobra.Command, service *servingv1.Service, client client
 		return err
 	}
 
-	if !withRevisions {
-		return printer.PrintObj(exportLatestService(service.DeepCopy(), false), cmd.OutOrStdout())
-	}
-
 	mode, err := cmd.Flags().GetString("mode")
 	if err != nil {
 		return err
 	}
 
-	switch mode {
-	case "replay":
-		svcList, err := exportServiceListForReplay(service.DeepCopy(), client)
+	if mode == ModeReplay {
+		svcList, err := exportServiceListForReplay(service.DeepCopy(), client, withRevisions)
 		if err != nil {
 			return err
 		}
 		return printer.PrintObj(svcList, cmd.OutOrStdout())
-	case "export":
-		knExport, err := exportForKNImport(service.DeepCopy(), client)
-		if err != nil {
-			return err
-		}
-		//print kn export
-		if err := printer.PrintObj(knExport, cmd.OutOrStdout()); err != nil {
-			return err
-		}
-	default:
-		return errors.New("'kn service export --with-revisions' requires a mode, please specify one of replay|export")
 	}
-	return nil
+	// default is export mode
+	knExport, err := exportForKNImport(service.DeepCopy(), client, withRevisions)
+	if err != nil {
+		return err
+	}
+	//print kn export
+	return printer.PrintObj(knExport, cmd.OutOrStdout())
 }
 
 func exportLatestService(latestSvc *servingv1.Service, withRoutes bool) *servingv1.Service {
@@ -223,14 +219,17 @@ func constructServiceFromRevision(latestSvc *servingv1.Service, revision *servin
 	return exportedSvc
 }
 
-func exportServiceListForReplay(latestSvc *servingv1.Service, client clientservingv1.KnServingClient) (*servingv1.ServiceList, error) {
+func exportServiceListForReplay(latestSvc *servingv1.Service, client clientservingv1.KnServingClient, withRevisions bool) (runtime.Object, error) {
+	if !withRevisions {
+		return exportLatestService(latestSvc, false), nil
+	}
+	var exportedSvcItems []servingv1.Service
+
 	revisionList, revsMap, err := getRevisionsToExport(latestSvc, client)
 	if err != nil {
 		return nil, err
 	}
 
-	var exportedSvcItems []servingv1.Service
-
 	for _, revision := range revisionList.Items {
 		//construct service only for active revisions
 		if revsMap[revision.ObjectMeta.Name] && revision.ObjectMeta.Name != latestSvc.Spec.Template.ObjectMeta.Name {
@@ -253,19 +252,22 @@ func exportServiceListForReplay(latestSvc *servingv1.Service, client clientservi
 	return exportedSvcList, nil
 }
 
-func exportForKNImport(latestSvc *servingv1.Service, client clientservingv1.KnServingClient) (*clientv1alpha1.Export, error) {
-	revisionList, revsMap, err := getRevisionsToExport(latestSvc, client)
-	if err != nil {
-		return nil, err
-	}
-
+func exportForKNImport(latestSvc *servingv1.Service, client clientservingv1.KnServingClient, withRevisions bool) (*clientv1alpha1.Export, error) {
 	var exportedRevItems []servingv1.Revision
-
-	for _, revision := range revisionList.Items {
-		//append only active revisions, no latest revision
-		if revsMap[revision.ObjectMeta.Name] && revision.ObjectMeta.Name != latestSvc.Spec.Template.ObjectMeta.Name {
-			exportedRevItems = append(exportedRevItems, exportRevision(revision.DeepCopy()))
+	revisionHistoryCount := 0
+	if withRevisions {
+		revisionList, revsMap, err := getRevisionsToExport(latestSvc, client)
+		if err != nil {
+			return nil, err
 		}
+
+		for _, revision := range revisionList.Items {
+			//append only active revisions, no latest revision
+			if revsMap[revision.ObjectMeta.Name] && revision.ObjectMeta.Name != latestSvc.Spec.Template.ObjectMeta.Name {
+				exportedRevItems = append(exportedRevItems, exportRevision(revision.DeepCopy()))
+			}
+		}
+		revisionHistoryCount = len(revisionList.Items)
 	}
 
 	typeMeta := metav1.TypeMeta{
@@ -275,7 +277,7 @@ func exportForKNImport(latestSvc *servingv1.Service, client clientservingv1.KnSe
 	knExport := &clientv1alpha1.Export{
 		TypeMeta: typeMeta,
 		Spec: clientv1alpha1.ExportSpec{
-			Service:   *(exportLatestService(latestSvc, len(revisionList.Items) > 1)),
+			Service:   *(exportLatestService(latestSvc, revisionHistoryCount > 1)),
 			Revisions: exportedRevItems,
 		},
 	}

pkg/kn/commands/service/export_test.go

@@ -56,12 +56,6 @@ func TestServiceExportError(t *testing.T) {
 
 	_, err := executeServiceExportCommand(t, tc, "export", tc.latestSvc.ObjectMeta.Name)
 	assert.Error(t, err, "'kn service export' requires output format")
-
-	_, err = executeServiceExportCommand(t, tc, "export", tc.latestSvc.ObjectMeta.Name, "--with-revisions", "-o", "json")
-	assert.Error(t, err, "'kn service export --with-revisions' requires a mode, please specify one of replay|export")
-
-	_, err = executeServiceExportCommand(t, tc, "export", tc.latestSvc.ObjectMeta.Name, "--with-revisions", "--mode", "k8s", "-o", "yaml")
-	assert.Error(t, err, "'kn service export --with-revisions' requires a mode, please specify one of replay|export")
 }
 
 func TestServiceExport(t *testing.T) {
@@ -73,12 +67,16 @@ func TestServiceExport(t *testing.T) {
 		{latestSvc: libtest.BuildServiceWithOptions("foo", servingtest.WithConfigSpec(buildConfiguration()), servingtest.WithServiceLabel("a", "mouse"), servingtest.WithServiceAnnotation("a", "mouse"))},
 		{latestSvc: libtest.BuildServiceWithOptions("foo", servingtest.WithConfigSpec(buildConfiguration()), servingtest.WithVolume("secretName", "/mountpath", volumeSource("secretName")))},
 	} {
-		exportServiceTest(t, &tc)
+		exportServiceTestForReplay(t, &tc)
+		tc.expectedKNExport = libtest.BuildKNExportWithOptions()
+		exportServiceTest(t, &tc, true)
+		//test default
+		exportServiceTest(t, &tc, false)
 	}
 }
 
-func exportServiceTest(t *testing.T, tc *testCase) {
-	output, err := executeServiceExportCommand(t, tc, "export", tc.latestSvc.ObjectMeta.Name, "-o", "yaml")
+func exportServiceTestForReplay(t *testing.T, tc *testCase) {
+	output, err := executeServiceExportCommand(t, tc, "export", tc.latestSvc.ObjectMeta.Name, "--mode", "replay", "-o", "yaml")
 	assert.NilError(t, err)
 
 	actSvc := servingv1.Service{}
@@ -88,6 +86,23 @@ func exportServiceTest(t *testing.T, tc *testCase) {
 	assert.DeepEqual(t, tc.latestSvc, &actSvc)
 }
 
+func exportServiceTest(t *testing.T, tc *testCase, addMode bool) {
+	args := []string{"export", tc.latestSvc.ObjectMeta.Name, "-o", "json"}
+	if addMode {
+		args = append(args, []string{"--mode", "export"}...)
+	}
+	output, err := executeServiceExportCommand(t, tc, args...)
+	assert.NilError(t, err)
+
+	tc.expectedKNExport.Spec.Service = *tc.latestSvc
+
+	actKNExport := &clientv1alpha1.Export{}
+	err = json.Unmarshal([]byte(output), actKNExport)
+	assert.NilError(t, err)
+
+	assert.DeepEqual(t, tc.expectedKNExport, actKNExport)
+}
+
 func TestServiceExportwithMultipleRevisions(t *testing.T) {
 	for _, tc := range []testCase{{
 		name: "test 2 revisions with traffic split",

test/e2e/service_export_test.go

@@ -60,7 +60,7 @@ func TestServiceExport(t *testing.T) {
 		servingtest.WithConfigSpec(test.BuildConfigurationSpec()),
 		servingtest.WithBYORevisionName("hello-rev1"),
 		test.WithRevisionAnnotations(map[string]string{"client.knative.dev/user-image": pkgtest.ImagePath("helloworld")}),
-	), "-o", "json")
+	), "--mode", "replay", "-o", "json")
 
 	t.Log("update service - add env variable")
 	test.ServiceUpdate(r, "hello", "--env", "a=mouse", "--revision-name", "rev2", "--no-lock-to-digest")
@@ -68,7 +68,7 @@ func TestServiceExport(t *testing.T) {
 		servingtest.WithConfigSpec(test.BuildConfigurationSpec()),
 		servingtest.WithBYORevisionName("hello-rev2"),
 		servingtest.WithEnv(corev1.EnvVar{Name: "a", Value: "mouse"}),
-	), "-o", "json")
+	), "--mode", "replay", "-o", "json")
 
 	t.Log("export service-revision2 with kubernetes-resources")
 	serviceExportWithServiceList(r, "hello", test.BuildServiceListWithOptions(