Skip to content
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.

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

GH#51152: document Single Node OKD #71324

Closed
wants to merge 10 commits into from
Closed

Conversation

peterroth
Copy link
Contributor

Version(s):
all OKD 4 versions

Issue:
GH#51152, okd-project/planning #27

QE review:

  • QE has approved this change.

@openshift-ci openshift-ci bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Feb 7, 2024
Copy link

openshift-ci bot commented Feb 7, 2024

Hi @peterroth. Thanks for your PR.

I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@peterroth
Copy link
Contributor Author

Please check if this change is OK, @mburke5678 @vrutkovs

and revert deleted empty line
ifndef::openshift-origin[]
You can install {sno} using the web-based Assisted Installer and a discovery ISO that you generate using the Assisted Installer. You can also install {sno} by using `coreos-installer` to generate the installation ISO.
Copy link
Member

Choose a reason for hiding this comment

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

This bit would need "how to setup Assisted Installer" section for OKD, as users can't use one from console.redhat.com.

I think a simple ref to https://github.com/openshift/assisted-service/tree/master/deploy/podman#okd-configuration would be good for a start

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, I wasn't aware of that document.
I just updated the file and pushed the changes.

@vrutkovs
Copy link
Member

vrutkovs commented Feb 8, 2024

/ok-to-test

Looks good to me

@openshift-ci openshift-ci bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Feb 8, 2024
@ocpdocs-previewbot
Copy link

ocpdocs-previewbot commented Feb 8, 2024

🤖 Wed Feb 14 05:54:27 - Prow CI generated the docs preview: https://71324--ocpdocs-pr.netlify.app

@mburke5678
Copy link
Contributor

@peterroth @vrutkovs I want to make sure what the PR is trying to do.

  • Adds a link to the upstream Install OKD using Assisted Service docs.
  • Remove the == Installing {sno} using the Assisted Installer heading.
  • Leaves two cross-references (but, should be combined into one Additional references section).
  • Removes the Installing single-node OpenShift using the Assisted Installer module and two submodules.
  • Removes the Installing single-node OpenShift manually module and two submodules.
  • Leaves the Installing single-node OpenShift on cloud providers and all five submodules.
  • Leaves the Creating a bootable ISO image on a USB drive module.
  • Removes the final 8 modules.

Here is an internel-only (sorry!) preview: https://file.rdu.redhat.com/mburke/71324/installing/installing_sno/install-sno-installing-sno.html#install-sno-installing-sno-on-cloud-providers

Here is a screenshot that shows the new link, the two xrefs, and the TOC of remaining modules (on the right).

image

Please let me know if this is what is expected.

@peterroth
Copy link
Contributor Author

peterroth commented Feb 8, 2024

Hello @mburke5678 ,

I can only say a partial yes, hence I'll go back into the code.

Adds a link to the upstream Install OKD using Assisted Service docs. -- Yes, Assisted Installer isn't available for OKD
Remove the == Installing {sno} using the Assisted Installer heading. -- Maybe a new header is required, using Assisted Service
Leaves two cross-references (but, should be combined into one Additional references section). -- Definitely no, I'll combine them
Removes the Installing single-node OpenShift using the Assisted Installer module and two submodules. -- Yes
Removes the Installing single-node OpenShift manually module and two submodules. -- the "Install sno manually" part isn't removed, accessible for both OCP and OKD
Leaves the Installing single-node OpenShift on cloud providers and all five submodules. -- correct, because OKD supports those cloud providers
Leaves the Creating a bootable ISO image on a USB drive module. -- correct
Removes the final 8 modules. -- correct, OKD doesn't support IBM Z and IBM Power

edit: taking a look on the screenshot I'd like to ask where can I set the reference for OKD instead of OpenShift? I couldn't find it and thought that happens somewhere automagically, but it seems it does not.

@mburke5678
Copy link
Contributor

mburke5678 commented Feb 8, 2024

where can I set the reference for OKD instead of OpenShift?

That would require a new {sno} attribute in the _attributes/common-attributes.adoc file, and more ifdefs to apply the different attributes.

:sno: single-node OpenShift
:sno-caps: Single-node OpenShift
:sno-okd: single-node OKD
:sno-caps-okd: Single-node OKD

image

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 10, 2024
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 11, 2024
@peterroth
Copy link
Contributor Author

/retest

@peterroth
Copy link
Contributor Author

It seems the last test failed with the below error:

INFO[2024-02-11T10:45:31Z] Step validate-asciidoc-openshift-docs-asciidoctor failed after 1h2m22s. 
INFO[2024-02-11T10:45:31Z] Step phase test failed after 1h2m22s.        
INFO[2024-02-11T10:45:31Z] Running multi-stage phase post               
INFO[2024-02-11T10:45:31Z] Step phase post succeeded after 0s.          
INFO[2024-02-11T10:45:31Z] Ran for 1h4m17s                              
ERRO[2024-02-11T10:45:31Z] Some steps failed:                           
ERRO[2024-02-11T10:45:31Z] 
  * could not run steps: step validate-asciidoc failed: "validate-asciidoc" test steps failed: "validate-asciidoc" pod "validate-asciidoc-openshift-docs-asciidoctor" failed: pod pending for more than 30m0s: containers have not started in 30m0.006857311s: sidecar, test: 
* Container sidecar is not ready with reason PodInitializing
* Container test is not ready with reason PodInitializing

The pod didn't start, it stuck in PodInitializing state for 30 min.

@peterroth
Copy link
Contributor Author

/retest

@peterroth
Copy link
Contributor Author

peterroth commented Feb 13, 2024

@mburke5678 I pushed some changes to fix the issues you mentioned previously. Can you please check if the preview looks better/promising after the modifications?
Thank you in advance!

@mburke5678
Copy link
Contributor

@peterroth Looking good! If you can just put an asterisk * before Install OKD using Assisted Service and I think we've got it!

@peterroth
Copy link
Contributor Author

Sure, just added it, @mburke5678.

@peterroth
Copy link
Contributor Author

/retest

Copy link

openshift-ci bot commented Feb 14, 2024

@peterroth: all tests passed!

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

@peterroth
Copy link
Contributor Author

Hi @mburke5678,
Please let me know if there are things I should add to the changes. The tests passed and based on your feedback from last week I added the missing asterisk.

@mburke5678
Copy link
Contributor

mburke5678 commented Feb 19, 2024

@peterroth I noticed on issue with your edit. You need a carriage return before the asterisk. And we squash comments down to one before merging in the docs repo. But, no worry. I took the liberty of taking a copy of your PR and made the changes locally, to save any further back and forth. See :#71809

image

@peterroth
Copy link
Contributor Author

Sounds good. As the changes will be merged with your PR, should I close this one?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch/enterprise-4.13 branch/enterprise-4.14 branch/enterprise-4.15 branch/enterprise-4.16 ok-to-test Indicates a non-member PR verified by an org member that is safe to test. 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.

5 participants