Allow kubectl get to fetch multiple resource types by label #3004

smarterclayton · 2014-12-17T20:43:34Z

Like Delete, which can now run over multiple types:

kubectl delete pods,services -l name=foo

Get should be able to span items for label selection

kubectl get pods,services -l name=foo

Not yet complete, want feedback from @bgrant0607.

Follow up to #2751

bgrant0607 · 2014-12-18T18:21:37Z

@smarterclayton What exactly would you like feedback on at this point? I think the capability makes a lot of sense, and the example syntax looks fine (modulo the issue that -l increases confusion regarding the difference between labels and selectors, IMO).

I'd also like to be able to get all kinds of objects within a namespace matching a label selector, though that would require resolving #2057.

smarterclayton · 2014-12-18T18:40:32Z

And #991. We could handle that with storing config locally for all the endpoints and resources we know about.

----- Original Message -----

@smarterclayton What exactly would you like feedback on at this point? I
think the capability makes a lot of sense, and the example syntax looks fine
(modulo the issue that -l increases confusion regarding the difference
between labels and selectors, IMO).

I'd also like to be able to get all kinds of objects within a namespace
matching a label selector, though that would require resolving #2057.

Reply to this email directly or view it on GitHub:
#3004 (comment)

smarterclayton · 2014-12-18T22:04:26Z

Have it now so that you can do:

kubectl get services,rc -l provider=kubernetes -o json

and get

{
    "kind": "List",
    "creationTimestamp": null,
    "apiVersion": "v1beta1",
    "items": [
        {
            "kind": "Service",
            "id": "kubernetes-ro",
            "uid": "cf26c187-86ff-11e4-95de-080027ce4245",
            "creationTimestamp": "2014-12-18T16:49:57-05:00",
            "selfLink": "/api/v1beta1/services/kubernetes-ro?namespace=default",
            "resourceVersion": 3,
            "apiVersion": "v1beta1",
            "namespace": "default",
            "port": 80,
            "protocol": "TCP",
            "labels": {
                "component": "apiserver",
                "provider": "kubernetes"
            },
            "selector": null,
            "containerPort": 0,
            "portalIP": "10.0.0.61"
        },
        {
            "kind": "Service",
            "id": "kubernetes",
            "uid": "cf27b128-86ff-11e4-95de-080027ce4245",
            "creationTimestamp": "2014-12-18T16:49:57-05:00",
            "selfLink": "/api/v1beta1/services/kubernetes?namespace=default",
            "resourceVersion": 4,
            "apiVersion": "v1beta1",
            "namespace": "default",
            "port": 443,
            "protocol": "TCP",
            "labels": {
                "component": "apiserver",
                "provider": "kubernetes"
            },
            "selector": null,
            "containerPort": 0,
            "portalIP": "10.0.0.232"
        }
    ]
}

which you can then pipe to enscope or to something that would strip specifics.

smarterclayton · 2014-12-31T01:17:48Z

Only the last commit is belongs to this pull, the others are part of #3152

smarterclayton · 2014-12-31T01:23:03Z

