Skip to content

Conversation

@LikiosSedo
Copy link
Contributor

@LikiosSedo LikiosSedo commented Oct 30, 2025

Ⅰ. Motivation

Based on community feedback, refine KEP-8 API design and preview/diff tooling for better naming consistency and usability.

Ⅱ. Modifications

  1. API Design: Introduce role.patchTemplate field with mutual exclusivity with role.template, ensuring complete naming consistency with patchLeaderTemplate/patchWorkerTemplate
  2. Preview/Diff Tools: Design kubectl-style commands (kubectl rbg preview, kubectl rbg diff) matching RBG's existing plugin pattern
  3. Update all examples, validation rules, and documentation accordingly

Ⅲ. 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

  • Update the documentation related to the change

@RongGu RongGu requested a review from Copilot October 30, 2025 10:10
@RongGu
Copy link
Collaborator

RongGu commented Oct 30, 2025

@LikiosSedo Please fix the conflict issue.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

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.roleTemplates field for defining reusable base configurations
  • Introduces role.templateRef and role.patchTemplate for 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
Copy link

Copilot AI Oct 30, 2025

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
- /models/Qwen3-32B/
- --enable-dp-attention
- --enable-dp-lm-head
- --enable-deepep-moe
Copy link

Copilot AI Oct 30, 2025

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
  - 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
Copy link
Contributor Author

@LikiosSedo Please fix the conflict issue.

Conflict resolved. Rebased on latest main and ready for review.

@codecov-commenter
Copy link

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.
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.

3 participants