Skip to content

TELCODOCS-258: D/S Docs & RN: MPHARDWARE-11 (KNIDEPLOY-4526), Bare Metal IPI Support on IBM Cloud #36529

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
merged 1 commit into from
Oct 8, 2021
Merged

TELCODOCS-258: D/S Docs & RN: MPHARDWARE-11 (KNIDEPLOY-4526), Bare Metal IPI Support on IBM Cloud #36529

merged 1 commit into from
Oct 8, 2021

Conversation

johnwilkins
Copy link
Contributor

@johnwilkins johnwilkins commented Sep 17, 2021 •
edited
Loading

Provides documentation for deploying an installer-provisioned cluster on IBM Cloud.

Fixes: TELCODOCS-258

See https://issues.redhat.com/browse/TELCODOCS-258 for additional details.

Preview URL: https://deploy-preview-36529--osdocs.netlify.app/openshift-enterprise/latest/installing/installing_ibm_cloud/install-ibm-cloud-prerequisites.html

For release(s): 4.9
Signed-off-by: John Wilkins [email protected]

@openshift-ci openshift-ci bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Sep 17, 2021
@johnwilkins johnwilkins changed the title TELCODOCS-258: D/S Docs & RN: MPHARDWARE-11 (KNIDEPLOY-4526), Bare Metal IPI Support on IBM Cloud WIP: TELCODOCS-258: D/S Docs & RN: MPHARDWARE-11 (KNIDEPLOY-4526), Bare Metal IPI Support on IBM Cloud Sep 17, 2021
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 17, 2021
@johnwilkins johnwilkins changed the title WIP: TELCODOCS-258: D/S Docs & RN: MPHARDWARE-11 (KNIDEPLOY-4526), Bare Metal IPI Support on IBM Cloud TELCODOCS-258: D/S Docs & RN: MPHARDWARE-11 (KNIDEPLOY-4526), Bare Metal IPI Support on IBM Cloud Sep 17, 2021
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 17, 2021
@netlify
Copy link

netlify bot commented Sep 17, 2021

✔️ Deploy Preview for osdocs ready!

🔨 Explore the source changes: 40eca033f01f3d0c3d93a421c46a4dd483d0cbd7

🔍 Inspect the deploy log: https://app.netlify.com/sites/osdocs/deploys/61450646138cfa0007d01709

😎 Browse the preview: https://deploy-preview-36529--osdocs.netlify.app

@netlify
Copy link

netlify bot commented Sep 17, 2021 •
edited
Loading

✔️ Deploy Preview for osdocs ready!

🔨 Explore the source changes: b91399d

🔍 Inspect the deploy log: https://app.netlify.com/sites/osdocs/deploys/6160874cc7f9350007dcfbcd

😎 Browse the preview: https://deploy-preview-36529--osdocs.netlify.app/openshift-enterprise/latest/installing/installing_ibm_cloud/install-ibm-cloud-prerequisites

derekhiggins
derekhiggins reviewed Sep 20, 2021
View reviewed changes
Copy link

@derekhiggins derekhiggins left a comment

Choose a reason for hiding this comment

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

Some comments added, I'll run through the instruction on a new environment to ensure it all works

derekhiggins
derekhiggins reviewed Sep 21, 2021
View reviewed changes
Copy link

@derekhiggins derekhiggins left a comment

Choose a reason for hiding this comment

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

Notes added inline
I think all of the content around creating the kni user and configuring sudo and ssh keys is duplicated, we probably don't need this in here

Also we should explicitly mention somewhere that only ipmi/pxe is supported in 4.9 (i.e. redfish support has not been added/tested)

@johnwilkins
Copy link
Contributor Author

Notes added inline
I think all of the content around creating the kni user and configuring sudo and ssh keys is duplicated, we probably don't need this in here

Also we should explicitly mention somewhere that only ipmi/pxe is supported in 4.9 (i.e. redfish support has not been added/tested)

Per the release notes, I'm replacing the entire procedure for the IBM steps. I'm inclined to completely review the baremetal steps too in case we no longer need the pool steps. There are a few comments in that procedure from Dieter and others that I'm resolving as well.

I will make a few additional comments to indicate that we are completely substituting a procedure.

@johnwilkins
Copy link
Contributor Author

@derekhiggins I've made the noted changes. I've also added in statements and links to the main IPI docs. I still think this could use a summary telling them what to do next, and what's NOT supported in the main IPI docs--e.g., disconnected installation.

