Skip to content

Commit 5b1e2a4

Browse files
michaelawyumichaelawyu
authored
test: add E2E tests for using SKU based properties for scheduling (limited) + apply minor changes (#283)
* Added E2E tests for SKU properties + applied minor changes (webhook validation logic + name change) Signed-off-by: michaelawyu <chenyu1@microsoft.com> * Minor fixes Signed-off-by: michaelawyu <chenyu1@microsoft.com> --------- Signed-off-by: michaelawyu <chenyu1@microsoft.com>
1 parent 595a499 commit 5b1e2a4

File tree

7 files changed

+362
-20
lines changed

7 files changed

+362
-20
lines changed

‎apis/cluster/v1/membercluster_types.go‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,20 @@ type MemberClusterSpec struct {
7575
Taints []Taint `json:"taints,omitempty"`
7676
}
7777

78-
// PropertyName is the name of a cluster property; it should be a Kubernetes label name.
78+
// PropertyName is the name of a cluster property.
79+
//
80+
// A valid name should be a string of one or more segments, separated by slashes (/) if applicable.
81+
//
82+
// Each segment must be 63 characters or less, start and end with an alphanumeric character,
83+
// and can include dashes (-), underscores (_), dots (.), and alphanumerics in between.
84+
//
85+
// Optionally, the property name can have a prefix, which must be a DNS subdomain up to 253 characters,
86+
// followed by a slash (/).
87+
//
88+
// Examples include:
89+
// - "avg-resource-pressure"
90+
// - "kubernetes-fleet.io/node-count"
91+
// - "kubernetes.azure.com/vm-sizes/Standard_D2s_v3/count"
7992
type PropertyName string
8093

8194
// PropertyValue is the value of a cluster property.

‎go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ require (
2222
go.goms.io/fleet-networking v0.3.3
2323
go.uber.org/atomic v1.11.0
2424
go.uber.org/zap v1.27.0
25-
golang.org/x/exp v0.0.0-20250305212735-054e65f0b394
2625
golang.org/x/sync v0.15.0
2726
golang.org/x/time v0.11.0
2827
gomodules.xyz/jsonpatch/v2 v2.4.0
@@ -106,6 +105,7 @@ require (
106105
go.uber.org/mock v0.5.1 // indirect
107106
go.uber.org/multierr v1.11.0 // indirect
108107
golang.org/x/crypto v0.38.0 // indirect
108+
golang.org/x/exp v0.0.0-20250305212735-054e65f0b394 // indirect
109109
golang.org/x/net v0.40.0 // indirect
110110
golang.org/x/oauth2 v0.27.0 // indirect
111111
golang.org/x/sys v0.33.0 // indirect

‎pkg/propertyprovider/azure/provider.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ const (
5353
// a Kubernetes cluster.
5454
PerGBMemoryCostProperty = "kubernetes.azure.com/per-gb-memory-cost"
5555

56-
NodeCountPerSKUPropertyTmpl = "kubernetes.azure.com/vm-size/%s/count"
56+
NodeCountPerSKUPropertyTmpl = "kubernetes.azure.com/vm-sizes/%s/count"
5757

5858
CostPrecisionTemplate = "%.3f"
5959
)

‎pkg/utils/validator/clusterresourceplacement.go‎

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -476,10 +476,44 @@ func validateName(name string) error {
476476
if !supportedResourceCapacityTypesMap[segments[0]] {
477477
return fmt.Errorf("invalid capacity type in resource property name %s, supported values are %+v", name, resourceCapacityTypes)
478478
}
479+
480+
if errs := validation.IsQualifiedName(name); errs != nil {
481+
return fmt.Errorf("property name %s is not valid: %s", name, strings.Join(errs, "; "))
482+
}
483+
return nil
479484
}
480485

481-
if err := validation.IsQualifiedName(name); err != nil {
482-
return fmt.Errorf("name is not a valid Kubernetes label name: %v", err)
486+
// For other properties, they should have a name that is formatted as follows:
487+
//
488+
// It should be a string of one or more segments, separated by slashes (/) if applicable;
489+
// each segment must be 63 characters or less, start and end with an alphanumeric character,
490+
// and can include dashes (-), underscores (_), dots (.), and alphanumerics in between.
491+
//
492+
// Optionally, the property name can have a prefix, which must be a DNS subdomain up to 253 characters,
493+
// followed by a slash (/).
494+
segs := strings.Split(name, "/")
495+
if len(segs) <= 1 {
496+
// The property name does not have a slash; it has no prefix.
497+
if errs := validation.IsQualifiedName(name); errs != nil {
498+
return fmt.Errorf("property name %s is not valid: %s", name, strings.Join(errs, "; "))
499+
}
500+
} else {
501+
// The property name might have a prefix.
502+
possiblePrefix := segs[0]
503+
504+
subDomainErrs := validation.IsDNS1123Subdomain(possiblePrefix)
505+
qualifiedNameErrs := validation.IsQualifiedName(possiblePrefix)
506+
if len(subDomainErrs) != 0 && len(qualifiedNameErrs) != 0 {
507+
return fmt.Errorf("property name first segment %s is not valid: it is neither a valid DNS subdomain (%s) nor a valid qualified name (%s)", possiblePrefix, strings.Join(subDomainErrs, "; "), strings.Join(qualifiedNameErrs, "; "))
508+
}
509+
510+
segsLeft := segs[1:]
511+
for idx := range segsLeft {
512+
seg := segsLeft[idx]
513+
if errs := validation.IsQualifiedName(seg); errs != nil {
514+
return fmt.Errorf("property name segment %s is not valid: %s", seg, strings.Join(errs, "; "))
515+
}
516+
}
483517
}
484518
return nil
485519
}

‎pkg/utils/validator/clusterresourceplacement_test.go‎

Lines changed: 156 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1225,9 +1225,9 @@ func TestValidateClusterResourcePlacement_PickNPlacementPolicy(t *testing.T) {
12251225
},
12261226
},
12271227
wantErr: true,
1228-
wantErrMsg: "name is not a valid Kubernetes label name",
1228+
wantErrMsg: "property name resources.kubernetes-fleet.io/total-nospecialchars%^=@ is not valid",
12291229
},
1230-
"valid placement policy - PickN with valid property selector name": {
1230+
"valid placement policy - PickN with valid property selector name (non-resource, single segment)": {
12311231
policy: &placementv1beta1.PlacementPolicy{
12321232
PlacementType: placementv1beta1.PickNPlacementType,
12331233
NumberOfClusters: &positiveNumberOfClusters,
@@ -1242,7 +1242,100 @@ func TestValidateClusterResourcePlacement_PickNPlacementPolicy(t *testing.T) {
12421242
PropertySelector: &placementv1beta1.PropertySelector{
12431243
MatchExpressions: []placementv1beta1.PropertySelectorRequirement{
12441244
{
1245-
Name: "resources.kubernetes-fleet.io/total-allocatable-cpu",
1245+
Name: "k8s-minor-version",
1246+
Operator: placementv1beta1.PropertySelectorEqualTo,
1247+
Values: []string{"28"},
1248+
},
1249+
},
1250+
},
1251+
},
1252+
},
1253+
},
1254+
},
1255+
},
1256+
},
1257+
wantErr: false,
1258+
},
1259+
"valid placement policy - PickN with valid property selector name (non-resource, multiple segments)": {
1260+
policy: &placementv1beta1.PlacementPolicy{
1261+
PlacementType: placementv1beta1.PickNPlacementType,
1262+
NumberOfClusters: &positiveNumberOfClusters,
1263+
Affinity: &placementv1beta1.Affinity{
1264+
ClusterAffinity: &placementv1beta1.ClusterAffinity{
1265+
RequiredDuringSchedulingIgnoredDuringExecution: &placementv1beta1.ClusterSelector{
1266+
ClusterSelectorTerms: []placementv1beta1.ClusterSelectorTerm{
1267+
{
1268+
LabelSelector: &metav1.LabelSelector{
1269+
MatchLabels: map[string]string{"test-key1": "test-value1"},
1270+
},
1271+
PropertySelector: &placementv1beta1.PropertySelector{
1272+
MatchExpressions: []placementv1beta1.PropertySelectorRequirement{
1273+
{
1274+
// Note that the first segment is both a valid DNS subdomain name and a qualified
1275+
// name.
1276+
Name: "kubernetes.azure.com/vm-sizes/Standard_D8s_v3/count",
1277+
Operator: placementv1beta1.PropertySelectorEqualTo,
1278+
Values: []string{"2"},
1279+
},
1280+
},
1281+
},
1282+
},
1283+
},
1284+
},
1285+
},
1286+
},
1287+
},
1288+
wantErr: false,
1289+
},
1290+
"valid placement policy - PickN with valid property selector name (non-resource, multiple segments with prefix)": {
1291+
policy: &placementv1beta1.PlacementPolicy{
1292+
PlacementType: placementv1beta1.PickNPlacementType,
1293+
NumberOfClusters: &positiveNumberOfClusters,
1294+
Affinity: &placementv1beta1.Affinity{
1295+
ClusterAffinity: &placementv1beta1.ClusterAffinity{
1296+
RequiredDuringSchedulingIgnoredDuringExecution: &placementv1beta1.ClusterSelector{
1297+
ClusterSelectorTerms: []placementv1beta1.ClusterSelectorTerm{
1298+
{
1299+
LabelSelector: &metav1.LabelSelector{
1300+
MatchLabels: map[string]string{"test-key1": "test-value1"},
1301+
},
1302+
PropertySelector: &placementv1beta1.PropertySelector{
1303+
MatchExpressions: []placementv1beta1.PropertySelectorRequirement{
1304+
{
1305+
// Note that the first segment is longer than 63 characters; it is a valid
1306+
// DNS subdomain name, but not a qualified name.
1307+
Name: "a.very.loooooooong.suuuuuuuub.doooooooomain.kubernetes.azure.com/vm-sizes/Standard_D8s_v3/count",
1308+
Operator: placementv1beta1.PropertySelectorEqualTo,
1309+
Values: []string{"2"},
1310+
},
1311+
},
1312+
},
1313+
},
1314+
},
1315+
},
1316+
},
1317+
},
1318+
},
1319+
wantErr: false,
1320+
},
1321+
"valid placement policy - PickN with valid property selector name (non-resource, multiple segments with no prefix)": {
1322+
policy: &placementv1beta1.PlacementPolicy{
1323+
PlacementType: placementv1beta1.PickNPlacementType,
1324+
NumberOfClusters: &positiveNumberOfClusters,
1325+
Affinity: &placementv1beta1.Affinity{
1326+
ClusterAffinity: &placementv1beta1.ClusterAffinity{
1327+
RequiredDuringSchedulingIgnoredDuringExecution: &placementv1beta1.ClusterSelector{
1328+
ClusterSelectorTerms: []placementv1beta1.ClusterSelectorTerm{
1329+
{
1330+
LabelSelector: &metav1.LabelSelector{
1331+
MatchLabels: map[string]string{"test-key1": "test-value1"},
1332+
},
1333+
PropertySelector: &placementv1beta1.PropertySelector{
1334+
MatchExpressions: []placementv1beta1.PropertySelectorRequirement{
1335+
{
1336+
// Note that DNS subdomain names do not allow underscores, so the first
1337+
// segment cannot be a prefix.
1338+
Name: "Standard_D8s_v3/count",
12461339
Operator: placementv1beta1.PropertySelectorEqualTo,
12471340
Values: []string{"2"},
12481341
},
@@ -1256,6 +1349,66 @@ func TestValidateClusterResourcePlacement_PickNPlacementPolicy(t *testing.T) {
12561349
},
12571350
wantErr: false,
12581351
},
1352+
"invalid placement policy - PickN with invalid property selector name (non-resource, invalid prefix)": {
1353+
policy: &placementv1beta1.PlacementPolicy{
1354+
PlacementType: placementv1beta1.PickNPlacementType,
1355+
NumberOfClusters: &positiveNumberOfClusters,
1356+
Affinity: &placementv1beta1.Affinity{
1357+
ClusterAffinity: &placementv1beta1.ClusterAffinity{
1358+
RequiredDuringSchedulingIgnoredDuringExecution: &placementv1beta1.ClusterSelector{
1359+
ClusterSelectorTerms: []placementv1beta1.ClusterSelectorTerm{
1360+
{
1361+
LabelSelector: &metav1.LabelSelector{
1362+
MatchLabels: map[string]string{"test-key1": "test-value1"},
1363+
},
1364+
PropertySelector: &placementv1beta1.PropertySelector{
1365+
MatchExpressions: []placementv1beta1.PropertySelectorRequirement{
1366+
{
1367+
Name: "St@ndard_D8s_v3/count",
1368+
Operator: placementv1beta1.PropertySelectorEqualTo,
1369+
Values: []string{"2"},
1370+
},
1371+
},
1372+
},
1373+
},
1374+
},
1375+
},
1376+
},
1377+
},
1378+
},
1379+
wantErr: true,
1380+
wantErrMsg: "property name first segment St@ndard_D8s_v3 is not valid",
1381+
},
1382+
"invalid placement policy - PickN with invalid property selector name (non-resource, invalid non-prefix segment)": {
1383+
policy: &placementv1beta1.PlacementPolicy{
1384+
PlacementType: placementv1beta1.PickNPlacementType,
1385+
NumberOfClusters: &positiveNumberOfClusters,
1386+
Affinity: &placementv1beta1.Affinity{
1387+
ClusterAffinity: &placementv1beta1.ClusterAffinity{
1388+
RequiredDuringSchedulingIgnoredDuringExecution: &placementv1beta1.ClusterSelector{
1389+
ClusterSelectorTerms: []placementv1beta1.ClusterSelectorTerm{
1390+
{
1391+
LabelSelector: &metav1.LabelSelector{
1392+
MatchLabels: map[string]string{"test-key1": "test-value1"},
1393+
},
1394+
PropertySelector: &placementv1beta1.PropertySelector{
1395+
MatchExpressions: []placementv1beta1.PropertySelectorRequirement{
1396+
{
1397+
Name: "Standard_D8s_v3/count/$",
1398+
Operator: placementv1beta1.PropertySelectorEqualTo,
1399+
Values: []string{"2"},
1400+
},
1401+
},
1402+
},
1403+
},
1404+
},
1405+
},
1406+
},
1407+
},
1408+
},
1409+
wantErr: true,
1410+
wantErrMsg: "property name segment $ is not valid",
1411+
},
12591412
}
12601413

12611414
for testName, testCase := range tests {

0 commit comments

Comments
 (0)