Add mutating webhook re-invocation details #1049
Conversation
k8s-ci-robot
added
cncf-cla: yes
|
/assign |
|
cc @yliaog |
liggitt
force-pushed
the
admission-ordering
branch
2 times, most recently
from
bcff6b6
to
fe5bfe7
Compare
|
addressed comments, renamed v1beta1 field, outlined plan for dropping the field and requiring idempotence in v1 |
liggitt
force-pushed
the
admission-ordering
branch
from
a5db4bd
to
4a47764
Compare
liggitt
changed the title
liggitt
force-pushed
the
admission-ordering
branch
from
4a47764
to
19fd9af
Compare
k8s-ci-robot
added
size/L
|
/assign @deads2k |
k8s-ci-robot
added
the
needs-rebase
| 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. |
There was a problem hiding this comment.
Webhooks with side effects should not double-actuate their side effects, so I think it's not quite as separate as described. Do we need to tell webhooks whether they are looking at the first or second invocation?
There was a problem hiding this comment.
Do we need to tell webhooks whether they are looking at the first or second invocation?
I would avoid that if at all possible. Admission can already be invoked multiple times per API request in case of conflicts encountered inside a GuaranteedUpdate loop.
There was a problem hiding this comment.
That's a good point. Maybe we should say, "It's already the responsibility of webhooks with side effects to avoid double-actuating those side effects if they are invoked multiple times for the same attempted change (which can already happen if a PATCH is retried due to an etcd conflict)."
There was a problem hiding this comment.
How can a webhook detect this condition, though?
There was a problem hiding this comment.
I still think it'd be good to add a blurb about this.
It continues to be the responsibility of webhooks with side effects to avoid
double-actuating those side effects. Note that it is already possible for all
webhooks to be invoked multiple times for the same change, if the entire
admission process is retried due to an etcd conflict.
b5f9ae1
to
05248ef
Compare
k8s-ci-robot
removed
the
needs-rebase
liggitt
force-pushed
the
admission-ordering
branch
from
05248ef
to
97332ab
Compare
|
/approve |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: lavalamp, liggitt 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
Adds details of the opt-in re-invocation mechanism
/cc @deads2k @lavalamp