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.

Sign up for GitHub

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 local copy of JobStatus by mpi-operator #514

Merged
merged 2 commits into from
Feb 3, 2023
Merged

Use local copy of JobStatus by mpi-operator #514

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
897b800
Use local copy of JobStatus by mpi-operator
tenzen-y Feb 2, 2023
bc8288c
address comments
tenzen-y Feb 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions crd/kubeflow.org_mpijobs.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7925,9 +7925,6 @@ spec:
type:
description: Type of job condition.
type: string
required:
- status
- type
type: object
type: array
lastReconcileTime:
Expand Down Expand Up @@ -8015,9 +8012,6 @@ spec:
and is in UTC.
format: date-time
type: string
required:
- conditions
- replicaStatuses
type: object
type: object
served: true
Expand Down
96 changes: 95 additions & 1 deletion pkg/apis/kubeflow/v2beta1/swagger.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,71 @@
}
}
},
"v2beta1.JobCondition": {
"description": "JobCondition describes the state of the job at a certain point.",
"type": "object",
"properties": {
"lastTransitionTime": {
"description": "Last time the condition transitioned from one status to another.",
"default": {},
"$ref": "#/definitions/v1.Time"
},
"lastUpdateTime": {
"description": "The last time this condition was updated.",
"default": {},
"$ref": "#/definitions/v1.Time"
},
"message": {
"description": "A human readable message indicating details about the transition.",
"type": "string"
},
"reason": {
"description": "The reason for the condition's last transition.",
"type": "string"
},
"status": {
"description": "Status of the condition, one of True, False, Unknown.",
"type": "string"
},
"type": {
"description": "Type of job condition.",
"type": "string"
}
}
},
"v2beta1.JobStatus": {
"description": "JobStatus represents the current observed state of the training Job.",
"type": "object",
"properties": {
"completionTime": {
"description": "Represents time when the job was completed. It is not guaranteed to be set in happens-before order across separate operations. It is represented in RFC3339 form and is in UTC.",
"$ref": "#/definitions/v1.Time"
},
"conditions": {
"description": "Conditions is an array of current observed job conditions.",
"type": "array",
"items": {
"default": {},
"$ref": "#/definitions/v2beta1.JobCondition"
}
},
"lastReconcileTime": {
"description": "Represents last time when the job was reconciled. It is not guaranteed to be set in happens-before order across separate operations. It is represented in RFC3339 form and is in UTC.",
"$ref": "#/definitions/v1.Time"
},
"replicaStatuses": {
"description": "ReplicaStatuses is map of ReplicaType and ReplicaStatus, specifies the status of each replica.",
"type": "object",
"additionalProperties": {
"$ref": "#/definitions/v2beta1.ReplicaStatus"
}
},
"startTime": {
"description": "Represents time when the job was acknowledged by the job controller. It is not guaranteed to be set in happens-before order across separate operations. It is represented in RFC3339 form and is in UTC.",
"$ref": "#/definitions/v1.Time"
}
}
},
"v2beta1.MPIJob": {
"type": "object",
"properties": {
Expand All @@ -208,7 +273,7 @@
},
"status": {
"default": {},
"$ref": "#/definitions/v1.JobStatus"
"$ref": "#/definitions/v2beta1.JobStatus"
}
}
},
Expand Down Expand Up @@ -273,6 +338,35 @@
}
}
},
"v2beta1.ReplicaStatus": {
"description": "ReplicaStatus represents the current observed state of the replica.",
"type": "object",
"properties": {
"active": {
"description": "The number of actively running pods.",
"type": "integer",
"format": "int32"
},
"failed": {
"description": "The number of pods which reached phase Failed.",
"type": "integer",
"format": "int32"
},
"labelSelector": {
"description": "Deprecated: Use Selector instead",
"$ref": "#/definitions/v1.LabelSelector"
},
"selector": {
"description": "A Selector is a label query over a set of resources. The result of matchLabels and matchExpressions are ANDed. An empty Selector matches all objects. A null Selector matches no objects.",
"type": "string"
},
"succeeded": {
"description": "The number of pods which reached phase Succeeded.",
"type": "integer",
"format": "int32"
}
}
},
"v2beta1.RunPolicy": {
"description": "RunPolicy encapsulates various runtime policies of the distributed training job, for example how to clean up resources and how long the job can stay active.",
"type": "object",
Expand Down
119 changes: 116 additions & 3 deletions pkg/apis/kubeflow/v2beta1/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ import (
type MPIJob struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`
Spec MPIJobSpec `json:"spec,omitempty"`
Status common.JobStatus `json:"status,omitempty"`
Spec MPIJobSpec `json:"spec,omitempty"`
Status JobStatus `json:"status,omitempty"`
}

// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
Expand Down Expand Up @@ -114,7 +114,7 @@ type MPIJobSpec struct {
}

// MPIReplicaType is the type for MPIReplica.
type MPIReplicaType common.ReplicaType
type MPIReplicaType string

const (
// MPIReplicaTypeLauncher is the type for launcher replica.
Expand All @@ -130,3 +130,116 @@ const (
MPIImplementationOpenMPI MPIImplementation = "OpenMPI"
MPIImplementationIntel MPIImplementation = "Intel"
)

// JobStatus represents the current observed state of the training Job.
type JobStatus struct {
// Conditions is an array of current observed job conditions.
// +options
Conditions []JobCondition `json:"conditions,omitempty"`

// ReplicaStatuses is map of ReplicaType and ReplicaStatus,
// specifies the status of each replica.
// +options
ReplicaStatuses map[MPIReplicaType]*ReplicaStatus `json:"replicaStatuses,omitempty"`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

another violation of API conventions T_T
But we can only change it in a new API version.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's right :(
For v2beta1, we need to keep using this API for compatibility.


// Represents time when the job was acknowledged by the job controller.
// It is not guaranteed to be set in happens-before order across separate operations.
// It is represented in RFC3339 form and is in UTC.
// +options
StartTime *metav1.Time `json:"startTime,omitempty"`

// Represents time when the job was completed. It is not guaranteed to
// be set in happens-before order across separate operations.
// It is represented in RFC3339 form and is in UTC.
// +options
CompletionTime *metav1.Time `json:"completionTime,omitempty"`

// Represents last time when the job was reconciled. It is not guaranteed to
// be set in happens-before order across separate operations.
// It is represented in RFC3339 form and is in UTC.
// +options
LastReconcileTime *metav1.Time `json:"lastReconcileTime,omitempty"`
}

// ReplicaStatus represents the current observed state of the replica.
type ReplicaStatus struct {
// The number of actively running pods.
// +options
Active int32 `json:"active,omitempty"`

// The number of pods which reached phase Succeeded.
// +options
Succeeded int32 `json:"succeeded,omitempty"`

// The number of pods which reached phase Failed.
// +options
Failed int32 `json:"failed,omitempty"`

// Deprecated: Use Selector instead
// +options
LabelSelector *metav1.LabelSelector `json:"labelSelector,omitempty"`

// A Selector is a label query over a set of resources. The result of matchLabels and
// matchExpressions are ANDed. An empty Selector matches all objects. A null
// Selector matches no objects.
// +options
Selector string `json:"selector,omitempty"`
}

// JobCondition describes the state of the job at a certain point.
type JobCondition struct {
// Type of job condition.
// +options
Type JobConditionType `json:"type,omitempty"`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can be a type, but we shouldn't add an enum annotation (which you don't currently have)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see.
SGTM.


// Status of the condition, one of True, False, Unknown.
// +options
Status v1.ConditionStatus `json:"status,omitempty"`

// The reason for the condition's last transition.
// +options
Reason string `json:"reason,omitempty"`

// A human readable message indicating details about the transition.
// +options
Message string `json:"message,omitempty"`

// The last time this condition was updated.
// +options
LastUpdateTime metav1.Time `json:"lastUpdateTime,omitempty"`

// Last time the condition transitioned from one status to another.
// +options
LastTransitionTime metav1.Time `json:"lastTransitionTime,omitempty"`
}

// JobConditionType defines all kinds of types of JobStatus.
type JobConditionType string

const (
// JobCreated means the job has been accepted by the system,
// but one or more of the pods/services has not been started.
// This includes time before pods being scheduled and launched.
JobCreated JobConditionType = "Created"

// JobRunning means all sub-resources (e.g. services/pods) of this job
// have been successfully scheduled and launched.
// The training is running without error.
JobRunning JobConditionType = "Running"

// JobRestarting means one or more sub-resources (e.g. services/pods) of this job
// reached phase failed but maybe restarted according to it's restart policy
// which specified by user in v1.PodTemplateSpec.
// The training is freezing/pending.
JobRestarting JobConditionType = "Restarting"

// JobSucceeded means all sub-resources (e.g. services/pods) of this job
// reached phase have terminated in success.
// The training is complete without error.
JobSucceeded JobConditionType = "Succeeded"

// JobFailed means one or more sub-resources (e.g. services/pods) of this job
// reached phase failed with no restarting.
// The training has failed its execution.
JobFailed JobConditionType = "Failed"
)
90 changes: 90 additions & 0 deletions pkg/apis/kubeflow/v2beta1/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading