Skip to content

api: add coordination api for rbg #84

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

api: add coordination api for rbg #84

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions api/workloads/v1alpha1/rolebasedgroup_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,44 @@ type RoleBasedGroupSpec struct {

// Configuration for the PodGroup to enable gang-scheduling via supported plugins.
PodGroupPolicy *PodGroupPolicy `json:"podGroupPolicy,omitempty"`

// Coordination describes the requirements of coordination strategies for some specified roles.
// +patchMergeKey=name
// +patchStrategy=merge
// +listType=map
// +listMapKey=name
Coordination []Coordination `json:"coordination,omitempty" patchStrategy:"merge" patchMergeKey:"name"`
}

// Coordination describes the requirements of coordination strategies for roles.
type Coordination struct {
// Name of the coordination.
Name string `json:"name"`

// Roles that should be constrained by this coordination.
Comment on lines +52 to +55
Copy link

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

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

The Coordination struct has required fields Name and Roles but lacks kubebuilder validation markers (e.g., +kubebuilder:validation:Required, +kubebuilder:validation:MinItems=1 for Roles). Consider adding these to ensure proper validation at the API level.

Suggested change
// Name of the coordination.
Name string `json:"name"`
// Roles that should be constrained by this coordination.
// Name of the coordination.
// +kubebuilder:validation:Required
Name string `json:"name"`
// Roles that should be constrained by this coordination.
// +kubebuilder:validation:Required
// +kubebuilder:validation:MinItems=1

Copilot uses AI. Check for mistakes.
Roles []string `json:"roles"`

// RolloutStrategy describes the coordination strategy for rolling update.
RolloutStrategy *CoordinationRolloutStrategy `json:"rolloutStrategy,omitempty"`
}

