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

Create a doc to describe the deployment process. #1159

Merged
merged 1 commit into from
Jul 27, 2018

Conversation

jlewi
Copy link
Contributor

@jlewi jlewi commented Jul 10, 2018

  • This is not a proposal but a description of the current state.

/assign @ankushagarwal
/assign @kunmingg


This change is Reviewable


1. We provide simple, platform deployment scripts like this [one for GKE](https://github.com/kubeflow/kubeflow/blob/master/docs/gke/configs/deploy.sh)

1. A corresponding platform specific getting started page ([see here]](https://github.com/kubeflow/website/tree/master/content/docs/started) provides platform specific instructions
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo near see here


1. We provide simple, platform deployment scripts like this [one for GKE](https://github.com/kubeflow/kubeflow/blob/master/scripts/gke/deploy.sh)

1. A corresponding platform specific getting started page ([see here]](https://github.com/kubeflow/website/tree/master/content/docs/started) provides platform specific instructions
Copy link
Contributor

Choose a reason for hiding this comment

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

Replace [see here]] with [see here]

@pdmack
Copy link
Member

pdmack commented Jul 12, 2018

/retest


The scaffolding/prototype for this (bootstrapper) is still in place and we haven't
rejected this idea completely. So contributions pursuing this idea further would
be welcome.
Copy link
Contributor

Choose a reason for hiding this comment

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

@jlewi I liked the design of the bootstrapper running in the 'kubeflow' namespace with elevated privileges. It did seem to have a limitation of only creating/deploying one kubeflow instance/namespace with no specific rbac mapping to users in that namespace. i wonder if this design might be continued with a local command communicating with the bootstrapper for create, delete, list, add <user/group>, and other subcommands. The bootstrapper would leverage the application as a bundling construct for different types of applications that have different users, cloud-providers and components.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See #1151 The bootstrapper is turning into a wrapper around ksonnet to support click to deploy. And moves it into the direction of being an app manager. (I hope providing a REST server in front of ks will eventually be officially part of ksonnet).

This is needed to support click to deploy which uses a javascript client side web app. So we can't run ks in that case.

We can also avoid providing elevated permissions via a service account and instead use a bearer token containing user credentials in the request. Ideally that credential could be scoped (e.g. in GCP don't want to pass a GCP credential with access to other resources).

I think we should avoid duplicating the functions provided by existing tools e.g. ks and kubectl.

I think the next piece of functionality that will go into it will be monitoring functionality see #1106.


Here are some **guidelines (not requirements)** for creating the above scripts and instructions

* Platform scripts should assume users are starting from scratch
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO platform scripts which create the cluster and its node pools (eg deployment manager) should be separate from scripts which create/deploy a kubeflow instance which should also be separate from scripts which create the user/group/PV/PVCs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See comment below about the scripts being linear. My thinking is that providing a single script optimizes for getting started "one command". If the scripts are linear it should be pretty straightforward.

If we can have a module approach where the scripts are split up into separate scripts that can then be called from a single uber script that might be nice too. That would probably help with code reuse as well. But I also don't want to make our scripts super complicated.

opening more.

The current thinking is to follow the guidance of sig-apps and use an [application resource](https://github.com/kubernetes-sigs/application)
to represent Kubeflow and attach events, status, and other metrics to that application as appropriate.
Copy link
Contributor

Choose a reason for hiding this comment

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

In addition to the application crd noted above - could the SecurityProfile also be leveraged?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Assuming you mean for security purposes (as opposed to monitoring) then I think so. It looks like SecurityProfile is still very early though and not even supported by sig-apps yet.

Copy link
Contributor

Choose a reason for hiding this comment

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

yes. It is early but it seems like it could be combined with application resource - where a UI could allow different components and security profiles to be bundled and deployed.

Is the bootstrapper assuming that only one kubeflow instance would be deployed or many? EG:

  • kubeflow-admin
    above privileged namespace could deploy kubeflow 'instances' like below and have different directories (that could include gitops) for each deployment
  • kubeflow-teamX
  • kubeflow-datascientistY
  • kubeflow-pytorch-gpu-1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

bootstrapper is becoming a server around ksonnet see #1151. So in this case the server can be used with manager different ksonnet applications and not just deploying kubeflow. Its primary function will be as a templating microservice e.g.

paramaters, ksonnetapp -> YAML manifests.

@jlewi
Copy link
Contributor Author

jlewi commented Jul 18, 2018

@kkasravi Would you mind LGTM'ing since you're the only who provide significant feedback? We can continue to discuss/iterate but it would be good to get it checked in.

/assign @kkasravi

@jlewi
Copy link
Contributor Author

jlewi commented Jul 21, 2018

/assign @kunmingg Can you LGTM this? Would be good to get it committed.

@kkasravi
Copy link
Contributor

/lgtm

@jlewi
Copy link
Contributor Author

jlewi commented Jul 22, 2018

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jlewi

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

@pdmack
Copy link
Member

pdmack commented Jul 22, 2018

/retest

1 similar comment
@pdmack
Copy link
Member

pdmack commented Jul 23, 2018

/retest

@jlewi
Copy link
Contributor Author

jlewi commented Jul 23, 2018

Rebase'd to pick up improvements to the tests.

@kkasravi
Copy link
Contributor

/lgtm

@jlewi
Copy link
Contributor Author

jlewi commented Jul 25, 2018

/retest

* This is not a proposal but a description of the current state.
@k8s-ci-robot k8s-ci-robot removed the lgtm label Jul 27, 2018
@lluunn
Copy link
Contributor

lluunn commented Jul 27, 2018

/lgtm

@k8s-ci-robot k8s-ci-robot merged commit 645db22 into kubeflow:master Jul 27, 2018
saffaalvi pushed a commit to StatCan/kubeflow that referenced this pull request Feb 11, 2021
* This is not a proposal but a description of the current state.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants