[OSDOCS#17012]: Bare metal ipi CQA workflow 1 - #117340
Conversation
|
🤖 Wed Aug 05 15:49:02 - Prow CI generated the docs preview: |
178c6d2 to
4f647e9
Compare
4f647e9 to
dba3672
Compare
|
@rh-sgehlot: 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. |
skopacz1
left a comment
There was a problem hiding this comment.
Left some comments, but I think you did a nice job with the work here. Will take another look once feedback is addressed
|
|
||
| toc::[] | ||
| [role="_abstract"] | ||
| Review the different methods for installing {product-title} on bare metal and setup your environment for installation. |
There was a problem hiding this comment.
I don't see anything on this assembly that talks about different install methods (which makes sense - by the time you're on a page in the bare metal IPI install section, you've already decided on this method). I would suggest focusing on the environment setup only:
| Review the different methods for installing {product-title} on bare metal and setup your environment for installation. | |
| Before you can install an {product-title} cluster on bare metal, you must set up your environment for installation. |
(also a nit here that "set up" is two words as a verb, and "setup" is only used as a noun, per the ISG)
| @@ -68,9 +69,9 @@ interfaces: | |||
| # ... | |||
| ---- | |||
| + | |||
| .. For a network using static IP addressing and no DHCP network, create an NMState YAML file and specify the bare-metal bridge interface details in the file: | |||
| .. For a network using static IP addressing and no DHCP network, create an NMState YAML file and specify the bare metal bridge interface details in the file: | |||
| + | |||
| .Example bare-metal bridge interface configuration that uses static IP addressing and no DHCP network | |||
| .Example bare metal bridge interface configuration that uses static IP addressing and no DHCP network | |||
There was a problem hiding this comment.
Just an FYI that if you changed all these "bare-metal" instances to "bare metal" because a bot on this PR commented to do so, I think that bot is incorrect. This is "bare-metal" used as a (compound) adjective, which, when preceding the noun it modifies, should be hyphenated. See the SSG entries on bare metal for more info
So I would revert these changes to say "bare-metal", and if you made similar changes anywhere else I would revert those too (do note - using it as a noun like "install on bare metal" is valid to have as two unhyphenated words)
| # Serve time even if not synchronized to a time source. | ||
| local stratum 3 orphan | ||
| ---- | ||
| + |
There was a problem hiding this comment.
Lol I am confused by how these description lists still indent correctly and don't restart the number order without the first +, but I guess if it works it works
| [id="ipi-install-creating-a-disconnected-registry_{context}"] | ||
| = Creating a disconnected registry | ||
|
|
||
| [] |
| ==== | ||
| Creating a disconnected registry on a registry node is optional. If you need to create a disconnected registry on a registry node, you must complete all of the following sub-sections. | ||
| * Creating a disconnected registry on a registry node is optional. If you need to create a disconnected registry on a registry node, you must complete all of the following sub-sections. | ||
| * If you have already prepared a mirror registry for a disconnected installation by mirroring images, you can skip directly to "Modify the install-config.yaml file to use the disconnected registry" section. For more information about preparing a mirror registry for a disconnected installation by mirroring images see, "Mirroring images for a disconnected installation". |
There was a problem hiding this comment.
Yeah this is a smart way to deal with that "prereqs" section that didn't really seem to be written like a prereqs section
| :_mod-docs-content-type: PROCEDURE | ||
| :_mod-docs-content-type: CONCEPT | ||
| [id="installing-rhel-on-the-provisioner-node_{context}"] | ||
| = Installing {op-system-base} on the provisioner node |
There was a problem hiding this comment.
I think you did good work here and the module sucks to work with. Changing the content type to a CONCEPT is a good idea. I think one thing to be extra safe here is to change this title so it doesn't use a gerund like our procedure titles do:
| = Installing {op-system-base} on the provisioner node | |
| = {op-system-base} installations on the provisioner node |
|
|
||
| Perform the following steps to prepare the environment. | ||
| [role="_abstract"] | ||
| Before you proceed, perform the following steps to prepare the environment. |
There was a problem hiding this comment.
Can't have the abstract be self referential, maybe something like this instead:
| Before you proceed, perform the following steps to prepare the environment. | |
| Begin to set up your environment for cluster installation by preparing the provisioner node. |
| [NOTE] | ||
| ==== | ||
| For more information about Red Hat Subscription Manager, see link:https://docs.redhat.com/en/documentation/subscription_central/1-latest/html/getting_started_with_rhel_system_registration/basic-reg-rhel-cli[Registering a {op-system-base} system with command-line tools]. | ||
| For more information about Red Hat Subscription Manager, see "Registering a {op-system-base} system with command-line tools". |
There was a problem hiding this comment.
This is a procedure module, which can keep its external links like these. I would revert the removal of this link and the one further down in the file
| [source,terminal,subs="attributes+"] | ||
| ---- | ||
| $ export VERSION=stable-{product-version} | ||
| ---- | ||
| [source,terminal,subs="attributes+"] | ||
| ---- | ||
| $ export RELEASE_ARCH=<architecture> | ||
| ---- | ||
| [source,terminal,subs="attributes+"] |
There was a problem hiding this comment.
This procedure sucks to have to work with, and I think you did a decent job. But this is a case where I think we need to carefully format this to be more like a proper procedure instead of making it a concept. So I would revert the content type change above and then modify this part like so:
| .Procedure | |
| * Retrieve the installation program by running one of the following commands: | |
| + | |
| [source,terminal,subs="attributes+"] | |
| ---- | |
| $ export VERSION=stable-{product-version} | |
| ---- | |
| + | |
| [source,terminal,subs="attributes+"] | |
| ---- | |
| $ export RELEASE_ARCH=<architecture> | |
| ---- | |
| + | |
| [source,terminal,subs="attributes+"] |
| // * installing/installing_bare_metal/ipi/ipi-install-installation-workflow.adoc | ||
|
|
||
| :_mod-docs-content-type: PROCEDURE | ||
| :_mod-docs-content-type: CONCEPT |
There was a problem hiding this comment.
Reversion based on my advice below:
| :_mod-docs-content-type: CONCEPT | |
| :_mod-docs-content-type: PROCEDURE |
Version(s):
4.20 +
Issue:
https://redhat.atlassian.net/browse/OSDOCS-17012
Link to docs preview:
https://117340--ocpdocs-pr.netlify.app/openshift-enterprise/latest/installing/installing_bare_metal/ipi/ipi-install-installation-workflow
QE review:
Additional information:
Assemby and modules till "Configuring host network interfaces" are covered here. Second PR - #117420