Skip to content

feat: migrate to Helm charts, add LLMInferenceService, and support deploy on existing OpenShift#4

Merged
itay1551 merged 27 commits into
mainfrom
feat/helm-chart-migration
Jul 6, 2026
Merged

feat: migrate to Helm charts, add LLMInferenceService, and support deploy on existing OpenShift#4
itay1551 merged 27 commits into
mainfrom
feat/helm-chart-migration

Conversation

@itay1551

@itay1551 itay1551 commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

Summary

This PR restructures the entire deployment stack and adds a new deployment mode:

  • Migrate ArgoCD manifests to Helm charts — both ARO and ROSA deployments now use proper Helm charts (pca-operators, pca-platform-config, pca-ai-serving, pca-devspaces) with templated values, replacing the raw ArgoCD YAML manifests.
  • Replace InferenceService workaround with LLMInferenceService — migrates from the manual ServingRuntime + InferenceService + Gateway + TLS Job workaround to the native OpenShift AI LLMInferenceService CRD (serving.kserve.io/v1alpha2), which automatically manages vLLM runtime, EPP router, InferencePool, HTTPRoute, and TLS.
  • Add local deployment on existing OpenShift — new make deploy / make undeploy targets deploy the AI serving stack on any existing OpenShift cluster via Helm, without needing Terraform provisioning or ArgoCD. Values overrides in deploy_existing_openshift/ disable cluster-scoped resources that already exist.
  • Upgrade RHOAI from 2.19 to 3.4 and GPU Operator from v24.9 to v25.3 for LLMInferenceService support and CUDA 12.9+ driver compatibility.
  • Add missing operators — NFD, LeaderWorkerSet, and cert-manager subscriptions added to the operators chart.
  • Fix pre-commit CI — exclude Helm templates/ from yamllint/yamlfmt (Go template syntax is not valid YAML).

Test plan

  • helm template renders all charts without errors
  • LLMInferenceService deployed on ROSA cluster via make run
  • vLLM pod running on GPU node, model loaded and serving inference
  • /v1/models and /v1/chat/completions return valid responses
  • Pre-commit hooks pass locally and in CI
  • Full Gateway routing path (Gateway → EPP → vLLM) once cluster has CPU headroom for the router

@itay1551 itay1551 marked this pull request as draft July 2, 2026 12:24
@itay1551 itay1551 force-pushed the feat/helm-chart-migration branch from 5b33860 to 7101c4d Compare July 2, 2026 13:27
itay1551 added 5 commits July 2, 2026 16:29
Move raw YAML manifests from argocd/ wave directories into Helm chart
structures under charts/. Only REPLACE_WITH_* placeholders and hardcoded
values are converted to Helm template expressions — all other manifests
remain unchanged.

Changes:
- ROSA: 5 charts (app-of-apps, operators, platform-config, ai-serving, devspaces)
- ARO: 6 charts (same + benchmarks)
- Escape Go template syntax in llminferenceservice.yaml for Helm compatibility
- Update Terraform gitops-bootstrap.tf: directory source -> helm source
- Add gitops_charts_path variable to both platforms
LeaderWorkerSetOperator and ClusterPolicy CRs depend on CRDs that are
installed by operators in wave 1. Moving them to wave 2 (platform-config)
ensures the CRDs exist before the CRs are applied.

Also adds required spec.daemonsets field to nvidia ClusterPolicy for
GPU operator v24.9+ compatibility.
…adlock

- cert-manager and leader-worker-set ArgoCD Applications referenced
  non-existent repo (llm-d/llm-d-playbook). Replace with OLM
  Subscriptions from Red Hat Operators catalog.
- Move model-cache PVC to sync-wave 2 (same as LLMInferenceService) to
  avoid deadlock: WaitForFirstConsumer PVC stays Pending in wave 0,
  blocking the pod that would trigger binding in wave 2.
….io/v1

The Red Hat LWS operator CRD uses operator.openshift.io group, not
leaderworkerset.x-k8s.io as originally assumed from the llm-d playbook.
- Add DSCInitialization template with serviceMesh.managementState=Unmanaged.
  RHOAI 2.19 defaults to Managed which requires SM 2.x, but our cluster
  uses SM 3.x + Gateway API. Setting Unmanaged unblocks KServe deployment.
- Fix HardwareProfile apiVersion from infrastructure.opendatahub.io/v1
  to dashboard.opendatahub.io/v1alpha1 (matching the actual CRD).
@itay1551 itay1551 force-pushed the feat/helm-chart-migration branch from 7101c4d to 7fe7804 Compare July 2, 2026 13:31
itay1551 added 16 commits July 2, 2026 16:32
…hema

- RHOAI 2.19 KServe requires serviceMesh.managementState=Managed with
  SM 2.x. Switch subscription from servicemeshoperator3 to
  servicemeshoperator.
