Skip to content

Conversation

@tmalove
Copy link
Contributor

@tmalove tmalove commented Aug 17, 2022

Version(s): 4.11+

Issue: OSDOCS-3891

Link to docs preview: http://file.rdu.redhat.com/tlove/opp/architecture/opp-architecture.html#opp-architecture-compatibility-matrix_opp-architecture (Updated: 11/11)

Reviews:

  • [ x] Peer view
  • [ x] SME review
  • [ x] QE review

Additional information:

  • This is the initial published version of the OPP Product Guide.
  • This guide is located in the opp-docs repo.

@openshift-ci openshift-ci bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Aug 17, 2022
@tmalove tmalove changed the title add assembly, modules and image [OSDOCS-3819]: Create the OpenShift Platform Plus product guide Aug 18, 2022
@tmalove tmalove changed the title [OSDOCS-3819]: Create the OpenShift Platform Plus product guide [WIP] [OSDOCS-3819]: Create the OpenShift Platform Plus product guide Aug 18, 2022
@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 Aug 18, 2022
Copy link
Contributor

@kalexand-rh kalexand-rh left a comment

Choose a reason for hiding this comment

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

I have some formatting suggestions and a couple of questions. I think this is shaping up well!

Copy link
Contributor

Choose a reason for hiding this comment

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

Will you make these list items use parallel structure? And I strongly suggest using the attributes that define your product names throughout the PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

These should be links, not xrefs. And, to keep users from bouncing between sites, please use access.redhat.com links instead of the docs.openshift ones.

Because these are all links, please incorporate them into the module.

Copy link
Contributor

Choose a reason for hiding this comment

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

Because these are all links, please incorporate them directly into the modules instead.

Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest expanding your module titles.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Completed

Copy link
Contributor

Choose a reason for hiding this comment

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

Please confirm that "Registry" be capitalized here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Verified on the portal and corrected.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do you need this file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure why this ocp file is included in this PR. You are correct, it is not a part of this guide. I will investigate.

Choose a reason for hiding this comment

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

@tmalove, this file is still present.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this needs to be removed before merging iiuc

/remove-label merge-review-in-progress
/remove-label merge-review-needed

Copy link
Contributor

Choose a reason for hiding this comment

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

Done.

Copy link
Contributor

Choose a reason for hiding this comment

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

You can put the link here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

Copy link
Contributor

Choose a reason for hiding this comment

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

Is your second {ocp} correct?

Copy link
Contributor Author

@tmalove tmalove Aug 26, 2022

Choose a reason for hiding this comment

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

Corrected

@tmalove tmalove changed the title [WIP] [OSDOCS-3819]: Create the OpenShift Platform Plus product guide [WIP] [OSDOCS-3891]: Create the OpenShift Platform Plus product guide Aug 29, 2022
Copy link
Contributor

@sagidlow sagidlow left a comment

Choose a reason for hiding this comment

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

Just some light comments about attributes, otherwise content looks great!

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
{product-title} is a single hybrid-cloud platform for enterprises to build, deploy, run, and manage intelligent applications securely across infrastructures. It is based on Red Hat Enterprise Linux, Kubernetes, and Red Hat {ocp}, and includes the following products:
{product-title} is a single hybrid-cloud platform for enterprises to build, deploy, run, and manage intelligent applications securely across infrastructures. It is based on {op-system-base-full}, Kubernetes, and Red Hat {ocp}, and includes the following products:

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* Red Hat Advanced Cluster Management for Kubernetes - Controls clusters and applications from a single console.
* {rh-rhacm-first} for Kubernetes - Controls clusters and applications from a single console.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
|Red Hat Advanced Cluster Management release
|{rh-rhacm-first} release

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
To install {product-title}, install {ocp}, Red Hat Advanced Cluster Management, Quay, OpenShift Data Foundation Essentials, and Red Hat Advanced Cluster Security. See the following detailed installation information for the {product-title} products:
To install {product-title}, install {ocp}, {rh-rhacm-first}, Quay, OpenShift Data Foundation Essentials, and Red Hat Advanced Cluster Security. See the following detailed installation information for the {product-title} products:

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
IMPORTANT: You must first install {ocp}, followed by Red Hat Advanced Cluster Management. The remaining products can be installed in any order.
IMPORTANT: You must first install {ocp}, followed by {rh-rhacm-first}. The remaining products can be installed in any order.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
. Install Red Hat Advanced Cluster Management. For detailed information, see _Installing Red Hat Advanced Cluster Management for Kubernetes_.
. Install {rh-rhacm-first}. For detailed information, see _Installing Red Hat Advanced Cluster Management for Kubernetes_.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would add Red Hat Advanced Cluster Security to your common-attributes.adoc file. Maybe coordinate with Kathryn and see what the best attribute would be for this? I didn't see this in the common-attributes.adoc file in OpenShift. If and when you add this to your common-attributes file just go through the whole doc and make sure you update any hardcoded referenced to "Red Hat Advanced Cluster Security" references to the attribute reference.

