Skip to content

Add GCP UPI install docs#17043

Merged
adellape merged 1 commit intoopenshift:masterfrom
adellape:gcp_upi
Oct 15, 2019
Merged

Add GCP UPI install docs#17043
adellape merged 1 commit intoopenshift:masterfrom
adellape:gcp_upi

Conversation

@adellape
Copy link
Copy Markdown
Contributor

@adellape adellape commented Oct 3, 2019

@adellape adellape added this to the Future Release milestone Oct 3, 2019
@openshift-ci-robot openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Oct 3, 2019
@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 4, 2019
@adellape adellape force-pushed the gcp_upi branch 2 times, most recently from 9c4e8a2 to 226d265 Compare October 4, 2019 20:16
@openshift-ci-robot openshift-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Oct 4, 2019
@adellape adellape changed the title [WIP] Add GCP UPI install docs Add GCP UPI install docs Oct 7, 2019
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 7, 2019
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 7, 2019
@adellape adellape force-pushed the gcp_upi branch 5 times, most recently from e1401f0 to 61a6e49 Compare October 7, 2019 15:41
@adellape
Copy link
Copy Markdown
Contributor Author

adellape commented Oct 7, 2019

@kalexand-rh PTAL for peer review.
@jstuever @wking PTAL for tech review.
@wmengRH PTAL for QE review.

Thanks! 🙇‍♂️

@wmengRH
Copy link
Copy Markdown

wmengRH commented Oct 7, 2019

@jiajliu help for UPI on GCP doc review, Thanks

Comment thread modules/installation-aws-user-infra-installation.adoc Outdated
Comment thread modules/installation-creating-gcp-bootstrap.adoc Outdated
Comment thread modules/installation-creating-gcp-bootstrap.adoc Outdated
Comment thread modules/installation-extracting-infraid.adoc Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

With these additions, modules/installation-extracting-infraid.adoc and the Extracting the infrastructure name subject are no longer such a good match. Maybe broaden the scope to... Basic installation configuration or some such? Or punt the variables other than INFRA_ID off to a different location.

Copy link
Copy Markdown
Contributor Author

@adellape adellape Oct 11, 2019

Choose a reason for hiding this comment

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

Created a generic modules/installating-user-infra-exporting-common-variables.adoc and couched them both in a new "Exporting common variables" section:

http://file.rdu.redhat.com/~adellape/100419/gcp_upi/installing/installing_gcp_user_infra/installing-gcp-user-infra.html#installation-gcp-user-infra-exporting-common-variables

cc @kalexand-rh

@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 14, 2019
Copy link
Copy Markdown
Contributor

@kalexand-rh kalexand-rh left a comment

Choose a reason for hiding this comment

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

I have a couple of suggestions and notes about places where we will need to untangle conflicting changes or further optimize reuse.

Comment thread installing/installing_gcp_user_infra/installing-gcp-user-infra.adoc Outdated
Comment thread installing/installing_gcp_user_infra/installing-gcp-user-infra.adoc Outdated
Comment thread modules/installation-creating-gcp-bootstrap.adoc Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Are you documenting how to enable the machine API for GCP UPI clusters?

Comment thread modules/installation-creating-gcp-worker.adoc Outdated
Comment thread modules/installation-gcp-user-infra-adding-ingress.adoc Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Comment thread modules/installation-gcp-user-infra-completing.adoc Outdated
Comment thread modules/installation-initializing.adoc Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@wking, we don't have this option for AWS UPI. Do we want to add it there or remove it from GCP?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this really optional?
We'll need to untangle the conflict with https://github.com/openshift/openshift-docs/pull/17016/files

@jstuever
Copy link
Copy Markdown

We use the wording The URI to the <object> a few times... we might want to modify to indicate that these are self-links. See: https://www.terraform.io/docs/providers/google/r/compute_firewall.html#self_link

@bergerhoffer bergerhoffer added peer-review-done Signifies that the peer review team has reviewed this PR and removed peer-review-needed Signifies that the peer review team needs to review this PR labels Oct 14, 2019
@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 15, 2019
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 15, 2019
@adellape
Copy link
Copy Markdown
Contributor Author

@jiajliu @kalexand-rh @jstuever @wking Thanks, updated per feedback and latest discussion. Will continue looking into the few suggestions still outstanding as a follow-up.

@adellape adellape merged commit b4d14e5 into openshift:master Oct 15, 2019
@adellape
Copy link
Copy Markdown
Contributor Author

/cherrypick enterprise-4.2

@openshift-cherrypick-robot
Copy link
Copy Markdown

@adellape: new pull request created: #17336

Details

In response to this:

/cherrypick enterprise-4.2

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

wking added a commit to wking/openshift-docs that referenced this pull request Jul 16, 2020
The script consuming the YAML landed without 'region' consumers in
bd08aca (Add GCP UPI install docs, 2019-10-03, openshift#17043), and still
has no consumers for that property.
openshift-cherrypick-robot pushed a commit to openshift-cherrypick-robot/openshift-docs that referenced this pull request Jul 27, 2020
The script consuming the YAML landed without 'region' consumers in
bd08aca (Add GCP UPI install docs, 2019-10-03, openshift#17043), and still
has no consumers for that property.
openshift-cherrypick-robot pushed a commit to openshift-cherrypick-robot/openshift-docs that referenced this pull request Jul 27, 2020
The script consuming the YAML landed without 'region' consumers in
bd08aca (Add GCP UPI install docs, 2019-10-03, openshift#17043), and still
has no consumers for that property.
kalexand-rh pushed a commit to kalexand-rh/openshift-docs that referenced this pull request Jul 27, 2020
The script consuming the YAML landed without 'region' consumers in
bd08aca (Add GCP UPI install docs, 2019-10-03, openshift#17043), and still
has no consumers for that property.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

branch/enterprise-4.2 peer-review-done Signifies that the peer review team has reviewed this PR size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.