// CoordinationRolloutStrategy describes the rolling update coordination strategy.
type CoordinationRolloutStrategy struct {
// MaxSkew defines the max skew requirement about updated replicas between the roles when rolling update.
// For example, one RoleBasedGroup with (200 prefills, 100 decodes) will have the
// constraint `abs(updated_prefills/200, updated_decodes/100) <= MaxSkew`.
// Only support percentage value, and defaults to nil.
MaxSkew *intstr.IntOrString `json:"maxSkew,omitempty"`

// Partition indicates the replicas at which the role should be partitioned for rolling update.
// If Partition is not nil, the Partition of the roles' rolloutStrategy will be overridden by this field.
// Only support percentage value, and defaults to nil.
Partition *intstr.IntOrString `json:"partition,omitempty"`

// MaxUnavailable defines the updating step during rolling. If MaxUnavailable is not nil,
// the MaxUnavailable of the roles' rolloutStrategy will be overridden by this field.
// Only support percentage value, and defaults to nil.
Comment on lines +67 to +77
Copy link

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

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

The comment states 'Only support percentage value' but the field type is *intstr.IntOrString which accepts both integer and string values. Consider adding validation (e.g., +kubebuilder:validation:Pattern) to enforce percentage format if only percentages are truly supported, or update the documentation to clarify what formats are accepted.

Suggested change
// Only support percentage value, and defaults to nil.
MaxSkew *intstr.IntOrString `json:"maxSkew,omitempty"`
// Partition indicates the replicas at which the role should be partitioned for rolling update.
// If Partition is not nil, the Partition of the roles' rolloutStrategy will be overridden by this field.
// Only support percentage value, and defaults to nil.
Partition *intstr.IntOrString `json:"partition,omitempty"`
// MaxUnavailable defines the updating step during rolling. If MaxUnavailable is not nil,
// the MaxUnavailable of the roles' rolloutStrategy will be overridden by this field.
// Only support percentage value, and defaults to nil.
// Only support percentage value, and defaults to nil.
// +kubebuilder:validation:Pattern=`^[0-9]+%$`
MaxSkew *intstr.IntOrString `json:"maxSkew,omitempty"`
// Partition indicates the replicas at which the role should be partitioned for rolling update.
// If Partition is not nil, the Partition of the roles' rolloutStrategy will be overridden by this field.
// Only support percentage value, and defaults to nil.
// +kubebuilder:validation:Pattern=`^[0-9]+%$`
Partition *intstr.IntOrString `json:"partition,omitempty"`
// MaxUnavailable defines the updating step during rolling. If MaxUnavailable is not nil,
// the MaxUnavailable of the roles' rolloutStrategy will be overridden by this field.
// Only support percentage value, and defaults to nil.
// +kubebuilder:validation:Pattern=`^[0-9]+%$`

Copilot uses AI. Check for mistakes.
Comment on lines +67 to +77
Copy link

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

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

The comment states 'Only support percentage value' but the field type is *intstr.IntOrString which accepts both integer and string values. Consider adding validation (e.g., +kubebuilder:validation:Pattern) to enforce percentage format if only percentages are truly supported, or update the documentation to clarify what formats are accepted.

Suggested change
// Only support percentage value, and defaults to nil.
MaxSkew *intstr.IntOrString `json:"maxSkew,omitempty"`
// Partition indicates the replicas at which the role should be partitioned for rolling update.
// If Partition is not nil, the Partition of the roles' rolloutStrategy will be overridden by this field.
// Only support percentage value, and defaults to nil.
Partition *intstr.IntOrString `json:"partition,omitempty"`
// MaxUnavailable defines the updating step during rolling. If MaxUnavailable is not nil,
// the MaxUnavailable of the roles' rolloutStrategy will be overridden by this field.
// Only support percentage value, and defaults to nil.
// Only support percentage value, and defaults to nil.
// +kubebuilder:validation:Pattern=`^\d+%$`
MaxSkew *intstr.IntOrString `json:"maxSkew,omitempty"`
// Partition indicates the replicas at which the role should be partitioned for rolling update.
// If Partition is not nil, the Partition of the roles' rolloutStrategy will be overridden by this field.
// Only support percentage value, and defaults to nil.
// +kubebuilder:validation:Pattern=`^\d+%$`
Partition *intstr.IntOrString `json:"partition,omitempty"`
// MaxUnavailable defines the updating step during rolling. If MaxUnavailable is not nil,
// the MaxUnavailable of the roles' rolloutStrategy will be overridden by this field.
// Only support percentage value, and defaults to nil.
// +kubebuilder:validation:Pattern=`^\d+%$`

Copilot uses AI. Check for mistakes.
Comment on lines +67 to +77
Copy link

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

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

The comment states 'Only support percentage value' but the field type is *intstr.IntOrString which accepts both integer and string values. Consider adding validation (e.g., +kubebuilder:validation:Pattern) to enforce percentage format if only percentages are truly supported, or update the documentation to clarify what formats are accepted.

Suggested change
// Only support percentage value, and defaults to nil.
MaxSkew *intstr.IntOrString `json:"maxSkew,omitempty"`
// Partition indicates the replicas at which the role should be partitioned for rolling update.
// If Partition is not nil, the Partition of the roles' rolloutStrategy will be overridden by this field.
// Only support percentage value, and defaults to nil.
Partition *intstr.IntOrString `json:"partition,omitempty"`
// MaxUnavailable defines the updating step during rolling. If MaxUnavailable is not nil,
// the MaxUnavailable of the roles' rolloutStrategy will be overridden by this field.
// Only support percentage value, and defaults to nil.
// Only support percentage value, and defaults to nil.
// +kubebuilder:validation:Pattern=^\d+%$
MaxSkew *intstr.IntOrString `json:"maxSkew,omitempty"`
// Partition indicates the replicas at which the role should be partitioned for rolling update.
// If Partition is not nil, the Partition of the roles' rolloutStrategy will be overridden by this field.
// Only support percentage value, and defaults to nil.
// +kubebuilder:validation:Pattern=^\d+%$
Partition *intstr.IntOrString `json:"partition,omitempty"`
// MaxUnavailable defines the updating step during rolling. If MaxUnavailable is not nil,
// the MaxUnavailable of the roles' rolloutStrategy will be overridden by this field.
// Only support percentage value, and defaults to nil.
// +kubebuilder:validation:Pattern=^\d+%$

Copilot uses AI. Check for mistakes.
MaxUnavailable *intstr.IntOrString `json:"maxUnavailable,omitempty"`
}

