🐛 Return NoResourceMatchError when appropriate for backwards compatibility.

[porridge]

Fixes: #2354

[k8s-ci-robot]

Hi @porridge. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

[alvaroaleman]

Thanks!

[alvaroaleman]

/ok-to-test
/cherrypick release-0.16

[k8s-infra-cherrypick-robot]

@alvaroaleman: once the present PR merges, I will cherry-pick it on top of release-0.16 in a new PR and assign it to you.

In response to this:

/ok-to-test
/cherrypick release-0.16

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.

[alvaroaleman]

/hold

[alvaroaleman]

This confuses me - In the initial issue description, @saschagrunert mentions that they expected to match through IsNotFound: #2354 (comment) - Which should this be?

[porridge]

@alvaroaleman good point, let's see.

I created a tiny repro program to investigate the behaviour of an actual client that uses the mapper, and ran it on a few versions:

• pre-v0.15

   deleg.go:124: INFO ret-error-demo: Kind is present but object was not found kind="ReplicaSet"
   deleg.go:124: INFO ret-error-demo: Kind does not exist kind="Badger"
   deleg.go:124: INFO ret-error-demo: Kind does not exist kind="Gherkin"
  

• curent tip of main (commit):

   deleg.go:127: I0911 12:22:01.453070] ret-error-demo: Kind is present but object was not found kind="ReplicaSet"
   deleg.go:127: I0911 12:22:01.453660] ret-error-demo: Kind does not exist kind="Badger"
   deleg.go:142: E0911 12:22:01.454161] ret-error-demo: Unexpected error err="failed to get API group resources: unable to retrieve the complete list of server APIs: inexistent.group.com/v1: the server could not find the requested resource"
  

• on top of this PR (commit):

   deleg.go:127: I0911 12:24:06.089190] ret-error-demo: Kind is present but object was not found kind="ReplicaSet"
   deleg.go:127: I0911 12:24:06.089777] ret-error-demo: Kind does not exist kind="Badger"
   deleg.go:127: I0911 12:24:06.090234] ret-error-demo: Kind does not exist kind="Gherkin"
  

BTW, this also shows that the regression is only for the case of missing group(version) - error returned on a missing kind in an existing group+version is ok w.r.t. backwards compatibility. Unfortunately the missing group/version case is the most interesting one, as it's used to discover capabilities supported by the cluster.

My theory on why @saschagrunert mentioned IsNotFound is that it was a mistake or simply lack of precision. At least judging by the code he mentions, it contains a single method that ORs together checks done by IsNoMatchError and IsNotFound.

The conditional branches in the demo program I created mimics actual the code in my operator which is careful to distinguish between errors that satisfy IsNoMatchError vs IsNotFound.

[alvaroaleman]

Thanks for fixing this!

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alvaroaleman, porridge

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:

• OWNERS [alvaroaleman]

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

[alvaroaleman]

/hold cancel

[k8s-infra-cherrypick-robot]

@alvaroaleman: new pull request created: #2492

In response to this:

/ok-to-test
/cherrypick release-0.16

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.