Step 5 – generic controplane: split aggregator API priorities #124613
Conversation
k8s-ci-robot
added
release-note-none
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sttts 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:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
sttts
added
priority/important-soon
k8s-ci-robot
removed
the
needs-priority
sttts
added
the
approved
Signed-off-by: Dr. Stefan Schimanski <stefan.schimanski@gmail.com>
sttts
force-pushed
the
sttts-generic-api-versions
branch
from
3cc5f33
to
03ae7fc
Compare
sttts
changed the title
| {Group: "autoscaling", Version: "v1"}: {Group: 17500, Version: 15}, | ||
| {Group: "autoscaling", Version: "v2"}: {Group: 17500, Version: 30}, | ||
| {Group: "autoscaling", Version: "v2beta1"}: {Group: 17500, Version: 9}, | ||
| {Group: "autoscaling", Version: "v2beta2"}: {Group: 17500, Version: 1}, |
There was a problem hiding this comment.
Any reason these are in the controlplane code and not in kube-apiserver?
There was a problem hiding this comment.
aren't these for kube-apiserver related aka workloads?
There was a problem hiding this comment.
Aren't they working against any resource with scale resource? Then they are generic.
There was a problem hiding this comment.
I think I agree with you, looking into that group which resources it exposes. Updated the PR.
Signed-off-by: Dr. Stefan Schimanski <stefan.schimanski@gmail.com>
|
/retest |
|
I think this impacts only controlplane as we might be missing resources there, but not in kube-apiserver. So we can always move from one bucket to another later on. |
k8s-ci-robot
added
the
lgtm
|
LGTM label has been added. Git tree hash: 00407496c307c135e97f221896c5ab87776cfbc9
|
|
/triage accepted |
k8s-ci-robot
added
triage/accepted
sttts
changed the title
| @@ -349,3 +317,10 @@ func apiServicesToRegister(delegateAPIServer genericapiserver.DelegationTarget, | |||
|
|
|||
| return apiServices | |||
| } | |||
|
|
|||
| func merge(a, b map[schema.GroupVersion]controlplaneapiserver.APIServicePriority) map[schema.GroupVersion]controlplaneapiserver.APIServicePriority { | |||
There was a problem hiding this comment.
now it's not a problem, but I was first thinking that it could be problematic if this function would be passed some generic global variable, as this func mutates a. You indeed solved it with having a function returning a new variable every time, but just something to be careful about, unless this function first does a deep copy of a.
| {Group: "flowcontrol.apiserver.k8s.io", Version: "v1beta1"}: {Group: 16100, Version: 12}, | ||
| {Group: "flowcontrol.apiserver.k8s.io", Version: "v1alpha1"}: {Group: 16100, Version: 9}, | ||
| {Group: "internal.apiserver.k8s.io", Version: "v1alpha1"}: {Group: 16000, Version: 9}, | ||
| {Group: "resource.k8s.io", Version: "v1alpha2"}: {Group: 15900, Version: 9}, |
There was a problem hiding this comment.
Is this really generic? This seems to be related to Dynamic Resource Allocation, which is pod-specific if I'm not mistaken?
There was a problem hiding this comment.
Good question. I can see it has few types we might want to use in building some quota machinery:
https://pkg.go.dev/k8s.io/api@v0.30.0/resource/v1alpha2
but in general I think you right.
What type of PR is this?
/kind feature
What this PR does / why we need it:
In a generic controlplane, the aggregator will need a partial set of API priorities. This PR splits up the big table, and merges non-generic API priorities into the generic priorities.
NO FUNCTIONAL CHANGE.
Part of #124530.
Which issue(s) this PR fixes:
Towards kubernetes/enhancements#4080.
Special notes for your reviewer:
This is just code move. No behaviour change.
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: