kubeflow / kubeflow/mpi-operator

Set the knowledge about Launcher and Worker to CRD

Open
#519 3 comments 0 reactions 0 assignees View on GitHub
kind/feature
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.