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

OSDOCS-4341 GCP: Authenticate using Service Account on a GCP VM #52170

Merged
merged 1 commit into from Nov 28, 2022
Merged

OSDOCS-4341 GCP: Authenticate using Service Account on a GCP VM #52170

merged 1 commit into from Nov 28, 2022

Conversation

bscott-rh
Copy link
Contributor

@bscott-rh bscott-rh commented Oct 25, 2022 •
edited

@openshift-ci openshift-ci bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Oct 25, 2022
@ocpdocs-previewbot
Copy link

ocpdocs-previewbot commented Oct 25, 2022 •
edited

🤖 Updated build preview is available at:
https://52170--docspreview.netlify.app

Build log: https://circleci.com/gh/ocpdocs-previewbot/openshift-docs/4184

@bscott-rh
Copy link
Contributor Author

@patrickdillon PTAL at this PR for CORS-2260. Thank you

@patrickdillon
Copy link
Contributor

LGTM

@bscott-rh
Copy link
Contributor Author

@jianli-wei PTAL at this PR for VM service account authentication for GCP installs. Thank you

@bscott-rh
Copy link
Contributor Author

@jianli-wei hello Jianli, have you had a chance to take a look at this PR? Thank you

@jianli-wei
Copy link

@bscott-rh @patrickdillon When I was testing the epic, it demands credentialsMode: Manual (see QE Tracker). It seems the doc doesn't mention it, any suggestion? Thanks!

@patrickdillon
Copy link
Contributor

Manual credential mode is indeed required.

@bscott-rh
Copy link
Contributor Author

Thanks @jianli-wei and Patrick. I've updated the PR with a note about manual credentials mode. PTAL

jianli-wei
jianli-wei suggested changes Nov 11, 2022
View reviewed changes
modules/installation-gcp-service-account.adoc
The service account key is required to create a cluster.
[NOTE]
====
If you use a virtual machine with an attached service account to create your cluster, you must set `credentialsMode: Manual` in the `install-config.yaml` file before installation.
Copy link

Choose a reason for hiding this comment

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

Better to tell what's needed when using credentialsMode: Manual, e.g. https://docs.openshift.com/container-platform/4.11/installing/installing_gcp/manually-creating-iam-gcp.html. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @jianli-wei , I have added a reference to the "Manually create IAM" page immediately after this note. I would prefer not to detail the steps of manual IAM when we have an entire page dedicated to that. Does this look good to you?

Copy link

Choose a reason for hiding this comment

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

/lgtm

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 22, 2022
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 22, 2022
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 23, 2022
@bscott-rh
Copy link
Contributor Author

/label peer-review-needed

@openshift-ci openshift-ci bot added the peer-review-needed Signifies that the peer review team needs to review this PR label Nov 23, 2022
@opayne1 opayne1 added peer-review-in-progress Signifies that the peer review team is reviewing this PR and removed peer-review-needed Signifies that the peer review team needs to review this PR labels Nov 23, 2022
opayne1
opayne1 reviewed Nov 23, 2022
View reviewed changes
Copy link
Contributor

@opayne1 opayne1 left a comment

Choose a reason for hiding this comment

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

Just two small sentence suggestions you can take or leave. :)

modules/installation-gcp-service-account.adoc Outdated Show resolved Hide resolved
modules/installation-gcp-service-account.adoc Outdated Show resolved Hide resolved
@opayne1 opayne1 added peer-review-done Signifies that the peer review team has reviewed this PR branch/enterprise-4.12 and removed peer-review-in-progress Signifies that the peer review team is reviewing this PR labels Nov 23, 2022
@opayne1 opayne1 added this to the Planned for 4.12 GA milestone Nov 23, 2022
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Nov 28, 2022
@openshift-ci
Copy link

openshift-ci bot commented Nov 28, 2022

New changes are detected. LGTM label has been removed.

@bscott-rh
Copy link
Contributor Author

/label merge-review-needed

@openshift-ci openshift-ci bot added the merge-review-needed Signifies that the merge review team needs to review this PR label Nov 28, 2022
@jldohmann jldohmann added the merge-review-in-progress Signifies that the merge review team is reviewing this PR label Nov 28, 2022
jldohmann
jldohmann approved these changes Nov 28, 2022
View reviewed changes
Copy link
Contributor

@jldohmann jldohmann left a comment

Choose a reason for hiding this comment

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

LGTM

@jldohmann jldohmann removed merge-review-in-progress Signifies that the merge review team is reviewing this PR merge-review-needed Signifies that the merge review team needs to review this PR labels Nov 28, 2022
@jldohmann jldohmann merged commit ddf5f9c into openshift:main Nov 28, 2022
@jldohmann
Copy link
Contributor

/cherrypick enterprise-4.12

2 similar comments
@jldohmann
Copy link
Contributor

/cherrypick enterprise-4.12

@jldohmann
Copy link
Contributor

/cherrypick enterprise-4.12

@openshift-cherrypick-robot openshift-cherrypick-robot mentioned this pull request Nov 29, 2022
Merged
@openshift-cherrypick-robot
Copy link

@jldohmann: new pull request created: #53292

In response to this:

/cherrypick enterprise-4.12

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.

@bscott-rh bscott-rh deleted the OSDOCS-4341 branch October 30, 2023 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@opayne1 opayne1 opayne1 left review comments

@jianli-wei jianli-wei jianli-wei requested changes

@jldohmann jldohmann jldohmann approved these changes

Assignees

@jianli-wei jianli-wei

Labels
branch/enterprise-4.12 peer-review-done Signifies that the peer review team has reviewed this PR size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
OCP 4.12 GA
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@bscott-rh @ocpdocs-previewbot @patrickdillon @jianli-wei @jldohmann @openshift-cherrypick-robot @opayne1 @openshift-merge-robot