Skip to content

Commit

Permalink
address comments about polarity
Browse files Browse the repository at this point in the history
  • Loading branch information
fabriziopandini committed May 20, 2020
1 parent 3a2e2fa commit 2918c84
Showing 1 changed file with 25 additions and 2 deletions.
27 changes: 25 additions & 2 deletions docs/proposals/20200506-conditions.md
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ type Condition struct {
LastTransitionTime metav1.Time `json:"lastTransitionTime,omitempty"`

// The reason for the condition's last transition.
// Reasons should be one-word CamelCase.
// Reasons should be CamelCase.
// +optional
Reason string `json:"reason,omitempty" description:"one-word CamelCase reason for the condition's last transition"`

Expand Down Expand Up @@ -234,11 +234,15 @@ const (

Condition types MUST have a consistent polarity (i.e. "True = good");

Condition types MUST have one of the following suffix:
Condition types SHOULD have one of the following suffix:

- `Ready`, for resources which represent an ongoing status, like `ControlplaneReady` or `MachineDeploymentsReady`.
- `Succeeded`, for resources which run-to-completion, e.g. `CreateVPCSucceeded`

When the above suffix are not adequate for a specific condition type, other suffix _with positive meaning_ COULD be used
(e.g. `Completed`, `Healthy`); however, it is recommended to balance this flexibility with the objective to provide
a consistent condition naming across all the Cluster API objects.

The `Severity` field MUST be set only when `Status=False` and it is designed to provide a standard classification
of possible conditions failure `Reason`.

Expand Down Expand Up @@ -477,6 +481,25 @@ enhance the condition utilities to handle those situations in a generalized way.
to ensure a consistent approach across all objects.
- Mitigation: Ensure all the implementations comply with the defined set of constraints/design principles.

- Risk: Having a consistent polarity ensures a simple and clear contract with the consumers, and it allows
processing conditions in a simple and consistent way without being forced to implements specific logic
for each condition type. However, we are aware about the fact that enforcing of consistent polarity (truthy)
combined with the usage of recommended suffix for condition types can lead to verbal contortions to express
conditions, especially in case of conditions designed to signal problems or in case of conditions
that might exists or not.
- Mitigation: We are relaxing the rule about recommended suffix and allowing usage of custom suffix.
- Mitigation: We are recommending the condition adhere to the design principle to express the operational state
of the component, and this should help in avoiding conditions name to surface internal implementation details.
- Mitigation: We should recommend condition implementers to clearly document the meaning of Unknown state, because as
discussed also in the recent [Kubernetes KEP about standardizing conditions](https://github.com/kubernetes/enhancements/pull/1624#pullrequestreview-388777427),
_"Unknown" is a fact about the writer of the condition, and not a claim about the object_.
- Mitigation: We should recommend developers of code relying on conditions to treat Unknown as a separated state vs
assimilating it to True or False, because this can vary case by case and generate confusion in readers.

As a final consideration about the risk related to using a consistent polarity, it is important to notice that a
consistent polarity ensure a clear meaning for True or o False states, which is already an improvement vs having
different interpretations for all the three possible condition states.

## Alternatives

### Kubernetes Conditions
Expand Down

0 comments on commit 2918c84

Please sign in to comment.