KEP-8: Refine API naming and preview/diff design based on community feedback#79
Conversation
|
@LikiosSedo Please fix the conflict issue. |
There was a problem hiding this comment.
Pull Request Overview
This PR introduces KEP-8, which proposes a RoleTemplate feature to reduce YAML duplication in RoleBasedGroup specifications. The proposal enables platform teams to define reusable Pod configurations that multiple roles can reference and patch, reducing configuration duplication by approximately 20%.
Key changes:
- Adds
spec.roleTemplatesfield for defining reusable base configurations - Introduces
role.templateRefandrole.patchTemplatefor referencing and customizing templates - Leverages Kubernetes Strategic Merge Patch for configuration merging with clear semantics
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| keps/8-reduce-yaml-duplication/kep.yaml | Metadata file defining KEP-8 with authors, reviewers, approvers, and milestone targets |
| keps/8-reduce-yaml-duplication/README.md | Complete KEP specification including motivation, design details, API examples, controller behavior, test plan, and implementation history |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - /models/Qwen3-32B/ | ||
| - --enable-dp-attention | ||
| - --enable-dp-lm-head | ||
| - --enable-deepep-moe |
There was a problem hiding this comment.
Likely typo in flag name: 'deepep' should probably be 'deepspeed' or similar. This appears to be '--enable-deepspeed-moe' or '--enable-deep-moe'. Please verify the correct SGlang flag name.
| - /models/Qwen3-32B/ | ||
| - --enable-dp-attention | ||
| - --enable-dp-lm-head | ||
| - --enable-deepep-moe |
There was a problem hiding this comment.
Likely typo in flag name: 'deepep' should probably be 'deepspeed' or similar. This appears to be '--enable-deepspeed-moe' or '--enable-deep-moe'. Please verify the correct SGlang flag name.
- Introduce role.patch field for clear semantic distinction - Add Preview/Diff tools for merge visualization - Update API examples: roleTemplate.template → roleTemplate.spec, role.template → role.patch - Add validation rules for template XOR patch
LikiosSedo
force-pushed
the
kep-8-roletemplates
branch
from
7b1244a to
2484501
Compare
Conflict resolved. Rebased on latest main and ready for review. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Introduce roleTemplates to eliminate config duplication across roles. Uses templatePatch field with Strategic Merge Patch semantics.
LikiosSedo
force-pushed
the
kep-8-roletemplates
branch
from
1c22a31 to
ad2888f
Compare
|
/lgtm |
Ⅰ. Motivation
Based on community feedback, refine KEP-8 API design and preview/diff tooling for better naming consistency and usability.
Ⅱ. Modifications
role.patchTemplatefield with mutual exclusivity withrole.template, ensuring complete naming consistency withpatchLeaderTemplate/patchWorkerTemplatekubectl rbg preview,kubectl rbg diff) matching RBG's existing plugin patternⅢ. Does this pull request fix one issue?
NONE
Ⅳ. List the added test cases (unit test/integration test) if any, please explain if no tests are needed.
No new tests - KEP design document refinement. Test plan updated for future implementation.
Ⅴ. Describe how to verify it
Review KEP document for API design completeness and naming consistency across all sections.
Checklist