-
Notifications
You must be signed in to change notification settings - Fork 49
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Mention management of metadata.generation
and status.observedGeneration
.
#31
base: main
Are you sure you want to change the base?
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: evankanderson 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 |
@@ -10,6 +8,20 @@ general Kubernetes recommendation with a `severity` field, and does not include | |||
described in Resource Overview MUST have a `conditions` field in `status`, which | |||
MUST be a list of `Condition` objects described by the following table. | |||
|
|||
## Client Determination of Readiness | |||
|
|||
Some servers (such as Kubernetes) may separate updates of the resource `spec` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not 100% sure but I think you mean for the "may" here to be "MAY" (meaning normative), right?
The stuff after the semi-colon doesn't flow right to me. What about:
Server implementations MAY separate updates of the resource spec
and the status
fields.
Whether an implementation does so or not, any update to the spec
of a resource MUST result in the spec.metadata.generation
field being updated (according to the Kubernetes rules). Implementation MUST update the status.observedGeneration
field when all updates related to a corresponding spec.metadata.generation
set of field values have been applied.
When determining whether a resource is "Ready", client implementations are expected to not only check the status.conditions
values, but also whether the status.observedGeneration
and spec.metadata.generation
values are equal.
However, the more I think about this, the more it bothers me. ;-) Let's take the case where a resource is Ready but for whatever reason the "spec" section is updated a lot, but in ways that don't really impact much... e.g. maybe annotations are added/removed because some 3rd party tool is using that to control some non-Knative aspects of the overall system. Should the two "generation" values being different really make clients believe that the resource isn't ready and therefore not do things like send events to it? Having clients switch between thinking the resource is ready and not continually even tho all of the Conditions say "true" sounds like we're asking for things to be racey and intermittent - and give spotty performance.
Why not keep it simple and have clients only check the conditions and then the server can decide when to change the conditions? If it determines that it wants to change it on ANY change to the spec, then it can do so - even w/o actually doing the reconciliation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW - clients should look at the generation values and compare them to understand if the changes applied to spec have been fullfilled.
This doesn't preclude clients from sending trafffic - but you're sending traffic to something that doesn't match the desired spec.
I believe Webhooks being invoked for |
@evankanderson do you still want this PR to merge? I didn't see a reply to @duglin's comment. |
Document management of
status.observedGeneration
and client behavior from this comment.TODO: experiment whether mutating webhooks could actually clear Conditions during PATCH of
spec
. If so, update webhooks and then we can remove this section.