expose discovery information on scalable resources #51703
Conversation
k8s-ci-robot
added
size/L
k8s-github-robot
added
the
do-not-merge/release-note-label-needed
deads2k
force-pushed
the
discovery-02-scale
branch
from
a4d3d5e to
465f02d
Compare
|
/retest |
|
Haven't looked in detail but I agree in theory. |
| @@ -778,6 +778,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.
Could you remove the extra space after the period? There are many occurrences. Is it an editor issue?
There was a problem hiding this comment.
Could you remove the extra space after the period? There are many occurrences. Is it an editor issue?
MLA when I was growing up I'm afraid.
|
A nit, otherwise lgtm. |
|
/approve Based on reviewer comments and general agreement, plus API is consistent with others |
deads2k
force-pushed
the
discovery-02-scale
branch
from
465f02d to
7b44627
Compare
k8s-ci-robot
added
size/M
|
/approve no-issue |
deads2k
added
release-note-none
k8s-github-robot
added
the
approved
|
/retest |
| gvk := gvkProvider.GroupVersionKind() | ||
| apiResource.Group = gvk.Group | ||
| apiResource.Version = gvk.Version | ||
| apiResource.Kind = gvk.Kind |
There was a problem hiding this comment.
Now we have a KindProvider and a GroupVersionKindProvider provider. Wouldn't it make things clearer to have a GroupVersionProvider to complement the KindProvider?
There was a problem hiding this comment.
Now we have a KindProvider and a GroupVersionKindProvider provider. Wouldn't it make things clearer to have a GroupVersionProvider to complement the KindProvider?
The other is for federation, I'd rather not change it for 1.8 since the intent is different.
There was a problem hiding this comment.
What do you mean with federation? Compare https://github.com/deads2k/kubernetes/blob/7b44627d0372a3febbe906441c28f43323760bf9/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go#L374. Isn't that the source of a custom kind? Your interface does gv in addition.
There was a problem hiding this comment.
What do you mean with federation? Compare https://github.com/deads2k/kubernetes/blob/7b44627d0372a3febbe906441c28f43323760bf9/staging/src/k8s.io/apiserver/pkg/endpoints/installer.go#L374. Isn't that the source of a custom kind? Your interface does gv in addition.
It's only implemented by federation types to lie about their kinds during discovery. Every interaction with federation reveals dragons that I'd prefer to slay early in 1.9 as opposed to late in 1.8.
There was a problem hiding this comment.
Ic, then just add a comment to your new interface that it trumps over the KindProvider. After that lgtm.
There was a problem hiding this comment.
Ic, then just add a comment to your new interface that it trumps over the KindProvider. After that lgtm.
done
|
/retest |
deads2k
force-pushed
the
discovery-02-scale
branch
from
7b44627 to
65d0f18
Compare
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: deads2k, smarterclayton, sttts Associated issue requirement bypassed by: deads2k 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 |
|
Automatic merge from submit-queue (batch tested with PRs 50602, 51561, 51703, 51748, 49142) |
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.