use specified discovery information if possible

[deads2k]

Fixes #49948

This uses the available discovery information if available, but it seems we never updated "normal" resources to show the singular name, so its often not available. I've left this code compatible.

@enisoc @ash2k
@kubernetes/sig-api-machinery-misc

custom resources that use unconventional pluralization now work properly with kubectl and garbage collection

[ncdc]

Is this easily unit testable?

[ash2k]

https://github.com/kubernetes/apiextensions-apiserver/blob/master/pkg/apis/apiextensions/v1beta1/types.go#L46

What if ListKind is not Kind + "List"? Looks like APIResource struct does not expose this information so we cannot use it here.

[deads2k]

What if ListKind is not Kind + "List"? Looks like APIResource struct does not expose this information so we cannot use it here.

Hrm. Not fixable in this pull.

[deads2k]

Is this easily unit testable?

I delegated one to the other, so the existing callers test the core functionality. The actual discovery bits? Not easily. An integration test can't, an e2e test maybe, but it would be a disruptive one (affects global discovery).

[ncdc]

You don't think you could unit test NewRESTMapper?

[mml]

/assign mbohlool

[liggitt]

The actual discovery bits? Not easily. An integration test can't, an e2e test maybe, but it would be a disruptive one (affects global discovery).

@ironcladlou has an integration test that could be made to exercise this :)

[ironcladlou]

Here's an integration test that exercises discovery and which will fail whenever the generated CRD's singular name ends with an "s".

[liggitt]

adding the Kind+List to the non-list singular/plural resources doesn't look right... the Add of the Kind+"List" previously did the following:

Add(schema.GroupVersionKind{Group:"mygroup.example.com", Version:"v1beta1", Kind:"MyObj"})
	m.singularToPlural[{mygroup.example.com v1beta1 myobj}] = {mygroup.example.com v1beta1 myobjs}
	m.pluralToSingular[{mygroup.example.com v1beta1 myobjs}] = {mygroup.example.com v1beta1 myobj}

	m.resourceToKind[{mygroup.example.com v1beta1 myobj}] = mygroup.example.com/v1beta1, Kind=MyObj
	m.resourceToKind[{mygroup.example.com v1beta1 myobjs}] = mygroup.example.com/v1beta1, Kind=MyObj

	m.kindToPluralResource[mygroup.example.com/v1beta1, Kind=MyObj] = {mygroup.example.com v1beta1 myobjs}
	m.kindToScope[mygroup.example.com/v1beta1, Kind=MyObj] = namespace

Add(schema.GroupVersionKind{Group:"mygroup.example.com", Version:"v1beta1", Kind:"MyObjList"})
	m.singularToPlural[{mygroup.example.com v1beta1 myobjlist}] = {mygroup.example.com v1beta1 myobjlists}
	m.pluralToSingular[{mygroup.example.com v1beta1 myobjlists}] = {mygroup.example.com v1beta1 myobjlist}

	m.resourceToKind[{mygroup.example.com v1beta1 myobjlist}] = mygroup.example.com/v1beta1, Kind=MyObjList
	m.resourceToKind[{mygroup.example.com v1beta1 myobjlists}] = mygroup.example.com/v1beta1, Kind=MyObjList

	m.kindToPluralResource[mygroup.example.com/v1beta1, Kind=MyObjList] = {mygroup.example.com v1beta1 myobjlists}
	m.kindToScope[mygroup.example.com/v1beta1, Kind=MyObjList] = namespace

with this change, it does:

Add(schema.GroupVersionKind{Group:"mygroup.example.com", Version:"v1beta1", Kind:"MyObj"})
	m.singularToPlural[{mygroup.example.com v1beta1 myobj}] = {mygroup.example.com v1beta1 myobjs}
	m.pluralToSingular[{mygroup.example.com v1beta1 myobjs}] = {mygroup.example.com v1beta1 myobj}

	m.resourceToKind[{mygroup.example.com v1beta1 myobj}] = mygroup.example.com/v1beta1, Kind=MyObj
	m.resourceToKind[{mygroup.example.com v1beta1 myobjs}] = mygroup.example.com/v1beta1, Kind=MyObj

	m.kindToPluralResource[mygroup.example.com/v1beta1, Kind=MyObj] = {mygroup.example.com v1beta1 myobjs}
	m.kindToScope[mygroup.example.com/v1beta1, Kind=MyObj] = namespace

