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.

Sign up for GitHub

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

Jump to bottom

(gke) Full Kubeflow Deployment guide on GKE (revision for Kubeflow 1.3) #2686

Merged
merged 25 commits into from
May 17, 2021
Merged

(gke) Full Kubeflow Deployment guide on GKE (revision for Kubeflow 1.3) #2686

merged 25 commits into from
May 17, 2021

Conversation

zijianjoy
Copy link
Contributor

@zijianjoy zijianjoy commented May 1, 2021 •
edited
Loading

Partial #2177
Partial kubeflow/manifests#1798

This is for preparing Kubeflow 1.3 release on GCP:

Updated Deployment sections:

  1. Overview
  2. Set up project
  3. OAuth
  4. Management Cluster
  5. Kubeflow Cluster
  6. Upgrade Kubeflow
  7. Delete Kubeflow
  8. Monitor IAP setup

Deleted deploy using UI and reasons sections since they are obsolete or merged to remaining sections.

@google-cla google-cla bot added the cla: yes label May 1, 2021
@zijianjoy zijianjoy changed the title WIP: (gke) Full Kubeflow Deployment on GKE (gke) Full Kubeflow Deployment guide on GKE (revision for Kubeflow 1.3) May 3, 2021
@zijianjoy zijianjoy requested a review from Bobgy May 4, 2021 17:14
rui-vas
rui-vas reviewed May 5, 2021
View reviewed changes
Copy link
Contributor

@rui-vas rui-vas left a comment

Choose a reason for hiding this comment

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

Thank you for this @zijianjoy !

/assign @Bobgy

Bobgy
Bobgy reviewed May 6, 2021
View reviewed changes
Copy link
Contributor

@Bobgy Bobgy left a comment

Choose a reason for hiding this comment

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

Great work @zijianjoy!
I finished a quick pass of reviews, please take a look at the comments -- mostly nitpickings

content/en/docs/distributions/gke/_index.md Outdated Show resolved Hide resolved
content/en/docs/distributions/gke/deploy/_index.md Outdated Show resolved Hide resolved
content/en/docs/distributions/gke/deploy/deploy-cli.md Outdated Show resolved Hide resolved
content/en/docs/distributions/gke/deploy/deploy-cli.md Show resolved Hide resolved
content/en/docs/distributions/gke/deploy/deploy-cli.md Outdated Show resolved Hide resolved
content/en/docs/distributions/gke/deploy/overview.md Outdated Show resolved Hide resolved
content/en/docs/distributions/gke/deploy/upgrade.md Outdated Show resolved Hide resolved
content/en/docs/distributions/gke/deploy/upgrade.md Show resolved Hide resolved
content/en/docs/distributions/gke/deploy/upgrade.md Outdated Show resolved Hide resolved
content/en/docs/distributions/gke/deploy/deploy-cli.md Outdated Show resolved Hide resolved
@zijianjoy zijianjoy requested a review from Bobgy May 12, 2021 18:21
Bobgy
Bobgy reviewed May 13, 2021
View reviewed changes
Copy link
Contributor

@Bobgy Bobgy left a comment

Choose a reason for hiding this comment

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

overall LGTM

content/en/docs/distributions/gke/deploy/deploy-cli.md Outdated Show resolved Hide resolved
content/en/docs/distributions/gke/deploy/deploy-cli.md

#### kpt setter config
Copy link
Contributor

Choose a reason for hiding this comment

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

no longer need to document this section, but going forward, I'd suggest not using env vars and stick to kpt setters as the only interface.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good, removing this. I agree not to use env vars but directly use kpt setters. In that case, we probably need to use other ways than environment variables to describe debugging and deployment steps in our documentation. Currently we rely on env var for documentation and kpt setter purpose.

@Bobgy
Copy link
Contributor

Bobgy commented May 13, 2021

Some instructions need an update after GoogleCloudPlatform/kubeflow-distribution#272 is merged

@zijianjoy
Copy link
Contributor Author

Some instructions need an update after kubeflow/gcp-blueprints#272 is merged

Thank you Yuan! Updated documentation accordingly.

@Bobgy
Copy link
Contributor

Bobgy commented May 14, 2021 •
edited
Loading

/lgtm
/approve
/hold
you can decide when to unhold (I guess after pinning gcp-blueprints to 1.3)

@google-oss-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Bobgy, zijianjoy

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

@Bobgy
Copy link
Contributor

Bobgy commented May 17, 2021

/lgtm
Great work!

@google-oss-robot
Copy link

New changes are detected. LGTM label has been removed.

@zijianjoy
Copy link
Contributor Author

Thank you Yuan for reviewing and approving! @Bobgy I will unhold and merge this change in 1 hour.

@zijianjoy
Copy link
Contributor Author

Kubeflow on GCP v1.3.0 has been cut: https://github.com/kubeflow/gcp-blueprints/releases/tag/v1.3.0

@zijianjoy zijianjoy added the lgtm label May 17, 2021
@zijianjoy
Copy link
Contributor Author

/unhold

Thank you again for the throughout document review! Releasing documentation update along with release cut.

@google-oss-robot google-oss-robot merged commit f50693a into kubeflow:master May 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rui-vas rui-vas rui-vas left review comments

@Bobgy Bobgy Bobgy left review comments

@dsdinter dsdinter Awaiting requested review from dsdinter

@joeliedtke joeliedtke Awaiting requested review from joeliedtke

Assignees

@Bobgy Bobgy

Labels
approved cla: yes lgtm size/XL
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@zijianjoy @Bobgy @google-oss-robot @rui-vas