derekhiggins
derekhiggins reviewed Sep 22, 2021
View reviewed changes
Copy link

@derekhiggins derekhiggins left a comment

Choose a reason for hiding this comment

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

Per the release notes, I'm replacing the entire procedure for the IBM steps. I'm inclined to completely review the baremetal steps too in case we no longer need the pool steps. There are a few comments in that procedure from Dieter and others that I'm resolving as well.

I'm a bit confused the Release notes you sent me says "The documentation provides complementary sections identifying the differences between installing on bare metal and installing on IBM Cloud."

So I'd be expecting that we only need to document the differences.

@derekhiggins
Copy link

@derekhiggins I've made the noted changes. I've also added in statements and links to the main IPI docs. I still think this could use a summary telling them what to do next, and what's NOT supported in the main IPI docs--e.g., disconnected installation.

thanks, looks pretty close to be, the only thing I know of that not supported is redfish and virtual media (we've only tested ipmi/pxe)

kquinn1204
kquinn1204 reviewed Sep 22, 2021
View reviewed changes
lshilin-r
lshilin-r reviewed Oct 4, 2021
View reviewed changes
lshilin-r
lshilin-r reviewed Oct 4, 2021
View reviewed changes
lshilin-r
lshilin-r reviewed Oct 4, 2021
View reviewed changes
lshilin-r
lshilin-r reviewed Oct 5, 2021
View reviewed changes
lshilin-r
lshilin-r reviewed Oct 5, 2021
View reviewed changes
@openshift-ci openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 7, 2021
@openshift-ci openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 7, 2021
derekhiggins
derekhiggins reviewed Oct 7, 2021
View reviewed changes
@johnwilkins johnwilkins force-pushed the TELCODOCS-258 branch 2 times, most recently from 8e27ea2 to 1be53dd Compare October 7, 2021 15:42
@bergerhoffer bergerhoffer added the peer-review-needed Signifies that the peer review team needs to review this PR label Oct 7, 2021
@bergerhoffer bergerhoffer added this to the Future Release milestone Oct 7, 2021
bergerhoffer
bergerhoffer reviewed Oct 7, 2021
View reviewed changes
@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 7, 2021
lshilin-r
lshilin-r reviewed Oct 7, 2021
View reviewed changes
lshilin-r
lshilin-r reviewed Oct 7, 2021
View reviewed changes
lshilin-r
lshilin-r reviewed Oct 7, 2021
View reviewed changes
@lshilin-r
Copy link

Please, correct as we discussed and lgtm

@derekhiggins
Copy link

lgtm, also confirmed the comments @lshilin-r mentioned have been addressed

bergerhoffer
bergerhoffer reviewed Oct 8, 2021
View reviewed changes
Copy link
Contributor

@bergerhoffer bergerhoffer left a comment

Choose a reason for hiding this comment

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

@johnwilkins thanks for the updates! LGTM. I pointed out just one thing, but it's not enough to hold the PR over.

However there are 2 commits in the PR now, so we can't merge until that is squashed. I'm out tomorrow, but if you ping me on slack once you squash I can try to hop on and merge it for ya!

@bergerhoffer
Copy link
Contributor

Thanks for the squash, merging!

@bergerhoffer bergerhoffer merged commit c29a389 into openshift:main Oct 8, 2021
@bergerhoffer
Copy link
Contributor

/cherrypick enterprise-4.9

@openshift-cherrypick-robot
Copy link

@bergerhoffer: new pull request created: #37262

In response to this:

/cherrypick enterprise-4.9

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.

@openshift-cherrypick-robot openshift-cherrypick-robot mentioned this pull request Oct 8, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bergerhoffer bergerhoffer bergerhoffer left review comments

+3 more reviewers

@derekhiggins derekhiggins derekhiggins left review comments

@lshilin-r lshilin-r lshilin-r left review comments

@kquinn1204 kquinn1204 kquinn1204 left review comments

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
branch/enterprise-4.9 peer-review-done Signifies that the peer review team has reviewed this PR size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Milestone
OCP 4.9 GA
Development

Successfully merging this pull request may close these issues.
7 participants
@johnwilkins @derekhiggins @lshilin-r @bergerhoffer @openshift-cherrypick-robot @kquinn1204 @vikram-redhat