-
Notifications
You must be signed in to change notification settings - Fork 78
docs: update and extend instructions #1223
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
base: master
Are you sure you want to change the base?
docs: update and extend instructions #1223
Conversation
|
Skipping CI for Draft Pull Request. |
WalkthroughRenames and restructures the CRC guide into an OpenShift Local (formerly CRC) workflow: updates terminology, install/start steps, cluster configuration, Quay repo creation/login flow, test invocation, and cleanup instructions; adds a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
Pre-merge checks❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (8)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
209c108 to
be5bc77
Compare
be5bc77 to
5bd6bc6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (8)
doc/images/crc_oc_login.pngis excluded by!**/*.pngdoc/images/crc_start_output.pngis excluded by!**/*.pngdoc/images/download.pngis excluded by!**/*.pngdoc/images/extract_crc.pngis excluded by!**/*.pngdoc/images/openshift_crc_download.pngis excluded by!**/*.pngdoc/images/quay_repo.pngis excluded by!**/*.pngdoc/images/quay_repo_detail.pngis excluded by!**/*.pngdoc/images/quay_repo_visibility.pngis excluded by!**/*.png
📒 Files selected for processing (1)
CRC.adoc(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build & push Developer Sandbox UI image for UI e2e tests
- GitHub Check: Build & push operator bundles for e2e tests
- GitHub Check: Build & push Developer Sandbox UI image for UI e2e tests
🔇 Additional comments (3)
CRC.adoc (3)
79-120: Clear and well-structured repository setup instructions.The Quay repository creation steps and repository cloning instructions are explicit, easy to follow, and properly formatted. The directory structure example is particularly helpful.
247-265: Cleanup section is clear and comprehensive.The cleanup instructions, including the fallback procedure for stuck finalizers, are well-documented and appropriately sequenced. This is helpful for users encountering cleanup issues.
1-265: Documentation reorganization substantially improves usability — fix minor issues before merge.The restructuring transforms the guide into a comprehensive, step-by-step workflow with explicit commands, clear prerequisites, and helpful context (e.g., CRC startup output, directory structure). The additions make it much easier for new contributors to set up and run E2E tests successfully.
Before merging, address two minor issues:
- Line 68-69: Terminology correction ("pull request secret" → "pull secret")
- Line 222-223: Formatting correction (double space in "clean up")
The document structure, command examples, and overall flow are well-organized and appropriate for the intended purpose.
09371e9 to
8a21ae3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (8)
doc/images/crc_oc_login.pngis excluded by!**/*.pngdoc/images/crc_start_output.pngis excluded by!**/*.pngdoc/images/download.pngis excluded by!**/*.pngdoc/images/extract_crc.pngis excluded by!**/*.pngdoc/images/openshift_crc_download.pngis excluded by!**/*.pngdoc/images/quay_repo.pngis excluded by!**/*.pngdoc/images/quay_repo_detail.pngis excluded by!**/*.pngdoc/images/quay_repo_visibility.pngis excluded by!**/*.png
📒 Files selected for processing (1)
CRC.adoc(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Unit Tests
- GitHub Check: Build & push operator bundles for e2e tests
- GitHub Check: Build & push Developer Sandbox UI image for UI e2e tests
🔇 Additional comments (6)
CRC.adoc (6)
20-41: Well-structured download and setup section.The extraction and binary setup steps are clear, with proper shell code blocks and permission handling.
43-76: Clear configuration and startup instructions.The CRC setup, configuration, and start commands are well-documented with inline comments explaining the rationale for each setting (e.g., cluster monitoring requirement, memory/CPU/disk recommendations).
79-103: Clear Quay repository preparation steps.The numbered instructions for creating and publicizing repositories are actionable and easy to follow. The list of required repository names is well-organized and complete.
104-121: Well-documented repository cloning with clear structure.The directory structure diagram and explicit instruction to clone repositories at the same level as
toolchain-e2eare clear and helpful for users setting up their environment.
122-250: Comprehensive final setup and test execution guide.The realistic CRC boot output sets proper expectations. The section clearly progresses through environment setup (oc), authentication (kubeadmin login and Quay login), and test execution with good practices documented (output redirection, environment variable export). All commands are well-formatted and placeholder variables are clearly used.
252-271: Complete cleanup procedure with edge-case handling.The section provides both standard cleanup and an edge-case solution for stuck finalizers, with clear command sequencing and explanations.
8a21ae3 to
64d0040
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (8)
doc/images/crc_oc_login.pngis excluded by!**/*.pngdoc/images/crc_start_output.pngis excluded by!**/*.pngdoc/images/download.pngis excluded by!**/*.pngdoc/images/extract_crc.pngis excluded by!**/*.pngdoc/images/openshift_crc_download.pngis excluded by!**/*.pngdoc/images/quay_repo.pngis excluded by!**/*.pngdoc/images/quay_repo_detail.pngis excluded by!**/*.pngdoc/images/quay_repo_visibility.pngis excluded by!**/*.png
📒 Files selected for processing (1)
CRC.adoc(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Unit Tests
- GitHub Check: GolangCI Lint
- GitHub Check: Build & push operator bundles for e2e tests
- GitHub Check: Build & push Developer Sandbox UI image for UI e2e tests
🔇 Additional comments (2)
CRC.adoc (2)
3-271: Well-structured documentation with comprehensive E2E test guidance.The refactored instructions effectively consolidate the end-to-end testing workflow. The additions include explicit cluster configuration steps, Quay repository setup with repository names, a clear directory structure diagram, Podman login guidance, and detailed cleanup procedures. The previous issues flagged in earlier reviews (terminology, verb form, spacing) appear to have been resolved. The progressive structure—from prerequisites through cluster setup to test execution and cleanup—provides clear guidance for users.
237-250: The review comment is incorrect—all referenced make targets and the external URL are valid and current.The verification confirms:
test-e2e-localis defined at make/test.mk:126clean-e2e-resourcesis defined at make/clean.mk:57force-remove-finalizers-from-e2e-resourcesis defined at make/clean.mk:106- The pull secret URL (https://console.redhat.com/openshift/create/local) is valid as of November 20, 2025
All referenced items exist in the repository and are current. No changes are needed.
Likely an incorrect or invalid review comment.
10a34c6 to
cf2b0ad
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
CRC.adoc (2)
28-41: Clarify OS-specific filename in extraction instructions.The extraction command uses a hardcoded Linux filename (
crc-linux-amd64.tar.xz), but the preceding instructions mention selecting "your OS" during download. Users on macOS or Windows will have different filenames and extraction methods. Consider adding a note that these commands are Linux-specific, or provide platform-specific variants.For example:
# For Linux users: tar -xvf crc-linux-amd64.tar.xz # For macOS users: tar -xvf crc-macos-amd64.tar.xz # For Windows users, extract using your preferred archive tool or WSL
215-228: Clarify the API endpoint placeholder in login command.Line 227 uses
${URL_SHOWN_IN_OUTPUT}as a placeholder, but it's not entirely clear which URL from the preceding output block should be used. The API endpoint (e.g.,https://api.crc.testing:6443) is actually shown on line 212 in the example output. Consider replacing the placeholder with the explicit endpoint or adding a clarifying comment.-oc login -u kubeadmin -p ${KUBEADMIN_PASSWORD} ${URL_SHOWN_IN_OUTPUT} +oc login -u kubeadmin -p ${KUBEADMIN_PASSWORD} https://api.crc.testing:6443Alternatively, add a comment above the command noting that the URL is shown in the CRC startup output.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (8)
doc/images/crc_oc_login.pngis excluded by!**/*.pngdoc/images/crc_start_output.pngis excluded by!**/*.pngdoc/images/download.pngis excluded by!**/*.pngdoc/images/extract_crc.pngis excluded by!**/*.pngdoc/images/openshift_crc_download.pngis excluded by!**/*.pngdoc/images/quay_repo.pngis excluded by!**/*.pngdoc/images/quay_repo_detail.pngis excluded by!**/*.pngdoc/images/quay_repo_visibility.pngis excluded by!**/*.png
📒 Files selected for processing (1)
CRC.adoc(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: GolangCI Lint
- GitHub Check: Unit Tests
- GitHub Check: Build & push Developer Sandbox UI image for UI e2e tests
🔇 Additional comments (4)
CRC.adoc (4)
1-18: Excellent structural improvements to introduction and prerequisites.The reorganized introduction clearly sets expectations, and the new "Install the required tools" section properly delegates prerequisites to external documentation. This follows a good pattern for cross-cutting dependencies.
43-75: Well-structured cluster setup with clear configuration rationale.The setup section clearly explains which settings are required versus recommended, and appropriately allows for parallel work on Quay repositories while CRC boots. All configuration commands and values appear correct.
79-103: Clear and actionable Quay repository setup instructions.The numbered steps and explicit list of required repositories make this section easy to follow. Proper emphasis on public visibility requirements is included.
252-271: Excellent addition of comprehensive cleanup instructions.The new cleanup section appropriately handles both normal cleanup and the edge case of stuck resources requiring finalizer removal. The conditional approach and reminder to rerun cleanup after finalizer removal are helpful.
cf2b0ad to
1187ac6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
CRC.adoc (2)
79-80: Consolidate redundant subsection headers.Lines 79 and 80 are two consecutive headers at the same level (===), but line 79 ("Prepare the projects") has no content under it. The content that follows (lines 82+) pertains only to line 80 ("Creating the Quay repositories and making them public"). This structure is unclear and may confuse readers.
Consider removing line 79 or restructuring if "Prepare the projects" is intended to be a parent section.
If you intended to group both Quay setup and operator cloning under a parent section, restructure as:
-=== Prepare the projects === Creating the Quay repositories and making them publicOr, if "Prepare the projects" is meant to encompass both Quay and cloning:
-=== Prepare the projects -=== Creating the Quay repositories and making them public +== Prepare the projects + +You need to create Quay repositories and clone the operator repositories. + +=== Creating the Quay repositories and making them public
61-61: Fix memory notation: use "GB" (gigabytes) instead of "Gb" (gigabits).Memory capacity is measured in GB (gigabytes), not Gb (gigabits). While context makes the intent clear, the notation is non-standard and could confuse readers unfamiliar with the distinction.
Apply these diffs:
-# A minimum of 14Gb of virtual machine memory is required to run by default, so it is recommended to change that +# A minimum of 14GB of virtual machine memory is required to run by default, so it is recommended to change that-crc config set memory 14500 # or 20000 for 20Gb +crc config set memory 14500 # or 20000 for 20GBAlso applies to: 65-65
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (8)
doc/images/crc_oc_login.pngis excluded by!**/*.pngdoc/images/crc_start_output.pngis excluded by!**/*.pngdoc/images/download.pngis excluded by!**/*.pngdoc/images/extract_crc.pngis excluded by!**/*.pngdoc/images/openshift_crc_download.pngis excluded by!**/*.pngdoc/images/quay_repo.pngis excluded by!**/*.pngdoc/images/quay_repo_detail.pngis excluded by!**/*.pngdoc/images/quay_repo_visibility.pngis excluded by!**/*.png
📒 Files selected for processing (1)
CRC.adoc(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build & push operator bundles for e2e tests
- GitHub Check: Build & push Developer Sandbox UI image for UI e2e tests
1187ac6 to
c735696
Compare
The instructions could use a little refactor to include some of the missing steps which are required to successfully run the E2E tests for Developer Sandbox.
c735696 to
40dbfa1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
CRC.adoc (2)
43-76: Consider clarifying memory unit on line 61.The comment states "A minimum of 14GB of virtual machine memory is required" but the config value is
14500, which is in MB (~14.3 GB). The subsequent comment "or 20000 for 20GB" suggests approximate sizing, but adding a unit annotation (e.g., "14500 MB ≈ 14GB") would eliminate ambiguity for users unfamiliar with memory units.Minor clarity improvement (not a blocker):
# A minimum of 14GB of virtual machine memory is required to run by default, so it is recommended to change that # setting. Tweaking the CPUs and disk size is optional, but also recommended to avoid having issues down the line. crc config set cpus 8 crc config set disk-size 50 -crc config set memory 14500 # or 20000 for 20GB +crc config set memory 14500 # in MB; or 20000 for ~20GB
77-102: Minor: Clarify "the local ones" on line 77.Line 77 uses the vague phrase "prepare the Quay repositories and the local ones"; replacing "the local ones" with "local operator repositories" or "operator repositories to clone locally" would improve clarity without adding length.
-While CRC boots, you can go ahead and prepare the Quay repositories and the local ones to be able to run the tests. +While CRC boots, you can go ahead and prepare the Quay repositories and clone the local operator repositories to be able to run the tests.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (8)
doc/images/crc_oc_login.pngis excluded by!**/*.pngdoc/images/crc_start_output.pngis excluded by!**/*.pngdoc/images/download.pngis excluded by!**/*.pngdoc/images/extract_crc.pngis excluded by!**/*.pngdoc/images/openshift_crc_download.pngis excluded by!**/*.pngdoc/images/quay_repo.pngis excluded by!**/*.pngdoc/images/quay_repo_detail.pngis excluded by!**/*.pngdoc/images/quay_repo_visibility.pngis excluded by!**/*.png
📒 Files selected for processing (1)
CRC.adoc(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build & push Developer Sandbox UI image for UI e2e tests
- GitHub Check: Build & push operator bundles for e2e tests
🔇 Additional comments (3)
CRC.adoc (3)
1-19: Clear introduction and prerequisites reference.The intro section effectively communicates the importance of running the latest CRC release, system requirements, and appropriately defers tool installation to the separate required_tools.adoc reference document. Good information architecture.
122-250: Comprehensive final steps with clear test execution.The section provides realistic CRC startup output, explicit login commands, environment setup, and test execution with helpful explanatory comments. The placeholder variables (e.g.,
${YOUR_QUAY_USERNAME}) are clearly distinguished, and the rationale for output redirection is explained. This significantly improves usability for first-time users.
252-271: New cleanup section with edge case handling.The addition of explicit cleanup instructions with finalizer removal guidance handles a realistic failure mode (cleanup hanging on finalizers). The two-step recovery process is clear and actionable. This is a valuable addition that prevents user frustration during iterative testing.
alexeykazakov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job! I have a few minor comments though.
CRC.adoc
Outdated
| :imagesdir: doc/images | ||
|
|
||
| ==== Step by Step Guide - CRC (CodeReady Containers) | ||
| == Step-by-step guide — CRC (CodeReady Containers) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CodeReady Containers is an old name. Let's maybe change it to:
| == Step-by-step guide — CRC (CodeReady Containers) | |
| == Step-by-step guide — OpenShift Local (formerly CodeReady Containers) |
I would also rename CodeReady Containers to OpenShift Local everywhere in the document.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good! Should I go ahead and rename the CRC.adoc file to step-by-step-guide.adoc, for example?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can keep the filename. But I'm fine with both options.
CRC.adoc
Outdated
| + | ||
| `crc config set enable-cluster-monitoring true` | ||
| === Download and install CRC | ||
| Download CodeReady Containers from https://developers.redhat.com/products/codeready-containers/overview[developers.redhat.com]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use the new link: https://developers.redhat.com/products/openshift-local/overview
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, done!
CRC.adoc
Outdated
|
|
||
| 1. Log in to https://quay.io. | ||
| 2. Go to the repository section, or click on the following link: https://quay.io/repository. | ||
| 3. Click on the "Create new repository" button. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternatively, you can run the e2e makefile targets first. It will create all the repos for you automatically when pushing the built images. Then you can go and change the created repos to public in quay.io. In this case you don't have to pre-create the repos.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initially I had written the steps so that you had to run the suite once for it to create the required repositories, but I decided to make it a "manual creation" because that way:
- You raise awareness of which repositories will be used in your Quay account, and what components are part of the tests.
- Tests take a relatively long time to run, so having to run them once just for the sake of creating the repos felt a bit wasteful - and confusing for the first time you run them.
Let me add a note though so that the reader can decide which path to take.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
may be we can mention this first - automatic creation of quay repos , and explain of manual creation as a 2nd option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
CRC.adoc (1)
68-70: Minor terminology consistency: consider updating "CRC download page" reference.Line 68–70 refers to "CRC download page" when discussing the pull secret. For consistency with the document's terminology shift to OpenShift Local, consider updating to "OpenShift local download page." This is minor since the context is clear, but would improve overall consistency.
Apply this diff if you'd like to align terminology:
-Now you can go ahead and start the cluster. The first time you will need to provide the pull secret you were presented -with in the CRC download page — if you didn't grab it you can come back and copy it. Run the following command to start +Now you can go ahead and start the cluster. The first time you will need to provide the pull secret you were presented +with in the OpenShift local download page — if you didn't grab it you can come back and copy it. Run the following command to start
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
doc/images/openshift_local_download.pngis excluded by!**/*.png
📒 Files selected for processing (1)
CRC.adoc(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Unit Tests
- GitHub Check: GolangCI Lint
- GitHub Check: Build & push operator bundles for e2e tests
- GitHub Check: Build & push Developer Sandbox UI image for UI e2e tests
🔇 Additional comments (5)
CRC.adoc (5)
3-13: Clear and well-structured introduction.The title and introductory sections appropriately reflect the terminology shift to OpenShift Local and establish context well. The reference to "Codeready Toolchain" on line 13 is correct as it denotes the project/repositories being tested, not the CRC tool itself.
20-41: Well-formatted installation instructions with updated links.The download and installation section correctly references the updated Red Hat portal and provides clear, executable commands for extraction and binary setup. The cross-reference to required tools on line 17 is properly formatted.
79-120: Well-structured project preparation with clear naming and layout guidance.The section provides explicit Quay repository names and a clear directory structure diagram. The repository links and setup instructions are detailed and actionable, reducing ambiguity for developers following the guide.
122-250: Comprehensive and practical test execution guidance.The section provides a realistic output example and clear step-by-step instructions for environment setup and test execution. The comments explaining output redirection and QUAY_NAMESPACE configuration enhance usability for developers unfamiliar with the process.
252-271: Practical cleanup guidance with troubleshooting path.The cleanup section provides both the primary cleanup target and a fallback approach for handling stuck finalizers, giving developers a clear recovery path if cleanup encounters issues.
CodeReady Containers was the old name for the "OpenShift local" product form nowadays, so it does not make sense to have it lying around the docs anymore.
5935555 to
9432f8b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
CRC.adoc (1)
28-41: Consider adding platform-specific guidance for macOS/Windows users.The document instructs users to select their OS during the download (line 23) but then shows only Linux-specific commands for extracting and configuring the binary. Users selecting macOS or Windows may be confused by the
tar,chmod, and/usr/local/binreferences, which don't apply to their platform.Consider one of these approaches:
- Briefly acknowledge that the commands shown are Linux-specific and reference the OpenShift Local documentation for macOS/Windows equivalents
- Add separate subsections or a note with platform-specific steps (e.g., macOS users would use
unzipand place the binary in/usr/local/binwithoutchmod, Windows users would unzip and add the directory to%PATH%)- Link to the official OpenShift Local installation guide for platform-specific details
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
doc/images/openshift_local_download.pngis excluded by!**/*.png
📒 Files selected for processing (1)
CRC.adoc(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Unit Tests
- GitHub Check: Build & push Developer Sandbox UI image for UI e2e tests
- GitHub Check: Build & push operator bundles for e2e tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
CRC.adoc(1 hunks)
🔇 Additional comments (4)
CRC.adoc (4)
1-278: Previous issues have been successfully resolved.All issues flagged in prior reviews have been addressed:
- "pull secret" terminology corrected throughout ✓
- Verb form agreement fixed ("downloading and installing") ✓
- Subject-verb agreement fixed ("is required") ✓
- Double-space formatting corrected ("clean up") ✓
- CRC renamed to "OpenShift local" throughout ✓
15-277: Documentation restructuring is clear and comprehensive.The reorganized guide effectively walks users through the complete E2E test setup process. The addition of runnable command blocks, explicit repository naming, directory structure visualization, and the workflow note (lines 84-87) providing the alternative
make e2e-runpath all significantly improve usability for first-time users.The section structure (Install tools → Download/install → Setup cluster → Prepare projects → Final steps → Cleanup) follows a logical progression that matches user workflow.
30-33: Command blocks are well-formatted and syntactically sound.The runnable commands provided throughout the guide are correct and properly formatted with appropriate bash syntax highlighting. The example output at lines 134-220 provides realistic reference material for users.
Also applies to: 38-41, 49-52, 57-67, 73-76, 224-227, 232-235, 239-242, 246-257
21-21: Links verified and current.Both referenced Red Hat URLs remain valid and active: the OpenShift Local overview page and the console download/creation flow are correctly documented.
fbm3307
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great work , just some minor changes
CRC.adoc
Outdated
| + | ||
| `crc config set enable-cluster-monitoring true` | ||
| === Download and install CRC | ||
| Download CodeReady Containers from https://developers.redhat.com/products/codeready-containers/overview[developers.redhat.com]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as alexey mentioned lets rename this to openshift local (new name)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, thanks!
CRC.adoc
Outdated
| Download CodeReady Containers from https://developers.redhat.com/products/codeready-containers/overview[developers.redhat.com]. | ||
| You will need to log in using your Red Hat SSO account, after which you may click on the `Install OpenShift on your | ||
| laptop` button which will take you to the download page for CRC. From here, select your OS before clicking `Download | ||
| CodeReady Containers`. You will also need to download your pull secret, and keep that in a safe place. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we give link to pull secret.?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is not necessary, since we provide a screenshot of the page which already shows the download button for the pull secret. What do you think?
CRC.adoc
Outdated
|
|
||
| 1. Log in to https://quay.io. | ||
| 2. Go to the repository section, or click on the following link: https://quay.io/repository. | ||
| 3. Click on the "Create new repository" button. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
may be we can mention this first - automatic creation of quay repos , and explain of manual creation as a 2nd option.
CRC.adoc
Outdated
|
|
||
| ==== Cloning the operator repositories | ||
|
|
||
| In order to run the tests, you need to clone the repositories listed below *at the same directory level* as the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets mention as a Note that if someone already has these repos. make sure they are on the latest changes or master branch with latest changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea, thank you!
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alexeykazakov, MikelAlejoBR, rajivnathan The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thanks a lot for updating the doc.
Some sections of the file overlap with other docs/README, let's not create a duplication of the same. Let's update the other files instead and focus this one only for configuring CRC.
CRC.adoc
Outdated
| image::crc_start_output.png[align=center] | ||
| # A minimum of 14GB of virtual machine memory is required to run by default, so it is recommended to change that | ||
| # setting. Tweaking the CPUs and disk size is optional, but also recommended to avoid having issues down the line. | ||
| crc config set cpus 8 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I use 6 and it's sufficient
| crc config set cpus 8 | |
| crc config set cpus 6 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
CRC.adoc
Outdated
| === Prepare the projects | ||
| ==== Creating the Quay repositories and making them public |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this doesn't seem to be CRC-specific steps. Also, we already have this doc where we list quay repos and what do do with them: https://github.com/codeready-toolchain/toolchain-e2e/blob/58301e81b3adfc88a5119f825b546abecf77e15e/quay.adoc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated the other doc too with a few more thingies, thank you!
CRC.adoc
Outdated
| ==== Cloning the operator repositories | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is needed only if you want to run any of the "*-local" makefile targets. This is also mentioned here https://github.com/codeready-toolchain/toolchain-e2e/blob/58301e81b3adfc88a5119f825b546abecf77e15e/README.adoc#running-end-to-end-tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is a "Step by step guide" which shows how to run the local E2E tests, I think we should keep this section too, what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when the CRC is configured and running, then there is nothing different compared to any other OpenShift cluster, thus I would "only" delegate to the main document where developers can read how to execute the tests.
Also, feel free to rename the doc to "setup CRC" so it's really focused on setting up CRC and not step-by-step description of what is also in the main doc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it. I've removed the section from the doc
d56fef0 to
6a8c2ca
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
quay.adoc (1)
30-40: Clarify the purpose of the "Public visibility" section.The "Public visibility" section appears to be a verification or post-creation confirmation step, since the repository creation instructions (line 11) already explicitly direct users to "choose Public as the repository's visibility." Consider adding a clarifying header such as "=== Verify public visibility" or a brief note explaining this is a verification checklist to reduce potential confusion about whether this is a separate requirement or a duplicate step.
CRC.adoc (1)
62-66: Clarify memory requirement messaging.Line 62 states "A minimum of 14GB of virtual machine memory is required," but the introduction (line 10) recommends "at least 32GB of memory." These statements appear contradictory. Consider clarifying whether 14GB is the bare minimum for tests to run (with caveats about performance) and 32GB is recommended for optimal development experience, or update to be consistent.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
CRC.adoc(1 hunks)quay.adoc(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build & push operator bundles for e2e tests
- GitHub Check: Build & push Developer Sandbox UI image for UI e2e tests
- GitHub Check: Unit Tests
- GitHub Check: GolangCI Lint
🔇 Additional comments (4)
quay.adoc (1)
14-28: Solid guidance on environment setup.The instructions for QUAY_NAMESPACE export and Podman login are clear and actionable. The repository list using
<quay-username>placeholder is consistent and well-formatted.CRC.adoc (3)
84-103: Project structure and repository guidance is clear.The cross-reference to quay.adoc is well-placed, and the directory structure diagram with explicit repository list provides clear guidance on the layout users should create for running E2E tests locally.
220-233: Test execution guidance is well-documented.The QUAY_NAMESPACE export, output redirection, and fallback inline usage guidance are clear and practical. The comments explaining the rationale (terminal buffer, checking output) add helpful context for users.
235-254: Cleanup guidance is comprehensive and includes fallback instructions.The two-step cleanup process (primary clean target + finalizer removal fallback) provides good error recovery guidance. The rerun instruction after finalizer cleanup is appropriate.
6a8c2ca to
6f0a589
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
quay.adoc (1)
31-31: Fix subject-verb agreement: plural "repositories" requires "have," not "has."Line 31 uses incorrect verb form with a plural subject. "All repositories" is plural and requires the plural verb form "have" instead of "has."
Apply this diff:
- All aforementioned repositories has to be public, so make sure that the visibility is set to `public` for all of them: + All aforementioned repositories have to be public, so make sure that the visibility is set to `public` for all of them:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
CRC.adoc(1 hunks)quay.adoc(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Unit Tests
- GitHub Check: GolangCI Lint
- GitHub Check: Build & push Developer Sandbox UI image for UI e2e tests
- GitHub Check: Build & push operator bundles for e2e tests
🔇 Additional comments (4)
CRC.adoc (4)
84-85: Approve Quay repository reference and structure.The refactored step that references the separate
quay.adocdocument is a clean approach for managing Quay configuration. The xref link to the external guide keeps the instructions maintainable and avoids duplication. The instruction to "then come back to this guide" provides clear continuity.
89-106: Approve repository cloning and directory structure guidance.The explicit guidance on directory structure and the requirement to clone repositories at the same level as
toolchain-e2eis clear and helpful. The note on lines 105-106 to ensure latest changes are fetched is a good addition for preventing stale repository issues during test runs.
223-236: Approve test execution environment setup.The instructions for setting
QUAY_NAMESPACEand redirecting test output are clear and well-documented. The comments explaining why environment variables are needed and why output redirection is recommended provide good context for users unfamiliar with these steps.
238-257: Approve cleanup workflow and recovery steps.The documentation of cleanup targets (
clean-e2e-resourcesandforce-remove-finalizers-from-e2e-resources) provides a complete workflow. The explanation of when to use each target and the sequential ordering for recovery scenarios is clear and helpful.
f7ffbd6 to
3386a02
Compare
The "e2e run" target also creates all the repositories for the user, and then they only have to make them public. Adding a note as an alternative to manage those repositories in case the user wants to follow that path instead.
a403871 to
eb0eccb
Compare
|



The instructions could use a little refactor to include some of the missing steps which are required to successfully run the E2E tests for Developer Sandbox.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.