kubeflow / pytorch-operator

PyTorch on Kubernetes
Apache License 2.0
307 stars 143 forks source link

pytorch-operator: Consolidate manifests #323

Closed yanniszark closed 3 years ago

yanniszark commented 3 years ago

Umbrella-Issue: https://github.com/kubeflow/manifests/issues/1769

PyTorch Operator

Current manifests structure

.
├── crd.yaml
├── deployment.yaml
├── kustomization.yaml
├── namespace.yaml
├── podgroup.yaml
├── pytorch-job-crds
│   ├── base
│   └── overlays
│       └── application
├── pytorch-operator
│   ├── base
│   └── overlays
│       └── application

Explanation

Recommended end state

The main diff is that the upstream manifests also install a namespace resource, which we don't want when installing with kubeflow. Following the standard base and overlays structure, I propose the following structure:

├── base
├── overlays
│   ├── kubeflow
│   └── standalone
yanniszark commented 3 years ago

cc @kubeflow/wg-training-leads

coveralls commented 3 years ago

Coverage Status

Coverage remained the same at 64.11% when pulling afb2445cb8540caa339dc21af6872dc8d93e11eb on arrikto:feature-consolidate-manifests into 147349ee731720e0d2b20171fe5a228a112f34d0 on kubeflow:master.

yanniszark commented 3 years ago

/retest

Jeffwan commented 3 years ago

See #325 for more details. I file #326 to mitigate this issue. This should be unblocked once #326 is merged

yanniszark commented 3 years ago

I have rebased and the tests now pass. Thanks @Jeffwan

Jeffwan commented 3 years ago

/lgtm /approve

google-oss-robot commented 3 years ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Jeffwan, yanniszark

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/kubeflow/pytorch-operator/blob/master/OWNERS)~~ [Jeffwan] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment