From 1306b060c124fd7abd4cfdab4f510631830842b7 Mon Sep 17 00:00:00 2001 From: itsmurugappan Date: Tue, 2 Feb 2021 16:01:17 -0800 Subject: [PATCH 1/5] kn export defect to honor mode --- pkg/kn/commands/service/export.go | 53 +++++++++++++------------- pkg/kn/commands/service/export_test.go | 6 --- 2 files changed, 27 insertions(+), 32 deletions(-) diff --git a/pkg/kn/commands/service/export.go b/pkg/kn/commands/service/export.go index 3d438f0497..0816b470e9 100644 --- a/pkg/kn/commands/service/export.go +++ b/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" @@ -129,24 +130,14 @@ 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 err != nil { - return err - } - return printer.PrintObj(svcList, cmd.OutOrStdout()) case "export": - knExport, err := exportForKNImport(service.DeepCopy(), client) + knExport, err := exportForKNImport(service.DeepCopy(), client, withRevisions) if err != nil { return err } @@ -155,7 +146,11 @@ func exportService(cmd *cobra.Command, service *servingv1.Service, client client return err } default: - return errors.New("'kn service export --with-revisions' requires a mode, please specify one of replay|export") + svcList, err := exportServiceListForReplay(service.DeepCopy(), client, withRevisions) + if err != nil { + return err + } + return printer.PrintObj(svcList, cmd.OutOrStdout()) } return nil } @@ -223,14 +218,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 +251,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 + 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())) + 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 +276,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, }, } diff --git a/pkg/kn/commands/service/export_test.go b/pkg/kn/commands/service/export_test.go index f60be43f5f..46df3fbcd5 100644 --- a/pkg/kn/commands/service/export_test.go +++ b/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) { From d4cdcf21fdf75ee98d09096c3b8b12a4d436f2a5 Mon Sep 17 00:00:00 2001 From: itsmurugappan Date: Wed, 17 Feb 2021 18:33:16 -0800 Subject: [PATCH 2/5] add test for export mode without revisions --- pkg/kn/commands/service/export_test.go | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/pkg/kn/commands/service/export_test.go b/pkg/kn/commands/service/export_test.go index 46df3fbcd5..ecb17c5905 100644 --- a/pkg/kn/commands/service/export_test.go +++ b/pkg/kn/commands/service/export_test.go @@ -67,12 +67,14 @@ 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")))}, } { + exportServiceTestForReplay(t, &tc) + tc.expectedKNExport = libtest.BuildKNExportWithOptions() exportServiceTest(t, &tc) } } -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{} @@ -82,6 +84,19 @@ func exportServiceTest(t *testing.T, tc *testCase) { assert.DeepEqual(t, tc.latestSvc, &actSvc) } +func exportServiceTest(t *testing.T, tc *testCase) { + output, err := executeServiceExportCommand(t, tc, "export", tc.latestSvc.ObjectMeta.Name, "--mode", "export", "-o", "json") + 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", From b0bb594e02feba11d668283bc90d99e23df17317 Mon Sep 17 00:00:00 2001 From: itsmurugappan Date: Mon, 1 Mar 2021 15:07:51 -0800 Subject: [PATCH 3/5] change default to export --- pkg/kn/commands/service/export.go | 19 +++++++++++-------- pkg/kn/commands/service/export_test.go | 12 +++++++++--- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/pkg/kn/commands/service/export.go b/pkg/kn/commands/service/export.go index 0816b470e9..58df3719d4 100644 --- a/pkg/kn/commands/service/export.go +++ b/pkg/kn/commands/service/export.go @@ -65,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 { @@ -136,21 +141,19 @@ func exportService(cmd *cobra.Command, service *servingv1.Service, client client } switch mode { - case "export": - knExport, err := exportForKNImport(service.DeepCopy(), client, withRevisions) + case ModeReplay: + svcList, err := exportServiceListForReplay(service.DeepCopy(), client, withRevisions) if err != nil { return err } - //print kn export - if err := printer.PrintObj(knExport, cmd.OutOrStdout()); err != nil { - return err - } + return printer.PrintObj(svcList, cmd.OutOrStdout()) default: - svcList, err := exportServiceListForReplay(service.DeepCopy(), client, withRevisions) + knExport, err := exportForKNImport(service.DeepCopy(), client, withRevisions) if err != nil { return err } - return printer.PrintObj(svcList, cmd.OutOrStdout()) + //print kn export + return printer.PrintObj(knExport, cmd.OutOrStdout()) } return nil } diff --git a/pkg/kn/commands/service/export_test.go b/pkg/kn/commands/service/export_test.go index ecb17c5905..68a4897655 100644 --- a/pkg/kn/commands/service/export_test.go +++ b/pkg/kn/commands/service/export_test.go @@ -69,7 +69,9 @@ func TestServiceExport(t *testing.T) { } { exportServiceTestForReplay(t, &tc) tc.expectedKNExport = libtest.BuildKNExportWithOptions() - exportServiceTest(t, &tc) + exportServiceTest(t, &tc, true) + //test default + exportServiceTest(t, &tc, false) } } @@ -84,8 +86,12 @@ func exportServiceTestForReplay(t *testing.T, tc *testCase) { assert.DeepEqual(t, tc.latestSvc, &actSvc) } -func exportServiceTest(t *testing.T, tc *testCase) { - output, err := executeServiceExportCommand(t, tc, "export", tc.latestSvc.ObjectMeta.Name, "--mode", "export", "-o", "json") +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 From 615e8f554d81fe0d11003dddfa8561fb2886474d Mon Sep 17 00:00:00 2001 From: itsmurugappan Date: Wed, 3 Mar 2021 06:55:44 -0800 Subject: [PATCH 4/5] add export as default --- pkg/kn/commands/service/export.go | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/pkg/kn/commands/service/export.go b/pkg/kn/commands/service/export.go index 58df3719d4..9cfe6d00cd 100644 --- a/pkg/kn/commands/service/export.go +++ b/pkg/kn/commands/service/export.go @@ -140,22 +140,20 @@ func exportService(cmd *cobra.Command, service *servingv1.Service, client client return err } - switch mode { - case ModeReplay: + if mode == ModeReplay { svcList, err := exportServiceListForReplay(service.DeepCopy(), client, withRevisions) if err != nil { return err } return printer.PrintObj(svcList, cmd.OutOrStdout()) - default: - knExport, err := exportForKNImport(service.DeepCopy(), client, withRevisions) - if err != nil { - return err - } - //print kn export - return printer.PrintObj(knExport, cmd.OutOrStdout()) } - 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 { From f5b1635c328240d29f1902cf28bc220a6e4bf32a Mon Sep 17 00:00:00 2001 From: itsmurugappan Date: Wed, 3 Mar 2021 07:50:04 -0800 Subject: [PATCH 5/5] change tests for export mode change --- test/e2e/service_export_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/e2e/service_export_test.go b/test/e2e/service_export_test.go index 154495dde1..a12cd2945f 100644 --- a/test/e2e/service_export_test.go +++ b/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(