From a198a620640688222fc5900334ba3316bcf3ef2b Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Tue, 11 Nov 2014 16:15:22 -0800 Subject: [PATCH] refactor resource printer's version handling (to make adding --watch feature easier) --- pkg/kubectl/cmd/get.go | 16 ++---- pkg/kubectl/resource_printer.go | 81 ++++++++++++++++++---------- pkg/kubectl/resource_printer_test.go | 28 +++++----- 3 files changed, 74 insertions(+), 51 deletions(-) diff --git a/pkg/kubectl/cmd/get.go b/pkg/kubectl/cmd/get.go index ea1f4dc0930..fd0af00c6a8 100644 --- a/pkg/kubectl/cmd/get.go +++ b/pkg/kubectl/cmd/get.go @@ -61,22 +61,16 @@ Examples: defaultPrinter, err := f.Printer(cmd, mapping, GetFlagBool(cmd, "no-headers")) checkErr(err) - printer, versioned, err := kubectl.GetPrinter(outputFormat, templateFile, defaultPrinter) + outputVersion := GetFlagString(cmd, "output-version") + if len(outputVersion) == 0 { + outputVersion = mapping.APIVersion + } + printer, err := kubectl.GetPrinter(outputVersion, outputFormat, templateFile, defaultPrinter) checkErr(err) obj, err := kubectl.NewRESTHelper(client, mapping).Get(namespace, name, labels) checkErr(err) - if versioned { - outputVersion := GetFlagString(cmd, "output-version") - if len(outputVersion) == 0 { - outputVersion = mapping.APIVersion - } - - obj, err = mapping.ObjectConvertor.ConvertToVersion(obj, outputVersion) - checkErr(err) - } - if err := printer.PrintObj(obj, out); err != nil { checkErr(fmt.Errorf("Unable to output the provided object: %v", err)) } diff --git a/pkg/kubectl/resource_printer.go b/pkg/kubectl/resource_printer.go index ec2f1978338..9f55f84d629 100644 --- a/pkg/kubectl/resource_printer.go +++ b/pkg/kubectl/resource_printer.go @@ -37,57 +37,64 @@ import ( // GetPrinter returns a resource printer and a bool indicating whether the object must be // versioned for the given format. -// TODO: remove the 'versioned' return value, replace with method on ResourcePrinter. -func GetPrinter(format, templateFile string, defaultPrinter ResourcePrinter) (ResourcePrinter, bool, error) { - versioned := true +func GetPrinter(version, format, templateFile string, defaultPrinter ResourcePrinter) (ResourcePrinter, error) { var printer ResourcePrinter switch format { case "json": - printer = &JSONPrinter{} + printer = &JSONPrinter{version} case "yaml": - printer = &YAMLPrinter{} + printer = &YAMLPrinter{version} case "template": if len(templateFile) == 0 { - return nil, false, fmt.Errorf("template format specified but no template given") + return nil, fmt.Errorf("template format specified but no template given") } var err error - printer, err = NewTemplatePrinter([]byte(templateFile)) + printer, err = NewTemplatePrinter(version, []byte(templateFile)) if err != nil { - return nil, false, fmt.Errorf("error parsing template %s, %v\n", templateFile, err) + return nil, fmt.Errorf("error parsing template %s, %v\n", templateFile, err) } case "templatefile": if len(templateFile) == 0 { - return nil, false, fmt.Errorf("templatefile format specified but no template file given") + return nil, fmt.Errorf("templatefile format specified but no template file given") } data, err := ioutil.ReadFile(templateFile) if err != nil { - return nil, false, fmt.Errorf("error reading template %s, %v\n", templateFile, err) + return nil, fmt.Errorf("error reading template %s, %v\n", templateFile, err) } - printer, err = NewTemplatePrinter(data) + printer, err = NewTemplatePrinter(version, data) if err != nil { - return nil, false, fmt.Errorf("error parsing template %s, %v\n", string(data), err) + return nil, fmt.Errorf("error parsing template %s, %v\n", string(data), err) } case "": printer = defaultPrinter - versioned = false default: - return nil, false, fmt.Errorf("output format %q not recognized", format) + return nil, fmt.Errorf("output format %q not recognized", format) } - return printer, versioned, nil + return printer, nil } // ResourcePrinter is an interface that knows how to print API resources. type ResourcePrinter interface { // Print receives an arbitrary object, formats it and prints it to a writer. PrintObj(runtime.Object, io.Writer) error + + // Returns true if this printer emits properly versioned output. + IsVersioned() bool } -// IdentityPrinter is an implementation of ResourcePrinter which simply copies the body out to the output stream. -type JSONPrinter struct{} +// JSONPrinter is an implementation of ResourcePrinter which prints as JSON. +type JSONPrinter struct { + version string +} // PrintObj is an implementation of ResourcePrinter.PrintObj which simply writes the object to the Writer. -func (i *JSONPrinter) PrintObj(obj runtime.Object, w io.Writer) error { - data, err := latest.Codec.Encode(obj) +func (j *JSONPrinter) PrintObj(obj runtime.Object, w io.Writer) error { + vi, err := latest.InterfacesFor(j.version) + if err != nil { + return err + } + + data, err := vi.Codec.Encode(obj) if err != nil { return err } @@ -98,8 +105,16 @@ func (i *JSONPrinter) PrintObj(obj runtime.Object, w io.Writer) error { return err } -func toVersionedMap(obj runtime.Object) (map[string]interface{}, error) { - data, err := latest.Codec.Encode(obj) +// IsVersioned returns true. +func (*JSONPrinter) IsVersioned() bool { return true } + +func toVersionedMap(version string, obj runtime.Object) (map[string]interface{}, error) { + vi, err := latest.InterfacesFor(version) + if err != nil { + return nil, err + } + + data, err := vi.Codec.Encode(obj) if err != nil { return nil, err } @@ -111,12 +126,14 @@ func toVersionedMap(obj runtime.Object) (map[string]interface{}, error) { return outObj, nil } -// YAMLPrinter is an implementation of ResourcePrinter which parsess JSON, and re-formats as YAML. -type YAMLPrinter struct{} +// YAMLPrinter is an implementation of ResourcePrinter which prints as YAML. +type YAMLPrinter struct { + version string +} // PrintObj prints the data as YAML. func (y *YAMLPrinter) PrintObj(obj runtime.Object, w io.Writer) error { - outObj, err := toVersionedMap(obj) + outObj, err := toVersionedMap(y.version, obj) if err != nil { return err } @@ -129,6 +146,9 @@ func (y *YAMLPrinter) PrintObj(obj runtime.Object, w io.Writer) error { return err } +// IsVersioned returns true. +func (*YAMLPrinter) IsVersioned() bool { return true } + type handlerEntry struct { columns []string printFunc reflect.Value @@ -140,6 +160,9 @@ type HumanReadablePrinter struct { noHeaders bool } +// IsVersioned returns false-- human readable printers do not make versioned output. +func (*HumanReadablePrinter) IsVersioned() bool { return false } + // NewHumanReadablePrinter creates a HumanReadablePrinter. func NewHumanReadablePrinter(noHeaders bool) *HumanReadablePrinter { printer := &HumanReadablePrinter{ @@ -337,20 +360,24 @@ func (h *HumanReadablePrinter) PrintObj(obj runtime.Object, output io.Writer) er // TemplatePrinter is an implementation of ResourcePrinter which formats data with a Go Template. type TemplatePrinter struct { + version string template *template.Template } -func NewTemplatePrinter(tmpl []byte) (*TemplatePrinter, error) { +func NewTemplatePrinter(version string, tmpl []byte) (*TemplatePrinter, error) { t, err := template.New("output").Parse(string(tmpl)) if err != nil { return nil, err } - return &TemplatePrinter{t}, nil + return &TemplatePrinter{version, t}, nil } +// IsVersioned returns true. +func (*TemplatePrinter) IsVersioned() bool { return true } + // PrintObj formats the obj with the Go Template. func (t *TemplatePrinter) PrintObj(obj runtime.Object, w io.Writer) error { - outObj, err := toVersionedMap(obj) + outObj, err := toVersionedMap(t.version, obj) if err != nil { return err } diff --git a/pkg/kubectl/resource_printer_test.go b/pkg/kubectl/resource_printer_test.go index b52580976a4..ac34c32c42e 100644 --- a/pkg/kubectl/resource_printer_test.go +++ b/pkg/kubectl/resource_printer_test.go @@ -42,9 +42,11 @@ type testStruct struct { func (ts *testStruct) IsAnAPIObject() {} +const TestVersion = latest.Version + func init() { api.Scheme.AddKnownTypes("", &testStruct{}) - api.Scheme.AddKnownTypes(latest.Version, &testStruct{}) + api.Scheme.AddKnownTypes(TestVersion, &testStruct{}) } var testData = testStruct{ @@ -55,20 +57,20 @@ var testData = testStruct{ } func TestYAMLPrinter(t *testing.T) { - testPrinter(t, &YAMLPrinter{}, yaml.Unmarshal) + testPrinter(t, &YAMLPrinter{TestVersion}, yaml.Unmarshal) } func TestJSONPrinter(t *testing.T) { - testPrinter(t, &JSONPrinter{}, json.Unmarshal) + testPrinter(t, &JSONPrinter{TestVersion}, json.Unmarshal) } func TestPrintJSON(t *testing.T) { buf := bytes.NewBuffer([]byte{}) - printer, versioned, err := GetPrinter("json", "", nil) + printer, err := GetPrinter(TestVersion, "json", "", nil) if err != nil { t.Errorf("unexpected error: %#v", err) } - if !versioned { + if !printer.IsVersioned() { t.Errorf("printer should be versioned") } printer.PrintObj(&api.Pod{ObjectMeta: api.ObjectMeta{Name: "foo"}}, buf) @@ -80,11 +82,11 @@ func TestPrintJSON(t *testing.T) { func TestPrintYAML(t *testing.T) { buf := bytes.NewBuffer([]byte{}) - printer, versioned, err := GetPrinter("yaml", "", nil) + printer, err := GetPrinter(TestVersion, "yaml", "", nil) if err != nil { t.Errorf("unexpected error: %#v", err) } - if !versioned { + if !printer.IsVersioned() { t.Errorf("printer should be versioned") } printer.PrintObj(&api.Pod{ObjectMeta: api.ObjectMeta{Name: "foo"}}, buf) @@ -96,11 +98,11 @@ func TestPrintYAML(t *testing.T) { func TestPrintTemplate(t *testing.T) { buf := bytes.NewBuffer([]byte{}) - printer, versioned, err := GetPrinter("template", "{{.id}}", nil) + printer, err := GetPrinter(TestVersion, "template", "{{.id}}", nil) if err != nil { t.Fatalf("unexpected error: %#v", err) } - if !versioned { + if !printer.IsVersioned() { t.Errorf("printer should be versioned") } err = printer.PrintObj(&api.Pod{ObjectMeta: api.ObjectMeta{Name: "foo"}}, buf) @@ -113,19 +115,19 @@ func TestPrintTemplate(t *testing.T) { } func TestPrintEmptyTemplate(t *testing.T) { - if _, _, err := GetPrinter("template", "", nil); err == nil { + if _, err := GetPrinter(TestVersion, "template", "", nil); err == nil { t.Errorf("unexpected non-error") } } func TestPrintBadTemplate(t *testing.T) { - if _, _, err := GetPrinter("template", "{{ .Name", nil); err == nil { + if _, err := GetPrinter(TestVersion, "template", "{{ .Name", nil); err == nil { t.Errorf("unexpected non-error") } } func TestPrintBadTemplateFile(t *testing.T) { - if _, _, err := GetPrinter("templatefile", "", nil); err == nil { + if _, err := GetPrinter(TestVersion, "templatefile", "", nil); err == nil { t.Errorf("unexpected non-error") } } @@ -234,7 +236,7 @@ func TestUnknownTypePrinting(t *testing.T) { func TestTemplateEmitsVersionedObjects(t *testing.T) { // kind is always blank in memory and set on the wire - printer, err := NewTemplatePrinter([]byte(`{{.kind}}`)) + printer, err := NewTemplatePrinter(TestVersion, []byte(`{{.kind}}`)) if err != nil { t.Fatalf("tmpl fail: %v", err) }