Openshift Container Platform (OCP) service #1380
Open
kaponco wants to merge 9 commits into
Open
Conversation
…ree Helm charts (NVIDIA#566) Add OCP as a new service type with OLM-based operator deployment using the existing KindLocalHelm bundler pipeline. Each OCP operator is modeled as two in-tree Helm chart components: an OLM installer phase and a CR configuration phase, with readiness gates between them. - Add CriteriaServiceOCP constant with "openshift" alias to criteria parser - Register six OCP components (OLM + CR pairs for NFD and GPU Operator) - Create OCP overlays (base, training, inference) that disable base components not applicable to the platform - Add Helm-templated manifests for OLM resources (OperatorGroup, Subscription) and operator CRs (NodeFeatureDiscovery, ClusterPolicy) - Add Chainsaw readiness gates that assert CSV phase Succeeded before CR deployment proceeds - Update OpenAPI spec, CLI docs, API docs, and issue templates with ocp service value - Add exhaustive switch coverage for CriteriaServiceOCP in validators - Update ADR-010 design document with implementation decisions and overlay merge constraints Signed-off-by: Shai Kapon <skapon@redhat.com>
…k Operator to base overlay (NVIDIA#566) Improve OCP recipe reliability and coverage by strengthening readiness gates, consolidating networking into the base overlay, and adding NIM Operator support for inference workloads. - Add Deployment availability checks to all OLM readiness gates (GPU, Network, NFD) to prevent premature CR creation before operator pods are ready - Register k8s-nim-operator-ocp-olm component and add ocp-inference-nim overlay for NIM-based inference on OpenShift - Expand Network Operator NicClusterPolicy values with upgrade policy, health probes, RDMA shared device plugin config, and NV-IPAM settings - Correct GPU Operator namespace from nvidia-gpu-operator to gpu-operator across values, readiness gates, and registry - Update design doc and added openshift integration specification Signed-off-by: Shai Kapon <skapon@redhat.com>
Signed-off-by: Shai Kapon <skapon@redhat.com>
…IA#566) Restructure OCP Helm component values to use flat top-level keys instead of nesting everything under a wrapper. This aligns the values files with their upstream Helm chart counterparts for easier side-by-side comparison - Flatten gpu-operator-ocp, network-operator-ocp, and nfd-ocp values from nested structure to top-level keys matching upstream charts - Expand ClusterPolicy, NicClusterPolicy, and NodeFeatureDiscovery CR templates to explicitly render each field instead of - Add template function to pkg/manifest/render.go for Helm compatibility in CR templates - Add DCGM exporter ConfigMap manifest and comprehensive metrics config to gpu-operator-ocp (dcgm-exporter-configmap.yaml) - Remove values-training.yaml — gdrcopy and migManager are now enabled in the base values, eliminating the training-specific override file - Update ocp-training overlay to drop the componentRef that pointed at the removed values-training.yaml - Register dcgm-exporter-configmap.yaml in the ocp overlay manifestFiles Signed-off-by: Shai Kapon <skapon@redhat.com>
Signed-off-by: Shai Kapon <skapon@redhat.com>
|
Caution Review failedAn error occurred during the review process. Please try again later. 📝 WalkthroughWalkthroughThis PR adds OpenShift Container Platform (OCP) as a supported service type across the codebase. A new Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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/design/010-ocp-helm.md`:
- Line 71: Add language identifiers to all unlabeled fenced code blocks to
comply with markdownlint MD040. In docs/design/010-ocp-helm.md at lines 71-71,
add appropriate language tags (such as yaml, go, or text) after the opening
triple backticks for each fenced code block. Similarly, in
docs/integrator/openshift.md at lines 25-25, add language identifiers to all
unlabeled fenced code blocks. Each opening fence should specify the code
language (e.g., ```yaml instead of just ```).
- Line 5: The ADR status in the header line currently states "Proposed —
2026-06-07 (design-only; not implemented)" but this PR is shipping the actual
implementation. Update this status line to change "Proposed" to "Implemented"
(or similar status indicating the feature is complete), remove the
"(design-only; not implemented)" qualifier, and update the date to reflect when
the implementation was completed. This ensures the architecture documentation
accurately reflects the current state of the OCP support feature.
In `@pkg/recipe/doc.go`:
- Line 71: The doc block in pkg/recipe/doc.go has been updated with
CriteriaServiceOCP documentation at one location, but other service enumeration
lists within the same doc block (such as the initial service list and other
enumeration descriptions) still lack the corresponding ocp entry and
description. Update all service enumeration lists and descriptions throughout
the doc block to consistently include CriteriaServiceOCP with its "Red Hat
OpenShift Container Platform" description, ensuring the package documentation
remains internally consistent across all service listings.
In `@recipes/components/gpu-operator-ocp-olm/readiness.yaml`:
- Around line 34-50: The assertions in the readiness checks lack explicit
resource names in their metadata, causing them to match any
ClusterServiceVersion or Deployment in the gpu-operator namespace with the
matching status conditions. This can cause false positives if unrelated
resources satisfy the conditions first. Add metadata.name constraints to both
assertions: add a name field to the ClusterServiceVersion resource metadata
(around line 37) and add a name field to the Deployment resource metadata in the
operator-deployment-available test (around line 47) to ensure the assertions
target only the specific GPU operator resources, not any resource that happens
to match the status conditions.
In `@recipes/components/gpu-operator-ocp/values.yaml`:
- Around line 57-58: The dcgmExporter.config.data section in values.yaml
contains duplicate DCGM metric IDs that will cause initialization failure:
DCGM_FI_PROF_PCIE_TX_BYTES appears at both line 57 and line 96, and
DCGM_FI_PROF_PCIE_RX_BYTES appears at both line 58 and line 97. Remove the
duplicate definitions of these two metric IDs and keep only one canonical
definition per metric. Review the descriptions at both locations to determine
which version is correct (based on whether they are NVML-sourced or DCP metrics)
and retain only that single definition while removing the redundant entries.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: b6b26084-e126-4918-8535-29d8ac6987cb
📒 Files selected for processing (40)
.claude/skills/creating-slide-decks/skeleton.html.github/ISSUE_TEMPLATE/bug_report.ymlapi/aicr/v1/server.yamlcmd/gate/chainsaw-config.yamldocs/contributor/recipe.mddocs/design/010-ocp-helm.mddocs/integrator/openshift.mddocs/user/api-reference.mddocs/user/cli-reference.mdpkg/manifest/render.gopkg/recipe/criteria.gopkg/recipe/criteria_test.gopkg/recipe/doc.gorecipes/checks/gpu-operator-ocp-olm/readiness.yamlrecipes/checks/network-operator-ocp-olm/readiness.yamlrecipes/checks/nfd-ocp-olm/readiness.yamlrecipes/components/gpu-operator-ocp-olm/manifests/operatorgroup.yamlrecipes/components/gpu-operator-ocp-olm/manifests/subscription.yamlrecipes/components/gpu-operator-ocp-olm/readiness.yamlrecipes/components/gpu-operator-ocp-olm/values.yamlrecipes/components/gpu-operator-ocp/manifests/clusterpolicy.yamlrecipes/components/gpu-operator-ocp/manifests/dcgm-exporter-configmap.yamlrecipes/components/gpu-operator-ocp/values.yamlrecipes/components/network-operator-ocp-olm/manifests/operatorgroup.yamlrecipes/components/network-operator-ocp-olm/manifests/subscription.yamlrecipes/components/network-operator-ocp-olm/readiness.yamlrecipes/components/network-operator-ocp-olm/values.yamlrecipes/components/network-operator-ocp/manifests/nicclusterpolicy.yamlrecipes/components/network-operator-ocp/values.yamlrecipes/components/nfd-ocp-olm/manifests/operatorgroup.yamlrecipes/components/nfd-ocp-olm/manifests/subscription.yamlrecipes/components/nfd-ocp-olm/readiness.yamlrecipes/components/nfd-ocp-olm/values.yamlrecipes/components/nfd-ocp/manifests/nodefeaturediscovery.yamlrecipes/components/nfd-ocp/values.yamlrecipes/overlays/ocp-inference.yamlrecipes/overlays/ocp-training.yamlrecipes/overlays/ocp.yamlrecipes/registry.yamlvalidators/performance/nccl_all_reduce_bw_constraint.go
Contributor
|
@kaponco this PR now has merge conflicts with |
unsupported DCGM PCIe metrics Signed-off-by: Shai Kapon <skapon@redhat.com>
…duleType (NVIDIA#566) - Add OPERATOR_NAMESPACE env var to OLM Subscription config so the GPU Operator resolves its install namespace at runtime instead of falling back to a hard-coded value - Add driver.kernelModuleType field to ClusterPolicy template (defaultsto auto) - Add clarifying comment on operatorGroup targetNamespace choice Signed-off-by: Shai Kapon <skapon@redhat.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add OpenShift Container Platform (OCP) as a first-class service type, deploying operators via OLM using the
existing Helm bundler pipeline.
Motivation / Context
AICR currently supports EKS, GKE and more cluster providers. OCP customers need GPU-accelerated recipes but
OCP deploys operators through OLM (Operator Lifecycle Manager) rather than direct Helm installs. This PR
models each OCP operator as two in-tree Helm chart phases — an OLM installer (Subscription + OperatorGroup)
and a CR configuration phase — with Chainsaw readiness gates ensuring operators are available before CRs are
applied.
Fixes: N/A
Related: N/A
Type of Change
Component(s) Affected
cmd/aicr,pkg/cli)cmd/aicrd,pkg/server)pkg/recipe)pkg/bundler,pkg/component/*)pkg/collector,pkg/snapshotter)pkg/validator)pkg/errors,pkg/k8s)docs/,examples/)Implementation Notes
Testing
# Commands run (prefer `make qualify` for non-trivial changes) make qualifyEverything is green — ready to commit.
Risk Assessment
Rollout notes:
Checklist
make testwith-race)make lint)git commit -S) — GPG signing info