Skip to content
This repository has been archived by the owner on Sep 19, 2022. It is now read-only.

pytorch-operator: Consolidate manifests #323

Merged

Conversation

yanniszark
Copy link
Contributor

Umbrella-Issue: kubeflow/manifests#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
  • overlays/standalone is used to install the operator in its own namespace, for testing or other purposes.
  • overlays/kubeflow is used to install the operator as part of kubeflow.

@yanniszark
Copy link
Contributor Author

cc @kubeflow/wg-training-leads

@coveralls
Copy link

coveralls commented Mar 17, 2021

Coverage Status

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

@yanniszark
Copy link
Contributor Author

/retest

@Jeffwan
Copy link
Member

Jeffwan commented Mar 17, 2021

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

@yanniszark yanniszark force-pushed the feature-consolidate-manifests branch from a9965c9 to 5071c9c Compare March 18, 2021 10:30
@yanniszark yanniszark force-pushed the feature-consolidate-manifests branch from 5071c9c to afb2445 Compare March 18, 2021 10:35
@yanniszark
Copy link
Contributor Author

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

@Jeffwan
Copy link
Member

Jeffwan commented Mar 18, 2021

/lgtm
/approve

@google-oss-robot
Copy link

[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:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants