Skip to content

upi/gcp: initial deployment manager templates #2117

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
Merged
openshift-merge-robot merged 1 commit into from
Aug 9, 2019
Merged

upi/gcp: initial deployment manager templates #2117

openshift-merge-robot merged 1 commit into from
Aug 9, 2019

Conversation

@jstuever
Copy link
Contributor

@jstuever jstuever commented Jul 30, 2019

This change adds the initial upi/gcp templates for ci.

@openshift-ci-robot openshift-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Jul 30, 2019
@wking
Copy link
Member

wking commented Jul 30, 2019 •
edited
Loading

I haven't had time to go through these templates in detail yet, but we'll want something like docs/user/aws for docs/user/gcp and an entry in the README's platform list. I'm fine nibbling away at that over a series of PRs, but this pull request should probably at least get an analog of docs/user/aws/install_upi.md to explain how to wire them together in a UPI install.

wking
wking reviewed Jul 30, 2019
View reviewed changes
wking
wking reviewed Jul 30, 2019
View reviewed changes
@jstuever jstuever changed the title upi/gcp: initial deployment manager templates [WIP] upi/gcp: initial deployment manager templates Jul 31, 2019
@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 31, 2019
@jstuever jstuever force-pushed the cors1167 branch 2 times, most recently from cee696e to 569dc26 Compare July 31, 2019 21:13
@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 Jul 31, 2019
@jstuever jstuever force-pushed the cors1167 branch 2 times, most recently from a71a9de to e86750e Compare July 31, 2019 21:54
@jstuever
Copy link
Contributor Author

jstuever commented Jul 31, 2019

@wking I have added an install_upi.md and made the other requested changes. PTAL.

@jstuever jstuever changed the title [WIP] upi/gcp: initial deployment manager templates upi/gcp: initial deployment manager templates Jul 31, 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 Jul 31, 2019
@jstuever jstuever force-pushed the cors1167 branch 3 times, most recently from 85345bc to 02e34fd Compare August 1, 2019 23:54
derekwaynecarr
derekwaynecarr previously requested changes Aug 2, 2019
View reviewed changes
@jstuever jstuever force-pushed the cors1167 branch 3 times, most recently from c5609f8 to e746404 Compare August 2, 2019 22:11
wking
wking reviewed Aug 2, 2019
View reviewed changes
wking
wking reviewed Aug 2, 2019
View reviewed changes
wking
wking reviewed Aug 2, 2019
View reviewed changes
wking
wking reviewed Aug 2, 2019
View reviewed changes
wking
wking reviewed Aug 3, 2019
View reviewed changes
wking
wking reviewed Aug 3, 2019
View reviewed changes
wking
wking reviewed Aug 8, 2019
View reviewed changes
@wking
Copy link
Member

wking commented Aug 8, 2019

I left a handful of nits inline, but I'm fine landing this as it stands and adding additional polish as we find time. Let me know if you'd rather address them now or punt. I think @csrwng was going to take a look too.

csrwng
csrwng reviewed Aug 8, 2019
View reviewed changes
Copy link
Contributor

@csrwng csrwng left a comment

Choose a reason for hiding this comment

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

This is really cool, and looks like it was a lot of work. A couple of comments
And a really minor nit: capitalize API and URL throughout (but feel free to ignore)

wking
wking reviewed Aug 8, 2019
View reviewed changes
wking
wking reviewed Aug 8, 2019
View reviewed changes
wking
wking reviewed Aug 8, 2019
View reviewed changes
wking
wking reviewed Aug 8, 2019
View reviewed changes
wking
wking reviewed Aug 8, 2019
View reviewed changes
wking
wking reviewed Aug 8, 2019
View reviewed changes
@wking
Copy link
Member

wking commented Aug 8, 2019

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 8, 2019
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Aug 8, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jstuever, wking

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details 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

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 8, 2019
@openshift-bot
Copy link
Contributor

openshift-bot commented Aug 9, 2019

/retest

Please review the full test history for this PR and help us cut down flakes.

1 similar comment
@openshift-bot
Copy link
Contributor

openshift-bot commented Aug 9, 2019

/retest

Please review the full test history for this PR and help us cut down flakes.

@sdodson
Copy link
Member

sdodson commented Aug 9, 2019

/test e2e-aws

@openshift-bot
Copy link
Contributor

openshift-bot commented Aug 9, 2019

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit 5458e78 into openshift:master Aug 9, 2019
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Aug 9, 2019

@jstuever: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
ci/prow/e2e-aws-scaleup-rhel7 cbe6f15 link /test e2e-aws-scaleup-rhel7

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Details

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. I understand the commands that are listed here.

@jstuever jstuever deleted the cors1167 branch August 19, 2019 18:45
@wking wking mentioned this pull request Jul 6, 2020
Merged
wking added a commit to wking/openshift-installer that referenced this pull request Jul 6, 2020
@wking
docs/user/gcp/install_upi: Drop unused 'region' from compute
19c91a2 
The consuming script has not been changed since it landed in
cbe6f15 (upi/gcp: initial deployment manager templates,
2019-07-29, openshift#2117).

The associated Markdown landed with a 'region' in the docs and YAML in
cbe6f15.  It was removed from the YAML in 998a518 (gcp upi:
split templates to simplify shared vpc workflow, 2019-10-25, openshift#2574),
but not the surrounding Markdown (until this commit).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wking wking wking left review comments

@csrwng csrwng csrwng left review comments

@derekwaynecarr derekwaynecarr derekwaynecarr left review comments

@jcpowermac jcpowermac Awaiting requested review from jcpowermac

@vrutkovs vrutkovs Awaiting requested review from vrutkovs

Assignees

@wking wking

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. platform/google size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@jstuever @wking @openshift-ci-robot @openshift-bot @sdodson @csrwng @derekwaynecarr @openshift-merge-robot