NO-JIRA: Add onboarding guide for new HCP team members#8132
NO-JIRA: Add onboarding guide for new HCP team members#8132jparrill wants to merge 1 commit intoopenshift:mainfrom
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
@jparrill: This pull request explicitly references no jira issue. 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. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new onboarding guide at docs/content/reference/onboarding-guide.md that documents HyperShift’s control-plane/data-plane decoupling, core CRDs and namespace topology (HostedCluster, HostedControlPlane, NodePool, control-plane namespace, operators), control-plane component model and status propagation, NodePool → CAPI → cloud provisioning and ignition/token flows, platform interfaces and per-cloud roles (credentials/encryption, CAPI/infra reconciliation, KubeVirt/CCM notes), development workflow and API-change rules, common controller patterns, architectural invariants, key file references, and a staged learning path. Also updates docs/mkdocs.yml to add the guide to Reference navigation. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jparrill 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.
Actionable comments posted: 1
🤖 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/content/reference/onboarding-guide.md`:
- Around line 293-295: The blockquote sections (the one that starts with
"Explore yourself" referencing registerComponents() and the later similar
blockquote) contain blank lines inside the quoted blocks which triggers
markdownlint MD028; remove the empty lines inside those blockquotes so every
quoted line directly follows the '>' prefix (no blank line between '> ...'
lines), ensure the text about registerComponents() and the kube-scheduler
example remains contiguous, and re-run markdownlint to confirm MD028 is
resolved.
🪄 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), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: d52f91d9-a5d5-44de-89e0-f6f2fb18bc09
⛔ Files ignored due to path filters (1)
docs/content/reference/aggregated-docs.mdis excluded by!docs/content/reference/aggregated-docs.md
📒 Files selected for processing (2)
docs/content/reference/onboarding-guide.mddocs/mkdocs.yml
81d3a3c to
922f217
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8132 +/- ##
==========================================
+ Coverage 27.50% 29.74% +2.24%
==========================================
Files 1096 1099 +3
Lines 107277 108949 +1672
==========================================
+ Hits 29503 32409 +2906
+ Misses 75240 73853 -1387
- Partials 2534 2687 +153 🚀 New features to boost your workflow:
|
922f217 to
82865e5
Compare
82865e5 to
ad5edc1
Compare
|
@bryan-cox Thanks for the feedback! I've pushed changes to address the points you raised on Slack: Content duplication concern:
This way the guide serves as a single entry point for onboarding while deferring to existing docs for deep dives, so we don't have to maintain the same content in two places. Let me know if you'd like further adjustments! |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
docs/content/reference/onboarding-guide.md (1)
299-301:⚠️ Potential issue | 🟡 MinorRemove blank lines inside blockquotes to fix MD028.
Line 300 and Line 893 break blockquote continuity, triggering
markdownlintMD028. Keep quoted lines contiguous.Proposed fix
> Light blue components have no KAS dependency. KAS (orange) is an implicit dependency for everything else. - > **Explore yourself**: Look at the `registerComponents()` function (~line 236 in `hostedcontrolplane_controller.go`) to see the full list of registered components. Then pick one simple component like `kube-scheduler` at `control-plane-operator/controllers/hostedcontrolplane/v2/kube_scheduler/` to understand the pattern.> **GOLDEN RULE**: After any change in `api/`, run `make update`. This runs: `api-deps` -> `workspace-sync` -> `deps` -> `api` -> `api-docs` -> `clients` -> `docs-aggregate`. - > **Explore yourself**: > - `api/go.mod` - the separate module definition > - `api/CLAUDE.md` - API backward compatibility rules (critical reading!)Also applies to: 892-894
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/content/reference/onboarding-guide.md` around lines 299 - 301, The blockquote in the onboarding guide contains empty lines that break continuity and trigger markdownlint MD028; edit the blockquote around the mention of registerComponents() and the kube-scheduler example to remove the blank lines so the quoted lines are contiguous (i.e., collapse the blank lines at the boundaries noted and ensure the two quoted paragraphs are immediately adjacent), preserving the text order and formatting so the reference to registerComponents() and the kube-scheduler example remains intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@docs/content/reference/onboarding-guide.md`:
- Around line 299-301: The blockquote in the onboarding guide contains empty
lines that break continuity and trigger markdownlint MD028; edit the blockquote
around the mention of registerComponents() and the kube-scheduler example to
remove the blank lines so the quoted lines are contiguous (i.e., collapse the
blank lines at the boundaries noted and ensure the two quoted paragraphs are
immediately adjacent), preserving the text order and formatting so the reference
to registerComponents() and the kube-scheduler example remains intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: f1872967-6edd-477c-8c45-1fb47ae2f322
⛔ Files ignored due to path filters (1)
docs/content/reference/aggregated-docs.mdis excluded by!docs/content/reference/aggregated-docs.md
📒 Files selected for processing (2)
docs/content/reference/onboarding-guide.mddocs/mkdocs.yml
✅ Files skipped from review due to trivial changes (1)
- docs/mkdocs.yml
ad5edc1 to
776ce15
Compare
776ce15 to
1717904
Compare
| @@ -0,0 +1,1315 @@ | |||
| # HyperShift / Hosted Control Planes (HCP) - Onboarding Guide | |||
|
|
|||
| > Ramp-up guide for new engineers joining the HyperShift/HCP team. | |||
There was a problem hiding this comment.
Maybe we can drop this? It seems like the note sufficiently explains this?
There was a problem hiding this comment.
Done — removed the blockquote. The admonition note covers it.
| - **Resource overhead**: 3+ master nodes per cluster just for the control plane | ||
| - **Provisioning time**: 30-45 minutes including bootstrap | ||
| - **Distributed operations**: each control plane is independently operated | ||
|
|
There was a problem hiding this comment.
Maybe it would be worth it to have a chart here on how standalone looks just to compare/contrast with the next chart?
There was a problem hiding this comment.
Done — split into two separate diagrams: standalone showing masters embedded per cluster, and HyperShift showing the shared management cluster.
|
|
||
| subgraph "HyperShift Model" | ||
| D[Management Cluster] | ||
| D --> E[CP 1 - Pods] |
There was a problem hiding this comment.
Should these be renamed to HCPs instead?
There was a problem hiding this comment.
Done — renamed to HCP 1/2/3.
| D --> E[CP 1 - Pods] | ||
| D --> F[CP 2 - Pods] | ||
| D --> G[CP 3 - Pods] | ||
| E -.-> H[Workers 1] |
There was a problem hiding this comment.
I think these should be labeled 1..N workers
There was a problem hiding this comment.
Done — labeled as Workers 1..N.
|
|
||
| ```mermaid | ||
| graph LR | ||
| subgraph "Standalone OpenShift" |
There was a problem hiding this comment.
Oh maybe move this chart up to https://github.com/openshift/hypershift/pull/8132/changes#r3045214587
There was a problem hiding this comment.
I think it would still be good to change the chart a bit and show how you need a management cluster for each standalone
There was a problem hiding this comment.
Done — the HyperShift diagram is now right below the standalone one for direct comparison.
There was a problem hiding this comment.
Done — standalone diagram now shows each cluster with its own Management / Masters x3 embedded.
| **Rollout detection**: `ConfigGenerator.Hash()` produces a new hash when config or version changes. New hash = new Secrets = new `DataSecretName` on MachineDeployment = CAPI rolling update. | ||
|
|
||
| > **Explore yourself**: Platform-specific machine template builders: | ||
| > - `hypershift-operator/controllers/nodepool/aws.go` - `awsMachineTemplateSpec()`: AMI resolution, instance type, root volume, security groups |
There was a problem hiding this comment.
List is not displaying correctly
There was a problem hiding this comment.
Fixed — added blank > line before the bullet list.
| > - `hypershift-operator/controllers/nodepool/gcp.go` - GCP machine config | ||
| > - `hypershift-operator/controllers/nodepool/openstack.go` - OpenStack config | ||
|
|
||
| ### 7.4 Auto-scaling |
There was a problem hiding this comment.
Should we not point to https://pr-8132.hypershift.pages.dev/how-to/resource-based-control-plane-autoscaling/?
There was a problem hiding this comment.
Done — added See Also link to the resource-based-control-plane-autoscaling page.
|
|
||
| --- | ||
|
|
||
| ## 8. Supported Cloud Platforms |
There was a problem hiding this comment.
IMO we should distinguish between self managed AWS and ROSA HCP
There was a problem hiding this comment.
Done — diagram and table now distinguish AWS Self-Managed vs AWS Managed (ROSA HCP).
|
|
||
| --- | ||
|
|
||
| ## 8. Supported Cloud Platforms |
There was a problem hiding this comment.
Azure is not managed only, there is self-managed Azure as well
There was a problem hiding this comment.
Good catch — fixed. Diagram and table now show Azure Self-Managed and Azure Managed (ARO HCP) as separate entries.
| | Infra provisioning | EC2, VPC, ELB | Azure VMs, VNet | KubeVirt VMs | Pre-provisioned | | ||
| | Cloud Controller Manager | aws-ccm | azure-ccm | kubevirt-ccm | none | | ||
|
|
||
| > **Explore yourself**: Each platform implementation is in its own directory: |
There was a problem hiding this comment.
Fixed — added blank > line before the bullet list in the blockquote.
Add a comprehensive onboarding guide covering HyperShift architecture, control plane and data plane internals, supported platforms, APIs, development workflow, and a recommended learning path with direct code references for self-guided exploration. Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com>
1717904 to
af7b12a
Compare
|
/uncc |
|
@jparrill: The following test failed, say
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. |
|
I now have a complete picture. Here is the analysis: Test Failure Analysis CompleteJob Information
Test Failure AnalysisErrorSummaryThis is a CI infrastructure flake unrelated to the PR content. The Root CauseTransient DNS resolution failure on the The complete timeline shows:
The DNS failure is an infrastructure-level issue on the This is not caused by the PR (which only adds markdown documentation files) and is not a product bug. Recommendations
Evidence
|
Summary
docs/content/reference/onboarding-guide.md) covering HyperShift architecture, control plane and data plane internals, supported platforms, APIs, development workflow, and a recommended learning pathTest plan
cd docs && mkdocs serve)🤖 Generated with Claude Code
Summary by CodeRabbit