From 1b68c328135fcaa1e8ccea86475a9e37631638fc Mon Sep 17 00:00:00 2001 From: Morten Torkildsen Date: Wed, 5 Jan 2022 10:13:31 -0800 Subject: [PATCH] fix: Remove preview/dry-run type from events output --- cmd/preview/cmdpreview.go | 9 +++++++ examples/alphaTestExamples/helloapp.md | 16 ++++++------ examples/alphaTestExamples/pruneAndDelete.md | 10 ++++---- pkg/printers/events/formatter.go | 26 ++++++-------------- pkg/printers/events/formatter_test.go | 24 +++++++++--------- pkg/printers/json/formatter.go | 10 +++----- 6 files changed, 46 insertions(+), 49 deletions(-) diff --git a/cmd/preview/cmdpreview.go b/cmd/preview/cmdpreview.go index cd7f858..9bb8d68 100644 --- a/cmd/preview/cmdpreview.go +++ b/cmd/preview/cmdpreview.go @@ -163,6 +163,15 @@ func (r *PreviewRunner) RunE(cmd *cobra.Command, args []string) error { }) } + // Print the preview strategy unless the output format is json. + if r.output != printers.JSONPrinter { + if drs.ServerDryRun() { + fmt.Println("Preview strategy: server") + } else { + fmt.Println("Preview strategy: client") + } + } + // The printer will print updates from the channel. It will block // until the channel is closed. printer := printers.GetPrinter(r.output, r.ioStreams) diff --git a/examples/alphaTestExamples/helloapp.md b/examples/alphaTestExamples/helloapp.md index 376c0c1..93ad929 100644 --- a/examples/alphaTestExamples/helloapp.md +++ b/examples/alphaTestExamples/helloapp.md @@ -174,11 +174,11 @@ Run preview to check which commands will be executed ``` kapply preview $BASE | tee $OUTPUT/status -expectedOutputLine "3 resource(s) applied. 3 created, 0 unchanged, 0 configured, 0 failed (preview)" +expectedOutputLine "3 resource(s) applied. 3 created, 0 unchanged, 0 configured, 0 failed" kapply preview $BASE --server-side | tee $OUTPUT/status -expectedOutputLine "3 resource(s) applied. 0 created, 0 unchanged, 0 configured, 0 failed, 3 serverside applied (preview-server)" +expectedOutputLine "3 resource(s) applied. 0 created, 0 unchanged, 0 configured, 0 failed, 3 serverside applied" # Verify that preview didn't create any resources. kubectl get all -n hellospace 2>&1 | tee $OUTPUT/status @@ -235,19 +235,19 @@ Clean-up the cluster ``` kapply preview $BASE --destroy | tee $OUTPUT/status -expectedOutputLine "deployment.apps/the-deployment deleted (preview)" +expectedOutputLine "deployment.apps/the-deployment deleted" -expectedOutputLine "configmap/the-map2 deleted (preview)" +expectedOutputLine "configmap/the-map2 deleted" -expectedOutputLine "service/the-service deleted (preview)" +expectedOutputLine "service/the-service deleted" kapply preview $BASE --destroy --server-side | tee $OUTPUT/status -expectedOutputLine "deployment.apps/the-deployment deleted (preview-server)" +expectedOutputLine "deployment.apps/the-deployment deleted" -expectedOutputLine "configmap/the-map2 deleted (preview-server)" +expectedOutputLine "configmap/the-map2 deleted" -expectedOutputLine "service/the-service deleted (preview-server)" +expectedOutputLine "service/the-service deleted" # Verify that preview all resources are still there after running preview. kubectl get --no-headers all -n hellospace | wc -l | xargs | tee $OUTPUT/status diff --git a/examples/alphaTestExamples/pruneAndDelete.md b/examples/alphaTestExamples/pruneAndDelete.md index fe3758e..cd65533 100644 --- a/examples/alphaTestExamples/pruneAndDelete.md +++ b/examples/alphaTestExamples/pruneAndDelete.md @@ -142,11 +142,11 @@ command. ``` kapply preview --destroy $BASE | tee $OUTPUT/status -expectedOutputLine "configmap/firstmap deleted (preview)" +expectedOutputLine "configmap/firstmap deleted" -expectedOutputLine "configmap/secondmap delete skipped (preview)" +expectedOutputLine "configmap/secondmap delete skipped" -expectedOutputLine "configmap/thirdmap delete skipped (preview)" +expectedOutputLine "configmap/thirdmap delete skipped" ``` We run the destroy command and see that the resource without the annotations (firstmap) @@ -193,9 +193,9 @@ will instead be skipped due to the lifecycle directive. ``` kapply preview $BASE | tee $OUTPUT/status -expectedOutputLine "configmap/secondmap prune skipped (preview)" +expectedOutputLine "configmap/secondmap prune skipped" -expectedOutputLine "configmap/thirdmap prune skipped (preview)" +expectedOutputLine "configmap/thirdmap prune skipped" ``` Run apply and verify that secondmap and thirdmap are still in the cluster. diff --git a/pkg/printers/events/formatter.go b/pkg/printers/events/formatter.go index cc0a207..20af66a 100644 --- a/pkg/printers/events/formatter.go +++ b/pkg/printers/events/formatter.go @@ -5,7 +5,6 @@ package events import ( "fmt" - "io" "strings" "k8s.io/apimachinery/pkg/runtime/schema" @@ -17,14 +16,14 @@ import ( ) func NewFormatter(ioStreams genericclioptions.IOStreams, - previewStrategy common.DryRunStrategy) list.Formatter { + _ common.DryRunStrategy) list.Formatter { return &formatter{ - print: getPrintFunc(ioStreams.Out, previewStrategy), + ioStreams: ioStreams, } } type formatter struct { - print printFunc + ioStreams genericclioptions.IOStreams } func (ef *formatter) FormatApplyEvent(ae event.ApplyEvent) error { @@ -112,7 +111,7 @@ func (ef *formatter) FormatActionGroupEvent( ps *list.PruneStats, ds *list.DeleteStats, ws *list.WaitStats, - c list.Collector, + _ list.Collector, ) error { if age.Action == event.ApplyAction && age.Type == event.Finished && @@ -152,20 +151,11 @@ func (ef *formatter) printResourceStatus(id object.ObjMetadata, se event.StatusE se.PollResourceInfo.Status.String(), se.PollResourceInfo.Message) } +func (ef *formatter) print(format string, a ...interface{}) { + _, _ = fmt.Fprintf(ef.ioStreams.Out, format+"\n", a...) +} + // resourceIDToString returns the string representation of a GroupKind and a resource name. func resourceIDToString(gk schema.GroupKind, name string) string { return fmt.Sprintf("%s/%s", strings.ToLower(gk.String()), name) } - -type printFunc func(format string, a ...interface{}) - -func getPrintFunc(w io.Writer, previewStrategy common.DryRunStrategy) printFunc { - return func(format string, a ...interface{}) { - if previewStrategy.ClientDryRun() { - format += " (preview)" - } else if previewStrategy.ServerDryRun() { - format += " (preview-server)" - } - fmt.Fprintf(w, format+"\n", a...) - } -} diff --git a/pkg/printers/events/formatter_test.go b/pkg/printers/events/formatter_test.go index 514eada..ef6fc02 100644 --- a/pkg/printers/events/formatter_test.go +++ b/pkg/printers/events/formatter_test.go @@ -42,7 +42,7 @@ func TestFormatter_FormatApplyEvent(t *testing.T) { Operation: event.Configured, Identifier: createIdentifier("apps", "Deployment", "", "my-dep"), }, - expected: "deployment.apps/my-dep configured (preview)", + expected: "deployment.apps/my-dep configured", }, "resource updated with server dryrun": { previewStrategy: common.DryRunServer, @@ -50,7 +50,7 @@ func TestFormatter_FormatApplyEvent(t *testing.T) { Operation: event.Configured, Identifier: createIdentifier("batch", "CronJob", "foo", "my-cron"), }, - expected: "cronjob.batch/my-cron configured (preview-server)", + expected: "cronjob.batch/my-cron configured", }, "apply event with error should display the error": { previewStrategy: common.DryRunServer, @@ -58,7 +58,7 @@ func TestFormatter_FormatApplyEvent(t *testing.T) { Identifier: createIdentifier("apps", "Deployment", "", "my-dep"), Error: fmt.Errorf("this is a test error"), }, - expected: "deployment.apps/my-dep apply failed: this is a test error (preview-server)", + expected: "deployment.apps/my-dep apply failed: this is a test error", }, } @@ -142,7 +142,7 @@ func TestFormatter_FormatPruneEvent(t *testing.T) { Operation: event.PruneSkipped, Identifier: createIdentifier("apps", "Deployment", "", "my-dep"), }, - expected: "deployment.apps/my-dep prune skipped (preview)", + expected: "deployment.apps/my-dep prune skipped", }, "resource with prune error": { previewStrategy: common.DryRunNone, @@ -190,7 +190,7 @@ func TestFormatter_FormatDeleteEvent(t *testing.T) { Identifier: createIdentifier("apps", "Deployment", "", "my-dep"), Object: createObject("apps", "Deployment", "", "my-dep"), }, - expected: "deployment.apps/my-dep delete skipped (preview)", + expected: "deployment.apps/my-dep delete skipped", }, "resource with delete error": { previewStrategy: common.DryRunServer, @@ -199,7 +199,7 @@ func TestFormatter_FormatDeleteEvent(t *testing.T) { Identifier: createIdentifier("apps", "Deployment", "", "my-dep"), Error: fmt.Errorf("this is a test"), }, - expected: "deployment.apps/my-dep deletion failed: this is a test (preview-server)", + expected: "deployment.apps/my-dep deletion failed: this is a test", }, } @@ -239,7 +239,7 @@ func TestFormatter_FormatWaitEvent(t *testing.T) { Operation: event.Reconciled, Identifier: createIdentifier("apps", "Deployment", "default", "my-dep"), }, - expected: "deployment.apps/my-dep reconciled (preview)", + expected: "deployment.apps/my-dep reconciled", }, "resource reconciled (server-side dry-run)": { previewStrategy: common.DryRunServer, @@ -248,7 +248,7 @@ func TestFormatter_FormatWaitEvent(t *testing.T) { Operation: event.Reconciled, Identifier: createIdentifier("apps", "Deployment", "default", "my-dep"), }, - expected: "deployment.apps/my-dep reconciled (preview-server)", + expected: "deployment.apps/my-dep reconciled", }, "resource reconcile timeout": { previewStrategy: common.DryRunNone, @@ -266,7 +266,7 @@ func TestFormatter_FormatWaitEvent(t *testing.T) { Identifier: createIdentifier("apps", "Deployment", "default", "my-dep"), Operation: event.ReconcileTimeout, }, - expected: "deployment.apps/my-dep reconcile timeout (preview)", + expected: "deployment.apps/my-dep reconcile timeout", }, "resource reconcile timeout (server-side dry-run)": { previewStrategy: common.DryRunServer, @@ -275,7 +275,7 @@ func TestFormatter_FormatWaitEvent(t *testing.T) { Identifier: createIdentifier("apps", "Deployment", "default", "my-dep"), Operation: event.ReconcileTimeout, }, - expected: "deployment.apps/my-dep reconcile timeout (preview-server)", + expected: "deployment.apps/my-dep reconcile timeout", }, "resource reconcile skipped": { previewStrategy: common.DryRunNone, @@ -293,7 +293,7 @@ func TestFormatter_FormatWaitEvent(t *testing.T) { Operation: event.ReconcileSkipped, Identifier: createIdentifier("apps", "Deployment", "default", "my-dep"), }, - expected: "deployment.apps/my-dep reconcile skipped (preview)", + expected: "deployment.apps/my-dep reconcile skipped", }, "resource reconcile skipped (server-side dry-run)": { previewStrategy: common.DryRunServer, @@ -302,7 +302,7 @@ func TestFormatter_FormatWaitEvent(t *testing.T) { Operation: event.ReconcileSkipped, Identifier: createIdentifier("apps", "Deployment", "default", "my-dep"), }, - expected: "deployment.apps/my-dep reconcile skipped (preview-server)", + expected: "deployment.apps/my-dep reconcile skipped", }, "resource reconcile failed": { previewStrategy: common.DryRunNone, diff --git a/pkg/printers/json/formatter.go b/pkg/printers/json/formatter.go index 86bd92e..0d7bdf2 100644 --- a/pkg/printers/json/formatter.go +++ b/pkg/printers/json/formatter.go @@ -16,16 +16,14 @@ import ( ) func NewFormatter(ioStreams genericclioptions.IOStreams, - previewStrategy common.DryRunStrategy) list.Formatter { + _ common.DryRunStrategy) list.Formatter { return &formatter{ - ioStreams: ioStreams, - previewStrategy: previewStrategy, + ioStreams: ioStreams, } } type formatter struct { - previewStrategy common.DryRunStrategy - ioStreams genericclioptions.IOStreams + ioStreams genericclioptions.IOStreams } func (jf *formatter) FormatApplyEvent(ae event.ApplyEvent) error { @@ -88,7 +86,7 @@ func (jf *formatter) FormatActionGroupEvent( ps *list.PruneStats, ds *list.DeleteStats, ws *list.WaitStats, - c list.Collector, + _ list.Collector, ) error { if age.Action == event.ApplyAction && age.Type == event.Finished && list.IsLastActionGroup(age, ags) {