Add initializer support to admission and uninitialized filtering to rest storage #36721
Conversation
smarterclayton
added
the
sig/api-machinery
|
@liggitt @bgrant0607 @derekwaynecarr @ncdc @kubernetes/sig-api-machinery as discussed at KubeCon there was a general feeling that initializers are technically solvable and a better path than more complexity in admission extension (make admission extensible like controllers is a better story than more web hooks, each of which is a net new API). I think from prototyping it that this will be no worse for performance than any out of process admission controller, and because it can leverage informers it's actually the most performant approach. This does not obviate cleaning up admission control in process - the most efficient way to have admission would still be one big chain, as opposed to many small ones. But I think it solves the fifth part of #34131 better than the other approaches. |
k8s-github-robot
added
kind/api-change
|
Jenkins GCI GKE smoke e2e failed for commit 2444005. Full PR test history. The magic incantation to run this job again is |
|
Jenkins GKE smoke e2e failed for commit 2444005. Full PR test history. The magic incantation to run this job again is |
|
(5) means that apiserver doesn't respond to the creation request until initialization has completed or failed, correct? |
|
Or timeout, correct. Will be trying to draft a larger proposal soon to cover the full scope of extension (on top of ncdc's previous work). |
| } | ||
|
|
||
| func (i initializer) Handles(op admission.Operation) bool { | ||
| return op == admission.Create |
There was a problem hiding this comment.
I thought we still have some objects that support create on update?
|
I like where this is going. Let me know if I can help in any particular area. |
Funny you should ask. :) Quota of non-count resources needs to be refactored to be an initializer so that it can run after all initializers have had a chance to update those resources. The admission path for "update things that haven't been initialized" is different than a normal "update" and could easily end up more noisy than it needs to be with multiple writes happening before the end of initialization which would require excessive compensation (openshift normally goes through two passes right?). Also, refactoring resource quota as initialization makes it possible to provide an easy entry point for any API server to ask for quota on an immutable field (or an expensive count implementation) without needing to compile in and configure an implementation of quota in its admission chain. Combine it with a way find resource-y things (same problem as our current podspec-y things) and it ought to just work. |
k8s-github-robot
added
the
needs-rebase
|
Started a proposal at kubernetes/community#132 |
|
Quota would not be an initializer - it doesn't need to be extensible in the
vast majority of cases (each api server only manages quota for its own
resources).
…On Sat, Dec 31, 2016 at 11:15 PM, Kubernetes Submit Queue < ***@***.***> wrote:
This PR hasn't been active in 30 days. It will be closed in 59 days (Mar
2, 2017).
cc @smarterclayton <https://github.com/smarterclayton>
You can add 'keep-open' label to prevent this from happening, or add a
comment to keep it open another 90 days
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#36721 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABG_pw_Rot5r3enVKOsEb_9elCM__fVHks5rNyhegaJpZM4Kw2LM>
.
|
You have to calculate quota after all limits have been set. That can't be done in the admission chain since an initializer can change it. Where would it happen? |
|
In the admission chain. Quota for Object count and quota for limits will
have to be done at two different points
On Jan 9, 2017, at 8:27 AM, David Eads <notifications@github.com> wrote:
Quota would not be an initializer - it doesn't need to be extensible in the
vast majority of cases (each api server only manages quota for its own
resources).
You have to calculate quota after all limits have been set. That can't be
done in the admission chain since an initializer can change it. Where would
it happen?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#36721 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABG_p6Pf4-o9geUs37Jqz76yP33f-_c9ks5rQjWmgaJpZM4Kw2LM>
.
|
k8s-github-robot
assigned
lavalamp,
csbell and
derekwaynecarr and unassigned
smarterclayton,
lavalamp and
csbell
smarterclayton
removed
the
needs-rebase
k8s-github-robot
removed
the
lgtm
smarterclayton
added
the
lgtm
|
Trivial rebase |
|
@k8s-bot pull-kubernetes-federation-e2e-gce test this |
|
@k8s-bot pull-kubernetes-unit test this |
|
@k8s-bot pull-kubernetes-verify test this |
Add support for creating resources that are not immediately visible to naive clients, but must first be initialized by one or more privileged cluster agents. These controllers can mark the object as initialized, allowing others to see them. Permission to override initialization defaults or modify an initializing object is limited per resource to a virtual subresource "RESOURCE/initialize" via RBAC. Initialization is currently alpha.
Running without an authorizer is a valid configuration.
smarterclayton
force-pushed
the
initializers
branch
from
959a6f9 to
b993f7d
Compare
k8s-github-robot
removed
the
lgtm
|
@k8s-bot pull-kubernetes-verify test this |
|
/lgtm |
|
@smarterclayton: you cannot LGTM your own PR. In response to this:
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. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: lavalamp, smarterclayton 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 |
smarterclayton
added
the
lgtm
|
Automatic merge from submit-queue |
|
yay, it merged. Time for me to rebase :) |
Automatic merge from submit-queue (batch tested with PRs 46967, 46992, 43338, 46717, 46672) Select initializers from the dynamic configuration Continues #36721 kubernetes/enhancements#209
…before-1.7 Automatic merge from submit-queue Remove Initializers from admission-control in kubernetes-master charm for pre-1.7 **What this PR does / why we need it**: This fixes a problem with the kubernetes-master charm where kube-apiserver never comes up: ``` failed to initialize admission: Unknown admission plugin: Initializers ``` The Initializers plugin does not exist before Kubernetes 1.7. The charm needs to support 1.6 as well. **Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes kubernetes#47062 **Special notes for your reviewer**: This fixes a problem introduced by kubernetes#36721 **Release note**: ```release-note Remove Initializers from admission-control in kubernetes-master charm for pre-1.7 ```
Automatic merge from submit-queue Remove Initializers from admission-control in kubernetes-master charm for pre-1.7 **What this PR does / why we need it**: This fixes a problem with the kubernetes-master charm where kube-apiserver never comes up: ``` failed to initialize admission: Unknown admission plugin: Initializers ``` The Initializers plugin does not exist before Kubernetes 1.7. The charm needs to support 1.6 as well. **Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #47062 **Special notes for your reviewer**: This fixes a problem introduced by kubernetes/kubernetes#36721 **Release note**: ```release-note Remove Initializers from admission-control in kubernetes-master charm for pre-1.7 ```
Initializers are the opposite of finalizers - they allow API clients to react to object creation and populate fields prior to other clients seeing them.
High level description:
metadata.initializersfield to all objectsinitializeverb on a resource)[]), or to have the result returned immediately when the object is created.The code here should be backwards compatible for all clients because they do not see partially initialized objects unless they GET the resource directly. The watch cache makes checking for partially initialized objects cheap. Some reflectors may need to change to ask for partially-initialized objects.