Barring changes to the predecessor pull based on API design, this roughly reflects the desired functionality. resource.Builder absorbs most of the work of taking user input (parameters, arguments) and normalizing and converting that output. The watch functionality in Get is the most complicated part (due to the current limitations of watch, and avoiding trying to solve mux'ing multiple watches, and ensuring the right resource version is set by the builder). This is a good demonstration of the most complex use of resource.Builder. Create and Update will use it for input only, and Delete will need to filter out items that were deleted by the server. Other commands can leverage parts of it for similar use as Delete.

bgrant0607 · 2015-01-08T21:47:44Z

Cool. Looks reasonable to me. We definitely need this functionality. Will wait for merge of #3152 before merging.

smarterclayton · 2015-01-09T17:19:15Z

This is now ready for merge, as are the commits above it (except maybe delete, Tim needs to ack the change).

bgrant0607 · 2015-01-09T17:29:33Z

pkg/kubectl/cmd/get.go

-				w, err := restHelper.Watch(namespace, rv, labelSelector, labels.Everything())
-				checkErr(err)
+	// use the default printer for each object
+	err = b.Do().Visit(func(r *resource.Info) error {


FYI: I'd eventually like it to be possible to invoke all of the business logic of kubectl via library call, not just through the command line. That will facilitate programmatic use of Kubernetes, such as to manage dataflow pipelines, CI pipelines, background worker pools, sophisticated rollouts, etc.

Command-line processing obviously has to be coupled to cobra and the kubectl command. Input/output should be less coupled. All the actual processing and client calls should be completely independently callable.

This particular batch of PRs don't need to be changed, but please keep this in mind as we evolve kubectl functionality.

Yeah, builder has two types of arguments *Arg/*Param which indicates it comes from a command line argument (so different failure rules apply), and programmatic "*" calls. The error messages on Do() and Visit() should eventually distinguish between those cases, so you get a different error message when you specify FilenameParam(...) vs ResourceTypeOrArgs vs URL or Stream.

bgrant0607 · 2015-01-09T17:31:36Z

LGTM. Reassigned to Tim for a look.

smarterclayton · 2015-01-09T18:28:45Z

pkg/kubectl/cmd/get.go

+		return nil, err
+	}
+	if ok {
+		printer = kubectl.NewVersionedPrinter(printer, mapping.ObjectConvertor, outputVersion(cmd))


@lavalamp note I moved versioning to a wrapped printer so that I could precisely control versioning here. You may want to take a look at the watch behavior and see whether it still makes sense.

Sorry I didn't see this mention of me. This breaks the template uses in cluster/kube-validate.sh. I'm still trying to figure out what happened here.

Which ones?

On Jan 13, 2015, at 5:47 PM, Daniel Smith notifications@github.com wrote:

In pkg/kubectl/cmd/get.go:

templateFile := GetFlagString(cmd, "template")

if len(outputFormat) == 0 && len(templateFile) != 0 {

outputFormat = "template"

}

return kubectl.GetPrinter(outputFormat, templateFile)
+}

+// printerForMapping returns a printer suitable for displaying the provided resource type.
+func printerForMapping(f *Factory, cmd *cobra.Command, mapping *meta.RESTMapping) (kubectl.ResourcePrinter, error) {

printer, ok, err := printerForCommand(cmd)

if err != nil {

return nil, err

}

if ok {

printer = kubectl.NewVersionedPrinter(printer, mapping.ObjectConvertor, outputVersion(cmd))
Sorry I didn't see this mention of me. This breaks the template uses in cluster/kube-validate.sh. I'm still trying to figure out what happened here.

—
Reply to this email directly or view it on GitHub.

At least the first one. It's giving either internal structs or v1beta3 structs, not sure yet

Will be back at my desk in a few minutes

On Jan 13, 2015, at 5:51 PM, Daniel Smith notifications@github.com wrote:

In pkg/kubectl/cmd/get.go:

templateFile := GetFlagString(cmd, "template")

if len(outputFormat) == 0 && len(templateFile) != 0 {

outputFormat = "template"

}

return kubectl.GetPrinter(outputFormat, templateFile)
+}

+// printerForMapping returns a printer suitable for displaying the provided resource type.
+func printerForMapping(f *Factory, cmd *cobra.Command, mapping *meta.RESTMapping) (kubectl.ResourcePrinter, error) {

printer, ok, err := printerForCommand(cmd)

if err != nil {

return nil, err

}

if ok {

printer = kubectl.NewVersionedPrinter(printer, mapping.ObjectConvertor, outputVersion(cmd))
At least the first one. It's giving either internal structs or v1beta3 structs, not sure yet

—
Reply to this email directly or view it on GitHub.

Like Delete, which can now run over multiple types: kubectl delete pods,services -l name=foo Get should be able to span items for label selection kubectl get pods,services -l name=foo

bgrant0607 · 2015-01-13T17:05:18Z

Rerunning Travis. Will merge after it passes, assuming this doesn't need to be rebased.

smarterclayton · 2015-01-13T18:21:20Z

Green across the board

Allow kubectl get to fetch multiple resource types by label

lavalamp · 2015-01-13T22:50:37Z

pkg/kubectl/resource_printer.go

+
+// VersionedPrinter takes runtime objects and ensures they are converted to a given API version
+// prior to being passed to a nested printer.
+type VersionedPrinter struct {


I maintain that it shouldn't be possible to make a non-versioned printer.

All the regular printers are not versioned

On Jan 13, 2015, at 5:50 PM, Daniel Smith notifications@github.com wrote:

In pkg/kubectl/resource_printer.go:

-// JSONPrinter is an implementation of ResourcePrinter which outputs an object as JSON.
-// The input object is assumed to be in the internal version of an API and is converted
-// to the given version first.
-type JSONPrinter struct {

version string
+// ResourcePrinterFunc is a function that can print objects
+type ResourcePrinterFunc func(runtime.Object, io.Writer) error

+// PrintObj implements ResourcePrinter
+func (fn ResourcePrinterFunc) PrintObj(obj runtime.Object, w io.Writer) error {

return fn(obj, w)
+}

+// VersionedPrinter takes runtime objects and ensures they are converted to a given API version
+// prior to being passed to a nested printer.
+type VersionedPrinter struct {
I maintain that it shouldn't be possible to make a non-versioned printer.

—
Reply to this email directly or view it on GitHub.

They were when I last touched this code! In fact the last thing I did to this code was to fix this very problem...

We should put that example into test-cmd.sh then. :)

-o json should be outputting a minionlist - is it?

On Jan 13, 2015, at 5:56 PM, Daniel Smith notifications@github.com wrote:

In pkg/kubectl/resource_printer.go:

-// JSONPrinter is an implementation of ResourcePrinter which outputs an object as JSON.
-// The input object is assumed to be in the internal version of an API and is converted
-// to the given version first.
-type JSONPrinter struct {

version string
+// ResourcePrinterFunc is a function that can print objects
+type ResourcePrinterFunc func(runtime.Object, io.Writer) error

+// PrintObj implements ResourcePrinter
+func (fn ResourcePrinterFunc) PrintObj(obj runtime.Object, w io.Writer) error {

return fn(obj, w)
+}

+// VersionedPrinter takes runtime objects and ensures they are converted to a given API version
+// prior to being passed to a nested printer.
+type VersionedPrinter struct {
They were when I last touched this code! In fact the last thing I did to this code was to fix this very problem...

—
Reply to this email directly or view it on GitHub.

VersionedPrinter may be getting empty string?

On Jan 13, 2015, at 5:56 PM, Daniel Smith notifications@github.com wrote:

In pkg/kubectl/resource_printer.go:

-// JSONPrinter is an implementation of ResourcePrinter which outputs an object as JSON.
-// The input object is assumed to be in the internal version of an API and is converted
-// to the given version first.
-type JSONPrinter struct {

version string
+// ResourcePrinterFunc is a function that can print objects
+type ResourcePrinterFunc func(runtime.Object, io.Writer) error

+// PrintObj implements ResourcePrinter
+func (fn ResourcePrinterFunc) PrintObj(obj runtime.Object, w io.Writer) error {

return fn(obj, w)
+}

+// VersionedPrinter takes runtime objects and ensures they are converted to a given API version
+// prior to being passed to a nested printer.
+type VersionedPrinter struct {
They were when I last touched this code! In fact the last thing I did to this code was to fix this very problem...

—
Reply to this email directly or view it on GitHub.

smarterclayton changed the title ~~Allow kubectl get to fetch multiple resource types by label~~ WIP - Allow kubectl get to fetch multiple resource types by label Dec 17, 2014

smarterclayton force-pushed the allow_get_to_span_resources branch from f20c2f2 to 3bd6717 Compare December 18, 2014 00:26

lavalamp assigned bgrant0607 Dec 18, 2014

smarterclayton force-pushed the allow_get_to_span_resources branch from 3bd6717 to eb5d3b6 Compare December 18, 2014 22:05

smarterclayton force-pushed the allow_get_to_span_resources branch from eb5d3b6 to c3c7ca8 Compare December 26, 2014 22:34

smarterclayton mentioned this pull request Dec 28, 2014

Let Kubectl deal with many objects at the same time #3152

Merged

smarterclayton force-pushed the allow_get_to_span_resources branch 3 times, most recently from 53ce361 to b026f9e Compare December 31, 2014 01:17

smarterclayton changed the title ~~WIP - Allow kubectl get to fetch multiple resource types by label~~ Allow kubectl get to fetch multiple resource types by label Dec 31, 2014

smarterclayton force-pushed the allow_get_to_span_resources branch 3 times, most recently from c2a9ac3 to 7e471f8 Compare December 31, 2014 18:07

smarterclayton mentioned this pull request Jan 3, 2015

Allow create to handle multiple resources, remove createall #3196

Merged

smarterclayton force-pushed the allow_get_to_span_resources branch from 7e471f8 to c845d09 Compare January 3, 2015 20:50

smarterclayton mentioned this pull request Jan 3, 2015

Allow delete to work from directories and multiple sources #3198

Merged

smarterclayton force-pushed the allow_get_to_span_resources branch from c845d09 to f0d805c Compare January 7, 2015 21:51

smarterclayton force-pushed the allow_get_to_span_resources branch 6 times, most recently from 976928c to 1fb7fcd Compare January 9, 2015 17:18

bgrant0607 reviewed Jan 9, 2015
View reviewed changes

bgrant0607 assigned thockin and unassigned bgrant0607 Jan 9, 2015

smarterclayton reviewed Jan 9, 2015
View reviewed changes

Allow kubectl get to fetch multiple resource types

2151afe

Like Delete, which can now run over multiple types: kubectl delete pods,services -l name=foo Get should be able to span items for label selection kubectl get pods,services -l name=foo

smarterclayton force-pushed the allow_get_to_span_resources branch from 1fb7fcd to 2151afe Compare January 9, 2015 18:30

bgrant0607 mentioned this pull request Jan 10, 2015

Add a resize command to kubectl. #3377

Merged

bgrant0607 assigned bgrant0607 and unassigned thockin Jan 13, 2015

bgrant0607 added a commit that referenced this pull request Jan 13, 2015

Merge pull request #3004 from smarterclayton/allow_get_to_span_resources

bb140d6

Allow kubectl get to fetch multiple resource types by label

bgrant0607 merged commit bb140d6 into kubernetes:master Jan 13, 2015

lavalamp reviewed Jan 13, 2015
View reviewed changes

smarterclayton deleted the allow_get_to_span_resources branch February 11, 2015 02:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow kubectl get to fetch multiple resource types by label #3004

Allow kubectl get to fetch multiple resource types by label #3004

smarterclayton commented Dec 17, 2014

bgrant0607 commented Dec 18, 2014

smarterclayton commented Dec 18, 2014

smarterclayton commented Dec 18, 2014

smarterclayton commented Dec 31, 2014

smarterclayton commented Dec 31, 2014

bgrant0607 commented Jan 8, 2015

smarterclayton commented Jan 9, 2015

bgrant0607 Jan 9, 2015

smarterclayton Jan 9, 2015

bgrant0607 commented Jan 9, 2015

smarterclayton Jan 9, 2015

lavalamp Jan 13, 2015

smarterclayton Jan 13, 2015

lavalamp Jan 13, 2015

smarterclayton Jan 13, 2015

bgrant0607 commented Jan 13, 2015

smarterclayton commented Jan 13, 2015

lavalamp Jan 13, 2015

smarterclayton Jan 13, 2015

lavalamp Jan 13, 2015

smarterclayton Jan 13, 2015

smarterclayton Jan 13, 2015

Allow kubectl get to fetch multiple resource types by label #3004

Allow kubectl get to fetch multiple resource types by label #3004

Conversation

smarterclayton commented Dec 17, 2014

bgrant0607 commented Dec 18, 2014

smarterclayton commented Dec 18, 2014

smarterclayton commented Dec 18, 2014

smarterclayton commented Dec 31, 2014

smarterclayton commented Dec 31, 2014

bgrant0607 commented Jan 8, 2015

smarterclayton commented Jan 9, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bgrant0607 commented Jan 9, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bgrant0607 commented Jan 13, 2015

smarterclayton commented Jan 13, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment