Fix legacy HWP tolerations not injected into ISVCs

[nananosirova]

https://issues.redhat.com/browse/RHOAIENG-50153

Description

The dashboard controller isn't injecting tolerations from legacy Hardware Profiles (HWPs) into ISVCs in RHOAI 2.25. When creating an ISVC using a legacy HWP, the tolerations are not applied to the ISVC predictor spec.

Root cause: In assembleInferenceService(), the condition if (!podSpecOptions.selectedHardwareProfile) skips injecting tolerations whenever any hardware profile is selected. This is correct for native HWPs (where the backend controller handles injection via the opendatahub.io/hardware-profile-name annotation), but wrong for legacy HWPs; there is no backend controller that injects tolerations for legacy profiles.

Fix: Added a legacy HWP detection check based on the absence of metadata.uid. Legacy HWPs are always in-memory objects (created by useMigratedHardwareProfiles), never persisted to the K8s API server, so they never have a uid. When a legacy HWP is detected, tolerations are injected directly into the ISVC predictor spec.

How Has This Been Tested?

Manual testing:

1. Ensure an AcceleratorProfile exists on the cluster (this will appear as a legacy HWP in the dashboard)
2. Go to Model Serving and deploy a model using KServe single-model serving
3. Select the legacy hardware profile
4. Deploy the model
5. Verify the resulting ISVC has the expected tolerations in spec.predictor.tolerations by running:

   oc get inferenceservice <name> -n <namespace> -o jsonpath='{.spec.predictor.tolerations}'
   

Test Impact

• 1 new unit test added covering the legacy HWP toleration injection scenario

Request review criteria:

Self checklist (all need to be checked):

• The developer has manually tested the changes and verified that the changes work
• Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
• The developer has added tests or explained why testing cannot be added (unit or cypress tests for related changes)
• The code follows our Best Practices (React coding standards, PatternFly usage, performance considerations)

After the PR is posted & before it merges:

• The developer has tested their solution on a cluster by using the image produced by the PR to main

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 714cb0f9-9e63-4795-8ac8-b2506d541350

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 67.77%. Comparing base (98cf02e) to head (28832b5).
⚠️ Report is 2 commits behind head on stable-2.x.

Additional details and impacted files

@@              Coverage Diff               @@
##           stable-2.x    #6892      +/-   ##
==============================================
- Coverage       67.93%   67.77%   -0.16%     
==============================================
  Files            2238     2238              
  Lines           50847    50851       +4     
  Branches        14252    14255       +3     
==============================================
- Hits            34541    34464      -77     
- Misses          16306    16387      +81     

Files with missing lines	Coverage Δ
frontend/src/api/k8s/inferenceServices.ts	95.03% <100.00%> (+2.12%)	⬆️
frontend/src/concepts/hardwareProfiles/utils.ts	92.30% <100.00%> (+0.35%)	⬆️

... and 19 files with indirect coverage changes

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 98cf02e...28832b5. Read the comment docs.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[DaoDaoNoCode]

Good targeted fix for the legacy HWP toleration injection bug. The root cause analysis is solid and the fix correctly addresses the issue by detecting legacy HWPs via missing metadata.uid. All CI checks pass (the E2E failure on dash-e2e-int appears pre-existing).

Left a couple of comments inline -- the main thing is an inconsistency between the two isLegacyHardwareProfile definitions in assembleInferenceService, and a few suggestions on the test coverage.

[DaoDaoNoCode]

This definition of isLegacyHardwareProfile only checks !metadata.uid, but the other one up at line 151-153 uses !!selectedAcceleratorProfile || !metadata.uid. In practice they produce the same result because the runtime hook (useModelServingPodSpecOptionsState) never sets both selectedAcceleratorProfile and selectedHardwareProfile at the same time. But it's confusing for future readers and fragile if that invariant ever changes.

Worth aligning the two -- either extract a shared helper or at least use the same condition in both places.

nit: not blocking since it's correct today, but would prevent a subtle future bug.

[DaoDaoNoCode]

The test verifies tolerations are injected for legacy HWPs, which is the core fix. Two additional things worth covering:

1. nodeSelector injection -- the test passes nodeSelector: undefined, so the nodeSelector path in the fix (line 240-242 in the source) is never exercised for the legacy case. Adding a non-undefined nodeSelector and asserting it appears on the result would cover the full fix.

2. legacy-hardware-profile-name annotation -- the annotation block (lines 158-161 in the source) sets opendatahub.io/legacy-hardware-profile-name for legacy HWPs, which is how the serving config remembers which legacy profile was used. An assertion on that annotation here would give confidence the full legacy path works end-to-end.

Neither is blocking, but they'd make the test more robust.

[DaoDaoNoCode]

All three comments from the first review are addressed. The isLegacyHardwareProfile inconsistency is resolved by extracting a shared isLegacyHardwareProfileSelected helper in utils.ts that both call sites now use. The delete cast has a clear explanatory comment. The test now covers nodeSelector injection and asserts the legacy-hardware-profile-name annotation value. Unit tests pass locally (85/85). The Cypress failure (ClusterStorage - Add cluster storage with multiple workbench connections) is unrelated to this PR.

Local checks: lint pass, type-check pass, unit tests pass (5 suites, 85 tests)

[DaoDaoNoCode]

I can see the tolerations after fix. Nice job!

Previously:

[DaoDaoNoCode]

/lgtm

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: DaoDaoNoCode

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [DaoDaoNoCode]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment