Create documentation to track conventional usage of Conditions

[mattmoor]

I think it would be good to formalize our conventions around the use of status (building on this).

@cooperneil and I were discussing this and want to make sure we build this consistently.

I'll post a blurb with my mental model next, but want to open this to track.

[mattmoor]

(Here is my $0.02 on the use of Conditions in Elafros, let's iterate on this and then get it written down so that we can be consistent in our use of Conditions across projects.)

It is my hope that with these guidelines we can make the Conditions we use more self-describing.

Type and Reason

Type is typically defined in terms of a Go-style enumeration (e.g. FooConditionType) and so the values this takes on may be reasoned about like values of an enum in a programmatic manner. Clients should guard against unrecognized condition types, in case of server/client version skew, however, we expect this set to grow slowly.

Reason is typically defined in terms of a string and so the values this takes on (while camelcase words) should be treated as opaque. Clients shouldn't programmatically act on their values, but bias towards using Reason as a terse explanation of the state for end-users (whereas Message is the long-form of this). The set of values these take on may change fairly rapidly.

Presence of Conditions

Status: False on a particular Type is taken to mean that the controller is actively driving this resource towards the state represented by Type.

Status: True on a particular Type is taken to mean that the resource is now observed to be in this state. It is possible for the resource to exit this state through observation, but from the perspective of the controller, this is the end state of a particular reconciliation (possibly unsuccessful if a failure Type).

Examples

With this world view the following combinations would not make sense for reasons inline:

conditions:
# We are not driving towards BuildComplete, since the build has failed.
- name: BuildComplete
  status: False
- name: BuildFailed
  status: True

conditions:
# Failure is not a condition the controller drives things towards.
- name: BuildFailed
  status: False

[cooperneil]

@evankanderson

I wonder then if a condition like this (from the API draft) need to be teased out into separate conditions, as the current form relies on the Reason to convey the problem:

kind: Configuration
...
status:
  latestReadyRevisionName: abc
  latestCreatedRevisionName: bcd  # Hasn't become became "Ready"
  conditions:
  - type: LatestRevisionReady
    status: False
    reason: ContainerMissing
    message: "Unable to start because container is missing and build failed."

[vaikas]

@cooperneil I don't think it's necessary bad to have a state: revision ready (false) and the reason why it's not ready. The reason could change. Otherwise we'll have another condition for every kind of failure? Or am I missing what you're proposing.

[vaikas]

LGTM

[evankanderson]

FWIW, I've started translating those docs into Markdown... I still need to move the pictures over and clean up a bit of formatting, but you can see the start here: https://github.com/evankanderson/elafros/tree/spec/docs/spec

In particular, we'd specified some of this in https://github.com/evankanderson/elafros/blob/spec/docs/spec/errors.md; I'll try to pry some time out tonight in addition to messing with Vault to get that into PR shape.

[sixolet]

@cooperneil @evankanderson For the use case of a client interested in determining when the latest revision is either ready or never-going-to-become-ready, do we then need to qualify each of the enum states as either final or not?
It might be useful to have a separate condition for RolloutStable or something.

[cooperneil]

@vaikas-google, my question is in regards to the guidance:

Reason is typically defined in terms of a string and so the values this takes on (while camelcase words) should be treated as opaque. Clients shouldn't programatically act on their values

Question is, how does a client tool differentiate an error False state due to a Reason like ContainerMissing vs an expected False state (e.g. the container is still being built), if Reason is opaque but the condition has the form:

  - type: LatestRevisionReady
    status: False
    reason: ContainerMissing
    message: "Unable to start because container is missing and build failed."

Do we not need separate conditions for failure if client tools can only rely on Type and Status=True/False?

[mattmoor]

ContainerMissing seems like a terminal state, and I think should correspond to a Condition like Failed with status: True.