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

[WIP] Reworked bootstrapper #262

Closed
wants to merge 1 commit into from
Closed

[WIP] Reworked bootstrapper #262

wants to merge 1 commit into from

Conversation

GiriSrini
Copy link

  • Removed the files from the outdated bootstrapper implementation.
  • Folded quickstart/ files into bootstrapper/ & replaced all references to quickstart/.
  • Both Kubernetes & Docker Compose setups are initialized with bootstrapper/init_catalog.sh, which takes options for host, port, & disabling KFP functionality from the user.

TO DO:

  • Rewrite Kubernetes instructions in bootstrapper/README.md.

- Removed the files from the outdated bootstrapper implementation.
- Folded `quickstart/` files into `bootstrapper/` & replaced all references to `quickstart/`.
- Both Kubernetes & Docker Compose setups are initialized with `bootstrapper/init_catalog.sh`, which takes options for host, port, & disabling KFP functionality from the user.

TO DO:
- Rewrite Kubernetes instructions in `bootstrapper/README.md`.
@mlx-bot
Copy link
Collaborator

mlx-bot commented Nov 22, 2021

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: girigsrini
To complete the pull request process, please assign animeshsingh after the PR has been reviewed.
You can assign the PR to them by writing /assign @animeshsingh in a comment when ready.

The full list of commands accepted by this bot can be found 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

@ckadner ckadner added the RCOS Potential work items for RCOS student interns label Dec 1, 2021
@ckadner ckadner requested review from ckadner and Tomcli and removed request for Tomcli and drewbutlerbb4 December 2, 2021 00:22
@ckadner ckadner linked an issue Dec 2, 2021 that may be closed by this pull request
Copy link
Member

@ckadner ckadner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Giri. Sorry for not responding sooner. We need to make a few more changes.

The Quickstart guide should stay in place. The only change under quickstart will be how the catalog gets initialized


For a full deployment, we use [Kubeflow Kfctl](https://github.com/kubeflow/kfctl) tooling.

* #### [MLX using Docker Compose (Asset Catalog Only)](./quickstart)
* #### [MLX using Docker Compose (Asset Catalog Only)](./bootstrapper)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these 2 links should still point to ./quickstart since the Quickstart as a concept and easy-install remains in place

@@ -100,7 +100,7 @@ Bring up the Quickstart without the `mlx-api` service, since we will run the MLX
from our local source code, instead of using the pre-built Docker image `mlexchange/mlx-api:nightly-main`.

# cd <mlx_root_dir>
cd quickstart
cd bootstrapper
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this folder needs to stay quickstart since that is where the docker compose app needs to start

After 2-5 minutes, the assets in [configmap.yaml](configmap.yaml) should be populated.
# Bootstrapper

## Running MLX with Docker Compose
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the bootstrapper/README.md should stay in place (with some modifications). it should not be replaced by the quickstart/README.md

@@ -1,12 +1,121 @@
# How to use it
1. Install MLX
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the install step should point to one of 3 options, like on the main README where we have kubernetes and Docker compose deployment options

@@ -1,12 +1,121 @@
# How to use it
1. Install MLX
2. Get the [github.ibm.com personal access token](https://github.ibm.com/settings/tokens/new) and give it access to read all public repos.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the remaining steps should describe how to run the bootstrapper script, i.e. set the host and port to point to the MLX server (either remote cluster or local Quickstart, but the local Quickstart catalog should be described in the quickstart/README.md)

@@ -95,7 +95,7 @@ from our local source code, instead of using the pre-built Docker image `mlexcha

```Bash
# cd <mlx_root_directory>
cd quickstart
cd bootstrapper
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stays quickstart

@ckadner
Copy link
Member

ckadner commented Dec 2, 2021

I think we should separate the changes for the bootstrapper and quickstart, starting by adding your new init_catalog.sh script under the bootstrapper folder.

The main idea for this PR should be that the bootstrapper folder would just keep the bootstrapper/catalog_upload.json and add a small Python or Bash script to call POST '/apis/v1alpha1/catalog' API endpoint with the catalog_upload.json file as payload.

We can then remove all of these files:

  • bootstrapper/bootstrap.yaml
  • bootstrapper/configmap.yaml
  • bootstrapper/Dockerfile
  • bootstrapper/start.py

We will keep:

  • bootstrapper/README.md
  • bootstrapper/catalog_upload.json

And add a script to call the /catalog API:

  • bootstrapper/init_catalog.sh

For the updates/simplifications to Quickstart we would open a follow up PR.

There are two recent changes that should make that rework easier:

So in the quickstart folder we would:

@ckadner
Copy link
Member

ckadner commented Dec 2, 2021

@girigsrini -- I took a look at the changes required for the Quickstart guide (files under quickstart) and they turned out to be quite comprehensive. So that should definitely be a separate PR.

For this PR, let's focus on changes under the bootstrapper folder as I described above

@ckadner ckadner changed the title Reworked bootstrapper [WIP] Reworked bootstrapper Dec 2, 2021
@ckadner ckadner mentioned this pull request Dec 2, 2021
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge/work-in-progress RCOS Potential work items for RCOS student interns
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rework bootstrapper
3 participants