Copy link
Contributor

Choose a reason for hiding this comment

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

You can make any attributes you need and name them whatever makes most sense! (If I were adding it, I might make it :acs:.)

Copy link
Contributor

Choose a reason for hiding this comment

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

I would add Quay to your common-attributes.adoc file. I don't see Red Hat Quay as a part of the common-attributes.adoc file in the main branch. Maybe coordinate with Steven Smith and Kathryn to see if it should be added? If and when you add this to your common-attributes file just go through the whole doc and make sure you update any hardcoded referenced to "Red Hat Quay" references to the attribute reference.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would add Red Hat OpenShift Data Foundation to your common-attributes.adoc file. In the OpenShift main branch for common-attributes.adoc file it is listed as

:rh-storage-first: Red Hat OpenShift Data Foundation

Maybe coordinate with Kathryn to see if it should be added? If and when you add this to your common-attributes file just go through the whole doc and make sure you update any hardcoded referenced to "Red Hat OpenShift Data Foundation" references to the attribute reference.

@ocpdocs-previewbot
Copy link

🤖 Bots are busy building the preview. It will be available soon at:
https://49279--docspreview.netlify.app

@tmalove tmalove marked this pull request as draft September 19, 2022 15:51
@tmalove
Copy link
Contributor Author

tmalove commented Sep 19, 2022

/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 Sep 19, 2022
@tmalove tmalove marked this pull request as ready for review September 19, 2022 15:56
Copy link

@jboxman-rh jboxman-rh left a comment

Choose a reason for hiding this comment

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

@tmalove, I've left some comments for your review.

Thanks for your work on this!

Choose a reason for hiding this comment

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

@tmalove, I'd uppercase compliance.

Choose a reason for hiding this comment

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

Do we want a single coma here? Maybe and? Not sure what kind of cache this means.

Choose a reason for hiding this comment

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

I think we're using plug-in.

Choose a reason for hiding this comment

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

This was a product name; I think we just rebranded this and the following bullet. Is it worth confirming?

Choose a reason for hiding this comment

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

I wonder if this should be {product_title} as well?

Choose a reason for hiding this comment

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

Do we want to use attributes where applicable for these link names?

Choose a reason for hiding this comment

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

@tmalove, this file is still present.

@jboxman-rh jboxman-rh added peer-review-in-progress Signifies that the peer review team is reviewing this PR branch/enterprise-4.11 branch/enterprise-4.12 and removed peer-review-needed Signifies that the peer review team needs to review this PR labels Sep 20, 2022
@jboxman-rh jboxman-rh added this to the Continuous Release milestone Sep 20, 2022
@tmalove
Copy link
Contributor Author

tmalove commented Sep 20, 2022

/label peer-review-done

@openshift-ci openshift-ci bot added the peer-review-done Signifies that the peer review team has reviewed this PR label Sep 20, 2022
@tmalove
Copy link
Contributor Author

tmalove commented Sep 21, 2022

/remove-label peer-review-in-progress

@openshift-ci openshift-ci bot removed the peer-review-in-progress Signifies that the peer review team is reviewing this PR label Sep 21, 2022
Copy link

Choose a reason for hiding this comment

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

Should we offer a more user friendly installation method in these docs as well?

Maybe this is just too early in the process but internally we have solutions using openshift-plus policyset and Helm-Charts for doing this deployment.

