Skip to content

Commit d72e59d

Browse files
committed
fix(api): allow PodGroup minMember of 0 (#1989)
Adapted backport: MinMember is a non-pointer int32 on this branch, so unset and explicit 0 are indistinguishable. The CRD now accepts 0, but the scheduler keeps defaulting minAvailable to 1 to preserve existing gang semantics for PodGroups created without minMember. Signed-off-by: lin121291 <4jp33f9e@gmail.com> (cherry picked from commit 62c594b) Signed-off-by: gshaibi <gshaibi@nvidia.com>
1 parent 74d0d93 commit d72e59d

6 files changed

Lines changed: 78 additions & 5 deletions

File tree

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
kind: Fixed
2+
body: |-
3+
Allow creating PodGroups with an explicit minMember of 0

deployments/kai-scheduler/crds/scheduling.run.ai_podgroups.yaml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,9 @@ spec:
6464
description: |-
6565
MinMember defines the minimal number of members to run the PodGroup;
6666
if there are not enough resources to start all required members, the scheduler will not start anyone.
67+
A value of 0 means no gang requirement: all pods are scheduled elastically (e.g. scale-to-zero workloads).
6768
format: int32
68-
minimum: 1
69+
minimum: 0
6970
type: integer
7071
parallelism:
7172
description: The number of pods which will try to run at any instant.

pkg/apis/scheduling/v2alpha2/podgroup_types.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,8 @@ import (
3434
type PodGroupSpec struct {
3535
// MinMember defines the minimal number of members to run the PodGroup;
3636
// if there are not enough resources to start all required members, the scheduler will not start anyone.
37-
// +kubebuilder:validation:Minimum=1
37+
// A value of 0 means no gang requirement: all pods are scheduled elastically (e.g. scale-to-zero workloads).
38+
// +kubebuilder:validation:Minimum=0
3839
MinMember int32 `json:"minMember,omitempty" protobuf:"bytes,1,opt,name=minMember"`
3940

4041
// Queue defines the queue to allocate resource for PodGroup; if queue does not exist,

pkg/podgrouper/podgrouper/plugins/knative/knative_test.go

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
package knative
55

66
import (
7-
"strconv"
87
"testing"
98

109
"github.com/stretchr/testify/assert"
@@ -113,6 +112,17 @@ func TestGetPodGroupMetadata(t *testing.T) {
113112
}
114113

115114
func TestGetPodGroupMetadata_MinScale(t *testing.T) {
115+
for minScale, expectedMinAvailable := range map[string]int32{
116+
"3": 3,
117+
"0": 0, // scale-to-zero: no gang requirement
118+
} {
119+
t.Run("min-scale="+minScale, func(t *testing.T) {
120+
testGetPodGroupMetadataMinScale(t, minScale, expectedMinAvailable)
121+
})
122+
}
123+
}
124+
125+
func testGetPodGroupMetadataMinScale(t *testing.T, minScale string, expectedMinAvailable int32) {
116126
service := &unstructured.Unstructured{
117127
Object: map[string]interface{}{
118128
"kind": "Service",
@@ -156,7 +166,7 @@ func TestGetPodGroupMetadata_MinScale(t *testing.T) {
156166
Namespace: "test_namespace",
157167
UID: "2",
158168
Annotations: map[string]string{
159-
"autoscaling.knative.dev/min-scale": "3",
169+
"autoscaling.knative.dev/min-scale": minScale,
160170
},
161171
},
162172
Spec: knative.RevisionSpec{},
@@ -197,7 +207,7 @@ func TestGetPodGroupMetadata_MinScale(t *testing.T) {
197207
assert.Equal(t, "pg-revision-2", metadata.Name)
198208
assert.Equal(t, constants.InferencePriorityClass, metadata.PriorityClassName)
199209
assert.Equal(t, "test_queue", metadata.Queue)
200-
assert.Equal(t, "3", strconv.Itoa(int(metadata.MinAvailable)))
210+
assert.Equal(t, expectedMinAvailable, metadata.MinAvailable)
201211
}
202212

203213
func TestGetPodGroupMetadataBackwardsCompatibility(t *testing.T) {

pkg/scheduler/api/podgroup_info/job_info_test.go

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
"k8s.io/apimachinery/pkg/types"
2929
"k8s.io/utils/ptr"
3030

31+
enginev2alpha2 "github.com/kai-scheduler/KAI-scheduler/pkg/apis/scheduling/v2alpha2"
3132
commonconstants "github.com/kai-scheduler/KAI-scheduler/pkg/common/constants"
3233
"github.com/kai-scheduler/KAI-scheduler/pkg/scheduler/api/common_info"
3334
"github.com/kai-scheduler/KAI-scheduler/pkg/scheduler/api/pod_info"
@@ -132,6 +133,37 @@ func TestAddTaskInfo(t *testing.T) {
132133
}
133134
}
134135

136+
func TestSetPodGroupMinMember(t *testing.T) {
137+
tests := []struct {
138+
name string
139+
minMember int32
140+
expectedMinAvail int32
141+
}{
142+
// int32 cannot distinguish unset from explicit 0; both default to a gang of 1
143+
{"zero or unset minMember defaults to 1", 0, 1},
144+
{"positive minMember is honored", 3, 3},
145+
}
146+
for _, test := range tests {
147+
t.Run(test.name, func(t *testing.T) {
148+
info := NewPodGroupInfo("group-1")
149+
info.SetPodGroup(&enginev2alpha2.PodGroup{
150+
ObjectMeta: metav1.ObjectMeta{
151+
Name: "group-1",
152+
Namespace: "ns-1",
153+
},
154+
Spec: enginev2alpha2.PodGroupSpec{
155+
Queue: "queue-1",
156+
MinMember: test.minMember,
157+
},
158+
})
159+
160+
if got := info.PodSets[DefaultSubGroup].GetMinAvailable(); got != test.expectedMinAvail {
161+
t.Errorf("expected minAvailable %d, got %d", test.expectedMinAvail, got)
162+
}
163+
})
164+
}
165+
}
166+
135167
func TestDeleteTaskInfo(t *testing.T) {
136168
// case1
137169
case01_uid := common_info.PodGroupID("owner1")

test/e2e/suites/integrations/third_party/knative/knative_specs.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,32 @@ func DescribeKnativeSpecs() bool {
110110
Expect(podGroups.Items[0].Spec.MinMember).To(Equal(int32(1)),
111111
"Expected minmember of podgroup to be 1")
112112
})
113+
114+
It("knative service with scale-to-zero - minscale 0", func(ctx context.Context) {
115+
namespace := queue.GetConnectedNamespaceToQueue(testCtx.Queues[0])
116+
serviceName := "knative-service-" + utils.GenerateRandomK8sName(10)
117+
118+
knativeService := GetKnativeServiceObject(serviceName, namespace, testCtx.Queues[0].Name)
119+
120+
knativeService.Spec.ConfigurationSpec.Template.Annotations[minScaleAnnotation] = "0"
121+
122+
Expect(testCtx.ControllerClient.Create(ctx, knativeService)).To(Succeed())
123+
defer func() {
124+
Expect(testCtx.ControllerClient.Delete(ctx, knativeService)).To(Succeed())
125+
}()
126+
127+
// Knative still creates the initial pod on deployment even with min-scale 0
128+
pods := GetServicePods(ctx, testCtx, serviceName, namespace, 1)
129+
130+
wait.ForPodsScheduled(ctx, testCtx.ControllerClient, namespace, pods)
131+
132+
var podGroups v2alpha2.PodGroupList
133+
Expect(testCtx.ControllerClient.List(ctx, &podGroups, client.InNamespace(namespace))).To(Succeed())
134+
Expect(len(podGroups.Items)).To(Equal(1),
135+
"Expected one podgroup for the revision")
136+
Expect(podGroups.Items[0].Spec.MinMember).To(Equal(int32(0)),
137+
"Expected minmember of podgroup to be 0")
138+
})
113139
})
114140
})
115141
}

0 commit comments

Comments
 (0)