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

Use conditions instead of phase #223

Closed
gaocegege opened this issue Dec 14, 2017 · 5 comments
Closed

Use conditions instead of phase #223

gaocegege opened this issue Dec 14, 2017 · 5 comments

Comments

@gaocegege
Copy link
Member

gaocegege commented Dec 14, 2017

Some resources in the v1 API contain fields called phase, and associated message, reason, and other status fields. The pattern of using phase is deprecated. Newer API types should use conditions instead. Phase was essentially a state-machine enumeration field, that contradicted system-design principles and hampered evolution, since adding new enum values breaks backward compatibility. Rather than encouraging clients to infer implicit properties from phases, we intend to explicitly expose the conditions that clients need to monitor. Conditions also have the benefit that it is possible to create some conditions with uniform meaning across all resource types, while still exposing others that are unique to specific resource types. See #7856 for more details and discussion.

https://github.com/kubernetes/community/blob/master/contributors/devel/api-conventions.md#typical-status-properties

kubernetes suggests us to use conditions instead of phase, now the code uses phase heavily, I think we should have a plan to deprecate phase.

/cc @DjangoPeng

label/enhancement

@jlewi
Copy link
Contributor

jlewi commented Dec 14, 2017

Makes sense to me.

Should we wait until #206 is fixed to fix this?

/cc @wackxu

@gaocegege
Copy link
Member Author

I think so 🤔

We could design the correspondence between phase and conditions first and the implementation may be blocked until #215 merged.

@DjangoPeng
Copy link
Member

Agree with @gaocegege. A slight question here: what's the design purpose of ReplicaStatuses and ReplicasStates here?

@jlewi
Copy link
Contributor

jlewi commented Dec 15, 2017

ReplicaStatuses and ReplicaStates provide information about each replica. Not all replicas might be in the same state; some might be crashing for example. So ReplicaStatuses provide finer grained information then just the overall status of the job.

@gaocegege
Copy link
Member Author

I think we could close the issue since v1alpha2 already implemented it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants