Add mutating webhook re-invocation details

keps/sig-api-machinery/00xx-admission-webhooks-to-ga.md

@@ -242,22 +242,103 @@ type AdmissionRequest struct {
 
 ### Mutating Plugin ordering
 
-Current ordering of the plugins (including webhooks) is fixed but may not work for all cases 
-(e.g. [this issue](https://github.com/kubernetes/kubernetes/issues/64333)). A mutating webhook 
-can add a completely new structure to the object (e.g. a `Container`) and other mutating plugins 
-may have opinion on those new structures (e.g. setting the image pull policy on all containers). 
-Although this problem can go deeper than two level (if the second mutating webhook added a new 
-structure based on the new `Container`), with current set of standard mutating plugins and use-cases, 
-it look like a rerun of mutating plugins is a sufficient fix. The proposal is to rerun all mutating 
-plugins (including webhooks) if there is any change by mutating webhooks. The assumption for this 
-process is all mutating plugins are idempotent and this must be clearly documented to users.
+A single ordering of mutating admissions plugins (including webhooks) does not work for all cases
+(see https://issue.k8s.io/64333 as an example). A mutating webhook can add a new sub-structure 
+to the object (like adding a `container` to a pod), and other mutating plugins which have already 
+run may have opinions on those new structures (like setting an `imagePullPolicy` on all containers). 
 
-Mutations from in-tree plugins will not trigger this process as they are well-ordered but if 
-there is any mutation by webhooks, all of the plugins including in-tree ones will be run again.
+To allow mutating admission plugins to observe changes made by other plugins, regardless of order,
+admission will add the ability to re-run mutating plugins (including webhooks) a single time if 
+there is any change made by a mutating webhook on the first pass.
 
-This feature would be would be opt in and defaulted to false for `v1beta1`.
+1. Initial mutating admission pass
+2. If any webhook plugins returned patches modifying the object, do a second mutating admission pass
+   1. Run all in-tree mutating admission plugins
+   2. Run all applicable webhook mutating admission plugins that indicate they want to be reinvoked
 
-The API representation and behavior for this feature is still under design and will be updated/approved here prior to implementation.
+Mutating plugins that are reinvoked must be idempotent, able to successfully process an object they have already 
+admitted and potentially modified. This will be clearly documented in admission webhook documentation,
+examples, and test guidelines. Mutating admission webhooks *should* already support this (since any 
+change they can make in an object could already exist in the user-provided object), and any webhooks
+that do not are broken for some set of user input.
+
+Note that idempotence (whether a webhook can successfully operate on its own output) is distinct from
+the `sideEffects` indicator, which describes whether a webhook can safely be called in `dryRun` mode.
+
+The reinvocation behavior will be opt-in, so `reinvocationPolicy` defaults to `"Never"`.
+
+```golang
+type Webhook struct {
+    ...
+    // reinvocationPolicy indicates whether this webhook should be called multiple times as part of a single admission evaluation.
+    // Allowed values are "Never" and "IfNeeded".
+    //
+    // Never: the webhook will not be called more than once in a single admission evaluation.
+    //
+    // IfNeeded: the webhook will be called at least one additional time as part of the admission evaluation
+    // if the object being admitted is modified by other admission plugins after the initial webhook call.
+    // Webhooks that specify this option *must* be idempotent, able to process objects they previously admitted.
+    // Note:
+    // * this policy does not apply to validating webhooks, because no modifications to the object being admitted are allowed during validation.
+    // * the number of additional invocations is not guaranteed to be exactly one.
+    // * if additional invocations result in further modifications to the object, webhooks are not guaranteed to be invoked again.
+    // * webhooks that use this option may be reordered to minimize the number of additional invocations.
+    // * to validate an object after all mutations are guaranteed complete, use a validating admission webhook instead (recommended for webhooks with side-effects).
+    //
+    // Defaults to "Never".
+    // +optional
+    ReinvocationPolicy *ReinvocationPolicyType `json:"reinvocationPolicy,omitempty"`
+}
+
+type ReinvocationPolicyType string
+
+var (
+  NeverReinvocationPolicy ReinvocationPolicyType = "Never"
+  IfNeededReinvocationPolicy ReinvocationPolicyType = "IfNeeded"
+)
+```
+
+Although this problem can go deeper than two levels (for example, if the second mutating webhook added
+a new structure based on the new `container`), a single re-run is sufficient for the current set of 
+in-tree plugins and webhook use-cases. If future use cases require additional or more targeted reinvocations,
+the `AdmissionReview` response can be enhanced to provide information to target those reinvocations.
+
+Test scenarios:
+
+1. No reinvocation
+   * in-tree -> mutation
+   * webhook -> no mutation
+   * complete (no reinvocation needed)
+
+2. In-tree reinvocation only
+   * in-tree -> mutation
+   * webhook -> mutation
+   * reinvoke in-tree -> no mutation
+   * complete (in-tree didn't change anything, so no webhook reinvocation required)
+
+3. Full reinvocation
+   * in-tree -> mutation
+   * webhook -> mutation
+   * reinvoke in-tree -> mutation
+   * reinvoke webhook -> mutation
+   * complete (both reinvoked once, no additional loops)
+
+4. Multiple webhooks, partial reinvocation
+   * in-tree -> mutation
+   * webhook A -> mutation
+   * webhook B -> mutation
+   * reinvoke in-tree -> no mutation
+   * reinvoke webhook A -> no mutation
+   * complete (no mutations occurred after webhook B was called, no reinvocation required)
+
+5. Multiple webhooks, full reinvocation
+   * in-tree -> mutation
+   * webhook A -> mutation
+   * webhook B -> mutation
+   * reinvoke in-tree -> no mutation
+   * reinvoke webhook A -> mutation
+   * reinvoke webhook B -> mutation (webhook A mutated after webhook B was called, one reinvocation required)
+   * complete (both reinvoked once, no additional loops)
 
 ### Passing {Operation}Option to Webhook
 
@@ -678,6 +759,24 @@ type Webhook struct {
      // +optional
      FailurePolicy *FailurePolicyType `json:"failurePolicy,omitempty" protobuf:"bytes,4,opt,name=failurePolicy,casttype=FailurePolicyType"`
 
+     // reinvocationPolicy indicates whether this webhook should be called multiple times as part of a single admission evaluation.
+     // Allowed values are "Never" and "IfNeeded".
+     //
+     // Never: the webhook will not be called more than once in a single admission evaluation.
+     //
+     // IfNeeded: the webhook will be called at least one additional time as part of the admission evaluation
+     // if the object being admitted is modified by other admission plugins after the initial webhook call.
+     // Webhooks that specify this option *must* be idempotent, able to process objects they previously admitted.
+     // Note:
+     // * the number of additional invocations is not guaranteed to be exactly one.
+     // * if additional invocations result in further modifications to the object, webhooks are not guaranteed to be invoked again.
+     // * webhooks that use this option may be reordered to minimize the number of additional invocations.
+     // * to validate an object after all mutations are guaranteed complete, use a validating admission webhook instead.
+     //
+     // Defaults to "Never".
+     // +optional
+     ReinvocationPolicy *ReinvocationPolicyType `json:"reinvocationPolicy,omitempty"`
+
      // NamespaceSelector decides whether to run the webhook on an object based
      // on whether the namespace for that object matches the selector. If the
      // object itself is a namespace, the matching is performed on
@@ -884,9 +983,6 @@ Also there need to be sufficient tests for any of these new features and all exi
 
 There are still open questions that need to be addressed and updated in this KEP before graduation:
 
-* Update with design and test details for "convert to webhook-requested version"
-* Update with design and test details for "mutating plugin ordering"
-
 ## Post-GA tasks
 
 These are related Post-GA tasks:
@@ -901,4 +997,5 @@ These are related Post-GA tasks:
 * First version of the KEP being merged: Jan 29th, 2019
 * The set of features for GA approved, initial set of features marked implementable: Feb 4th, 2019
 * Implementation start for all approved changes: Feb 4th, 2019
-* Target Kubernetes version for GA: TBD
+* Added details for auto-conversion: Apr 25th, 2019
+* Added details for mutating admission re-running: May 6th, 2019