add information for subresource kind determination #49971
Conversation
k8s-ci-robot
added
sig/api-machinery
k8s-github-robot
added
size/XS
|
This pull just shows the new fields without wiring or generation, so we can first sort out how to express it. @DirectXMan12 you want this too I think. |
deads2k
assigned lavalamp, cheftako and smarterclayton and unassigned janetkuo and davidopp
|
woot! This will eventually unblock me on a generic scale client. |
|
Seems very reasonable to me, do we need to qualify group / version with a prefix in order to distinguish them better? |
| // For subresources, this may have a different value, for example: Scale". | ||
| Group string `json:"group,omitempty" protobuf:"bytes,8,opt,name=group"` | ||
| // version is the preferred version of the resource. Empty implies the version of the containing resource list | ||
| // For subresources, this may have a different value, for example: Scale". |
There was a problem hiding this comment.
quibble but can we fix the example to be a version?
There was a problem hiding this comment.
quibble but can we fix the example to be a version?
Yes. I'll update during my rebase and generation cleanup today.
| @@ -750,6 +750,12 @@ type APIResource struct { | |||
| SingularName string `json:"singularName" protobuf:"bytes,6,opt,name=singularName"` | |||
| // namespaced indicates if a resource is namespaced or not. | |||
| Namespaced bool `json:"namespaced" protobuf:"varint,2,opt,name=namespaced"` | |||
| // group is the preferred group of the resource. Empty implies the group of the containing resource list. | |||
There was a problem hiding this comment.
Don't we expect there times when there are multiple versions of the APIResource? If we want to switch groups then there could even be multiple groups? Would preferred be the most stable? I assume that the preferred group must match the preferred version?
There was a problem hiding this comment.
I assume that the preferred group must match the preferred version?
I think this group and version should always match and be the preferred one (the one that most people will use. Our internal code already handles this case today.
Don't we expect there times when there are multiple versions of the APIResource?
Eventually, yes. Right now our code doesn't handle the condition and explicitly forbids it. I think that we should reflect what we have now (and realistically for the next few releases).
If we want to switch groups then there could even be multiple groups? Would preferred be the most stable?
I think we would handle this as we've done for our preferred versions. We would add an array of possible GroupVersions and say that they are in the preferred ordering, leaving the choice to the individual api server.
deads2k
force-pushed
the
discovery-01-group
branch
from
2bf40db to
02c270f
Compare
k8s-github-robot
added
size/L
|
Updated for comments and generated code. |
|
/retest |
|
@kubernetes/api-approvers |
Seems reasonable to me as well. I don't think a prefix is needed here... other discovery types use unprefixed |
|
Any other comments? I think i'm ok with this as is. |
|
/approve based on general agreement from participants |
liggitt
added
release-note
|
/lgtm |
k8s-ci-robot
added
the
lgtm
k8s-github-robot
added
the
approved
|
/test all |
|
/test pull-kubernetes-bazel-build |
|
/retest |
deads2k
force-pushed
the
discovery-01-group
branch
from
02c270f to
de4006b
Compare
deads2k
added
lgtm
|
/retest |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: deads2k, lavalamp, liggitt, smarterclayton Associated issue: 38810 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
|
/retest |
|
/retest Review the full test history for this PR. |
|
/test all [submit-queue is verifying that this PR is safe to merge] |
|
Automatic merge from submit-queue (batch tested with PRs 49971, 51357, 51616, 51649, 51372) |
|
@deads2k: The following tests failed, say
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. |
Automatic merge from submit-queue (batch tested with PRs 50602, 51561, 51703, 51748, 49142) expose discovery information on scalable resources Builds on #49971 and provides the GroupVersion information that can be used by a dynamic scale client. @kubernetes/sig-api-machinery-pr-reviews @foxish @DirectXMan12 since you both asked for it.
xref #38810 #38756
Polymorphic subresources usually have different groupVersions for their discovery kinds than their "native" groupVersions. Even though the APIResourceList shows the kind properly, it does not reflect the group or version of that kind, which makes it impossible to unambiguously determine if the subresource matches you and it is impossible to determine how to serialize your data. See HPA controller.
This adds an optional Group and Version to the discovery doc, which can be used to communicate the "native" groupversion of an endpoint. Doing this does not preclude fancier contenttype negotiation in the future and doesn't prevent future expansion from indicating equivalent types, but it does make it possible to solve the problem we have today or polymorphic categorization.
@kubernetes/sig-api-machinery-misc @smarterclayton
@cheftako since @lavalamp is out.