fix(podgrouper): skip WorkloadRunner wrapper when selecting the grouping plugin (v0.16 backport) - #2078
Conversation
v0.16 registers the DGD at exact v1alpha1, but Dynamo 1.2.0+ serves and owns it as v1beta1. Required alongside the WorkloadRunner fix: granting podgrouper read access on dynamographdeployments lets the chain walk climb past the DGD instead of truncating there, so a v1beta1 DGD that misses the table now falls to the default grouper and flattens the gang. Matches the widening already on v0.17. Signed-off-by: gshaibi <gshaibi@nvidia.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
📝 Changelog fragment recordedThanks! This PR added the changelog fragment(s) below. Pending fragments are folded into |
Description
Manual backport of #2067 to
v0.16. The automated backport failed to cherry-pick;v0.17succeeded as #2070.Conflicts resolved —
v0.16predates the Karta fallback hub, so the incoming changes had to be adapted:hub.gostruct field isdefaultPlugin, notdefaultGroupingHandler.kartaFallbackPluginassignment and therun.ai/kartasRBAC marker; neither exists on this branch.hub_test.go: dropped the "Generic Karta Fallback Tests" context and theruntime/types/ptrimports it needed. Kept the new "Skip Top Owner Resolution Tests" case and adjusted it tov0.16's 7-argumentNewDefaultPluginsHub(nogenericKartaFallback).Second commit is required, not optional
v0.16registers the DGD at exactv1alpha1;main/v0.17already useVersion: "*"(widened by #1870, which shipped in v0.17.0 and so is absent here).This matters because the backport also grants podgrouper read access on
dynamographdeployments. Today that read 403s, andhandleGetOwnerErrortruncates the chain to PodCliqueSet — the Grove grouper runs and grouping is accidentally correct. Once the grant lands, the walk climbs past the DGD to the WorkloadRunner, skips back down, and asks the table fornvidia.com/v1beta1/DynamoGraphDeployment. On an unwidenedv0.16that misses, falls to the default grouper, and produces the flatminMember: 1this PR is meant to fix — for anyone on Dynamo ≥1.2.0.So cherry-pick alone would regress
v1beta1users. The two commits must land together.Related Issues
Backport of #2067. Related to #856.
Checklist
make changelog(or applied theskip-changeloglabel). Do not editCHANGELOG.mddirectly — pending fragments are folded into it at release time.Breaking Changes
None beyond #2067. Note the behavior change is intentional and coupled to the new RBAC: clusters that were previously grouping correctly because the DGD read was denied will now walk the full chain. That is the point of the fix, and it is why both commits ship together.
Additional Notes
Verified on this branch:
go build ./pkg/podgrouper/...clean, full./pkg/podgrouper/...suite passes,gofmtclean, andmake manifestsreproduces the cherry-pickedpodgrouper.yamlwith no drift (workloadrunnersanddynamographdeploymentsboth present).The changelog fragment came across with the cherry-pick. I did not add a second one for the widening:
v0.16'smake changelogtarget ischangie newwith noKIND/BODYsupport, so it cannot be driven non-interactively here, and hand-writing fragments is against the contributor guide. Happy to add one if a maintainer runs it interactively.Consumers to update after this merges:
runai-enginepins KAI as a submodule (v3.41→v0.16.3) and hand-maintains its own podgrouper ClusterRole, so theworkloadrunnersanddynamographdeploymentsgrants do not propagate from the kubebuilder markers here — they need a separate engine-side change, shipped in the same release as the submodule bump.