Asking our customers to do 5 separate product installs for a single bundle that they have purchased just doesn't feel right. I'm curious if we are open to exploring documentation for a deployment that will be easier on the user.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @ascerra This is an item that we should discuss during the team meeting. I agree that a single deployment would be easier, seamless, and more receptive to customers and our internal teams. I will request a discussion as soon as possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ascerra We discussed this in the latest team meeting and the answer is that we will not have a universal installer; we want people to use policy sets for the installation.

Copy link

@ascerra ascerra Sep 22, 2022

Choose a reason for hiding this comment

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

I believe having a section about minimum resource requirements specifically for OPP would go a long way to the user experience here. It took me a lot of trial and error going through all the different products documentation to finally get to a point where I was deploying these products with enough capacity on my underlying infrastructure. My experience was with AWS & Openstack, which brings up the difficulty of this ask. Would we need specifics for minimum specs for each public cloud? This would be most helpful but tough to achieve and maintain I'm sure. ACM docs attempt to do this for the big 3 here

Having something that describes the absolute minimum resource requirements for OPP to install sucessfully like we see in ODF resource requirements would be very helpful.

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 for this @ascerra. This is on the list to discuss with the engineering to get in the guide soon after the first publishing. I will include you on the ODCBUGS Jira for transparency.

@BillDett
Copy link

/label quay-eng-ack

@openshift-ci
Copy link

openshift-ci bot commented Nov 11, 2022

@BillDett: The label(s) /label quay-eng-ack cannot be applied. These labels are supported: platform/aws, platform/azure, platform/baremetal, platform/google, platform/libvirt, platform/openstack, ga, tide/merge-method-merge, tide/merge-method-rebase, tide/merge-method-squash, px-approved, docs-approved, qe-approved, downstream-change-needed, approved, backport-risk-assessed, bugzilla/valid-bug, cherry-pick-approved, cnv, dev-tools, distributed-tracing, ims, jira/valid-bug, merge-review-in-progress, merge-review-needed, mtc, multi-arch, oadp, peer-review-done, peer-review-in-progress, peer-review-needed, rhacs, rhv, serverless, service-mesh, staff-eng-approved, telco. Is this label configured under labels -> additional_labels or labels -> restricted_labels in plugin.yaml?

In response to this:

/label quay-eng-ack

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.

Copy link

Choose a reason for hiding this comment

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

It looks like this was changed to rh-rhacm causing it to not show the real value in the preview when compiled.

Copy link

Choose a reason for hiding this comment

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

It looks like rh-rhacm-first was changed to rh-rhacm causing it to not show the real value in the preview when compiled.

@tmalove tmalove changed the title [WIP] [OSDOCS-3891]: Create the OpenShift Platform Plus product guide [OSDOCS-3891]: Create the OpenShift Platform Plus product guide Nov 11, 2022
@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 Nov 11, 2022
@tmalove
Copy link
Contributor Author

tmalove commented Nov 11, 2022

/label quay-eng-ack

Hi @BillDett you can apply the '/LGTM' label and that will suffice for your ack...thanks!

@BillDett
Copy link

/LGTM

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 11, 2022
@tmalove
Copy link
Contributor Author

tmalove commented Nov 11, 2022

/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 11, 2022
@jeana-redhat jeana-redhat added the merge-review-in-progress Signifies that the merge review team is reviewing this PR label Nov 11, 2022
@openshift-ci openshift-ci bot 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 11, 2022
@ascerra
Copy link

ascerra commented Nov 11, 2022

/LGTM

@kalexand-rh kalexand-rh force-pushed the opp-product-guide-osdocs-3892 branch from e41554d to 0bfd7bf Compare November 11, 2022 21:08
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Nov 11, 2022
@openshift-ci
Copy link

openshift-ci bot commented Nov 11, 2022

New changes are detected. LGTM label has been removed.

@openshift-ci openshift-ci bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Nov 11, 2022
@kalexand-rh kalexand-rh merged commit c5afef7 into openshift:opp-docs Nov 11, 2022
@kalexand-rh
Copy link
Contributor

OPP uses its own branch and is versionless, so no CPs are required.

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

Labels

peer-review-done Signifies that the peer review team has reviewed this PR size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants