From 04ab96d2bdbd3d613a944888d9925b9e6b377113 Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Tue, 14 Nov 2017 23:03:06 -0500 Subject: [PATCH] Unify Object and UnstructuredObject The unified RESTMapper and Typer follow the new rules, but on error will fallback to the legacy path (while still supporting Unstructured objects). This allows callers to handle the appropriate distinction themselves if necessary. Add a LocalParam() method to the resource.Builder that DRYs up a large chunk of complicated code in set commands. --- pkg/kubectl/cmd/annotate.go | 11 +-- pkg/kubectl/cmd/apply_set_last_applied.go | 2 +- pkg/kubectl/cmd/convert.go | 8 +- pkg/kubectl/cmd/convert_test.go | 45 +++++------ pkg/kubectl/cmd/label.go | 20 ++--- pkg/kubectl/cmd/resource/get_test.go | 2 +- pkg/kubectl/cmd/set/set_env.go | 9 +-- pkg/kubectl/cmd/set/set_image.go | 6 +- pkg/kubectl/cmd/set/set_resources.go | 8 +- pkg/kubectl/cmd/set/set_selector.go | 5 +- pkg/kubectl/cmd/set/set_serviceaccount.go | 6 +- pkg/kubectl/cmd/set/set_subject.go | 20 +++-- pkg/kubectl/cmd/testing/fake.go | 77 +++++++++++++------ pkg/kubectl/cmd/util/editor/editoptions.go | 2 +- pkg/kubectl/cmd/util/factory.go | 4 - pkg/kubectl/cmd/util/factory_builder.go | 20 +---- .../cmd/util/factory_object_mapping.go | 38 +++------ pkg/kubectl/resource/builder.go | 8 ++ 18 files changed, 135 insertions(+), 156 deletions(-) diff --git a/pkg/kubectl/cmd/annotate.go b/pkg/kubectl/cmd/annotate.go index d80071ea6dc..09541ea0e24 100644 --- a/pkg/kubectl/cmd/annotate.go +++ b/pkg/kubectl/cmd/annotate.go @@ -189,13 +189,10 @@ func (o AnnotateOptions) RunAnnotate(f cmdutil.Factory, cmd *cobra.Command) erro changeCause := f.Command(cmd, false) includeUninitialized := cmdutil.ShouldIncludeUninitialized(cmd, false) - b := f.NewBuilder() - if o.local { - b = b.Local() - } else { - b = b.Unstructured() - } - b = b.ContinueOnError(). + b := f.NewBuilder(). + Unstructured(). + LocalParam(o.local). + ContinueOnError(). NamespaceParam(namespace).DefaultNamespace(). FilenameParam(enforceNamespace, &o.FilenameOptions). IncludeUninitialized(includeUninitialized). diff --git a/pkg/kubectl/cmd/apply_set_last_applied.go b/pkg/kubectl/cmd/apply_set_last_applied.go index 8e936e75bd9..640009b2788 100644 --- a/pkg/kubectl/cmd/apply_set_last_applied.go +++ b/pkg/kubectl/cmd/apply_set_last_applied.go @@ -112,7 +112,7 @@ func (o *SetLastAppliedOptions) Complete(f cmdutil.Factory, cmd *cobra.Command) o.Codec = f.JSONEncoder() var err error - o.Mapper, o.Typer = f.UnstructuredObject() + o.Mapper, o.Typer = f.Object() if err != nil { return err } diff --git a/pkg/kubectl/cmd/convert.go b/pkg/kubectl/cmd/convert.go index 51ffe603668..f65a5e6a45a 100644 --- a/pkg/kubectl/cmd/convert.go +++ b/pkg/kubectl/cmd/convert.go @@ -128,22 +128,20 @@ func (o *ConvertOptions) Complete(f cmdutil.Factory, out io.Writer, cmd *cobra.C } // build the builder - o.builder = f.NewBuilder() + o.builder = f.NewBuilder().LocalParam(o.local) if !o.local { schema, err := f.Validator(cmdutil.GetFlagBool(cmd, "validate")) if err != nil { return err } - o.builder = o.builder.Schema(schema) - } else { - o.builder = o.builder.Local() + o.builder.Schema(schema) } cmdNamespace, _, err := f.DefaultNamespace() if err != nil { return err } - o.builder = o.builder.NamespaceParam(cmdNamespace). + o.builder.NamespaceParam(cmdNamespace). ContinueOnError(). FilenameParam(false, &o.FilenameOptions). Flatten() diff --git a/pkg/kubectl/cmd/convert_test.go b/pkg/kubectl/cmd/convert_test.go index c6d1ed7d9dd..39e368749a7 100644 --- a/pkg/kubectl/cmd/convert_test.go +++ b/pkg/kubectl/cmd/convert_test.go @@ -18,6 +18,7 @@ package cmd import ( "bytes" + "fmt" "net/http" "testing" @@ -98,30 +99,30 @@ func TestConvertObject(t *testing.T) { }, } - f, tf, _, _ := cmdtesting.NewAPIFactory() - tf.UnstructuredClient = &fake.RESTClient{ - Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) { - t.Fatalf("unexpected request: %#v\n%#v", req.URL, req) - return nil, nil - }), - } - tf.Namespace = "test" - buf := bytes.NewBuffer([]byte{}) - for _, tc := range testcases { - cmd := NewCmdConvert(f, buf) - cmd.Flags().Set("filename", tc.file) - cmd.Flags().Set("output-version", tc.outputVersion) - cmd.Flags().Set("local", "true") - cmd.Flags().Set("output", "go-template") - for _, field := range tc.fields { - buf.Reset() - tf.Printer, _ = printers.NewTemplatePrinter([]byte(field.template)) - cmd.Run(cmd, []string{}) - if buf.String() != field.expected { - t.Errorf("unexpected output when converting %s to %q, expected: %q, but got %q", tc.file, tc.outputVersion, field.expected, buf.String()) - } + t.Run(fmt.Sprintf("%s %s", tc.name, field), func(t *testing.T) { + f, tf, _, _ := cmdtesting.NewAPIFactory() + tf.UnstructuredClient = &fake.RESTClient{ + Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) { + t.Fatalf("unexpected request: %#v\n%#v", req.URL, req) + return nil, nil + }), + } + tf.Namespace = "test" + + buf := bytes.NewBuffer([]byte{}) + cmd := NewCmdConvert(f, buf) + cmd.Flags().Set("filename", tc.file) + cmd.Flags().Set("output-version", tc.outputVersion) + cmd.Flags().Set("local", "true") + cmd.Flags().Set("output", "go-template") + tf.Printer, _ = printers.NewTemplatePrinter([]byte(field.template)) + cmd.Run(cmd, []string{}) + if buf.String() != field.expected { + t.Errorf("unexpected output when converting %s to %q, expected: %q, but got %q", tc.file, tc.outputVersion, field.expected, buf.String()) + } + }) } } } diff --git a/pkg/kubectl/cmd/label.go b/pkg/kubectl/cmd/label.go index f21d38a7e27..786fbe6afcb 100644 --- a/pkg/kubectl/cmd/label.go +++ b/pkg/kubectl/cmd/label.go @@ -191,13 +191,9 @@ func (o *LabelOptions) RunLabel(f cmdutil.Factory, cmd *cobra.Command) error { changeCause := f.Command(cmd, false) includeUninitialized := cmdutil.ShouldIncludeUninitialized(cmd, false) - b := f.NewBuilder() - if o.local { - b = b.Local() - } else { - b = b.Unstructured() - } - b = b. + b := f.NewBuilder(). + Unstructured(). + LocalParam(o.local). ContinueOnError(). NamespaceParam(cmdNamespace).DefaultNamespace(). FilenameParam(enforceNamespace, &o.FilenameOptions). @@ -304,16 +300,10 @@ func (o *LabelOptions) RunLabel(f cmdutil.Factory, cmd *cobra.Command) error { return nil } - var mapper meta.RESTMapper - if o.local { - mapper, _ = f.Object() - } else { - mapper, _ = f.UnstructuredObject() - } if o.outputFormat != "" { - return f.PrintObject(cmd, o.local, mapper, outputObj, o.out) + return f.PrintObject(cmd, o.local, r.Mapper().RESTMapper, outputObj, o.out) } - f.PrintSuccess(mapper, false, o.out, info.Mapping.Resource, info.Name, o.dryrun, dataChangeMsg) + f.PrintSuccess(r.Mapper().RESTMapper, false, o.out, info.Mapping.Resource, info.Name, o.dryrun, dataChangeMsg) return nil }) } diff --git a/pkg/kubectl/cmd/resource/get_test.go b/pkg/kubectl/cmd/resource/get_test.go index e840e2813d8..9d30530aa96 100644 --- a/pkg/kubectl/cmd/resource/get_test.go +++ b/pkg/kubectl/cmd/resource/get_test.go @@ -230,7 +230,7 @@ func TestGetUnknownSchemaObject(t *testing.T) { tf.Namespace = "test" tf.ClientConfig = defaultClientConfig() - mapper, _ := f.UnstructuredObject() + mapper, _ := f.Object() m, err := mapper.RESTMapping(schema.GroupKind{Group: "apitest", Kind: "Type"}) if err != nil { t.Fatal(err) diff --git a/pkg/kubectl/cmd/set/set_env.go b/pkg/kubectl/cmd/set/set_env.go index 8378b97eddc..e107c0cf191 100644 --- a/pkg/kubectl/cmd/set/set_env.go +++ b/pkg/kubectl/cmd/set/set_env.go @@ -236,6 +236,7 @@ func (o *EnvOptions) RunEnv(f cmdutil.Factory) error { if len(o.From) != 0 { b := f.NewBuilder(). + LocalParam(o.Local). ContinueOnError(). NamespaceParam(cmdNamespace).DefaultNamespace(). FilenameParam(enforceNamespace, &o.FilenameOptions). @@ -246,8 +247,6 @@ func (o *EnvOptions) RunEnv(f cmdutil.Factory) error { LabelSelectorParam(o.Selector). ResourceTypeOrNameArgs(o.All, o.From). Latest() - } else { - b = b.Local() } infos, err := b.Do().Infos() @@ -304,18 +303,16 @@ func (o *EnvOptions) RunEnv(f cmdutil.Factory) error { } b := f.NewBuilder(). + LocalParam(o.Local). ContinueOnError(). NamespaceParam(cmdNamespace).DefaultNamespace(). FilenameParam(enforceNamespace, &o.FilenameOptions). Flatten() if !o.Local { - b = b. - LabelSelectorParam(o.Selector). + b.LabelSelectorParam(o.Selector). ResourceTypeOrNameArgs(o.All, o.Resources...). Latest() - } else { - b = b.Local() } o.Infos, err = b.Do().Infos() diff --git a/pkg/kubectl/cmd/set/set_image.go b/pkg/kubectl/cmd/set/set_image.go index e6636620ed7..f29735de01a 100644 --- a/pkg/kubectl/cmd/set/set_image.go +++ b/pkg/kubectl/cmd/set/set_image.go @@ -143,6 +143,7 @@ func (o *ImageOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []st includeUninitialized := cmdutil.ShouldIncludeUninitialized(cmd, false) builder := f.NewBuilder(). + LocalParam(o.Local). ContinueOnError(). NamespaceParam(cmdNamespace).DefaultNamespace(). FilenameParam(enforceNamespace, &o.FilenameOptions). @@ -150,8 +151,7 @@ func (o *ImageOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []st Flatten() if !o.Local { - builder = builder. - LabelSelectorParam(o.Selector). + builder.LabelSelectorParam(o.Selector). ResourceTypeOrNameArgs(o.All, o.Resources...). Latest() } else { @@ -161,8 +161,6 @@ func (o *ImageOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args []st if len(o.Resources) > 0 { return resource.LocalResourceError } - - builder = builder.Local() } o.Infos, err = builder.Do().Infos() diff --git a/pkg/kubectl/cmd/set/set_resources.go b/pkg/kubectl/cmd/set/set_resources.go index bfa1fbfa023..1eb1cba7c56 100644 --- a/pkg/kubectl/cmd/set/set_resources.go +++ b/pkg/kubectl/cmd/set/set_resources.go @@ -145,6 +145,7 @@ func (o *ResourcesOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args includeUninitialized := cmdutil.ShouldIncludeUninitialized(cmd, false) builder := f.NewBuilder(). + LocalParam(o.Local). ContinueOnError(). NamespaceParam(cmdNamespace).DefaultNamespace(). FilenameParam(enforceNamespace, &o.FilenameOptions). @@ -152,19 +153,18 @@ func (o *ResourcesOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args Flatten() if !o.Local { - builder = builder. - LabelSelectorParam(o.Selector). + builder.LabelSelectorParam(o.Selector). ResourceTypeOrNameArgs(o.All, args...). Latest() } else { // if a --local flag was provided, and a resource was specified in the form // /, fail immediately as --local cannot query the api server // for the specified resource. + // TODO: this should be in the builder - if someone specifies tuples, fail when + // local is true if len(args) > 0 { return resource.LocalResourceError } - - builder = builder.Local() } o.Infos, err = builder.Do().Infos() diff --git a/pkg/kubectl/cmd/set/set_selector.go b/pkg/kubectl/cmd/set/set_selector.go index f4c5009b1c2..b9b2be82854 100644 --- a/pkg/kubectl/cmd/set/set_selector.go +++ b/pkg/kubectl/cmd/set/set_selector.go @@ -129,6 +129,7 @@ func (o *SelectorOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args [ includeUninitialized := cmdutil.ShouldIncludeUninitialized(cmd, false) o.builder = f.NewBuilder(). + LocalParam(o.local). ContinueOnError(). NamespaceParam(cmdNamespace).DefaultNamespace(). FilenameParam(enforceNamespace, &o.fileOptions). @@ -136,7 +137,7 @@ func (o *SelectorOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args [ Flatten() if !o.local { - o.builder = o.builder. + o.builder. ResourceTypeOrNameArgs(o.all, o.resources...). Latest() } else { @@ -146,8 +147,6 @@ func (o *SelectorOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args [ if len(o.resources) > 0 { return resource.LocalResourceError } - - o.builder = o.builder.Local() } o.PrintObject = func(obj runtime.Object) error { diff --git a/pkg/kubectl/cmd/set/set_serviceaccount.go b/pkg/kubectl/cmd/set/set_serviceaccount.go index b499074965f..b02a48372a4 100644 --- a/pkg/kubectl/cmd/set/set_serviceaccount.go +++ b/pkg/kubectl/cmd/set/set_serviceaccount.go @@ -129,7 +129,9 @@ func (saConfig *serviceAccountConfig) Complete(f cmdutil.Factory, cmd *cobra.Com saConfig.serviceAccountName = args[len(args)-1] resources := args[:len(args)-1] includeUninitialized := cmdutil.ShouldIncludeUninitialized(cmd, false) - builder := f.NewBuilder().ContinueOnError(). + builder := f.NewBuilder(). + LocalParam(saConfig.local). + ContinueOnError(). NamespaceParam(cmdNamespace).DefaultNamespace(). FilenameParam(enforceNamespace, &saConfig.fileNameOptions). IncludeUninitialized(includeUninitialized). @@ -137,8 +139,6 @@ func (saConfig *serviceAccountConfig) Complete(f cmdutil.Factory, cmd *cobra.Com if !saConfig.local { builder.ResourceTypeOrNameArgs(saConfig.all, resources...). Latest() - } else { - builder = builder.Local() } saConfig.infos, err = builder.Do().Infos() if err != nil { diff --git a/pkg/kubectl/cmd/set/set_subject.go b/pkg/kubectl/cmd/set/set_subject.go index a76fbf507fe..8f88dd59005 100644 --- a/pkg/kubectl/cmd/set/set_subject.go +++ b/pkg/kubectl/cmd/set/set_subject.go @@ -125,7 +125,14 @@ func (o *SubjectOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args [] } includeUninitialized := cmdutil.ShouldIncludeUninitialized(cmd, false) - builder := f.NewBuilder() + builder := f.NewBuilder(). + LocalParam(o.Local). + ContinueOnError(). + NamespaceParam(cmdNamespace).DefaultNamespace(). + FilenameParam(enforceNamespace, &o.FilenameOptions). + IncludeUninitialized(includeUninitialized). + Flatten() + if o.Local { // if a --local flag was provided, and a resource was specified in the form // /, fail immediately as --local cannot query the api server @@ -133,16 +140,7 @@ func (o *SubjectOptions) Complete(f cmdutil.Factory, cmd *cobra.Command, args [] if len(args) > 0 { return resource.LocalResourceError } - builder = builder.Local() - } - builder = builder. - ContinueOnError(). - NamespaceParam(cmdNamespace).DefaultNamespace(). - FilenameParam(enforceNamespace, &o.FilenameOptions). - IncludeUninitialized(includeUninitialized). - Flatten() - - if !o.Local { + } else { builder = builder. LabelSelectorParam(o.Selector). ResourceTypeOrNameArgs(o.All, args...). diff --git a/pkg/kubectl/cmd/testing/fake.go b/pkg/kubectl/cmd/testing/fake.go index 4729141cb93..7ded4b9e9c2 100644 --- a/pkg/kubectl/cmd/testing/fake.go +++ b/pkg/kubectl/cmd/testing/fake.go @@ -244,6 +244,7 @@ type TestFactory struct { Command string TmpDir string CategoryExpander categories.CategoryExpander + SkipDiscovery bool ClientForMappingFunc func(mapping *meta.RESTMapping) (resource.RESTClient, error) UnstructuredClientForMappingFunc func(mapping *meta.RESTMapping) (resource.RESTClient, error) @@ -282,10 +283,9 @@ func (f *FakeFactory) FlagSet() *pflag.FlagSet { } func (f *FakeFactory) Object() (meta.RESTMapper, runtime.ObjectTyper) { - return legacyscheme.Registry.RESTMapper(), f.tf.Typer -} - -func (f *FakeFactory) UnstructuredObject() (meta.RESTMapper, runtime.ObjectTyper) { + if f.tf.SkipDiscovery { + return legacyscheme.Registry.RESTMapper(), f.tf.Typer + } groupResources := testDynamicResources() mapper := discovery.NewRESTMapper(groupResources, meta.InterfacesForUnstructuredConversion(legacyscheme.Registry.InterfacesFor)) typer := discovery.NewUnstructuredObjectTyper(groupResources) @@ -519,7 +519,6 @@ func (f *FakeFactory) PrinterForMapping(cmd *cobra.Command, isLocal bool, output func (f *FakeFactory) NewBuilder() *resource.Builder { mapper, typer := f.Object() - unstructuredMapper, unstructuredTyper := f.UnstructuredObject() return resource.NewBuilder( &resource.Mapper{ @@ -529,8 +528,8 @@ func (f *FakeFactory) NewBuilder() *resource.Builder { Decoder: f.Decoder(true), }, &resource.Mapper{ - RESTMapper: unstructuredMapper, - ObjectTyper: unstructuredTyper, + RESTMapper: mapper, + ObjectTyper: typer, ClientMapper: resource.ClientMapperFunc(f.UnstructuredClientForMapping), Decoder: unstructured.UnstructuredJSONScheme, }, @@ -605,10 +604,9 @@ type fakeAPIFactory struct { } func (f *fakeAPIFactory) Object() (meta.RESTMapper, runtime.ObjectTyper) { - return testapi.Default.RESTMapper(), legacyscheme.Scheme -} - -func (f *fakeAPIFactory) UnstructuredObject() (meta.RESTMapper, runtime.ObjectTyper) { + if f.tf.SkipDiscovery { + return testapi.Default.RESTMapper(), legacyscheme.Scheme + } groupResources := testDynamicResources() mapper := discovery.NewRESTMapper( groupResources, @@ -800,11 +798,7 @@ func (f *fakeAPIFactory) LogsForObject(object, options runtime.Object, timeout t } return c.Core().Pods(f.tf.Namespace).GetLogs(t.Name, opts), nil default: - fqKinds, _, err := legacyscheme.Scheme.ObjectKinds(object) - if err != nil { - return nil, err - } - return nil, fmt.Errorf("cannot get the logs from %v", fqKinds[0]) + return nil, fmt.Errorf("cannot get the logs from %T", object) } } @@ -813,11 +807,7 @@ func (f *fakeAPIFactory) AttachablePodForObject(object runtime.Object, timeout t case *api.Pod: return t, nil default: - gvks, _, err := legacyscheme.Scheme.ObjectKinds(object) - if err != nil { - return nil, err - } - return nil, fmt.Errorf("cannot attach to %v: not implemented", gvks[0]) + return nil, fmt.Errorf("cannot attach to %T: not implemented", object) } } @@ -865,7 +855,6 @@ func (f *fakeAPIFactory) PrinterForMapping(cmd *cobra.Command, isLocal bool, out func (f *fakeAPIFactory) NewBuilder() *resource.Builder { mapper, typer := f.Object() - unstructuredMapper, unstructuredTyper := f.UnstructuredObject() return resource.NewBuilder( &resource.Mapper{ @@ -875,8 +864,8 @@ func (f *fakeAPIFactory) NewBuilder() *resource.Builder { Decoder: f.Decoder(true), }, &resource.Mapper{ - RESTMapper: unstructuredMapper, - ObjectTyper: unstructuredTyper, + RESTMapper: mapper, + ObjectTyper: typer, ClientMapper: resource.ClientMapperFunc(f.UnstructuredClientForMapping), Decoder: unstructured.UnstructuredJSONScheme, }, @@ -953,6 +942,46 @@ func testDynamicResources() []*discovery.APIGroupResources { }, }, }, + { + Group: metav1.APIGroup{ + Name: "apps", + Versions: []metav1.GroupVersionForDiscovery{ + {Version: "v1beta1"}, + {Version: "v1beta2"}, + {Version: "v1"}, + }, + PreferredVersion: metav1.GroupVersionForDiscovery{Version: "v1"}, + }, + VersionedResources: map[string][]metav1.APIResource{ + "v1beta1": { + {Name: "deployments", Namespaced: true, Kind: "Deployment"}, + }, + "v1beta2": { + {Name: "deployments", Namespaced: true, Kind: "Deployment"}, + }, + "v1": { + {Name: "deployments", Namespaced: true, Kind: "Deployment"}, + }, + }, + }, + { + Group: metav1.APIGroup{ + Name: "autoscaling", + Versions: []metav1.GroupVersionForDiscovery{ + {Version: "v1"}, + {Version: "v2beta1"}, + }, + PreferredVersion: metav1.GroupVersionForDiscovery{Version: "v2beta1"}, + }, + VersionedResources: map[string][]metav1.APIResource{ + "v1": { + {Name: "horizontalpodautoscalers", Namespaced: true, Kind: "HorizontalPodAutoscaler"}, + }, + "v2beta1": { + {Name: "horizontalpodautoscalers", Namespaced: true, Kind: "HorizontalPodAutoscaler"}, + }, + }, + }, { Group: metav1.APIGroup{ Name: "storage.k8s.io", diff --git a/pkg/kubectl/cmd/util/editor/editoptions.go b/pkg/kubectl/cmd/util/editor/editoptions.go index 19a4f666b3c..cb4d527ea22 100644 --- a/pkg/kubectl/cmd/util/editor/editoptions.go +++ b/pkg/kubectl/cmd/util/editor/editoptions.go @@ -107,7 +107,7 @@ func (o *EditOptions) Complete(f cmdutil.Factory, out, errOut io.Writer, args [] if err != nil { return err } - mapper, _ := f.UnstructuredObject() + mapper, _ := f.Object() b := f.NewBuilder().Unstructured() if o.EditMode == NormalEditMode || o.EditMode == ApplyEditMode { // when do normal edit or apply edit we need to always retrieve the latest resource from server diff --git a/pkg/kubectl/cmd/util/factory.go b/pkg/kubectl/cmd/util/factory.go index 35cba682508..7ed72e4c6c2 100644 --- a/pkg/kubectl/cmd/util/factory.go +++ b/pkg/kubectl/cmd/util/factory.go @@ -189,11 +189,7 @@ type ClientAccessFactory interface { type ObjectMappingFactory interface { // Returns interfaces for dealing with arbitrary runtime.Objects. Object() (meta.RESTMapper, runtime.ObjectTyper) - // Returns interfaces for dealing with arbitrary - // runtime.Unstructured. This performs API calls to discover types. - UnstructuredObject() (meta.RESTMapper, runtime.ObjectTyper) // Returns interface for expanding categories like `all`. - // TODO: this should probably return an error if the full expander can't be loaded. CategoryExpander() categories.CategoryExpander // Returns a RESTClient for working with the specified RESTMapping or an error. This is intended // for working with arbitrary resources and is not guaranteed to point to a Kubernetes APIServer. diff --git a/pkg/kubectl/cmd/util/factory_builder.go b/pkg/kubectl/cmd/util/factory_builder.go index 413d764e2a6..08516705086 100644 --- a/pkg/kubectl/cmd/util/factory_builder.go +++ b/pkg/kubectl/cmd/util/factory_builder.go @@ -28,7 +28,6 @@ import ( "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/kubernetes/pkg/api/legacyscheme" "k8s.io/kubernetes/pkg/kubectl/plugins" "k8s.io/kubernetes/pkg/kubectl/resource" "k8s.io/kubernetes/pkg/printers" @@ -52,12 +51,8 @@ func (f *ring2Factory) PrinterForCommand(cmd *cobra.Command, isLocal bool, outpu var mapper meta.RESTMapper var typer runtime.ObjectTyper - if isLocal { - mapper = legacyscheme.Registry.RESTMapper() - typer = legacyscheme.Scheme - } else { - mapper, typer = f.objectMappingFactory.UnstructuredObject() - } + mapper, typer = f.objectMappingFactory.Object() + // TODO: used by the custom column implementation and the name implementation, break this dependency decoders := []runtime.Decoder{f.clientAccessFactory.Decoder(true), unstructured.UnstructuredJSONScheme} encoder := f.clientAccessFactory.JSONEncoder() @@ -131,12 +126,6 @@ func (f *ring2Factory) PrintObject(cmd *cobra.Command, isLocal bool, mapper meta _, typer := f.objectMappingFactory.Object() gvks, _, err := typer.ObjectKinds(obj) - // fall back to an unstructured object if we get something unregistered - if runtime.IsNotRegisteredError(err) { - _, typer := f.objectMappingFactory.UnstructuredObject() - gvks, _, err = typer.ObjectKinds(obj) - } - if err != nil { return err } @@ -178,7 +167,6 @@ func (f *ring2Factory) NewBuilder() *resource.Builder { mapper, typer := f.objectMappingFactory.Object() unstructuredClientMapperFunc := resource.ClientMapperFunc(f.objectMappingFactory.UnstructuredClientForMapping) - unstructuredMapper, unstructuredTyper := f.objectMappingFactory.Object() categoryExpander := f.objectMappingFactory.CategoryExpander() @@ -190,8 +178,8 @@ func (f *ring2Factory) NewBuilder() *resource.Builder { Decoder: f.clientAccessFactory.Decoder(true), }, &resource.Mapper{ - RESTMapper: unstructuredMapper, - ObjectTyper: unstructuredTyper, + RESTMapper: mapper, + ObjectTyper: typer, ClientMapper: unstructuredClientMapperFunc, Decoder: unstructured.UnstructuredJSONScheme, }, diff --git a/pkg/kubectl/cmd/util/factory_object_mapping.go b/pkg/kubectl/cmd/util/factory_object_mapping.go index 8bb2a1c5a70..5912c805360 100644 --- a/pkg/kubectl/cmd/util/factory_object_mapping.go +++ b/pkg/kubectl/cmd/util/factory_object_mapping.go @@ -73,33 +73,15 @@ func NewObjectMappingFactory(clientAccessFactory ClientAccessFactory) ObjectMapp return f } +// objectLoader attempts to perform discovery against the server, and will fall back to +// the built in mapper if necessary. It supports unstructured objects either way, since +// the underlying Scheme supports Unstructured. The mapper will return converters that can +// convert versioned types to unstructured and back. func (f *ring1Factory) objectLoader() (meta.RESTMapper, runtime.ObjectTyper, error) { - mapper := legacyscheme.Registry.RESTMapper() discoveryClient, err := f.clientAccessFactory.DiscoveryClient() if err != nil { - glog.V(5).Infof("Object loader was unable to retrieve a discovery client: %v", err) - return mapper, legacyscheme.Scheme, nil - } - mapper = meta.FirstHitRESTMapper{ - MultiRESTMapper: meta.MultiRESTMapper{ - discovery.NewDeferredDiscoveryRESTMapper(discoveryClient, legacyscheme.Registry.InterfacesFor), - legacyscheme.Registry.RESTMapper(), // hardcoded fall back - }, - } - - // wrap with shortcuts, they require a discoveryClient - mapper = NewShortcutExpander(mapper, discoveryClient) - return mapper, legacyscheme.Scheme, nil -} - -// TODO: unstructured and structured discovery should both gracefully degrade in the presence -// of errors, and it should be possible to get access to the unstructured object typers no matter -// whether the server responds or not. Make the legacy scheme recognize object typer, then we can -// safely fall back to the built in restmapper as a last resort. -func (f *ring1Factory) unstructuredObjectLoader() (meta.RESTMapper, runtime.ObjectTyper, error) { - discoveryClient, err := f.clientAccessFactory.DiscoveryClient() - if err != nil { - return nil, nil, err + glog.V(3).Infof("Unable to get a discovery client to find server resources, falling back to hardcoded types: %v", err) + return legacyscheme.Registry.RESTMapper(), legacyscheme.Scheme, nil } groupResources, err := discovery.GetAPIGroupResources(discoveryClient) @@ -108,12 +90,14 @@ func (f *ring1Factory) unstructuredObjectLoader() (meta.RESTMapper, runtime.Obje groupResources, err = discovery.GetAPIGroupResources(discoveryClient) } if err != nil { - return nil, nil, err + glog.V(3).Infof("Unable to retrieve API resources, falling back to hardcoded types: %v", err) + return legacyscheme.Registry.RESTMapper(), legacyscheme.Scheme, nil } // allow conversion between typed and unstructured objects interfaces := meta.InterfacesForUnstructuredConversion(legacyscheme.Registry.InterfacesFor) mapper := discovery.NewDeferredDiscoveryRESTMapper(discoveryClient, meta.VersionInterfacesFunc(interfaces)) + // TODO: should this also indicate it recognizes typed objects? typer := discovery.NewUnstructuredObjectTyper(groupResources) expander := NewShortcutExpander(mapper, discoveryClient) return expander, typer, err @@ -123,10 +107,6 @@ func (f *ring1Factory) Object() (meta.RESTMapper, runtime.ObjectTyper) { return NewLazyObjectLoader(f.objectLoader) } -func (f *ring1Factory) UnstructuredObject() (meta.RESTMapper, runtime.ObjectTyper) { - return NewLazyObjectLoader(f.unstructuredObjectLoader) -} - func (f *ring1Factory) CategoryExpander() categories.CategoryExpander { legacyExpander := categories.LegacyCategoryExpander diff --git a/pkg/kubectl/resource/builder.go b/pkg/kubectl/resource/builder.go index bb42534e26d..bcca08bab7d 100644 --- a/pkg/kubectl/resource/builder.go +++ b/pkg/kubectl/resource/builder.go @@ -177,6 +177,14 @@ func (b *Builder) Unstructured() *Builder { return b } +// LocalParam calls Local() if local is true. +func (b *Builder) LocalParam(local bool) *Builder { + if local { + b.Local() + } + return b +} + // Local will avoid asking the server for results. func (b *Builder) Local() *Builder { mapper := *b.mapper