- Revert DSCI to Managed with full SM control plane config.
- Fix HardwareProfile: use nodeSelector/tolerations at spec level
  (v1alpha1 schema has no scheduling field).
The dashboard.opendatahub.io/v1alpha1 HardwareProfile CRD requires
spec.displayName and spec.enabled fields.
The HardwareProfile was incorrectly defined for trn1.32xlarge (Trainium)
which doesn't exist in Terraform. Replace with g6e.2xlarge matching the
GPU machine pool: NVIDIA L40S, taint nvidia.com/gpu, 8 vCPU / 32 GiB.
- Change rhods-operator subscription channel from stable-2.19 to
  stable-3.4.
- Remove Service Mesh 2.x subscription (RHOAI 3.4 requires SM v2 to
  NOT be installed).
- Update DSC to v2 API with wva.managementState=Managed for llm-d
  workload variant autoscaler.
- Set DSCI serviceMesh.managementState=Removed.

RHOAI 3.4 includes native LLMInferenceService CRD and Gateway API
inference extension.
…mode

RHOAI 3.4 with rawDeploymentServiceConfig: Headed uses raw K8s
deployments, not KNative. Removing frees ~15 pods of CPU request
capacity needed to schedule the RHOAI 3.4 operator.
RHOAI 3.4 provides LLMInferenceService through the modelsAsService
component under kserve. Without it the CRD is never registered.
…ctivity Link

Red Hat Connectivity Link (Kuadrant/Authorino) is not available on ROSA,
blocking LLMInferenceService. Switch to standard KServe InferenceService
with raw deployment mode which works immediately.

Changes:
- Replace LLMInferenceService with InferenceService (v1beta1)
- Add vLLM NVIDIA ServingRuntime
- Fix HardwareProfile to infrastructure.opendatahub.io/v1 API
- Simplify HTTPRoute to point directly to predictor service
- Add maas-default-gateway to pca-platform-config
- Add TODO.md documenting the Connectivity Link dependency
PostSync prevents the job from blocking sync. BeforeHookCreation
deletes the old job before re-creating on next sync.
The NVIDIA GPU operator v24.9 on OpenShift doesn't bundle NFD.
Without NFD, nodes aren't labeled with nvidia.com/gpu.present=true
and DaemonSets (driver, device-plugin) never deploy.
Migrate from the manual InferenceService + custom ServingRuntime + Gateway
workaround to the proper OpenShift AI LLMInferenceService CRD which bundles
vLLM runtime, EPP router, and Gateway API routing automatically.

Changes:
- Add LLMInferenceService (serving.kserve.io/v1alpha2) for Qwen3-Coder-30B
- Delete obsolete workaround templates (vllm-nvidia-runtime, inferenceservice,
  llm-d-gateway, tls-secret-job)
- Upgrade GPU Operator subscription channel from v24.9 to v25.3
- Add Makefile deploy/undeploy targets for existing OpenShift clusters
- Add deploy_existing_openshift/ values overrides for helm-based deploys
- Update TODO.md to reflect deployed status
- Update README directory tree

Tested: LLMInferenceService deployed on ROSA cluster, vLLM pod running,
model loaded and serving inference successfully.
@itay1551 itay1551 changed the title feat: migrate ArgoCD manifests to Helm charts feat: deploy vLLM via LLMInferenceService (llm-d) with Helm charts Jul 2, 2026
@itay1551 itay1551 changed the title feat: deploy vLLM via LLMInferenceService (llm-d) with Helm charts feat: Migrate to Helm charts and add Openshift deployment without provisioning Jul 2, 2026
Helm templates contain Go template syntax ({{ }}) which is not valid
YAML, causing yamllint and yamlfmt to fail on every CI run.
@itay1551 itay1551 changed the title feat: Migrate to Helm charts and add Openshift deployment without provisioning feat: migrate to Helm charts, add LLMInferenceService, and support deploy on existing OpenShift Jul 2, 2026
@itay1551 itay1551 marked this pull request as ready for review July 2, 2026 19:34
@itay1551 itay1551 requested a review from Hadar301 July 2, 2026 19:34
@itay1551

itay1551 commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator Author

@Hadar301 This is ready for review,
This is working on ROSA and existing OpenShift for example I tested it on dev-2 .
Probably has some small bugs; we'll retest it when we're 98% done.

Just read the description to understand what is happending in high level.

Thanks!

Comment thread .gitignore
.terraform.lock.hcl
*.tfplan
tfplan
scripts/

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be PCA_Deployment_*/scripts/? in case we'll have scripts/ in the future

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was experimenting with local scripts, if someone needs this in the future the person who needs it will remove it.

