Repository navigation
CNTRLPLANE-3254: Sort how-to guides alphabetically and add CI enforcement - #8248
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
the
jira/valid-reference
|
@bryan-cox: This pull request references CNTRLPLANE-3254 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 task 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. |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
📝 WalkthroughWalkthroughThis pull request adds a documentation ordering validation: a new Python script ( Sequence Diagram(s)sequenceDiagram
participant Make as Makefile/CI
participant Script as verify-docs-nav (Python)
participant YAML as PyYAML Loader
participant FS as Filesystem (docs/mkdocs.yml + markdown files)
participant Console as Console/Exit
Make->>Script: invoke python3 hack/verify-docs-nav-order.py
Script->>YAML: load docs/mkdocs.yml (SafeLoader)
YAML-->>Script: parsed nav structure
Script->>FS: read referenced markdown files for titles
FS-->>Script: file contents / titles
Script->>Script: compute display titles, treat index entries, check alphabetical order
alt order correct
Script->>Console: print success (exit 0)
Console-->>Make: success
else order incorrect or errors
Script->>Console: print current vs expected order, error message (exit non-zero)
Console-->>Make: failure
end
🚥 Pre-merge checks | ✅ 9 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (9 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
openshift-ci
Bot
added
do-not-merge/needs-area
area/ci-tooling
|
@bryan-cox: This pull request references CNTRLPLANE-3254 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 task 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. |
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/mkdocs.yml`:
- Around line 169-172: In the mkdocs.yml nav entry for the 'None' section,
replace the Agent path currently listed as
"how-to/agent/exposing-services-from-hcp.md" (the 'Exposing HCP Services' item)
with the None-specific document "how-to/none/exposing-services-from-hcp.md" so
the None nav entries consistently reference files under how-to/none (same
pattern as 'how-to/none/global-pull-secret.md').
In `@hack/verify-docs-nav-order.py`:
- Around line 90-93: The loop over entries that calls is_index_entry(entry) and
get_display_title(entry) must also enforce that index pages come before
non-index pages: add logic in the iteration (in the same loop that processes
entries in verify-docs-nav-order) to track when the first non-index entry is
seen (e.g., seen_non_index flag) and if you encounter an index entry after
seen_non_index is true, report/fail with a clear message referencing the
offending entry; use the existing is_index_entry(entry) predicate to detect
index pages and get_display_title(entry) to include the title in the error
report so position violations are detected and surfaced.
- Around line 11-16: The current try/except around "import yaml" silently exits
with 0 which makes CI skip the docs nav check; change the behavior in the except
block to detect the OPENSHIFT_CI environment variable and exit non-zero in CI
while keeping the existing graceful exit for local runs—i.e., in the except
ImportError for the "import yaml" statement, if os.environ.get("OPENSHIFT_CI")
is truthy call sys.exit(1) after printing the warning, otherwise keep
sys.exit(0) for local dev. Ensure you reference the "import yaml" ImportError
handler and the OPENSHIFT_CI environment check when making the change.
🪄 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 YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 0b69192f-db79-49ce-9e2a-c2af50b0c858
📒 Files selected for processing (3)
Makefiledocs/mkdocs.ymlhack/verify-docs-nav-order.py
c95a837 to
53254d2
Compare
Sort all how-to guide entries in docs/mkdocs.yml alphabetically at every level, and group cloud provider sections (Agent, AWS, Azure, GCP, Kubevirt, None, OpenStack, PowerVS) under a new "Platform" parent section to reduce top-level clutter and improve discoverability. Fixes: CNTRLPLANE-3254 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
53254d2 to
57f8263
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8248 +/- ##
=======================================
Coverage 34.65% 34.65%
=======================================
Files 767 767
Lines 93263 93263
=======================================
Hits 32318 32318
Misses 58266 58266
Partials 2679 2679 🚀 New features to boost your workflow:
|
|
@bryan-cox: This pull request references CNTRLPLANE-3254 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 task 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. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
hack/verify-docs-nav-order.py (1)
64-64: Avoid temporary list allocations for single key/value access.Prefer iterator access (
next(iter(...))) overlist(...)[0]for clarity and lower overhead. This pattern appears in three places: lines 64, 78, and 120.Proposed refactor
- value = list(entry.values())[0] + value = next(iter(entry.values())) @@ - return list(entry.keys())[0] + return next(iter(entry.keys())) @@ - value = list(entry.values())[0] + value = next(iter(entry.values()))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hack/verify-docs-nav-order.py` at line 64, Replace the temporary list allocation pattern like list(entry.values())[0] with iterator-based access using next(iter(entry.values())) (or next(iter(entry.keys())) where keys are being accessed) to avoid creating an intermediate list; update the three occurrences in the script that currently assign via list(...)[0] (the lines that set value = list(entry.values())[0] and the two analogous places) to use next(iter(...)) instead while preserving the exact behavior and variable names (e.g., keep assigning to value/from entry).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@hack/verify-docs-nav-order.py`:
- Around line 43-49: The frontmatter end detection using content.find('---', 3)
is too permissive; replace that logic in the block that begins with if
content.startswith('---') so you locate the closing delimiter by scanning lines
and finding a line that equals '---' (after stripping) rather than any
occurrence of '---' within text. Collect lines between the first '---' and the
next delimiter line, then parse those lines for the title key
(line.startswith('title:')) as before, and handle the case where no closing
delimiter is found by returning None or skipping parsing.
---
Nitpick comments:
In `@hack/verify-docs-nav-order.py`:
- Line 64: Replace the temporary list allocation pattern like
list(entry.values())[0] with iterator-based access using
next(iter(entry.values())) (or next(iter(entry.keys())) where keys are being
accessed) to avoid creating an intermediate list; update the three occurrences
in the script that currently assign via list(...)[0] (the lines that set value =
list(entry.values())[0] and the two analogous places) to use next(iter(...))
instead while preserving the exact behavior and variable names (e.g., keep
assigning to value/from entry).
🪄 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 YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: e1879131-d1a4-4c32-b11a-7c1d7b28cc37
📒 Files selected for processing (4)
Makefiledocs/content/contribute/contribute-docs.mddocs/mkdocs.ymlhack/verify-docs-nav-order.py
✅ Files skipped from review due to trivial changes (1)
- docs/content/contribute/contribute-docs.md
🚧 Files skipped from review as they are similar to previous changes (2)
- Makefile
- docs/mkdocs.yml
| if content.startswith('---'): | ||
| end = content.find('---', 3) | ||
| if end != -1: | ||
| for line in content[3:end].strip().split('\n'): | ||
| if line.startswith('title:'): | ||
| return line[6:].strip().strip('"').strip("'") | ||
|
|
There was a problem hiding this comment.
Frontmatter end detection is fragile and can mis-parse titles.
Using content.find('---', 3) can stop on any --- sequence, not just a delimiter line, which can yield wrong titles and false sort failures.
Proposed fix
- if content.startswith('---'):
- end = content.find('---', 3)
- if end != -1:
- for line in content[3:end].strip().split('\n'):
- if line.startswith('title:'):
- return line[6:].strip().strip('"').strip("'")
+ if content.startswith('---'):
+ lines = content.splitlines()
+ if lines and lines[0].strip() == '---':
+ for idx in range(1, len(lines)):
+ if lines[idx].strip() == '---':
+ for line in lines[1:idx]:
+ if line.lstrip().startswith('title:'):
+ return line.split(':', 1)[1].strip().strip('"').strip("'")
+ break🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hack/verify-docs-nav-order.py` around lines 43 - 49, The frontmatter end
detection using content.find('---', 3) is too permissive; replace that logic in
the block that begins with if content.startswith('---') so you locate the
closing delimiter by scanning lines and finding a line that equals '---' (after
stripping) rather than any occurrence of '---' within text. Collect lines
between the first '---' and the next delimiter line, then parse those lines for
the title key (line.startswith('title:')) as before, and handle the case where
no closing delimiter is found by returning None or skipping parsing.
Add a Python script (hack/verify-docs-nav-order.py) that validates the how-to guides nav entries in docs/mkdocs.yml are sorted alphabetically. The script resolves display titles from markdown file frontmatter or H1 headings, exempts index pages from sorting, and recursively checks all subsections. Add a verify-docs-nav Makefile target and include it in verify-parallel so it runs as part of `make verify`. Document the alphabetical ordering convention in contribute-docs.md. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Align map literal spacing to satisfy go fmt. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
57f8263 to
fd1ede3
Compare
openshift-ci
Bot
added
the
area/testing
|
@bryan-cox: The 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. |
|
/override "Red Hat Konflux / enterprise-contract-mce-217 / hypershift-release-mce-217" |
|
@bryan-cox: Overrode contexts on behalf of bryan-cox: Red Hat Konflux / enterprise-contract-mce-217 / hypershift-release-mce-217 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 kubernetes-sigs/prow repository. |
|
/override ci/prow/e2e-aks |
|
/override ci/prow/e2e-azure-self-managed |
|
/override ci/prow/e2e-v2-aws |
|
@bryan-cox: Overrode contexts on behalf of bryan-cox: ci/prow/e2e-aks, ci/prow/e2e-aks-4-22, ci/prow/e2e-aws, ci/prow/e2e-aws-4-22, ci/prow/e2e-aws-upgrade-hypershift-operator 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 kubernetes-sigs/prow repository. |
|
@bryan-cox: Overrode contexts on behalf of bryan-cox: ci/prow/e2e-azure-self-managed, ci/prow/e2e-kubevirt-aws-ovn-reduced 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 kubernetes-sigs/prow repository. |
|
@bryan-cox: Overrode contexts on behalf of bryan-cox: ci/prow/e2e-v2-aws 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 kubernetes-sigs/prow repository. |
|
/override ci/prow/e2e-aks |
|
/override ci/prow/e2e-azure-self-managed |
|
@bryan-cox: Overrode contexts on behalf of bryan-cox: ci/prow/e2e-aks, ci/prow/e2e-aks-4-22, ci/prow/e2e-aws, ci/prow/e2e-aws-4-22, ci/prow/e2e-aws-upgrade-hypershift-operator 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 kubernetes-sigs/prow repository. |
|
@bryan-cox: Overrode contexts on behalf of bryan-cox: ci/prow/e2e-azure-self-managed, ci/prow/e2e-kubevirt-aws-ovn-reduced 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 kubernetes-sigs/prow repository. |
|
/override ci/prow/e2e-v2-aws |
|
@bryan-cox: Overrode contexts on behalf of bryan-cox: ci/prow/e2e-v2-aws 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 kubernetes-sigs/prow repository. |
|
/override Red Hat Konflux / enterprise-contract-mce-217 / hypershift-release-mce-217 |
|
@bryan-cox: /override requires failed status contexts, check run or a prowjob name to operate on.
Only the following failed contexts/checkruns were expected:
If you are trying to override a checkrun that has a space in it, you must put a double quote on the context. 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 kubernetes-sigs/prow repository. |
|
/override "Red Hat Konflux / enterprise-contract-mce-217 / hypershift-release-mce-217" |
|
@bryan-cox: /override requires failed status contexts, check run or a prowjob name to operate on.
Only the following failed contexts/checkruns were expected:
If you are trying to override a checkrun that has a space in it, you must put a double quote on the context. 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 kubernetes-sigs/prow repository. |
AI Test Failure AnalysisJob: Generated by hypershift-analyze-e2e-failure post-step using Claude claude-opus-4-6 |
|
This confirms the PR only changes documentation files, a Makefile target, a verify script, and a Karpenter test file — none of which affect DNS, networking, or cluster creation. The failure is clearly an Azure ExternalDNS infrastructure issue, not caused by the PR. Now I have all the evidence. Here's the final report: Test Failure Analysis CompleteJob Information
Test Failure AnalysisErrorSummaryAll 7 Azure-platform e2e tests failed because ExternalDNS did not create DNS A/CNAME records in the Root CauseThe ExternalDNS controller on the AKS management cluster failed to register DNS records for all 7 hosted clusters created during the e2e test run. The specific mechanism is:
This is an Azure ExternalDNS infrastructure issue — either the ExternalDNS pod was not running, its Azure credentials expired, or there was an Azure DNS zone API issue. The PR changes (docs, Makefile, verify script, karpenter test) have zero overlap with DNS, networking, or cluster provisioning code paths. Recommendations
Evidence
|
Test Resultse2e-aks
Failed TestsTotal failed tests: 15
... and 10 more failed tests |
1 similar comment
Test Resultse2e-aks
Failed TestsTotal failed tests: 15
... and 10 more failed tests |
|
/override ci/prow/e2e-aks-4-22 |
|
@bryan-cox: Overrode contexts on behalf of bryan-cox: ci/prow/e2e-aks, ci/prow/e2e-aks-4-22 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 kubernetes-sigs/prow repository. |
Test Resultse2e-aks
Failed TestsTotal failed tests: 15
... and 10 more failed tests |
f059233
into
openshift:main
|
@bryan-cox: 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. |
AI Test Failure AnalysisJob: Generated by hypershift-analyze-e2e-failure post-step using Claude claude-opus-4-6 |
This branch was successfully deployed
4 participants
What this PR does / why we need it:
docs/mkdocs.ymlalphabetically at every level of the nav hierarchyverify-docs-navMakefile target with a Python verification script (hack/verify-docs-nav-order.py) that enforces alphabetical ordering, included inmake verifyviaverify-parallelThe how-to guides at https://hypershift.pages.dev/how-to/ were not sorted alphabetically, making it difficult to find specific guides. This change ensures alphabetical ordering both now and going forward through CI enforcement.
Which issue(s) this PR fixes:
Fixes CNTRLPLANE-3254
Special notes for your reviewer:
index.md,*-index.md) are exempt from sorting and kept first in each sectionChecklist:
🤖 Generated with Claude Code via
/jira:solve [CNTRLPLANE-3254](https://redhat.atlassian.net/browse/CNTRLPLANE-3254)Summary by CodeRabbit
Documentation
Chores
Tests