Allow to specify categories for custom resources #59561

nikhita · 2018-02-08T11:11:09Z

Allow to specify categories for custom resources so that we can get it working with kubectl get all.

Adds a new field Categories in the CRD spec.

Release note:

Custom resources can be listed with a set of grouped resources (category) by specifying the categories in the CustomResourceDefinition spec. Example: They can be used with `kubectl get all`, where `all` is a category.

/cc sttts liggitt deads2k

sttts · 2018-02-08T11:38:10Z

staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/etcd.go

+
+// Categories implements the CategoriesProvider interface. Returns a list of categories a resource is part of.
+func (r *REST) Categories() []string {
+	return []string{"all"}


Not sure we really want that. The resources in all are manually curated because we don't want to see certain resources in there that are internal and not relevant for a user.

Yeah, we don't really want this here. I could see allowing a user to specify their categories.

I could see allowing a user to specify their categories.

Maybe as another field in the CRD spec (?)

deads2k · 2018-02-08T13:13:44Z

staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/etcd.go

+
+// Categories implements the CategoriesProvider interface. Returns a list of categories a resource is part of.
+func (r *REST) Categories() []string {
+	return []string{"all"}


all is for user facing resources. CRDs are highly privileged. They don't belong in the all list.

Right, came across https://github.com/kubernetes/community/blob/master/contributors/devel/kubectl-conventions.md#rules-for-extending-special-resource-alias---all now. 👍

/cc @liggitt since you mentioned about this on slack.

@deads2k: I could see allowing a user to specify their categories.

that is what I meant

liggitt · 2018-02-12T16:01:35Z

staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/types.go

@@ -30,6 +30,8 @@ type CustomResourceDefinitionSpec struct {
 	Scope ResourceScope
 	// Validation describes the validation methods for CustomResources
 	Validation *CustomResourceValidation
+	// Categories is a list of grouped resources custom resources belong to (e.g. 'all')
+	Categories []string


I expected this next to ShortNames, is there a reason not to put it there?

I expected this next to ShortNames, is there a reason not to put it there?

My first instinct was to include it next to ShortNames as well.

However, CustomResourceDefinitionNames is unique to the CRD and is used in the naming_controller too. Categories can be the same across CRDs and we don't really need to include it in the naming_controller. So I thought we could include it as a field in the CRD Spec...

Putting it in CustomResourceDefinitionNames should also be fine. @liggitt Wdyt?

liggitt · 2018-02-12T16:03:34Z

staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go

@@ -158,7 +158,6 @@ func ValidateCustomResourceDefinitionNames(names *apiextensions.CustomResourceDe
 		if errs := validationutil.IsDNS1035Label(shortName); len(errs) > 0 {
 			allErrs = append(allErrs, field.Invalid(fldPath.Child("shortNames").Index(i), shortName, strings.Join(errs, ",")))
 		}
-


need to validate categories (probably the same validation we apply to shortnames)

nikhita · 2018-02-12T21:27:23Z

To confirm - do we need a feature gate for this? Technically, it is an alpha field in an already beta API...

nikhita · 2018-02-13T07:52:05Z

/kind api-change

sttts · 2018-02-19T12:53:41Z

Needs a release note.

sttts · 2018-02-19T12:55:17Z

do we need a feature gate for this? Technically, it is an alpha field in an already beta API...

I tend to add it as a beta field. This is simple enough IMO.

@deads2k @liggitt opinions?

nikhita · 2018-02-19T13:01:46Z

Needs a release note.

Added. Ptal.

deads2k · 2018-02-20T19:02:36Z

I tend to add it as a beta field. This is simple enough IMO.

I'm fine with it as beta. I also expected it to be near names though.

deads2k · 2018-02-22T19:59:14Z

staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/etcd.go

+}
+
+// WithCategories sets the categories for the resource and returns the RESTStorage object.
+func (r *REST) WithCategories(categories []string) *REST {


I don't think you need this

Removed and rebased.

deads2k · 2018-02-22T19:59:41Z

/lgtm

/assign @liggitt
@kubernetes/api-approvers

liggitt · 2018-02-23T16:31:44Z

staging/src/k8s.io/apiextensions-apiserver/pkg/controller/status/naming_controller.go

@@ -111,6 +111,7 @@ func (c *NamingConditionController) getAcceptedNamesForGroup(group string) (allR

 		allKinds.Insert(item.Status.AcceptedNames.Kind)
 		allKinds.Insert(item.Status.AcceptedNames.ListKind)
+		allKinds.Insert(item.Status.AcceptedNames.Categories...)


categories are not kinds... why are we adding them here? multiple CRDs can share the same categories... no duplicate checking is required

liggitt · 2018-02-23T16:43:11Z

code looks good. I expected a test exercising the function (ensuring a specified category makes it through to the discovery doc)

We can group custom resources into categories i.e. use them with kubectl get all.

nikhita · 2018-02-23T16:59:59Z

I expected a test exercising the function (ensuring a specified category makes it through to the discovery doc)

Added.

Also added a unit test.

liggitt · 2018-02-23T18:18:17Z

/lgtm

k8s-ci-robot · 2018-02-23T18:18:44Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, liggitt, nikhita

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~api/OWNERS~~ [liggitt]
~~staging/src/k8s.io/apiextensions-apiserver/OWNERS~~ [deads2k]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-github-robot · 2018-02-23T22:01:48Z

Automatic merge from submit-queue (batch tested with PRs 60302, 57921, 59042, 60126, 59561). If you want to cherry-pick this change to another branch, please follow the instructions here.

bgrant0607 · 2018-04-03T06:27:37Z

Do we plan to add categories to built-in types?

sttts · 2018-04-03T11:20:26Z

Do we plan to add categories to built-in types?

Technically we can (for quite some time already): https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/apiserver/pkg/registry/rest/rest.go#L76

Are you asking for more categories by default?

bgrant0607 · 2018-04-03T14:15:52Z

@sttts No, I'm not asking for more default categories. I'm trying to figure out why we need categories.

"all" (which is misleading) is the only default category?

This is just intended for CLI shortcuts similar to resource-type short names?

sttts · 2018-04-04T07:53:39Z

/cc @kubernetes/sig-cli-misc

k8s-ci-robot requested review from deads2k, liggitt and sttts February 8, 2018 11:12

k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Feb 8, 2018

sttts reviewed Feb 8, 2018

View reviewed changes

deads2k reviewed Feb 8, 2018

View reviewed changes

nikhita mentioned this pull request Feb 9, 2018

Umbrella issue for CRDs moving to GA #58682

Closed

54 tasks

nikhita force-pushed the customresources-kubectl-get-all branch from e36f1c8 to 8d2b8e1 Compare February 11, 2018 22:15

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Feb 11, 2018

nikhita changed the title ~~apiextensions: implement CategoriesProvider~~ [WIP] Allow to specify categories for custom resources Feb 12, 2018

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 12, 2018

liggitt reviewed Feb 12, 2018

View reviewed changes

nikhita force-pushed the customresources-kubectl-get-all branch 2 times, most recently from 4f00c4c to 0bf0507 Compare February 12, 2018 21:24

k8s-ci-robot added the kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API label Feb 13, 2018

nikhita changed the title ~~[WIP] Allow to specify categories for custom resources~~ Allow to specify categories for custom resources Feb 19, 2018

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 19, 2018

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Feb 19, 2018

nikhita force-pushed the customresources-kubectl-get-all branch from 0bf0507 to 7f0ec58 Compare February 20, 2018 21:45

deads2k reviewed Feb 22, 2018

View reviewed changes

k8s-ci-robot assigned liggitt and deads2k Feb 22, 2018

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 22, 2018

nikhita force-pushed the customresources-kubectl-get-all branch from 7f0ec58 to 957751a Compare February 23, 2018 04:58

k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 23, 2018

nikhita force-pushed the customresources-kubectl-get-all branch from 957751a to 8b02c95 Compare February 23, 2018 05:24

nikhita mentioned this pull request Feb 23, 2018

Add docs for CRD features for 1.10 kubernetes/website#7439

Merged

liggitt reviewed Feb 23, 2018

View reviewed changes

nikhita force-pushed the customresources-kubectl-get-all branch from 8b02c95 to 53452ab Compare February 23, 2018 16:36

nikhita force-pushed the customresources-kubectl-get-all branch from 53452ab to 7ac2b3c Compare February 23, 2018 16:58

nikhita added 2 commits February 23, 2018 22:29

Add Categories to CRD spec

e7341f4

We can group custom resources into categories i.e. use them with kubectl get all.

update generated files

7ac2b3c

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 23, 2018

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 23, 2018

k8s-github-robot merged commit 8d406bf into kubernetes:master Feb 23, 2018

nikhita deleted the customresources-kubectl-get-all branch February 23, 2018 22:03

nikhita mentioned this pull request Mar 5, 2018

Need a command to list all available/existing types (or resources) in a cluster kubernetes/kubectl#113

Closed

k8s-ci-robot added the sig/cli Categorizes an issue or PR as relevant to SIG CLI. label Apr 4, 2018

zytek mentioned this pull request Apr 10, 2018

kubectl get all stopped printing resource type after upgrading client from 1.9.3 to 1.10.0 #62340

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow to specify categories for custom resources #59561

Allow to specify categories for custom resources #59561

nikhita commented Feb 8, 2018 •

edited

Loading

sttts Feb 8, 2018 •

edited

Loading

deads2k Feb 8, 2018

nikhita Feb 9, 2018

liggitt Feb 9, 2018

deads2k Feb 8, 2018

nikhita Feb 8, 2018

liggitt Feb 8, 2018

liggitt Feb 12, 2018 •

edited

Loading

nikhita Feb 12, 2018 •

edited

Loading

nikhita Feb 20, 2018

liggitt Feb 12, 2018

nikhita Feb 12, 2018

nikhita commented Feb 12, 2018 •

edited

Loading

nikhita commented Feb 13, 2018

sttts commented Feb 19, 2018 •

edited

Loading

sttts commented Feb 19, 2018

nikhita commented Feb 19, 2018

deads2k commented Feb 20, 2018

deads2k Feb 22, 2018

nikhita Feb 23, 2018

deads2k commented Feb 22, 2018

liggitt Feb 23, 2018 •

edited

Loading

nikhita Feb 23, 2018

liggitt commented Feb 23, 2018

nikhita commented Feb 23, 2018

liggitt commented Feb 23, 2018

k8s-ci-robot commented Feb 23, 2018

k8s-github-robot commented Feb 23, 2018

bgrant0607 commented Apr 3, 2018

sttts commented Apr 3, 2018

bgrant0607 commented Apr 3, 2018

sttts commented Apr 4, 2018

Allow to specify categories for custom resources #59561

Allow to specify categories for custom resources #59561

Conversation

nikhita commented Feb 8, 2018 • edited Loading

sttts Feb 8, 2018 • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liggitt Feb 12, 2018 • edited Loading

Choose a reason for hiding this comment

nikhita Feb 12, 2018 • edited Loading

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

nikhita commented Feb 12, 2018 • edited Loading

nikhita commented Feb 13, 2018

sttts commented Feb 19, 2018 • edited Loading

sttts commented Feb 19, 2018

nikhita commented Feb 19, 2018

deads2k commented Feb 20, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deads2k commented Feb 22, 2018

liggitt Feb 23, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liggitt commented Feb 23, 2018

nikhita commented Feb 23, 2018

liggitt commented Feb 23, 2018

k8s-ci-robot commented Feb 23, 2018

k8s-github-robot commented Feb 23, 2018

bgrant0607 commented Apr 3, 2018

sttts commented Apr 3, 2018

bgrant0607 commented Apr 3, 2018

sttts commented Apr 4, 2018

nikhita commented Feb 8, 2018 •

edited

Loading

sttts Feb 8, 2018 •

edited

Loading

liggitt Feb 12, 2018 •

edited

Loading

nikhita Feb 12, 2018 •

edited

Loading

nikhita commented Feb 12, 2018 •

edited

Loading

sttts commented Feb 19, 2018 •

edited

Loading

liggitt Feb 23, 2018 •

edited

Loading