Comment thread Makefile Outdated
--namespace $(NAMESPACE) --create-namespace \
-f $(DEPLOY_VALUES_DIR)/values-platform-config.yaml \
--set namespace=$(NAMESPACE) \
--set hfToken.data=$$(echo -n '$(HF_TOKEN)' | base64)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recommendation:
Use the raw token in the Makefile and let the Helm template do the encoding.
Change the Secret template to token: {{ .Values.hfToken.raw | b64enc }} and pass
--set hfToken.raw=$(HF_TOKEN) without the shell base64.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If HF_TOKEN contains characters like +, /, or =, the shell expansion inside $$() may break (apparently it's an edge case, but I still think that the suggestion to convert to base64 in helm would be better)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, good catch, I will update it.
Thanks!

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it an empty file?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, will be used for future values,
What do you think, you perfer it empty, or should I just remove the file?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can keep it, maybe add a comment that states that purpose of the file

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here: Is it an empty file?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above^

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The old LLMInferenceService.yaml mounted a 100Gi PVC (model-cache) via kserve-provision-location for model weight caching. This is missing here — without it, the model gets re-downloaded from HuggingFace on every pod restart (7-13 min cold start vs 3-5 min with PVC). Is this intentional?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added it back, it was causing me some problems for different ArgoCD waves, I think I got it right right now

Hadar301
Hadar301 previously approved these changes Jul 5, 2026

@Hadar301 Hadar301 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Added a few comments with suggestions.

Hadar301 added a commit to Hadar301/private-coding-assistant that referenced this pull request Jul 6, 2026
Restructure PCA_Deployment_ARO from raw ArgoCD YAML manifests into six
Helm charts under charts/, matching the pattern from PR rh-ai-quickstart#4. Templatize
repo URL, target revision, chart paths, and HF token with Helm values.
Update Terraform gitops-bootstrap to pass Helm parameters instead of
using directory mode.

Operator changes:
- Replace dead llm-d-playbook ArgoCD Applications with OLM Subscriptions
  for cert-manager and LeaderWorkerSet
- Remove Service Mesh and Serverless operators (not needed with RHOAI 3.x)
- Add Node Feature Discovery operator (GPU Operator dependency)
- Remove invalid lws-operator-cr.yaml

Platform changes:
- Upgrade DataScienceCluster to v2 API with RHOAI 3.x schema
- Fix HardwareProfile to use scheduling.node (replaces nodeSelectors)
- Remove useOpenShiftDriverToolkit from ClusterPolicy
- Prefix DevWorkspace template with underscore (reference only)

Root changes:
- Add make deploy/undeploy targets for existing OpenShift clusters
- Exclude Helm templates from yamllint/yamlfmt
- Add deploy_existing_openshift/ values overrides

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
itay1551 added 4 commits July 6, 2026 14:34
Rename hfToken.data → hfToken.raw across ARO/ROSA charts, Terraform,
and Makefile. Templates now apply b64enc so callers pass plaintext
instead of pre-encoded base64. Default placeholders changed from
CHANGE_ME to __overridden_at_deploy_time__.
…uster deploy

Gate HardwareProfiles, Neuron runtime template, MaaS gateway, and NFD
instance behind clusterResources (default true). Add a model-cache PVC
template and mount it in LLMInferenceService. The existing-cluster
values override sets clusterResources: false.
@itay1551

itay1551 commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator Author

@Hadar301 I updated the PR with your suggestions,
In addition, I was adding a skill to deploy on an existing OpenShift cluster, the local deployment needs improvments,

That's going to be my next PR.

@Hadar301 Hadar301 self-requested a review July 6, 2026 11:47
@itay1551 itay1551 merged commit b531d38 into main Jul 6, 2026
1 check passed
@itay1551 itay1551 deleted the feat/helm-chart-migration branch July 6, 2026 11:48
Hadar301 added a commit to Hadar301/private-coding-assistant that referenced this pull request Jul 6, 2026
Restructure PCA_Deployment_ARO from raw ArgoCD YAML manifests into six
Helm charts under charts/, matching the pattern from PR rh-ai-quickstart#4. Templatize
repo URL, target revision, chart paths, and HF token with Helm values.
Update Terraform gitops-bootstrap to pass Helm parameters instead of
using directory mode.

Operator changes:
- Replace dead llm-d-playbook ArgoCD Applications with OLM Subscriptions
  for cert-manager and LeaderWorkerSet
- Remove Service Mesh and Serverless operators (not needed with RHOAI 3.x)
- Add Node Feature Discovery operator (GPU Operator dependency)
- Remove invalid lws-operator-cr.yaml

Platform changes:
- Upgrade DataScienceCluster to v2 API with RHOAI 3.x schema
- Fix HardwareProfile to use scheduling.node (replaces nodeSelectors)
- Remove useOpenShiftDriverToolkit from ClusterPolicy
- Prefix DevWorkspace template with underscore (reference only)

Root changes:
- Add make deploy/undeploy targets for existing OpenShift clusters
- Exclude Helm templates from yamllint/yamlfmt
- Add deploy_existing_openshift/ values overrides

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants