Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Mention management of metadata.generation and status.observedGeneration. #31

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Mention management of metadata.generation and status.observedGeneration. #31

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions specs/common/error-signalling.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
# Error Signalling

<!-- copied from ../serving/knative-api-specification-1.0.md#error-signalling -->

Knative APIs use the
[Kubernetes Conditions convention](https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#typical-status-properties)
to communicate errors and problems to the user. Note that Knative customizes the
Expand All @@ -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`
Copy link
Contributor

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.

Copy link
Member

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.

and `status` fields; servers MUST also update `metadata.generation` on each
change to `spec` fields. To accomodate this split, server-side resources MUST
report `observedGeneration` in `status` to detect changes to `spec` which have
not yet been reported in `status`.

Clients SHOULD check that `status.observedGeneration` is equal to
`metadata.generation` before computing Conditions; if the two are not equal, the
resource is in the process of being updated.

## Condition Schema

Fields in the condition which are not marked as "REQUIRED" MAY be omitted to
indicate the default value (i.e. a Condition which does not include a `status`
field is equivalent to a `status` of `"Unknown"`). As `Conditions` are an output
Expand Down