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.

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

Kubeflow contains 2 controllers for launching Tensorboards #5921

Closed
davidspek opened this issue May 15, 2021 · 8 comments
Closed

Kubeflow contains 2 controllers for launching Tensorboards #5921

davidspek opened this issue May 15, 2021 · 8 comments

Comments

@davidspek
Copy link
Contributor

In the 1.3 release, the Notebooks Working Group added the Tensorboards Web App that uses the tensorboard controller to launch tensorboard instances. However, Kubeflow Pipelines already contains the viewers.kubeflow.org CRD as part of its controller, which currently is only used to start Tensorboard instances. As a result, there are now 2 controllers, along with 2 CRDs and 2 controller deployments that are serving the same purpose. Except for the obvious duplication of code and increased maintenance and deployment complexity, this is not great for the user experience. Many users will not know or expect that there are 2 controllers and CRDs that are used to start Tensorboard instances, and will also wonder why the Tensorboards Web App does not list the tensorboard instances created as part of a pipeline. The tensorboards controller also has the image it uses for the tensorboard pod hardcoded in the controller, so there is no way for the user or the cluster admin to change what image is being used for launching a tensorboard instance (particularly problematic for those behind a firewall or in an airgapped environment).

For this reason, I would like to suggest to use the viewers CRD that is part of Kubeflow Pipelines in the Tensorboards Web App. Along with that, I would like to suggest to port the feature from the tensorboard controller to the KFP viewers controller if they are not already present (for example RWO scheduling logic).
The 2 main reason for using the KFP viewers CRD over the tensorboard controller are:

  1. The viewers CRD is setup to be a more general CRD that is built to be extended to other types of viewers, such as a file browser for example.
  2. Kubeflow Pipelines sees many standalone deployments as well, while the Tensorboards Web App does not (and most, if not all, people deploying Kubeflow will also be deploying Kubeflow Pipelines). Therefore, it will more of a problem for KFP to rely on an external controller as a dependency. While this is not a great reason, and there are other possibilities that would need discussing, it is still something to take into account.

/cc @kubeflow/wg-notebooks-leads @kubeflow/wg-pipeline-leads

@davidspek davidspek added this to Controllers Stream in Notebooks WG May 15, 2021
@kubeflow-bot kubeflow-bot added this to To Do in Needs Triage May 15, 2021
@Bobgy
Copy link
Contributor

Bobgy commented May 15, 2021

Thank you for raising the issue!
I agree 💯.

Recently from KFP side we noticed a need for more generic viewer, e.g. other visualization tools besides tensorboard.

Also, KFP doesn't have a good CRUD UI for tensorboard. So it seems there are a lot of value to unify and make this more generic.

@davidspek
Copy link
Contributor Author

In general, I think there should be a focus on improving the integration between the components to make everything feel like a more cohesive and polished platform, which would be done through the UIs.

@davidspek
Copy link
Contributor Author

Another option would be to create a new controller that combines the notebook controller and the KFP viewer controller under a more general common user created web apps controller, since they are essentially performing the same tasks (pod deployment and creating an Istio VirtualService for the routing). This would allow for a single controller that could be extended with whatever user created web applications a component of Kubeflow might want to deploy in the future.

@Bobgy Do you think it would be acceptable for KFP to rely on such a common controller, mainly in terms of standalone deployments? I don't think it would be very different from the current situation with Argo Workflows for example.

@Bobgy
Copy link
Contributor

Bobgy commented May 16, 2021

For KFP standalone, there is some special proxy setup in KFP UI deployment, it doesn't use VirtualService.

Other than this, all the requirements seem to match very well. I personally think this is worth doing and KFP standalone can just take a third-party viewer controller too.

@Bobgy
Copy link
Contributor

Bobgy commented May 16, 2021

/cc @zijianjoy

@Bobgy
Copy link
Contributor

Bobgy commented May 19, 2021

I'm sending out a generic viewer controller proposal in kubeflow/pipelines#5681

@jbottum
Copy link
Contributor

jbottum commented May 23, 2021

/kind feature
/priority p2

@stale
Copy link

stale bot commented Aug 22, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot closed this as completed Mar 3, 2022
Needs Triage automation moved this from To Do to Closed Mar 3, 2022
Notebooks WG automation moved this from Controllers Stream to Done Mar 3, 2022
@kubeflow-bot kubeflow-bot removed this from Closed in Needs Triage Mar 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

No branches or pull requests

5 participants