Add(schema.GroupVersionKind{Group:"mygroup.example.com", Version:"v1beta1", Kind:"MyObjList"})
	m.singularToPlural[{mygroup.example.com v1beta1 myobj}] = {mygroup.example.com v1beta1 myobjs}
	m.pluralToSingular[{mygroup.example.com v1beta1 myobjs}] = {mygroup.example.com v1beta1 myobj}

	m.resourceToKind[{mygroup.example.com v1beta1 myobj}] = mygroup.example.com/v1beta1, Kind=MyObjList
	m.resourceToKind[{mygroup.example.com v1beta1 myobjs}] = mygroup.example.com/v1beta1, Kind=MyObjList

	m.kindToPluralResource[mygroup.example.com/v1beta1, Kind=MyObjList] = {mygroup.example.com v1beta1 myobjs}
	m.kindToScope[mygroup.example.com/v1beta1, Kind=MyObjList] = namespace

so it's overwriting the resourceToKind maps to point to the List kinds, which is not right.

[liggitt]

I'm actually not sure what the purpose of adding the Kind+"List" was previously, given that the resource names it guessed were so wrong. the only thing correct was the scope

[liggitt]

@deads2k would be good to get this fixed for 1.8 (the plural thing, not necessarily the list bit)

[mbohlool]

Are we still shooting for 1.7 here? I also have the same question about Kind+"List". It looks to me that if we added unnecessary endpoints or changed the surface of APIs in any way we should have seen some swagger/OpenAPI changes but there is none which is good. If we remove those Kind+"List" does it change the surface of APIs?

If we don't want to merge this for 1.7 please remove it from v1.7 milestone.

[k8s-github-robot]

Adding do-not-merge/release-note-label-needed because the release note process has not been followed.
One of the following labels is required "release-note", "release-note-action-required", "release-note-experimental" or "release-note-none".
Please see: https://github.com/kubernetes/community/blob/master/contributors/devel/pull-requests.md#write-release-notes-if-needed.

[deads2k]

I'm actually not sure what the purpose of adding the Kind+"List" was previously, given that the resource names it guessed were so wrong. the only thing correct was the scope

kubectl doesn't respect kubectl get podlist (bad mapping as you noted). I'm happy to simplify our RESTMapper.

[liggitt]

/lgtm

[k8s-github-robot]

[APPROVALNOTIFIER] This PR is APPROVED

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

Associated issue: 49948

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

Needs approval from an approver in each of these OWNERS Files:

• staging/src/k8s.io/apimachinery/pkg/OWNERS [deads2k,liggitt]
• staging/src/k8s.io/client-go/OWNERS [deads2k,liggitt]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

[liggitt]

verified by making NewRandomNameCustomResourceDefinition return a resource with unconventional pluralization and running

go test -v k8s.io/kubernetes/test/integration/garbagecollector -run TestCustomResourceCascadingDeletion$

would like to see a follow up that exercises unconventional pluralization in a test

[fejta-bot]

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to @fejta).

Review the full test history for this PR.

[liggitt]

/retest

[liggitt]

/retest

[fejta]

/retest
ref kubernetes/test-infra#4568

[fejta-bot]

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to @fejta).

Review the full test history for this PR.

[k8s-github-robot]

/test all [submit-queue is verifying that this PR is safe to merge]

[k8s-github-robot]

Automatic merge from submit-queue

[k8s-ci-robot]

@deads2k: The following test failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
pull-kubernetes-e2e-gce-bazel	1c8a9e4	link	/test pull-kubernetes-e2e-gce-bazel

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

[liggitt]

picked to 1.7 in #52545

[k8s-cherrypick-bot]

Commit found in the "release-1.7" branch appears to be this PR. Removing the "cherrypick-candidate" label. If this is an error find help to get your PR picked.