// PodGroupPolicy represents a PodGroup configuration for gang-scheduling.
Expand Down
62 changes: 62 additions & 0 deletions api/workloads/v1alpha1/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions client-go/applyconfiguration/utils.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

58 changes: 58 additions & 0 deletions client-go/applyconfiguration/workloads/v1alpha1/coordination.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

60 changes: 60 additions & 0 deletions client-go/applyconfiguration/workloads/v1alpha1/coordinationrolloutstrategy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 14 additions & 0 deletions client-go/applyconfiguration/workloads/v1alpha1/rolebasedgroupspec.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

51 changes: 51 additions & 0 deletions config/crd/bases/workloads.x-k8s.io_rolebasedgroups.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,57 @@ spec:
spec:
description: RoleBasedGroupSpec defines the desired state of RoleBasedGroup.
properties:
coordination:
description: Coordination describes the requirements of coordination
strategies for some specified roles.
items:
description: Coordination describes the requirements of coordination
strategies for roles.
properties:
name:
description: Name of the coordination.
type: string
roles:
description: Roles that should be constrained by this coordination.
items:
type: string
type: array
rolloutStrategy:
description: RolloutStrategy describes the coordination strategy
for rolling update.
properties:
maxSkew:
anyOf:
- type: integer
- type: string
description: MaxSkew defines the max skew requirement about
updated replicas between the roles when rolling update.
x-kubernetes-int-or-string: true
maxUnavailable:
anyOf:
- type: integer
- type: string
description: |-
MaxUnavailable defines the updating step during rolling. If MaxUnavailable is not nil,
the MaxUnavailable of the roles' rolloutStrategy will be overridden by this field.
x-kubernetes-int-or-string: true
partition:
anyOf:
- type: integer
- type: string
description: |-
Partition indicates the replicas at which the role should be partitioned for rolling update.
If Partition is not nil, the Partition of the roles' rolloutStrategy will be overridden by this field.
x-kubernetes-int-or-string: true
type: object
required:
- name
- roles
type: object
type: array
x-kubernetes-list-map-keys:
- name
x-kubernetes-list-type: map
podGroupPolicy:
description: Configuration for the PodGroup to enable gang-scheduling
via supported plugins.
Expand Down
52 changes: 52 additions & 0 deletions config/crd/bases/workloads.x-k8s.io_rolebasedgroupsets.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,58 @@ spec:
template:
description: Template describes the RoleBasedGroup that will be created.
properties:
coordination:
description: Coordination describes the requirements of coordination
strategies for some specified roles.
items:
description: Coordination describes the requirements of coordination
strategies for roles.
properties:
name:
description: Name of the coordination.
type: string
roles:
description: Roles that should be constrained by this coordination.
items:
type: string
type: array
rolloutStrategy:
description: RolloutStrategy describes the coordination
strategy for rolling update.
properties:
maxSkew:
anyOf:
- type: integer
- type: string
description: MaxSkew defines the max skew requirement
about updated replicas between the roles when rolling
update.
x-kubernetes-int-or-string: true
maxUnavailable:
anyOf:
- type: integer
- type: string
description: |-
MaxUnavailable defines the updating step during rolling. If MaxUnavailable is not nil,
the MaxUnavailable of the roles' rolloutStrategy will be overridden by this field.
x-kubernetes-int-or-string: true
partition:
anyOf:
- type: integer
- type: string
description: |-
Partition indicates the replicas at which the role should be partitioned for rolling update.
If Partition is not nil, the Partition of the roles' rolloutStrategy will be overridden by this field.
x-kubernetes-int-or-string: true
type: object
required:
- name
- roles
type: object
type: array
x-kubernetes-list-map-keys:
- name
x-kubernetes-list-type: map
podGroupPolicy:
description: Configuration for the PodGroup to enable gang-scheduling
via supported plugins.
Expand Down
Loading
Loading