CLID-643: Build relevant additional documentation on oc-mirror#1462
CLID-643: Build relevant additional documentation on oc-mirror#1462dorzel wants to merge 1 commit into
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@dorzel: This pull request references CLID-643 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
WalkthroughThis PR removes stale engineering notes from NOTES.md, substantially rewrites TROUBLESHOOTING.md with expanded diagnostic sections, and adds five new documentation pages under docs/features/ covering mirroring workflows, archive management, cluster resources, dry-run, and filtering. ChangesDocumentation updates
Estimated code review effort: 2 (Simple) | ~15 minutes Suggested labels: documentation Suggested reviewers: none identified PoemA rabbit hopped through docs anew, 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/features/archive-management.md`:
- Around line 46-53: Update the archive-management docs to describe the resolved
cache directory instead of claiming a fixed ~/.oc-mirror default: explain that
--cache-dir falls back to $HOME when unset and OC_MIRROR_CACHE overrides it, and
revise the rm -rf and tar xf restore examples to use that resolved path. Also
update the restore snippet in the relevant section to be tagged as bash.
In `@docs/features/cluster-resources.md`:
- Around line 13-15: The directory listing fence in the cluster resources docs
is unlabeled, which triggers markdownlint. Update the fenced block under the
output-path section to include a language label such as text so the markdown
renderer and linter treat it as a proper code fence. Locate the fenced listing
by its <workspace>/working-dir/cluster-resources/ content and adjust only the
fence declaration.
In `@docs/features/dry-run.md`:
- Line 45: The status sentence in the dry-run docs is using the wrong workflow
wording by saying “disk to a disconnected registry” even though this section is
about mirror-to-disk dry runs. Update the text in the dry-run documentation to
describe the mirror-to-disk workflow consistently, keeping the reference to the
missing.txt check and the confirmation message aligned with that flow.
- Around line 28-33: Both literal-output code fences in the dry-run
documentation are missing a language identifier, so update each plain output
fence to use a text fence. Fix the fenced blocks in the dry-run example sections
by adding a markdown language tag such as text to the output snippets so
markdownlint accepts them. Keep the visible content unchanged and ensure both
affected fences are updated consistently.
In `@docs/features/mirroring-workflows.md`:
- Around line 113-119: The directory tree in the mirroring workflows doc is an
unlabeled fenced block, which triggers markdownlint. Update the code fence
around the working-dir listing to include a text language tag (or equivalent) so
the tree block is explicitly labeled while keeping the content unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: ed238ec0-14e6-4bd2-a391-82b774f92e9c
📒 Files selected for processing (7)
NOTES.mdTROUBLESHOOTING.mddocs/features/archive-management.mddocs/features/cluster-resources.mddocs/features/dry-run.mddocs/features/filtering.mddocs/features/mirroring-workflows.md
💤 Files with no reviewable changes (1)
- NOTES.md
| oc-mirror maintains a local cache for image data. The default cache location is `~/.oc-mirror`, and it can be configured with: | ||
|
|
||
| - The `--cache-dir` flag: `oc-mirror -c ./isc.yaml --cache-dir /custom/cache file:///home/user/output` | ||
| - The `OC_MIRROR_CACHE` environment variable | ||
|
|
||
| These two options are mutually exclusive. | ||
|
|
||
| The cache stores downloaded image layers and manifests between runs. In mirror-to-disk workflows, the archive contains only the blobs that are new since the last run, while the cache retains the complete set of previously downloaded data. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use the actual cache directory here.
--cache-dir falls back to $HOME when unset, with OC_MIRROR_CACHE as the env override; it does not default to ~/.oc-mirror. Please update the prose and the rm -rf / tar xf examples to reference the resolved cache path, or these instructions will point users at the wrong directory. Also tag the restore snippet as bash.
Also applies to: 115-128
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/features/archive-management.md` around lines 46 - 53, Update the
archive-management docs to describe the resolved cache directory instead of
claiming a fixed ~/.oc-mirror default: explain that --cache-dir falls back to
$HOME when unset and OC_MIRROR_CACHE overrides it, and revise the rm -rf and tar
xf restore examples to use that resolved path. Also update the restore snippet
in the relevant section to be tagged as bash.
| ``` | ||
| <workspace>/working-dir/cluster-resources/ | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Label the output-path fence.
The directory listing needs a fence language (for example, text) so markdownlint stops flagging it.
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 13-13: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/features/cluster-resources.md` around lines 13 - 15, The directory
listing fence in the cluster resources docs is unlabeled, which triggers
markdownlint. Update the fenced block under the output-path section to include a
language label such as text so the markdown renderer and linter treat it as a
proper code fence. Locate the fenced listing by its
<workspace>/working-dir/cluster-resources/ content and adjust only the fence
declaration.
Source: Linters/SAST tools
| Lists every image that would be mirrored, in `source=destination` format: | ||
|
|
||
| ``` | ||
| quay.io/openshift-release-dev/ocp-release@sha256:abc123...=registry.example.com/openshift-release-dev/ocp-release@sha256:abc123... | ||
| registry.redhat.io/redhat/redhat-operator-index:v4.18=registry.example.com/redhat/redhat-operator-index:v4.18 | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Tag both literal-output fences.
These blocks are plain output, so add a fence language such as text to satisfy markdownlint.
Also applies to: 41-43
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 30-30: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/features/dry-run.md` around lines 28 - 33, Both literal-output code
fences in the dry-run documentation are missing a language identifier, so update
each plain output fence to use a text fence. Fix the fenced blocks in the
dry-run example sections by adding a markdown language tag such as text to the
output snippets so markdownlint accepts them. Keep the visible content unchanged
and ensure both affected fences are updated consistently.
Source: Linters/SAST tools
| quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:def456...=docker://localhost:55000/openshift-release-dev/ocp-v4.0-art-dev@sha256:def456... | ||
| ``` | ||
|
|
||
| If all required images are already in the cache, `missing.txt` is not created and a message confirms that mirroring from disk to a disconnected registry can proceed. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Fix the workflow wording.
This sentence says “disk to a disconnected registry,” but this section is describing mirror-to-disk dry runs. Reword it so the status message matches the workflow.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/features/dry-run.md` at line 45, The status sentence in the dry-run docs
is using the wrong workflow wording by saying “disk to a disconnected registry”
even though this section is about mirror-to-disk dry runs. Update the text in
the dry-run documentation to describe the mirror-to-disk workflow consistently,
keeping the reference to the missing.txt check and the confirmation message
aligned with that flow.
| ``` | ||
| working-dir/ | ||
| cluster-resources/ # Generated IDMS, ITMS, CatalogSource, etc. | ||
| logs/ # Detailed operation logs | ||
| dry-run/ # Dry-run output (when --dry-run is used) | ||
| delete/ # Delete operation artifacts | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Label the tree block.
The directory listing needs a fence language (for example, text) to clear markdownlint.
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 113-113: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/features/mirroring-workflows.md` around lines 113 - 119, The directory
tree in the mirroring workflows doc is an unlabeled fenced block, which triggers
markdownlint. Update the code fence around the working-dir listing to include a
text language tag (or equivalent) so the tree block is explicitly labeled while
keeping the content unchanged.
Source: Linters/SAST tools
|
@dorzel: 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. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: adolfo-ab, dorzel The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
3 participants
Description
Adds additional docs in docs/features. Removes old NOTES.md. Updates TROUBLESHOOTING.md.
Github / Jira issue: https://redhat.atlassian.net/browse/CLID-643
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Expected Outcome
Summary by CodeRabbit