OSDOCS-17898# Add bare metal nodes on vSphere documentation. - #105101
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@sslocket: This pull request references OSDOCS-17898 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target either version "4.22." or "openshift-4.22.", but it targets "openshift-4.21" instead. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci
Bot
added
the
size/L
3dd481a to
503a2d9
Compare
sslocket
changed the title
|
@sslocket: No Jira issue is referenced in the title of this pull request. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci-robot
removed
the
jira/valid-reference
731d5d0 to
f02d9da
Compare
|
|
||
| . Boot the bare-metal machine using the {op-system} ISO image. | ||
|
|
||
| . From the installation console, run the `coreos-installer` command. You must specify the `--platform=metal` argument to ensure the machine is correctly identified as a physical host and ignored by the vSphere cloud components. |
There was a problem hiding this comment.
I did platform=none. Not sure if metal is the preferred, but just want us to make sure this is correct.
There was a problem hiding this comment.
Committed suggestion.
2be4181 to
40cc0b5
Compare
|
@WenXinWei PTAL |
|
/lgtm |
40cc0b5 to
7f74522
Compare
|
New changes are detected. LGTM label has been removed. |
7f74522 to
6f2f834
Compare
|
/label merge-review-needed |
openshift-ci
Bot
added
the
merge-review-needed
|
/label merge-review-in-progress |
openshift-ci
Bot
added
the
merge-review-in-progress
bergerhoffer
left a comment
bergerhoffer
left a comment
There was a problem hiding this comment.
I added a few bits of feedback that aren't normally worried about during merge review, but I think are good for you to know.
Most of them I think are fine to consider following up on in a future PR. The one I think is actually most critical is that the prerequisites section should be in its own separate module.
But I do realize we're up against merge freeze tomorrow, so I'm going to ping you on slack and see what you want to do about if you can fix at least that now and fix others later or what you think.
| :FeatureName: Bare-metal nodes on vSphere clusters | ||
| include::snippets/technology-preview.adoc[] | ||
|
|
||
| [IMPORTANT] |
There was a problem hiding this comment.
Not critical to fix now, but in the future you should try to avoid back-to-back admonitions like this, per the SSG: https://redhat-documentation.github.io/supplementary-style-guide/#admonitions
| Bare-metal compute machines added to a {vmw-short} cluster are unmanaged by the Machine API. You cannot use compute machine sets or the cluster autoscaler to manage these compute machines. Lifecycle tasks such as provisioning and replacement must be performed manually. | ||
| ==== | ||
|
|
||
| == Prerequisites |
There was a problem hiding this comment.
This needs an ID, but really it should be modularized into its own module. Subsections shouldn't be directly in the assembly (per DITA migration prep instructions)
| * You have configured the network for the new bare-metal compute machines, including: | ||
| ** DHCP: Persistent IP addresses and hostname reservations. | ||
| ** DNS: Forward and reverse DNS resolution for the new hostnames. | ||
| * You have obtained the {op-system-first} ISO image that matches your cluster version. You can download this from the Cluster Details page on the {hybrid-console} or extract it from the cluster payload. |
There was a problem hiding this comment.
If it's a GUI item, be sure to bold it
| * You have obtained the {op-system-first} ISO image that matches your cluster version. You can download this from the Cluster Details page on the {hybrid-console} or extract it from the cluster payload. | |
| * You have obtained the {op-system-first} ISO image that matches your cluster version. You can download this from the *Cluster Details* page on the {hybrid-console} or extract it from the cluster payload. |
|
|
||
| .Verification | ||
|
|
||
| . Verify that the new compute machine has joined the cluster and is in the `Ready` state: |
There was a problem hiding this comment.
Since this is a single step, it should be an unnumbered bullet
| . Verify that the new compute machine has joined the cluster and is in the `Ready` state: | |
| * Verify that the new compute machine has joined the cluster and is in the `Ready` state: |
| `/dev/sda`:: Specifies the target install device for your hardware. | ||
| `<http_server>`:: Specifies the address of your web server. | ||
|
|
||
| . Reboot the machine: |
There was a problem hiding this comment.
I'd consider being consistent in how you introduce commands to be run. You do a few different ways throughout these steps - consider picking one and being consistent.
We have guidelines on best ones to use here: https://github.com/openshift/openshift-docs/blob/main/contributing_to_docs/doc_guidelines.adoc#procedure-step-that-introduces-a-command
| + | ||
| [source,terminal,subs="+quotes"] | ||
| ---- | ||
| $ curl -I http://_<http_server>_/worker.ign |
There was a problem hiding this comment.
Replaceable values shouldn't actually be italicized for us. OCP does has an exception noted here that we don't follow the SSG's rule about italicizing: https://github.com/openshift/openshift-docs/blob/main/contributing_to_docs/doc_guidelines.adoc#command-syntax-for-replaceable-values
In core OpenShift Container Platform documentation, do not italicize user-replaced values. This guideline is an exception to the Red Hat supplementary style guide for product documentation.
|
|
||
| . Boot the bare-metal machine using the {op-system} ISO image. | ||
|
|
||
| . From the installation console, run the `coreos-installer` command. |
There was a problem hiding this comment.
| . From the installation console, run the `coreos-installer` command. | |
| . From the installation console, run the `coreos-installer` command: |
| [source,terminal,subs="+quotes"] | ||
| ---- | ||
| $ sudo coreos-installer install /dev/sda \ | ||
| --ignition-url=http://_<http_server>_/worker.ign \ |
There was a problem hiding this comment.
Same about not italicizing the replacable value
| $ reboot | ||
| ---- | ||
|
|
||
| . Monitor the boot process. After the machine reboots, it attempts to join the cluster and generates Certificate Signing Requests (CSRs). |
There was a problem hiding this comment.
These don't need to be uppercase
| . Monitor the boot process. After the machine reboots, it attempts to join the cluster and generates Certificate Signing Requests (CSRs). | |
| . Monitor the boot process. After the machine reboots, it attempts to join the cluster and generates certificate signing requests (CSRs). |
bergerhoffer
removed
merge-review-in-progress
6f2f834 to
0e67fec
Compare
|
@sslocket: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions 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-sigs/prow repository. I understand the commands that are listed here. |
bergerhoffer
added
the
merge-review-in-progress
bergerhoffer
left a comment
bergerhoffer
left a comment
There was a problem hiding this comment.
LGTM to merge. Per slack convo - moving the prereqs to a module will be completed in a follow-up PR (jira to come to track it) since freeze is tomorrow.
|
/cherrypick enterprise-4.21 |
|
@bergerhoffer: new pull request created: #105689 DetailsIn response to this:
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-sigs/prow repository. |
@bergerhoffer following up on this: the requested prereq reformat is in the GA release documentation PR. #111626 |
11 participants
Version(s):
4.21+
Issue:
https://issues.redhat.com/browse/OSDOCS-17898
Link to docs preview:
QE review:
Additional information:
RN: #105106
Storage content PR: #104754