Skip to content

Commit 8181160

Browse files
committed
update scheduler changes
Signed-off-by: Britania Rodriguez Reyes <britaniar@microsoft.com>
1 parent 5fee34f commit 8181160

File tree

12 files changed

+223
-166
lines changed

12 files changed

+223
-166
lines changed

go.mod

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ require (
1010
github.com/evanphx/json-patch/v5 v5.9.11
1111
github.com/google/go-cmp v0.7.0
1212
github.com/grpc-ecosystem/grpc-gateway/v2 v2.20.0
13+
github.com/kubefleet-dev/kubefleet v0.0.1
1314
github.com/onsi/ginkgo/v2 v2.23.4
1415
github.com/onsi/gomega v1.37.0
1516
github.com/prometheus/client_golang v1.22.0

go.sum

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,8 @@ github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE=
201201
github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk=
202202
github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
203203
github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
204+
github.com/kubefleet-dev/kubefleet v0.0.1 h1:Xo/DvRlKq7YLvzjGp8GFD8G6SLAyI41l4Sw4+df534g=
205+
github.com/kubefleet-dev/kubefleet v0.0.1/go.mod h1:EXicokCzLknCzcpQ/8ehtXqBokr0/xUfzsSsU++rReY=
204206
github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0SNc=
205207
github.com/kylelemons/godebug v1.1.0/go.mod h1:9/0rRGxNHcop5bhtWyNeEfOS8JIWk580+fNqagV/RAw=
206208
github.com/leodido/go-urn v1.4.0 h1:WT9HwE9SGECu3lg4d/dIA+jxlljEa1/ffXKmRjqdmIQ=

pkg/clients/azure/compute/vmsizerecommenderclient_test.go

Lines changed: 2 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -7,18 +7,15 @@ package compute
77

88
import (
99
"context"
10-
"fmt"
11-
"io"
1210
"net/http"
13-
"net/http/httptest"
1411
"strings"
1512
"testing"
1613

1714
"github.com/google/go-cmp/cmp"
18-
"google.golang.org/protobuf/encoding/protojson"
1915
"google.golang.org/protobuf/proto"
2016

2117
computev1 "go.goms.io/fleet/apis/protos/azure/compute/v1"
18+
computeUtils "go.goms.io/fleet/test/utils/compute"
2219
)
2320

2421
func TestNewAttributeBasedVMSizeRecommenderClient(t *testing.T) {
@@ -205,50 +202,7 @@ func TestClient_GenerateAttributeBasedRecommendations(t *testing.T) {
205202
for _, tt := range tests {
206203
t.Run(tt.name, func(t *testing.T) {
207204
// Create mock server
208-
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
209-
// Verify request method
210-
if r.Method != http.MethodPost {
211-
t.Errorf("got %s, want POST request", r.Method)
212-
}
213-
214-
// Verify headers
215-
if r.Header.Get("Content-Type") != "application/json" {
216-
t.Errorf("got %s, want Content-Type: application/json", r.Header.Get("Content-Type"))
217-
}
218-
if r.Header.Get("Accept") != "application/json" {
219-
t.Errorf("got %s, want Accept: application/json", r.Header.Get("Accept"))
220-
}
221-
222-
// Verify URL path if request is not nil
223-
if tt.request != nil && tt.request.SubscriptionId != "" && tt.request.Location != "" {
224-
wantPath := fmt.Sprintf(recommendationsPathTemplate, tt.request.SubscriptionId, tt.request.Location)
225-
if r.URL.Path != wantPath {
226-
t.Errorf("got %s, want path %s", r.URL.Path, wantPath)
227-
}
228-
229-
// Verify request body using protojson for proper proto3 oneof support
230-
body, err := io.ReadAll(r.Body)
231-
if err != nil {
232-
t.Fatalf("failed to read request body: %v", err)
233-
}
234-
var req computev1.GenerateAttributeBasedRecommendationsRequest
235-
unmarshaler := protojson.UnmarshalOptions{
236-
DiscardUnknown: true,
237-
}
238-
if err := unmarshaler.Unmarshal(body, &req); err != nil {
239-
t.Fatalf("failed to unmarshal request body: %v", err)
240-
}
241-
if !proto.Equal(tt.request, &req) {
242-
t.Errorf("request body mismatch: got %+v, want %+v", &req, tt.request)
243-
}
244-
}
245-
246-
// Write mock response
247-
w.WriteHeader(tt.mockStatusCode)
248-
if _, err := w.Write([]byte(tt.mockResponse)); err != nil {
249-
t.Fatalf("failed to write response: %v", err)
250-
}
251-
}))
205+
server := computeUtils.CreateMockAttributeBasedVMSizeRecommenderServer(t, tt.request, tt.mockResponse, tt.mockStatusCode)
252206
defer server.Close()
253207

254208
// Create client

pkg/propertychecker/azure/checker_test.go

Lines changed: 26 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ import (
1616
placementv1beta1 "go.goms.io/fleet/apis/placement/v1beta1"
1717
"go.goms.io/fleet/pkg/clients/azure/compute"
1818
"go.goms.io/fleet/pkg/propertyprovider/azure"
19-
"go.goms.io/fleet/pkg/utils"
2019
"go.goms.io/fleet/pkg/utils/labels"
20+
computeUtils "go.goms.io/fleet/test/utils/compute"
2121
)
2222

2323
func TestValidateCapacity(t *testing.T) {
@@ -301,6 +301,7 @@ func TestCheckIfMeetSKUCapacityRequirement(t *testing.T) {
301301
name string
302302
cluster *clusterv1beta1.MemberCluster
303303
sku string
304+
targetCapacity uint32
304305
req placementv1beta1.PropertySelectorRequirement
305306
mockStatusCode int
306307
wantAvailable bool
@@ -311,15 +312,17 @@ func TestCheckIfMeetSKUCapacityRequirement(t *testing.T) {
311312
name: "valid capacity request",
312313
cluster: cluster,
313314
sku: validSKU,
315+
targetCapacity: 3,
314316
req: validPropertySelectorRequirement,
315317
mockStatusCode: http.StatusOK,
316318
wantAvailable: true,
317319
wantError: false,
318320
},
319321
{
320-
name: "unavailable SKU request",
321-
cluster: cluster,
322-
sku: "Standard_D2s_v4",
322+
name: "unavailable SKU request",
323+
cluster: cluster,
324+
sku: "Standard_D2s_v4",
325+
targetCapacity: 1,
323326
req: placementv1beta1.PropertySelectorRequirement{
324327
Name: fmt.Sprintf(azure.CapacityPerSKUPropertyTmpl, "Standard_D2s_v4"),
325328
Operator: placementv1beta1.PropertySelectorGreaterThanOrEqualTo,
@@ -339,6 +342,7 @@ func TestCheckIfMeetSKUCapacityRequirement(t *testing.T) {
339342
},
340343
},
341344
sku: validSKU,
345+
targetCapacity: 3,
342346
req: validPropertySelectorRequirement,
343347
wantError: true,
344348
errorSubstring: "failed to extract Azure location label from cluster : label \"fleet.azure.com/location\" not found in cluster",
@@ -353,6 +357,7 @@ func TestCheckIfMeetSKUCapacityRequirement(t *testing.T) {
353357
},
354358
},
355359
sku: validSKU,
360+
targetCapacity: 3,
356361
req: validPropertySelectorRequirement,
357362
wantError: true,
358363
errorSubstring: "failed to extract Azure location label from cluster",
@@ -367,6 +372,7 @@ func TestCheckIfMeetSKUCapacityRequirement(t *testing.T) {
367372
},
368373
},
369374
sku: validSKU,
375+
targetCapacity: 3,
370376
req: validPropertySelectorRequirement,
371377
wantError: true,
372378
errorSubstring: "failed to extract Azure subscription ID label from cluster",
@@ -375,28 +381,31 @@ func TestCheckIfMeetSKUCapacityRequirement(t *testing.T) {
375381
name: "Azure API returns error",
376382
cluster: cluster,
377383
sku: validSKU,
384+
targetCapacity: 3,
378385
req: validPropertySelectorRequirement,
379386
mockStatusCode: http.StatusInternalServerError,
380387
wantError: true,
381388
errorSubstring: "failed to generate VM size recommendations from Azure",
382389
},
383390
{
384-
name: "invalid operator in requirement",
385-
cluster: cluster,
386-
sku: validSKU,
391+
name: "invalid operator in requirement",
392+
cluster: cluster,
393+
sku: validSKU,
394+
targetCapacity: 2,
387395
req: placementv1beta1.PropertySelectorRequirement{
388396
Name: fmt.Sprintf(azure.CapacityPerSKUPropertyTmpl, validSKU),
389397
Operator: placementv1beta1.PropertySelectorEqualTo,
390-
Values: []string{"3"},
398+
Values: []string{"2"},
391399
},
392400
mockStatusCode: http.StatusOK,
393401
wantError: true,
394402
errorSubstring: "unsupported operator \"Eq\" for SKU capacity property, only GreaterThan (Gt) and GreaterThanOrEqualTo (Ge) are supported",
395403
},
396404
{
397-
name: "unsupported operator in requirement",
398-
cluster: cluster,
399-
sku: validSKU,
405+
name: "unsupported operator in requirement",
406+
cluster: cluster,
407+
sku: validSKU,
408+
targetCapacity: 0,
400409
req: placementv1beta1.PropertySelectorRequirement{
401410
Name: fmt.Sprintf(azure.CapacityPerSKUPropertyTmpl, validSKU),
402411
Operator: placementv1beta1.PropertySelectorGreaterThanOrEqualTo,
@@ -407,9 +416,10 @@ func TestCheckIfMeetSKUCapacityRequirement(t *testing.T) {
407416
errorSubstring: "capacity value cannot be zero for operator",
408417
},
409418
{
410-
name: "cases-insensitive request - unavailable SKU",
411-
cluster: cluster,
412-
sku: "STANDARD_D2S_V3",
419+
name: "cases-insensitive request - unavailable SKU",
420+
cluster: cluster,
421+
sku: "STANDARD_D2S_V3",
422+
targetCapacity: 1,
413423
req: placementv1beta1.PropertySelectorRequirement{
414424
Name: fmt.Sprintf(azure.CapacityPerSKUPropertyTmpl, "STANDARD_D2S_V3"),
415425
Operator: placementv1beta1.PropertySelectorGreaterThanOrEqualTo,
@@ -423,7 +433,8 @@ func TestCheckIfMeetSKUCapacityRequirement(t *testing.T) {
423433
for _, tt := range tests {
424434
t.Run(tt.name, func(t *testing.T) {
425435
// Create mock server.
426-
server := utils.CreateMockAttributeBasedVMSizeRecommenderServer(t, tt.mockStatusCode)
436+
mockRequest := computeUtils.GenerateMockAttributeBasedVMSizeRecommenderRequest(tt.cluster.Labels[labels.AzureSubscriptionIDLabel], tt.cluster.Labels[labels.AzureLocationLabel], tt.sku, tt.targetCapacity)
437+
server := computeUtils.CreateMockAttributeBasedVMSizeRecommenderServer(t, mockRequest, computeUtils.MockAttributeBasedVMSizeRecommenderResponse, tt.mockStatusCode)
427438
defer server.Close()
428439

429440
client, err := compute.NewAttributeBasedVMSizeRecommenderClient(server.URL, http.DefaultClient)

pkg/scheduler/framework/plugins/clusteraffinity/filtering.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ func (p *Plugin) Filter(
6161
t := &ps.GetPolicySnapshotSpec().Policy.Affinity.ClusterAffinity.RequiredDuringSchedulingIgnoredDuringExecution.ClusterSelectorTerms[idx]
6262
r := clusterRequirement{
6363
ClusterSelectorTerm: *t,
64+
PropertyChecker: p.PropertyChecker,
6465
}
6566
isMatched, err := r.Matches(cluster)
6667
if err != nil {

pkg/scheduler/framework/plugins/clusteraffinity/filtering_test.go

Lines changed: 38 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,8 @@ import (
3535
"go.goms.io/fleet/pkg/propertyprovider"
3636
"go.goms.io/fleet/pkg/propertyprovider/azure"
3737
"go.goms.io/fleet/pkg/scheduler/framework"
38-
"go.goms.io/fleet/pkg/utils"
3938
"go.goms.io/fleet/pkg/utils/labels"
39+
computeUtils "go.goms.io/fleet/test/utils/compute"
4040
)
4141

4242
const (
@@ -756,6 +756,35 @@ func TestFilter(t *testing.T) {
756756
},
757757
wantStatus: framework.NewNonErrorStatus(framework.ClusterUnschedulable, p.Name(), "cluster does not match with any of the required cluster affinity terms"),
758758
},
759+
}
760+
761+
for _, tc := range testCases {
762+
t.Run(tc.name, func(t *testing.T) {
763+
ctx := context.Background()
764+
state := framework.NewCycleState(nil, nil, nil)
765+
status := p.Filter(ctx, state, tc.ps, tc.cluster)
766+
767+
if diff := cmp.Diff(
768+
status, tc.wantStatus,
769+
cmp.AllowUnexported(framework.Status{}),
770+
ignoreStatusErrorField,
771+
); diff != "" {
772+
t.Errorf("Filter() unexpected status (-got, +want):\n%s", diff)
773+
}
774+
})
775+
}
776+
}
777+
778+
func TestFilter_PropertyChecker(t *testing.T) {
779+
// This test ensures that the property checker is invoked correctly.
780+
testCases := []struct {
781+
name string
782+
ps *placementv1beta1.ClusterSchedulingPolicySnapshot
783+
cluster *clusterv1beta1.MemberCluster
784+
vmSize string
785+
targetCapacity uint32
786+
wantStatus *framework.Status
787+
}{
759788
{
760789
name: "single cluster capacity based term, matched",
761790
ps: &placementv1beta1.ClusterSchedulingPolicySnapshot{
@@ -800,6 +829,8 @@ func TestFilter(t *testing.T) {
800829
},
801830
},
802831
},
832+
vmSize: "Standard_D2s_v3",
833+
targetCapacity: 1,
803834
},
804835
{
805836
name: "single cluster capacity based term, not matched",
@@ -845,21 +876,23 @@ func TestFilter(t *testing.T) {
845876
},
846877
},
847878
},
848-
wantStatus: framework.NewNonErrorStatus(framework.ClusterUnschedulable, p.Name(), "cluster does not match with any of the required cluster affinity terms"),
879+
vmSize: "Standard_B2ms",
880+
targetCapacity: 3,
881+
wantStatus: framework.NewNonErrorStatus(framework.ClusterUnschedulable, p.Name(), "cluster does not match with any of the required cluster affinity terms"),
849882
},
850883
}
851-
852884
for _, tc := range testCases {
853885
t.Run(tc.name, func(t *testing.T) {
854886
// Create mock server
855-
server := utils.CreateMockAttributeBasedVMSizeRecommenderServer(t, http.StatusOK)
887+
mockRequest := computeUtils.GenerateMockAttributeBasedVMSizeRecommenderRequest(tc.cluster.Labels[labels.AzureSubscriptionIDLabel], tc.cluster.Labels[labels.AzureLocationLabel], tc.vmSize, tc.targetCapacity)
888+
server := computeUtils.CreateMockAttributeBasedVMSizeRecommenderServer(t, mockRequest, computeUtils.MockAttributeBasedVMSizeRecommenderResponse, http.StatusOK)
856889
defer server.Close()
857890

858891
client, err := compute.NewAttributeBasedVMSizeRecommenderClient(server.URL, http.DefaultClient)
859892
if err != nil {
860893
t.Fatalf("failed to create VM size recommender client: %v", err)
861894
}
862-
p.propertyChecker = checker.NewPropertyChecker(*client)
895+
p.PropertyChecker = checker.NewPropertyChecker(*client)
863896

864897
ctx := context.Background()
865898
state := framework.NewCycleState(nil, nil, nil)

pkg/scheduler/framework/plugins/clusteraffinity/plugin.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,9 @@ type Plugin struct {
3333
// The framework handle.
3434
handle framework.Handle
3535

36-
// Optional property checker for SKU validation.
37-
propertyChecker *azure.PropertyChecker
36+
// Optional PropertyChecker for validating properties not provided by the property provider.
37+
// Currently only supports checking SKU capacity, but can support additional property checks.
38+
PropertyChecker *azure.PropertyChecker
3839
}
3940

4041
var (
@@ -58,8 +59,8 @@ type clusterAffinityPluginOptions struct {
5859
// The name of the plugin.
5960
name string
6061

61-
// Optional property checker for SKU validation.
62-
propertyChecker *azure.PropertyChecker
62+
// The PropertyChecker of the plugin.
63+
PropertyChecker *azure.PropertyChecker
6364
}
6465

6566
type Option func(*clusterAffinityPluginOptions)
@@ -79,7 +80,7 @@ func WithName(name string) Option {
7980
// This enables specific cluster requirement validation.
8081
func WithPropertyChecker(checker *azure.PropertyChecker) Option {
8182
return func(o *clusterAffinityPluginOptions) {
82-
o.propertyChecker = checker
83+
o.PropertyChecker = checker
8384
}
8485
}
8586

@@ -92,7 +93,7 @@ func New(opts ...Option) Plugin {
9293

9394
return Plugin{
9495
name: options.name,
95-
propertyChecker: options.propertyChecker,
96+
PropertyChecker: options.PropertyChecker,
9697
}
9798
}
9899

pkg/scheduler/framework/plugins/clusteraffinity/types.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828

2929
clusterv1beta1 "go.goms.io/fleet/apis/cluster/v1beta1"
3030
placementv1beta1 "go.goms.io/fleet/apis/placement/v1beta1"
31+
"go.goms.io/fleet/pkg/propertychecker/azure"
3132
"go.goms.io/fleet/pkg/propertyprovider"
3233
)
3334

@@ -36,6 +37,9 @@ import (
3637
type clusterRequirement struct {
3738
// Embed the original ClusterSelectorTerm.
3839
ClusterSelectorTerm placementv1beta1.ClusterSelectorTerm
40+
41+
// Optional PropertyChecker for validating properties not provided by the property provider.
42+
PropertyChecker *azure.PropertyChecker
3943
}
4044

4145
// retrieveResourceUsageFrom retrieves a resource property value from a member cluster.

0 commit comments

Comments
 (0)