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.

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

Should we deprecate status.conditions? #50798

Closed
erictune opened this issue Aug 16, 2017 · 6 comments
Closed

Should we deprecate status.conditions? #50798

erictune opened this issue Aug 16, 2017 · 6 comments
Labels
kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/feature Categorizes issue or PR as related to a new feature. sig/apps Categorizes an issue or PR as relevant to SIG Apps. sig/architecture Categorizes an issue or PR as relevant to SIG Architecture.

Comments

@erictune
Copy link
Member

I've surveyed kubernetes API types to see how Status Conditions are used, and evaluated their usability as well.

Method: Examine 54 kubernetes API types, spanning core types, incubator project, and operator (TPR/CRD) types. Detailed Data. Deeper examination of condition subfield on 14 conditions across 4 api types.

Goal: see whether Status.Conditions is useful and how used.

Survey Findings:

  • Status is widely used across Kubernetes types. Half of the types surveyed have Status.
  • Status.Conditions is not widely used. 18% of types surveyed have Status.Conditions.
  • Ready is the most common condition type for Status.Conditions.
  • Only 5 types have a Condition type with a meaning other than Ready.
  • In many cases, there are non-Condition status items that convey some of the same information as the conditions.
  • Most .status.condition types do not use the Reason or Message fields.
  • Of those that do, the Reason often does not contain additional information beyond what can be inferred from Type and Status.
  • Of those that do, all but one have Reason and Message which contain the same information. One had some additional information in Message (the intended use of the field).
  • Times (Probe/Update/Transition) were not used by 2 of the 4 types with conditions that were closely examined.
  • Some conditions that are about Readiness do not provide Reason or Message

Usability Issues with Conditions:

  • A Condition is verbose: it takes 4 lines, whereas a normal k-v pair in Status can express the main thing (whether the condition is met) in 1 line. Some status stanzas don't fit on a screen (let alone in a UI Card).
  • Because it is in a list rather than a map, and information is spread across two lines, it is a lot harder to extract condition using common tools like grep, awk, jq or kubectl -o jsonpath, compared to single k-v pair Status items. Source code to evaluate truth of a condition is also verbose compare to plain fields (search array vs lookup).

Possible Conclusions:

  • All of those 5 types that do use conditions (other than Ready) were implemented in part by very early project members. These people were involved in the discussions about the Conditions type and so baised towards using it.
  • Many other types have been added since then, and nobody has followed the convention of using a list of several Conditions and most don't use conditions at all. This suggests that most people don't find them useful, and none find it useful outside of Readiness.
  • Readiness condition might be a generally useful concept. (It is discussed in these issues Allow users to wait for conditions from kubectl and using the API #1899 and Facilitate API orchestration #34363). It does not appear to strongly require the other fields of Conditions, though.

Possible next Steps:

  • We could make it official that use of Conditions is not recommended for new types.
  • When we move existing types to new stable versions (such as moving the core controllers to apps/v1, we could drop Status.Conditions.
  • Readiness can be expressed as a non-condition status field.
  • Other status can be dropped or expressed as a non-condition status field.
@k8s-github-robot k8s-github-robot added the needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. label Aug 16, 2017
@erictune
Copy link
Member Author

/sig apps

@kubernetes/sig-apps-feature-requests Suggest we don't include Conditions in Deployment and ReplicaSet when moving them to GA. Can express same information in plain status fields (or events for the times when things happened).

@k8s-ci-robot k8s-ci-robot added sig/apps Categorizes an issue or PR as relevant to SIG Apps. kind/feature Categorizes issue or PR as related to a new feature. labels Aug 16, 2017
@k8s-github-robot k8s-github-robot removed the needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. label Aug 16, 2017
@kubernetes kubernetes deleted a comment from k8s-github-robot Aug 16, 2017
@erictune
Copy link
Member Author

@kubernetes/sig-architecture-api-reviews

@k8s-ci-robot k8s-ci-robot added sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API labels Aug 16, 2017
@erictune
Copy link
Member Author

Closing in favor of #7856 which was updated to conclude that we should remove conditions.

@bgrant0607
Copy link
Member

@erictune Thanks for the analysis. Please paste it into #7856.

@bgrant0607
Copy link
Member

I agree with the conclusions here, in case that isn't obvious.

@dims
Copy link
Member

dims commented Sep 23, 2020

Please see newer discussion kubernetes/community#4521

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/feature Categorizes issue or PR as related to a new feature. sig/apps Categorizes an issue or PR as relevant to SIG Apps. sig/architecture Categorizes an issue or PR as relevant to SIG Architecture.
Projects
None yet
Development

No branches or pull requests

5 participants