Skip to content

OCP 4.13 - IBM Power Virtual Server using installer-provisioned infrastructure#47558

Merged
mburke5678 merged 1 commit intoopenshift:mainfrom
alishaIBM:ipi_deploy
May 15, 2023
Merged

OCP 4.13 - IBM Power Virtual Server using installer-provisioned infrastructure#47558
mburke5678 merged 1 commit intoopenshift:mainfrom
alishaIBM:ipi_deploy

Conversation

@alishaIBM
Copy link
Copy Markdown
Contributor

@alishaIBM alishaIBM commented Jul 7, 2022

@alishaIBM alishaIBM changed the title OCP 4.11 - IBM Power VS using installer-provisioned infrastructure [WIP] OCP 4.11 - IBM Power VS using installer-provisioned infrastructure Jul 7, 2022
@openshift-ci openshift-ci Bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 7, 2022
@alishaIBM alishaIBM force-pushed the ipi_deploy branch 10 times, most recently from 9d486d5 to 4e1daae Compare July 8, 2022 16:33
@openshift-ci openshift-ci Bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 8, 2022
@alishaIBM alishaIBM force-pushed the ipi_deploy branch 7 times, most recently from 4b5411f to e7007bd Compare July 11, 2022 19:18
Copy link
Copy Markdown
Contributor

@mjturek mjturek left a comment

Choose a reason for hiding this comment

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

Looking good but some questions and comments

Comment thread modules/installation-uninstall-clouds.adoc Outdated
Comment thread modules/installation-ibm-power-vs-config-yaml.adoc Outdated
Comment thread modules/installation-ibm-power-vs-config-yaml.adoc Outdated
@alishaIBM alishaIBM force-pushed the ipi_deploy branch 3 times, most recently from a84ab07 to 156d30d Compare July 12, 2022 10:46
Comment thread modules/installation-configuration-parameters.adoc Outdated
Comment thread modules/installation-configuration-parameters.adoc Outdated
@mjturek
Copy link
Copy Markdown
Contributor

mjturek commented Apr 30, 2023

/lgtm

1 similar comment
@mjturek
Copy link
Copy Markdown
Contributor

mjturek commented May 2, 2023

/lgtm

@openshift-ci
Copy link
Copy Markdown

openshift-ci Bot commented May 3, 2023

New changes are detected. LGTM label has been removed.

@alishaIBM
Copy link
Copy Markdown
Contributor Author

/label peer-review-needed

Comment thread _topic_maps/_topic_map.yml Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This suggestion was not made.

- Name: Installing a cluster on IBM Power Virtual Server with customizations
File: installing-ibm-power-vs-customizations
- Name: Installing a cluster on IBM Power Virtual Server into an existing VPC
File: installing-ibm-powervs-vpc
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

all other file names are written as "ibm-power-vs." This specific one is written as "ibm-powervs-vpc." I don't think there's an issue with this, but I am bring it to your attention.

@@ -0,0 +1,25 @@
:_content-type: PROCEDURE
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Everything that’s called out in the topic map must be an assembly. All modules must be in the modules directory. Items in the modules directory can’t be called out in the topic map.

SO, this particular file needs split into two:

  1. An assembly that is included in the topic_map.yaml file
  2. A procedural document that is included in the assembly.

The following metadata is included in your assembly: https://github.com/openshift/openshift-docs/blob/main/contributing_to_docs/doc_guidelines.adoc#assembly-file-metadata

The following metadata metadata is included in your module: https://github.com/openshift/openshift-docs/blob/main/contributing_to_docs/doc_guidelines.adoc#module-file-metadata

You have combined the two here, and it must be reworked.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@stevsmit thanks !

As explained above I have split it into two parts :

  1. creating-ibm-power-vs-workspace.adoc ( an assembly ).
  2. modules/creating-ibm-power-vs-workspace-procedure.adoc ( a module ).

Request you to please review. Thanks !

Comment thread installing/installing_ibm_power/creating-ibm-power-vs-workspace.adoc Outdated
Comment thread installing/installing_ibm_power/creating-ibm-power-vs-workspace.adoc Outdated
Comment thread modules/installation-ibm-power-vs-config-yaml.adoc Outdated
Comment thread modules/installation-ibm-power-vs-config-yaml.adoc Outdated
Comment thread modules/installation-ibm-power-vs-config-yaml.adoc Outdated
Comment thread modules/installation-ibm-power-vs-config-yaml.adoc Outdated
Comment thread modules/installation-ibm-power-vs-config-yaml.adoc Outdated
Copy link
Copy Markdown

