Use local copy of JobStatus by mpi-operator#514
Use local copy of JobStatus by mpi-operator#514google-oss-prow[bot] merged 2 commits intokubeflow:masterfrom
Conversation
Signed-off-by: Yuki Iwai <yuki.iwai.tz@gmail.com>
tenzen-y
changed the title
|
What is missing in this PR? |
tenzen-y
changed the title
@alculquicondor There are no missing. PTAL. |
| // ReplicaStatuses is map of ReplicaType and ReplicaStatus, | ||
| // specifies the status of each replica. | ||
| // +options | ||
| ReplicaStatuses map[MPIReplicaType]*ReplicaStatus `json:"replicaStatuses,omitempty"` |
There was a problem hiding this comment.
another violation of API conventions T_T
But we can only change it in a new API version.
There was a problem hiding this comment.
Yes, that's right :(
For v2beta1, we need to keep using this API for compatibility.
pkg/apis/kubeflow/v2beta1/types.go
Outdated
| type JobCondition struct { | ||
| // Type of job condition. | ||
| // +options | ||
| Type JobConditionType `json:"type,omitempty"` |
There was a problem hiding this comment.
It shouldn't be optional. Also, in general we don't restrict this to enums for Job or Pod, with the assumption that external controllers can add their own.
There was a problem hiding this comment.
It shouldn't be optional.
Makes sense.
Also, in general we don't restrict this to enums for Job or Pod, with the assumption that external controllers can add their own.
@alculquicondor Does that mean we want to change the type of Type to string?
There was a problem hiding this comment.
It can be a type, but we shouldn't add an enum annotation (which you don't currently have)
Signed-off-by: Yuki Iwai <yuki.iwai.tz@gmail.com>
|
Let's see which one merges first 😂 #511 /assign @terrytangyuan |
Haha. I trust @terrytangyuan :) |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alculquicondor, terrytangyuan The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
3 participants
Signed-off-by: Yuki Iwai yuki.iwai.tz@gmail.com
I copied the
JobStatusapi to this repository.Currently, we can not use auto-generated CRDs in our unit and E2E tests since the
common.JobStatusrequiresJobStatus.ConditionandJobStatus.ReplicaStatusesby default in the following:https://github.com/kubeflow/common/blob/9ec55d141f90faaf52fd6df271e987e5a6781945/pkg/apis/common/v1/types.go#L25-L31
So, if we use auto-generated CRDs, we face the following errors in tests:
https://github.com/kubeflow/mpi-operator/actions/runs/4071386113/jobs/7013139204#step:8:39
Blocking: #510