kubeflow / kubeflow/mpi-operator
Set the knowledge about Launcher and Worker to CRD
- Dominant language
- Go
- Stars
- 535
- Forks
- 238
- Avg merge
- 2d 3h
- Merged PRs (30d)
- 10
Description
/kind feature
We have API violations related to maps in the following:
https://github.com/kubeflow/mpi-operator/blob/31d4575066770756cc8ca347cc6d5ba3113b1e02/pkg/apis/kubeflow/v2beta1/types.go#L112-L114
https://github.com/kubeflow/mpi-operator/blob/31d4575066770756cc8ca347cc6d5ba3113b1e02/pkg/apis/kubeflow/v2beta1/types.go#L154-L157
So we should use named subobjects instead of maps in the following:
```go
type MPIReplicaSpecs struct {
Launcher common.ReplicaSpec `json:"launcher"`
Worker common.ReplicaSpec `json:"worker"`
}
```
```go
type MPIReplicaStatuses struct {
Launcher *ReplicaStatus `json:"launcher"`
Worker *ReplicaStatus `json:"worker"`
}
```
However, we should work on this in the new API version such as `v2beta2` or `v2` since this change doesn't have backward compatibility.
Original comments by @alculquicondor.
> We are losing the knowledge about Launcher and Worker here.
> But this is a problem with the API structs themselves.
>
> We shouldn't be using a map https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#lists-of-named-subobjects-preferred-over-maps
>
> Maybe we can fix it in a v2(beta2) API, but it would be nice to fix other training job objects, like so:
>
> ```go
> type MPIReplicaSpecs struct {
> Launcher common.ReplicaSpec `json:"launcher"`
> Worker common.ReplicaSpec `json:"worker"`
> }
> ```
https://github.com/kubeflow/mpi-operator/pull/510#discussion_r1090974669
> another violation of API conventions T_T
But we can only change it in a new API version.
https://github.com/kubeflow/mpi-operator/pull/514#discussion_r1094826593
Contributor guide
Research direction
Start with pkg/apis/kubeflow/v2beta1/types.go at the referenced map fields and read the Kubernetes API conventions linked in the issue. Determine the new API version and the affected Launcher and Worker spec and status types, then verify that the new CRD shape preserves the named subobjects without changing the existing version's compatibility.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go, kubernetes
- Domain
- backend-api-design
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 30/100