@clnperez clnperez left a comment

Choose a reason for hiding this comment

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

change to control plane instead of master

Comment thread modules/quotas-and-limits-ibm-power-vs.adoc Outdated
Comment thread modules/machineset-yaml-ibm-power-vs.adoc
|VPC Infrastructure Services service in <resource_group> resource group

|Viewer
|<resource_group> resource group: resourceType string equals resource-group, resource string equals <resource_group>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
|<resource_group> resource group: resourceType string equals resource-group, resource string equals <resource_group>
|<resource_group> resource group: `resourceType` string equals resource group. Resource string equals <resource_group>

^ I think this particular description is confusing to understand, but it might be my lack of knowledge on the topic. Would we also but "resource string" in back ticks? eg, should it be "resourceString" ?

:context: creating-ibm-power-vs-workspace

:FeatureName: {ibmpowerProductName} Virtual Server using installer-provisioned infrastructure
include::snippets/technology-preview.adoc[]
Copy link
Copy Markdown
Member

@stevsmit stevsmit May 11, 2023

Choose a reason for hiding this comment

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

The answer to this question is "No," you would not remove the following information:

:content-type: PROCEDURE
[id="creating-ibm-power-vs-workspace-procedure
{context}"]
== Steps to create an {ibmpowerProductName} Virtual Server workspace

It's not uncommon for the title of the Assembly to to match the title of the Procedure, for example:

https://47558--docspreview.netlify.app/openshift-enterprise/latest/backup_and_restore/graceful-cluster-shutdown.html

You should add content to describe the purpose of the Assembly. For example...

"This document describes how to create an IBM Power Virtual Server workspace."

Like the above example, you might include prerequisites here if there are any.

For more information: https://github.com/openshift/openshift-docs/blob/main/contributing_to_docs/doc_guidelines.adoc#assemblymodule-titles-and-section-headings

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@stevsmit thanks. Incorporated the above suggestion.


:_content-type: PROCEDURE
[id="creating-ibm-power-vs-workspace-procedure_{context}"]
= Steps to create the workspace
Copy link
Copy Markdown
Member

@stevsmit stevsmit May 11, 2023

Choose a reason for hiding this comment

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

Apologies, but this must be a gerund. Eg:

"Creating the IBM Power VS workspace."

Followed by a short description.

"Use the following procedure to create the IBM Power VS workspace.

.Procedure

For more information: https://github.com/openshift/openshift-docs/blob/main/contributing_to_docs/doc_guidelines.adoc#assemblymodule-titles-and-section-headings

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@stevsmit thanks. Incorporated the above suggestion.

Copy link
Copy Markdown

@clnperez clnperez left a comment

Choose a reason for hiding this comment

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

i put in different language for the resource group questions. hopefully that is more clear

@alishaIBM
Copy link
Copy Markdown
Contributor Author

/label merge-review-needed

|Viewer, Operator, Editor, Administrator, Reader, Writer, Manager, Console Administrator
|VPC Infrastructure Services service <resource_group> resource group
|===
[.small]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think you need this [.small]. I don't know this for certain. It appears that we only use this tag for footnotes, according to the Doc Guidelines and a quick review of files that I did in Atom.
In the preview, it looks like the text in the following section, Access policy management is smaller than the rest of the assembly.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ack. I have removed [.small] as suggested above. FYI : @mburke5678

* You read the documentation on xref:../../installing/installing-preparing.adoc#installing-preparing[selecting a cluster installation method and preparing it for users].
* You xref:../../installing/installing_ibm_powervs/installing-ibm-cloud-account-power-vs.adoc#installing-ibm-cloud-account-power-vs[configured an IBM Cloud account] to host the cluster.
* If you use a firewall, you xref:../../installing/install_config/configuring-firewall.adoc#configuring-firewall[configured it to allow the sites] that your cluster requires access to.
* You configured the `ccoctl` utility before you installed the cluster. For more information, see xref:../../installing/installing_ibm_powervs/preparing-to-install-on-ibm-power-vs.adoc#cco-ccoctl-configuring_preparing-to-install-on-ibm-power-vs[Configuring the Cloud Credential Operator utility].
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This prerequisite is different in the preview. Perhaps Netlify didn't update it? Can you check that this prereq is correct?

image

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@mburke5678 yes , Netlify did not update the changes.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It is apprearing like below :

* You configured the `ccoctl` utility before you installed the cluster. For more information, see Configuring the Cloud Credential Operator utility.
* 

The code is pointing to the ID in the prerequisites assembly.

Code : * You configured the ccoctl utility before you installed the cluster. For more information, see xref:../../installing/installing_ibm_powervs/preparing-to-install-on-ibm-power-vs.adoc#choosing-an-method-to-install-ocp-on-power-vs-installer-provisioned[Configuring the Cloud Credential Operator utility].

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@alishaIBM Should this link go directly to the Configuring the Cloud Credentials Operator module?

installing/installing_ibm_power/preparing-to-install-on-ibm-power-vs.adoc#cco-ccoctl-configuring_preparing-to-install-on-ibm-power-vs

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Few more updates w.r.to above are addressed in this PR : https://github.com/openshift/openshift-docs/pull/60086/files fyi.

Comment thread modules/installation-ibm-cloud-export-variables.adoc Outdated
Comment thread modules/installation-uninstall-clouds.adoc Outdated
@mburke5678
Copy link
Copy Markdown
Contributor

@alishaIBM Amazing work! I had a few small issues. When you are ready, let me know and we can possibly merge this PR before its first birthday!

@alishaIBM
Copy link
Copy Markdown
Contributor Author

@alishaIBM Amazing work! I had a few small issues. When you are ready, let me know and we can possibly merge this PR before its first birthday!

@mburke5678 Thank you !

@mburke5678
Copy link
Copy Markdown
Contributor

/cherrypick enterprise-4.13

@openshift-cherrypick-robot
Copy link
Copy Markdown

@mburke5678: #47558 failed to apply on top of branch "enterprise-4.13":

Applying: OCP 4.13 - IBM PowerVS using installer-provisioned infrastructure
.git/rebase-apply/patch:2302: trailing whitespace.
          image: 
warning: 1 line adds whitespace errors.
Using index info to reconstruct a base tree...
M	_topic_maps/_topic_map.yml
M	modules/installation-about-restricted-network.adoc
M	modules/installation-configuration-parameters.adoc
M	modules/installation-configure-proxy.adoc
M	modules/installation-obtaining-installer.adoc
M	welcome/index.adoc
Falling back to patching base and 3-way merge...
Auto-merging welcome/index.adoc
Auto-merging modules/installation-obtaining-installer.adoc
Auto-merging modules/installation-configure-proxy.adoc
Auto-merging modules/installation-configuration-parameters.adoc
Auto-merging modules/installation-about-restricted-network.adoc
CONFLICT (content): Merge conflict in modules/installation-about-restricted-network.adoc
Auto-merging _topic_maps/_topic_map.yml
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 OCP 4.13 - IBM PowerVS using installer-provisioned infrastructure
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Details

In response to this:

/cherrypick enterprise-4.13

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.

<1> Required.
<2> If you do not provide these parameters and values, the installation program provides the default value.
<3> The `controlPlane` section is a single mapping, but the `compute` section is a sequence of mappings. To meet the requirements of the different data structures, the first line of the `compute` section must begin with a hyphen, `-`, and the first line of the `controlPlane` section must not. Only one control plane pool is used.
//<4> Enables or disables simultaneous multithreading, also known as Hyper-Threading. By default, simultaneous multithreading is enabled to increase the performance of your machines' cores. You can disable it by setting the parameter value to `Disabled`. If you disable simultaneous multithreading in some cluster machines, you must disable it in all cluster machines.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@alishaIBM With this call-out commented out, it affects the numbering in the docs.

image

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@mburke5678 thanks for the correction. Incorporated the changes in this PR : https://github.com/openshift/openshift-docs/pull/60086/files Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

branch/enterprise-4.13 peer-review-done Signifies that the peer review team has reviewed this PR size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.