From ac6ca38dd72c0a503310818606baebcbe1558292 Mon Sep 17 00:00:00 2001 From: juanvallejo Date: Tue, 24 Apr 2018 15:38:38 -0400 Subject: [PATCH] report outputFormat in PrintFlags err --- hack/make-rules/test-cmd-util.sh | 7 +++--- pkg/kubectl/cmd/certificates.go | 27 ++++++++++++------------ pkg/kubectl/cmd/clusterinfo_dump_test.go | 21 +++++++++--------- pkg/kubectl/cmd/cp_test.go | 7 +++--- pkg/kubectl/cmd/drain_test.go | 10 ++++----- pkg/kubectl/cmd/label_test.go | 23 ++++++++++---------- pkg/printers/flags.go | 12 ++++++++--- pkg/printers/json_yaml_flags.go | 2 +- pkg/printers/jsonpath_flags.go | 4 ++-- pkg/printers/name_flags.go | 2 +- pkg/printers/template_flags.go | 4 ++-- 11 files changed, 60 insertions(+), 59 deletions(-) diff --git a/hack/make-rules/test-cmd-util.sh b/hack/make-rules/test-cmd-util.sh index 74527f4faea..4313295782d 100755 --- a/hack/make-rules/test-cmd-util.sh +++ b/hack/make-rules/test-cmd-util.sh @@ -693,7 +693,7 @@ run_pod_tests() { kube::test::get_object_assert pods "{{range.items}}{{$id_field}}:{{end}}" 'valid-pod:' ## Patch can modify a local object - kubectl patch --local -f pkg/kubectl/validation/testdata/v1/validPod.yaml --patch='{"spec": {"restartPolicy":"Never"}}' -o jsonpath='{.spec.restartPolicy}' | grep -q "Never" + kubectl patch --local -f pkg/kubectl/validation/testdata/v1/validPod.yaml --patch='{"spec": {"restartPolicy":"Never"}}' -o yaml | grep -q "Never" ## Patch fails with error message "not patched" and exit code 1 output_message=$(! kubectl patch "${kube_flags[@]}" pod valid-pod -p='{"spec":{"replicas":7}}' 2>&1) @@ -2139,8 +2139,7 @@ run_recursive_resources_tests() { kube::test::get_object_assert deployment "{{range.items}}{{$id_field}}:{{end}}" 'nginx:' kube::test::get_object_assert deployment "{{range.items}}{{$image_field0}}:{{end}}" "${IMAGE_DEPLOYMENT_R1}:" # Command - output_message=$(kubectl convert --local -f hack/testdata/deployment-revision1.yaml --output-version=apps/v1beta1 -o go-template='{{ .apiVersion }}' "${kube_flags[@]}") - echo $output_message + output_message=$(kubectl convert --local -f hack/testdata/deployment-revision1.yaml --output-version=apps/v1beta1 -o yaml "${kube_flags[@]}") # Post-condition: apiVersion is still extensions/v1beta1 in the live deployment, but command output is the new value kube::test::get_object_assert 'deployment nginx' "{{ .apiVersion }}" 'extensions/v1beta1' kube::test::if_has_string "${output_message}" "apps/v1beta1" @@ -2851,7 +2850,7 @@ run_rc_tests() { # Pre-condition: don't use --port flag output_message=$(kubectl expose -f test/fixtures/doc-yaml/admin/high-availability/etcd.yaml --selector=test=etcd 2>&1 "${kube_flags[@]}") # Post-condition: expose succeeded - kube::test::if_has_string "${output_message}" 'etcd-server\" exposed' + kube::test::if_has_string "${output_message}" 'etcd-server exposed' # Post-condition: generated service has both ports from the exposed pod kube::test::get_object_assert 'service etcd-server' "{{$port_name}} {{$port_field}}" 'port-1 2380' kube::test::get_object_assert 'service etcd-server' "{{$second_port_name}} {{$second_port_field}}" 'port-2 2379' diff --git a/pkg/kubectl/cmd/certificates.go b/pkg/kubectl/cmd/certificates.go index 2a0861b5da7..83a940fef56 100644 --- a/pkg/kubectl/cmd/certificates.go +++ b/pkg/kubectl/cmd/certificates.go @@ -60,8 +60,8 @@ type CertificateOptions struct { csrNames []string outputStyle string - clientSetFunc func() (internalclientset.Interface, error) - builderFunc func() *resource.Builder + clientSet internalclientset.Interface + builder *resource.Builder genericclioptions.IOStreams } @@ -79,8 +79,11 @@ func (o *CertificateOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, arg return printer.PrintObj(obj, out) } - o.builderFunc = f.NewBuilder - o.clientSetFunc = f.ClientSet + o.builder = f.NewBuilder() + o.clientSet, err = f.ClientSet() + if err != nil { + return err + } return nil } @@ -127,7 +130,7 @@ func NewCmdCertificateApprove(f cmdutil.Factory, ioStreams genericclioptions.IOS } func (o *CertificateOptions) RunCertificateApprove(force bool) error { - return o.modifyCertificateCondition(o.builderFunc, o.clientSetFunc, force, func(csr *certificates.CertificateSigningRequest) (*certificates.CertificateSigningRequest, bool) { + return o.modifyCertificateCondition(o.builder, o.clientSet, force, func(csr *certificates.CertificateSigningRequest) (*certificates.CertificateSigningRequest, bool) { var alreadyApproved bool for _, c := range csr.Status.Conditions { if c.Type == certificates.CertificateApproved { @@ -177,7 +180,7 @@ func NewCmdCertificateDeny(f cmdutil.Factory, ioStreams genericclioptions.IOStre } func (o *CertificateOptions) RunCertificateDeny(force bool) error { - return o.modifyCertificateCondition(o.builderFunc, o.clientSetFunc, force, func(csr *certificates.CertificateSigningRequest) (*certificates.CertificateSigningRequest, bool) { + return o.modifyCertificateCondition(o.builder, o.clientSet, force, func(csr *certificates.CertificateSigningRequest) (*certificates.CertificateSigningRequest, bool) { var alreadyDenied bool for _, c := range csr.Status.Conditions { if c.Type == certificates.CertificateDenied { @@ -197,13 +200,9 @@ func (o *CertificateOptions) RunCertificateDeny(force bool) error { }) } -func (options *CertificateOptions) modifyCertificateCondition(builderFunc func() *resource.Builder, clientSetFunc func() (internalclientset.Interface, error), force bool, modify func(csr *certificates.CertificateSigningRequest) (*certificates.CertificateSigningRequest, bool)) error { +func (options *CertificateOptions) modifyCertificateCondition(builder *resource.Builder, clientSet internalclientset.Interface, force bool, modify func(csr *certificates.CertificateSigningRequest) (*certificates.CertificateSigningRequest, bool)) error { var found int - c, err := clientSetFunc() - if err != nil { - return err - } - r := builderFunc(). + r := builder. Internal(). ContinueOnError(). FilenameParam(false, &options.FilenameOptions). @@ -212,14 +211,14 @@ func (options *CertificateOptions) modifyCertificateCondition(builderFunc func() Flatten(). Latest(). Do() - err = r.Visit(func(info *resource.Info, err error) error { + err := r.Visit(func(info *resource.Info, err error) error { if err != nil { return err } csr := info.Object.(*certificates.CertificateSigningRequest) csr, hasCondition := modify(csr) if !hasCondition || force { - csr, err = c.Certificates(). + csr, err = clientSet.Certificates(). CertificateSigningRequests(). UpdateApproval(csr) if err != nil { diff --git a/pkg/kubectl/cmd/clusterinfo_dump_test.go b/pkg/kubectl/cmd/clusterinfo_dump_test.go index 3e603d93a64..22a951edbb4 100644 --- a/pkg/kubectl/cmd/clusterinfo_dump_test.go +++ b/pkg/kubectl/cmd/clusterinfo_dump_test.go @@ -17,7 +17,6 @@ limitations under the License. package cmd import ( - "bytes" "io/ioutil" "os" "path" @@ -30,15 +29,15 @@ import ( func TestSetupOutputWriterNoOp(t *testing.T) { tests := []string{"", "-"} for _, test := range tests { - out := &bytes.Buffer{} + _, _, buf, _ := genericclioptions.NewTestIOStreams() f := cmdtesting.NewTestFactory() defer f.Cleanup() - cmd := NewCmdClusterInfoDump(f, genericclioptions.IOStreams{Out: os.Stdout, ErrOut: os.Stderr}) + cmd := NewCmdClusterInfoDump(f, genericclioptions.NewTestIOStreamsDiscard()) cmd.Flag("output-directory").Value.Set(test) - writer := setupOutputWriter(cmd, out, "/some/file/that/should/be/ignored") - if writer != out { - t.Errorf("expected: %v, saw: %v", out, writer) + writer := setupOutputWriter(cmd, buf, "/some/file/that/should/be/ignored") + if writer != buf { + t.Errorf("expected: %v, saw: %v", buf, writer) } } } @@ -52,15 +51,15 @@ func TestSetupOutputWriterFile(t *testing.T) { fullPath := path.Join(dir, file) defer os.RemoveAll(dir) - out := &bytes.Buffer{} + _, _, buf, _ := genericclioptions.NewTestIOStreams() f := cmdtesting.NewTestFactory() defer f.Cleanup() - cmd := NewCmdClusterInfoDump(f, genericclioptions.IOStreams{Out: os.Stdout, ErrOut: os.Stderr}) + cmd := NewCmdClusterInfoDump(f, genericclioptions.NewTestIOStreamsDiscard()) cmd.Flag("output-directory").Value.Set(dir) - writer := setupOutputWriter(cmd, out, file) - if writer == out { - t.Errorf("expected: %v, saw: %v", out, writer) + writer := setupOutputWriter(cmd, buf, file) + if writer == buf { + t.Errorf("expected: %v, saw: %v", buf, writer) } output := "some data here" writer.Write([]byte(output)) diff --git a/pkg/kubectl/cmd/cp_test.go b/pkg/kubectl/cmd/cp_test.go index b24b9766178..24b7d7d6e38 100644 --- a/pkg/kubectl/cmd/cp_test.go +++ b/pkg/kubectl/cmd/cp_test.go @@ -525,10 +525,9 @@ func TestCopyToPod(t *testing.T) { } tf.ClientConfigVal = defaultClientConfig() - buf := bytes.NewBuffer([]byte{}) - errBuf := bytes.NewBuffer([]byte{}) + ioStreams, _, _, _ := genericclioptions.NewTestIOStreams() - cmd := NewCmdCp(tf, genericclioptions.IOStreams{Out: buf, ErrOut: errBuf}) + cmd := NewCmdCp(tf, ioStreams) srcFile, err := ioutil.TempDir("", "test") if err != nil { @@ -556,7 +555,7 @@ func TestCopyToPod(t *testing.T) { } for name, test := range tests { - opts := NewCopyOptions(genericclioptions.IOStreams{Out: buf, ErrOut: errBuf}) + opts := NewCopyOptions(ioStreams) src := fileSpec{ File: srcFile, } diff --git a/pkg/kubectl/cmd/drain_test.go b/pkg/kubectl/cmd/drain_test.go index 3869b5a35c3..962a3379d6c 100644 --- a/pkg/kubectl/cmd/drain_test.go +++ b/pkg/kubectl/cmd/drain_test.go @@ -17,7 +17,6 @@ limitations under the License. package cmd import ( - "bytes" "errors" "fmt" "io" @@ -198,8 +197,8 @@ func TestCordon(t *testing.T) { } tf.ClientConfigVal = defaultClientConfig() - buf := bytes.NewBuffer([]byte{}) - cmd := test.cmd(tf, genericclioptions.IOStreams{Out: buf, ErrOut: buf}) + ioStreams, _, _, _ := genericclioptions.NewTestIOStreams() + cmd := test.cmd(tf, ioStreams) saw_fatal := false func() { @@ -708,9 +707,8 @@ func TestDrain(t *testing.T) { } tf.ClientConfigVal = defaultClientConfig() - buf := bytes.NewBuffer([]byte{}) - errBuf := bytes.NewBuffer([]byte{}) - cmd := NewCmdDrain(tf, genericclioptions.IOStreams{Out: buf, ErrOut: errBuf}) + ioStreams, _, _, errBuf := genericclioptions.NewTestIOStreams() + cmd := NewCmdDrain(tf, ioStreams) saw_fatal := false fatal_msg := "" diff --git a/pkg/kubectl/cmd/label_test.go b/pkg/kubectl/cmd/label_test.go index fdf2630e798..69ca68b480e 100644 --- a/pkg/kubectl/cmd/label_test.go +++ b/pkg/kubectl/cmd/label_test.go @@ -328,14 +328,15 @@ func TestLabelErrors(t *testing.T) { tf.Namespace = "test" tf.ClientConfigVal = defaultClientConfig() + ioStreams, _, _, _ := genericclioptions.NewTestIOStreams() buf := bytes.NewBuffer([]byte{}) - cmd := NewCmdLabel(tf, genericclioptions.IOStreams{Out: buf, ErrOut: buf}) + cmd := NewCmdLabel(tf, ioStreams) cmd.SetOutput(buf) for k, v := range testCase.flags { cmd.Flags().Set(k, v) } - opts := NewLabelOptions(genericclioptions.IOStreams{Out: buf, ErrOut: buf}) + opts := NewLabelOptions(ioStreams) err := opts.Complete(tf, cmd, testCase.args) if err == nil { err = opts.Validate() @@ -390,9 +391,9 @@ func TestLabelForResourceFromFile(t *testing.T) { tf.Namespace = "test" tf.ClientConfigVal = defaultClientConfig() - buf := bytes.NewBuffer([]byte{}) - cmd := NewCmdLabel(tf, genericclioptions.IOStreams{Out: buf, ErrOut: buf}) - opts := NewLabelOptions(genericclioptions.IOStreams{Out: buf, ErrOut: buf}) + ioStreams, _, buf, _ := genericclioptions.NewTestIOStreams() + cmd := NewCmdLabel(tf, ioStreams) + opts := NewLabelOptions(ioStreams) opts.Filenames = []string{"../../../test/e2e/testing-manifests/statefulset/cassandra/controller.yaml"} err := opts.Complete(tf, cmd, []string{"a=b"}) if err == nil { @@ -423,9 +424,9 @@ func TestLabelLocal(t *testing.T) { tf.Namespace = "test" tf.ClientConfigVal = defaultClientConfig() - buf := bytes.NewBuffer([]byte{}) - cmd := NewCmdLabel(tf, genericclioptions.IOStreams{Out: buf, ErrOut: buf}) - opts := NewLabelOptions(genericclioptions.IOStreams{Out: buf, ErrOut: buf}) + ioStreams, _, buf, _ := genericclioptions.NewTestIOStreams() + cmd := NewCmdLabel(tf, ioStreams) + opts := NewLabelOptions(ioStreams) opts.Filenames = []string{"../../../test/e2e/testing-manifests/statefulset/cassandra/controller.yaml"} opts.local = true err := opts.Complete(tf, cmd, []string{"a=b"}) @@ -481,10 +482,10 @@ func TestLabelMultipleObjects(t *testing.T) { tf.Namespace = "test" tf.ClientConfigVal = defaultClientConfig() - buf := bytes.NewBuffer([]byte{}) - opts := NewLabelOptions(genericclioptions.IOStreams{Out: buf, ErrOut: buf}) + ioStreams, _, buf, _ := genericclioptions.NewTestIOStreams() + opts := NewLabelOptions(ioStreams) opts.all = true - cmd := NewCmdLabel(tf, genericclioptions.IOStreams{Out: buf, ErrOut: buf}) + cmd := NewCmdLabel(tf, ioStreams) err := opts.Complete(tf, cmd, []string{"pods", "a=b"}) if err == nil { err = opts.Validate() diff --git a/pkg/printers/flags.go b/pkg/printers/flags.go index 95e45f82980..0ab5df8be8e 100644 --- a/pkg/printers/flags.go +++ b/pkg/printers/flags.go @@ -23,11 +23,17 @@ import ( ) type NoCompatiblePrinterError struct { - Options interface{} + OutputFormat *string + Options interface{} } func (e NoCompatiblePrinterError) Error() string { - return fmt.Sprintf("unable to match a printer suitable for the options specified: %#v", e.Options) + output := "" + if e.OutputFormat != nil { + output = *e.OutputFormat + } + + return fmt.Sprintf("unable to match a printer suitable for the output format %q and the options specified: %#v", output, e.Options) } func IsNoCompatiblePrinterError(err error) bool { @@ -67,7 +73,7 @@ func (f *PrintFlags) ToPrinter() (ResourcePrinter, error) { return p, err } - return nil, NoCompatiblePrinterError{f} + return nil, NoCompatiblePrinterError{Options: f, OutputFormat: f.OutputFormat} } func (f *PrintFlags) AddFlags(cmd *cobra.Command) { diff --git a/pkg/printers/json_yaml_flags.go b/pkg/printers/json_yaml_flags.go index e74a11bb315..6cb6a4b5d50 100644 --- a/pkg/printers/json_yaml_flags.go +++ b/pkg/printers/json_yaml_flags.go @@ -44,7 +44,7 @@ func (f *JSONYamlPrintFlags) ToPrinter(outputFormat string) (ResourcePrinter, er case "yaml": printer = &YAMLPrinter{} default: - return nil, NoCompatiblePrinterError{f} + return nil, NoCompatiblePrinterError{Options: f, OutputFormat: &outputFormat} } // wrap the printer in a versioning printer that understands when to convert and when not to convert diff --git a/pkg/printers/jsonpath_flags.go b/pkg/printers/jsonpath_flags.go index b9443a5546e..636d6cd0158 100644 --- a/pkg/printers/jsonpath_flags.go +++ b/pkg/printers/jsonpath_flags.go @@ -39,7 +39,7 @@ type JSONPathPrintFlags struct { // Returns false if the specified templateFormat does not match a template format. func (f *JSONPathPrintFlags) ToPrinter(templateFormat string) (ResourcePrinter, error) { if (f.TemplateArgument == nil || len(*f.TemplateArgument) == 0) && len(templateFormat) == 0 { - return nil, NoCompatiblePrinterError{f} + return nil, NoCompatiblePrinterError{Options: f, OutputFormat: &templateFormat} } templateValue := "" @@ -66,7 +66,7 @@ func (f *JSONPathPrintFlags) ToPrinter(templateFormat string) (ResourcePrinter, } if _, supportedFormat := templateFormats[templateFormat]; !supportedFormat { - return nil, NoCompatiblePrinterError{f} + return nil, NoCompatiblePrinterError{Options: f, OutputFormat: &templateFormat} } if len(templateValue) == 0 { diff --git a/pkg/printers/name_flags.go b/pkg/printers/name_flags.go index 17deb358534..193b4e5947f 100644 --- a/pkg/printers/name_flags.go +++ b/pkg/printers/name_flags.go @@ -63,7 +63,7 @@ func (f *NamePrintFlags) ToPrinter(outputFormat string) (ResourcePrinter, error) case "": return namePrinter, nil default: - return nil, NoCompatiblePrinterError{f} + return nil, NoCompatiblePrinterError{Options: f, OutputFormat: &outputFormat} } } diff --git a/pkg/printers/template_flags.go b/pkg/printers/template_flags.go index 8fce11f827e..6d713e415fa 100644 --- a/pkg/printers/template_flags.go +++ b/pkg/printers/template_flags.go @@ -39,7 +39,7 @@ type GoTemplatePrintFlags struct { // Returns false if the specified templateFormat does not match a template format. func (f *GoTemplatePrintFlags) ToPrinter(templateFormat string) (ResourcePrinter, error) { if (f.TemplateArgument == nil || len(*f.TemplateArgument) == 0) && len(templateFormat) == 0 { - return nil, NoCompatiblePrinterError{f} + return nil, NoCompatiblePrinterError{Options: f, OutputFormat: &templateFormat} } templateValue := "" @@ -68,7 +68,7 @@ func (f *GoTemplatePrintFlags) ToPrinter(templateFormat string) (ResourcePrinter } if _, supportedFormat := supportedFormats[templateFormat]; !supportedFormat { - return nil, NoCompatiblePrinterError{f} + return nil, NoCompatiblePrinterError{Options: f, OutputFormat: &templateFormat} } if len(templateValue) == 0 {