Skip to content

Eliminate Phase and simplify Conditions #7856

Closed
@bgrant0607

Description

Forked from #6951.

This is also discussed #1899 (comment) and elsewhere.

Users and developers apparently think of phases as states in a state machine, regardless of how much we try to dissuade them:
https://github.com/GoogleCloudPlatform/kubernetes/blob/master/docs/api-conventions.md#spec-and-status

This interpretation conflicts with the declarative, level-based, observation-driven design, as well as limiting evolution. In particular, the phase should be derivable by observing other state and shouldn't require durable persistence for correctness.

In my experience, the danger to system extensibility is significant, and results from assumptions baked into clients.

Phases aren't themselves properties or conditions of their objects, but clients inevitably infer properties from them.

For example, a client might try to determine whether a pod is active, or whether it has reached a permanent, terminal state, such as with a switch statement, as follows:

switch pod.Status.Phase {
case api.PodPending:
    // not active yet
    ...
case api.PodRunning:
    // pod active
    ...
case api.PodSucceeded:
    fallthrough
case api.PodFailed:
    // terminated and won't re-activate
    ...
case api.PodUnknown:
    // WTF!
    ...
default:
    glog.Fatalf("unexpected phase: %s", pod.Status.Phase)
}

However, let's say someone wanted to add a new Phase where the pod would also be active (containers running/restarting, etc.), as in #6951. That would mean that every client/component that made decisions based on the phase would need to also then consider the new phase.

Enums aren't extensible. Every addition is a breaking, non-backward-compatible API change. We could create a new major API version for every such addition, but that would be very expensive and disruptive. At some point, it will just become too painful to add new phases, and then we'll be left with an unprincipled distinction between phases and non-phases. Creating all imaginable phases up front would not only create a lot of complexity prematurely, but would also almost certainly not be correct.

Conditions are more extensible since the addition of new conditions doesn't (shouldn't) invalidate decisions based on existing conditions, and are also better suited to reflect conditions/properties that may toggle back and forth and/or that may not be mutually exclusive.

Rather than distributing logic that combines multiple conditions or other properties for common inferences, such as whether the scheduler may schedule new pods to a node, we should expose those properties/conditions explicitly, for similar reasons as why we explicitly return fields containing default values.

Proposal:

  1. Add LastTransitionTime, Reason, and Message to PodCondition (similar to NodeCondition)
  2. Add new Condition types (e.g., PodCondition and NodeCondition), as needed. At minimum, we need to be able to identify whether the pod has reached a terminal condition or not. We could add a Terminated condition for that. The Reason could distinguish Succeeded from failed -- any reason other than Succeeded would be considered a failure. We have at least a couple dozen such failure reasons internally.
  3. Add Conditions to whatever other objects currently have Phase but not Conditions
  4. Eliminate *.Phase and PodStatus.Message from the v1 API

cc @smarterclayton @derekwaynecarr @thockin @timothysc @davidopp @markturansky

Metadata

Assignees

No one assigned

    Labels

    area/apiIndicates an issue on api area.lifecycle/frozenIndicates that an issue or PR should not be auto-closed due to staleness.priority/important-soonMust be staffed and worked on either currently, or very soon, ideally in time for the next release.sig/api-machineryCategorizes an issue or PR as relevant to SIG API Machinery.sig/appsCategorizes an issue or PR as relevant to SIG Apps.sig/architectureCategorizes an issue or PR as relevant to SIG Architecture.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions