fix(validator): harden Trainer controller toleration handling

[mikecook]

• TestApplyControllerTolerations_Isolation was a false green: unstructured.NestedSlice deep-copies its result, so mutating it never reached the live Deployment object or the shared controllerTolerateAll global. Switch to NestedFieldNoCopy, which returns the same map reference podSpec["tolerations"] actually holds, so the guard can observe aliasing. Verified by temporarily reverting cloneControllerTolerateAll to alias the shared slice: the fixed test fails, confirming it now catches that regression.

• Moved a doc comment that described TestApplyControllerTolerations but sat above the unrelated TestApplyControllerTolerations_Isolation, leaving the former undocumented.

• decodeTrainerObjects's seenControllers warn-loop recorded any Kind=="Deployment" as "seen" regardless of API group, while applyControllerTolerations gates the actual toleration stamp on Group=="apps" too. A Deployment-kind resource in a non-apps group named like a controller would mark that name seen and suppress the warning without the controller ever receiving the toleration - the exact silent-miss the warning exists to catch. Gated the seenControllers write the same way, and extracted the now thrice-repeated "apps" literal into an apiGroupApps const (goconst).

Summary

Harden the Trainer/JobSet controller toleration handling introduced in #2445: fix a false-green isolation test, correct a misplaced doc comment, and close a gap where the controller-missing warning could be silently suppressed.

Motivation / Context

A follow-up look at the toleration-handling code in validators/performance/trainer_lifecycle.go turned up three issues worth fixing before they bite:

1. TestApplyControllerTolerations_Isolation read tolerations back via unstructured.NestedSlice, which deep-copies its result. Mutating that copy can never reach the live Deployment object or the shared controllerTolerateAll global, so the test could not actually detect an aliasing regression — it was a false green.
2. A doc comment describing TestApplyControllerTolerations was misplaced above the unrelated TestApplyControllerTolerations_Isolation, leaving the former undocumented.
3. decodeTrainerObjects's seenControllers tracking (used to warn when an expected controller Deployment is missing) matched on Kind=="Deployment" alone, while applyControllerTolerations additionally gates on Group=="apps". A Deployment-kind resource in a non-apps group named like a controller would mark that name "seen" and suppress the warning, even though it never received the toleration — the exact silent-miss the warning exists to catch.

Fixes: N/A
Related: N/A

Type of Change

• Bug fix (non-breaking change that fixes an issue)
• New feature (non-breaking change that adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation update
• Refactoring (no functional changes)
• Build/CI/tooling

Component(s) Affected

• CLI (cmd/aicr, pkg/cli)
• API server (cmd/aicrd, pkg/server)
• Recipe engine / data (pkg/recipe)
• Bundlers (pkg/bundler, pkg/component/*)
• Collectors / snapshotter (pkg/collector, pkg/snapshotter)
• Validator (pkg/validator)
• Core libraries (pkg/errors, pkg/k8s)
• Docs/examples (docs/, examples/)
• Other: ____________

Implementation Notes

Switched the isolation test from unstructured.NestedSlice to unstructured.NestedFieldNoCopy, which returns the same map reference podSpec["tolerations"] actually holds, so mutating it in place can observe aliasing. Verified this closes the gap by temporarily reverting cloneControllerTolerateAll to alias the shared slice: the fixed test failed as expected, confirming it now catches the regression it's meant to guard, then restored the correct code.

Gated the seenControllers write in decodeTrainerObjects on Group=="apps" in addition to Kind=="Deployment", matching applyControllerTolerations's existing gate. This pushed the "apps" string literal to a third non-test occurrence, tripping golangci-lint's goconst threshold, so extracted it into an apiGroupApps constant in consts.go and reused it at all three call sites.

Testing

make qualify

make qualify passed clean:

• make test-coverage: all packages pass with -race
• make lint (golangci-lint + yamllint): clean, 0 issues
• make tuning-check: clean
• make e2e (chainsaw, --no-cluster): 24/24 passed, 0 failed, 0 skipped
• make scan (grype): no new vulnerabilities introduced by this change
• make license-check: clean
• make api-diff: no incompatible SDK facade / transparent-alias changes since v0.20.0

Also manually verified the isolation-test fix by temporarily reintroducing the aliasing bug it guards against (reverting cloneControllerTolerateAll to return the shared slice) and confirming TestApplyControllerTolerations_Isolation fails, then restored the fix.

Risk Assessment

• Low — Isolated change, well-tested, easy to revert
• Medium — Touches multiple components or has broader impact
• High — Breaking change, affects critical paths, or complex rollout

Rollout notes: N/A — test-only and internal-validator-logic changes; no recipe/API/CLI surface change.

Checklist

• Tests pass locally (make test with -race)
• Linter passes (make lint)
• I did not skip/disable tests to make CI green
• I added/updated tests for new functionality
• I updated docs if user-facing behavior changed
• Changes follow existing patterns in the codebase
• Commits are cryptographically signed (git commit -S) — GPG signing info

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 722f130b-b8dc-4d84-b51d-ebe142515acc

📥 Commits

Reviewing files that changed from the base of the PR and between a792a9a and d292d8d.

📒 Files selected for processing (3)

• validators/performance/consts.go
• validators/performance/trainer_lifecycle.go
• validators/performance/trainer_lifecycle_test.go

Included review availability: Your plan provides up to 12 included reviews per hour; 8 remain after this review.

---

📝 Walkthrough

Walkthrough

The performance validator adds a shared apiGroupApps constant. Deployment toleration handling, Deployment GVR resolution, and controller tracking now use this constant and require the Apps API group. The toleration isolation test reads live toleration fields with NestedFieldNoCopy through a new helper. The test also validates missing, malformed, and empty toleration data.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to d292d

This PR hardens controller toleration handling and its regression coverage without changing public interfaces; no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: almaslennikov

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly and concisely summarizes the validator changes to Trainer controller toleration handling.
Description check	✅ Passed	The description directly explains the test fix, API group filtering, shared constant, testing, and risk assessment for the changeset.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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