Skip to content

Commit 39ed26b

Browse files
michaelawyuryanzhang-oss
michaelawyu
and
ryanzhang-oss
authored
feat: add per SKU node count properties (#262)
* Added support for new properties Signed-off-by: michaelawyu <chenyu1@microsoft.com> * Minor fixes Signed-off-by: michaelawyu <chenyu1@microsoft.com> * Minor fixes Signed-off-by: michaelawyu <chenyu1@microsoft.com> --------- Signed-off-by: michaelawyu <chenyu1@microsoft.com> Co-authored-by: Ryan Zhang <yangzhangrice@hotmail.com>
1 parent 106381f commit 39ed26b

File tree

6 files changed

+193
-17
lines changed

6 files changed

+193
-17
lines changed

‎pkg/propertyprovider/azure/provider.go‎

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@ const (
5353
// a Kubernetes cluster.
5454
PerGBMemoryCostProperty = "kubernetes.azure.com/per-gb-memory-cost"
5555

56+
NodeCountPerSKUPropertyTmpl = "kubernetes.azure.com/vm-size/%s/count"
57+
5658
CostPrecisionTemplate = "%.3f"
5759
)
5860

@@ -284,13 +286,10 @@ func (p *PropertyProvider) Collect(ctx context.Context) propertyprovider.Propert
284286
conds := make([]metav1.Condition, 0, 1)
285287

286288
// Collect the non-resource properties.
287-
288-
// Collect the node count property.
289289
properties := make(map[clusterv1beta1.PropertyName]clusterv1beta1.PropertyValue)
290-
properties[propertyprovider.NodeCountProperty] = clusterv1beta1.PropertyValue{
291-
Value: fmt.Sprintf("%d", p.nodeTracker.NodeCount()),
292-
ObservationTime: metav1.Now(),
293-
}
290+
291+
// Collect node-count related properties.
292+
p.collectNodeCountRelatedProperties(ctx, properties)
294293

295294
// Collect the cost properties (if enabled).
296295
if p.isCostCollectionEnabled {
@@ -318,6 +317,27 @@ func (p *PropertyProvider) Collect(ctx context.Context) propertyprovider.Propert
318317
}
319318
}
320319

320+
// collectNodeCountRelatedProperties collects the node-count related properties.
321+
func (p *PropertyProvider) collectNodeCountRelatedProperties(_ context.Context, properties map[clusterv1beta1.PropertyName]clusterv1beta1.PropertyValue) {
322+
now := metav1.Now()
323+
324+
// Collect the total node count as a property.
325+
properties[propertyprovider.NodeCountProperty] = clusterv1beta1.PropertyValue{
326+
Value: fmt.Sprintf("%d", p.nodeTracker.NodeCount()),
327+
ObservationTime: now,
328+
}
329+
330+
// Collect the per-SKU node counts as properties.
331+
nodeCountPerSKU := p.nodeTracker.NodeCountPerSKU()
332+
for sku, count := range nodeCountPerSKU {
333+
pName := fmt.Sprintf(NodeCountPerSKUPropertyTmpl, sku)
334+
properties[clusterv1beta1.PropertyName(pName)] = clusterv1beta1.PropertyValue{
335+
Value: fmt.Sprintf("%d", count),
336+
ObservationTime: now,
337+
}
338+
}
339+
}
340+
321341
// collectCosts collects the cost information.
322342
func (p *PropertyProvider) collectCosts(_ context.Context, properties map[clusterv1beta1.PropertyName]clusterv1beta1.PropertyValue) []metav1.Condition {
323343
conds := make([]metav1.Condition, 0, 1)

‎pkg/propertyprovider/azure/provider_integration_test.go‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,16 @@ var (
187187
isPricingDataStale = true
188188
}
189189

190+
nodeCountPerSKU := map[string]int{}
191+
for idx := range nodes {
192+
node := nodes[idx]
193+
sku := node.Labels[trackers.AKSClusterNodeSKULabelName]
194+
if sku == "" {
195+
sku = trackers.ReservedNameForUndefinedSKU
196+
}
197+
nodeCountPerSKU[sku]++
198+
}
199+
190200
for idx := range nodes {
191201
node := nodes[idx]
192202
sku := node.Labels[trackers.AKSClusterNodeSKULabelName]
@@ -230,6 +240,11 @@ var (
230240
Value: fmt.Sprintf("%d", len(nodes)),
231241
},
232242
}
243+
for sku, count := range nodeCountPerSKU {
244+
wantProperties[clusterv1beta1.PropertyName(fmt.Sprintf(NodeCountPerSKUPropertyTmpl, sku))] = clusterv1beta1.PropertyValue{
245+
Value: fmt.Sprintf("%d", count),
246+
}
247+
}
233248

234249
if costCollectionErr == nil && isCostsEnabled {
235250
wantProperties[PerCPUCoreCostProperty] = clusterv1beta1.PropertyValue{

‎pkg/propertyprovider/azure/provider_test.go‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,12 @@ func TestCollect(t *testing.T) {
220220
propertyprovider.NodeCountProperty: {
221221
Value: "2",
222222
},
223+
clusterv1beta1.PropertyName(fmt.Sprintf(NodeCountPerSKUPropertyTmpl, nodeSKU1)): {
224+
Value: "1",
225+
},
226+
clusterv1beta1.PropertyName(fmt.Sprintf(NodeCountPerSKUPropertyTmpl, nodeSKU2)): {
227+
Value: "1",
228+
},
223229
PerCPUCoreCostProperty: {
224230
Value: "0.167",
225231
},
@@ -311,6 +317,12 @@ func TestCollect(t *testing.T) {
311317
propertyprovider.NodeCountProperty: {
312318
Value: "2",
313319
},
320+
clusterv1beta1.PropertyName(fmt.Sprintf(NodeCountPerSKUPropertyTmpl, nodeSKU1)): {
321+
Value: "1",
322+
},
323+
clusterv1beta1.PropertyName(fmt.Sprintf(NodeCountPerSKUPropertyTmpl, nodeSKU2)): {
324+
Value: "1",
325+
},
314326
PerCPUCoreCostProperty: {
315327
Value: "0.167",
316328
},
@@ -391,6 +403,9 @@ func TestCollect(t *testing.T) {
391403
propertyprovider.NodeCountProperty: {
392404
Value: "2",
393405
},
406+
clusterv1beta1.PropertyName(fmt.Sprintf(NodeCountPerSKUPropertyTmpl, nodeSKU3)): {
407+
Value: "2",
408+
},
394409
},
395410
Resources: clusterv1beta1.ResourceUsage{
396411
Capacity: corev1.ResourceList{
@@ -467,6 +482,12 @@ func TestCollect(t *testing.T) {
467482
propertyprovider.NodeCountProperty: {
468483
Value: "2",
469484
},
485+
clusterv1beta1.PropertyName(fmt.Sprintf(NodeCountPerSKUPropertyTmpl, nodeSKU1)): {
486+
Value: "1",
487+
},
488+
clusterv1beta1.PropertyName(fmt.Sprintf(NodeCountPerSKUPropertyTmpl, nodeSKU3)): {
489+
Value: "1",
490+
},
470491
},
471492
Resources: clusterv1beta1.ResourceUsage{
472493
Capacity: corev1.ResourceList{
@@ -540,6 +561,12 @@ func TestCollect(t *testing.T) {
540561
propertyprovider.NodeCountProperty: {
541562
Value: "2",
542563
},
564+
clusterv1beta1.PropertyName(fmt.Sprintf(NodeCountPerSKUPropertyTmpl, nodeSKU1)): {
565+
Value: "1",
566+
},
567+
clusterv1beta1.PropertyName(fmt.Sprintf(NodeCountPerSKUPropertyTmpl, trackers.ReservedNameForUndefinedSKU)): {
568+
Value: "1",
569+
},
543570
},
544571
Resources: clusterv1beta1.ResourceUsage{
545572
Capacity: corev1.ResourceList{
@@ -616,6 +643,12 @@ func TestCollect(t *testing.T) {
616643
propertyprovider.NodeCountProperty: {
617644
Value: "2",
618645
},
646+
clusterv1beta1.PropertyName(fmt.Sprintf(NodeCountPerSKUPropertyTmpl, nodeSKU1)): {
647+
Value: "1",
648+
},
649+
clusterv1beta1.PropertyName(fmt.Sprintf(NodeCountPerSKUPropertyTmpl, nodeKnownMissingSKU)): {
650+
Value: "1",
651+
},
619652
},
620653
Resources: clusterv1beta1.ResourceUsage{
621654
Capacity: corev1.ResourceList{
@@ -653,6 +686,12 @@ func TestCollect(t *testing.T) {
653686
propertyprovider.NodeCountProperty: {
654687
Value: "2",
655688
},
689+
clusterv1beta1.PropertyName(fmt.Sprintf(NodeCountPerSKUPropertyTmpl, nodeSKU1)): {
690+
Value: "1",
691+
},
692+
clusterv1beta1.PropertyName(fmt.Sprintf(NodeCountPerSKUPropertyTmpl, nodeSKU2)): {
693+
Value: "1",
694+
},
656695
PerCPUCoreCostProperty: {
657696
Value: "0.167",
658697
},
@@ -791,6 +830,9 @@ func TestCollectWithDisabledFeatures(t *testing.T) {
791830
propertyprovider.NodeCountProperty: {
792831
Value: "1",
793832
},
833+
clusterv1beta1.PropertyName(fmt.Sprintf(NodeCountPerSKUPropertyTmpl, nodeSKU1)): {
834+
Value: "1",
835+
},
794836
},
795837
Resources: clusterv1beta1.ResourceUsage{
796838
Capacity: corev1.ResourceList{
@@ -819,6 +861,9 @@ func TestCollectWithDisabledFeatures(t *testing.T) {
819861
propertyprovider.NodeCountProperty: {
820862
Value: "1",
821863
},
864+
clusterv1beta1.PropertyName(fmt.Sprintf(NodeCountPerSKUPropertyTmpl, nodeSKU1)): {
865+
Value: "1",
866+
},
822867
PerCPUCoreCostProperty: {
823868
Value: "0.250",
824869
},
@@ -856,6 +901,9 @@ func TestCollectWithDisabledFeatures(t *testing.T) {
856901
propertyprovider.NodeCountProperty: {
857902
Value: "1",
858903
},
904+
clusterv1beta1.PropertyName(fmt.Sprintf(NodeCountPerSKUPropertyTmpl, nodeSKU1)): {
905+
Value: "1",
906+
},
859907
},
860908
Resources: clusterv1beta1.ResourceUsage{
861909
Capacity: corev1.ResourceList{

‎pkg/propertyprovider/azure/trackers/nodes.go‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ const (
3636
// AKSClusterNodeSKULabelName is the node label added by AKS, which indicated the SKU
3737
// of the node.
3838
AKSClusterNodeSKULabelName = "beta.kubernetes.io/instance-type"
39+
40+
ReservedNameForUndefinedSKU = "undefined"
3941
)
4042

4143
const (
@@ -582,3 +584,20 @@ func (nt *NodeTracker) Costs() (perCPUCoreCost, perGBMemoryCost float64, warning
582584
}
583585
return nt.costs.perCPUCoreHourlyCost, nt.costs.perGBMemoryHourlyCost, nt.costs.warnings, nt.costs.err
584586
}
587+
588+
// NodeCountPerSKU returns a counter that tracks the number of nodes per SKU in the cluster.
589+
func (nt *NodeTracker) NodeCountPerSKU() map[string]int {
590+
nt.mu.RLock()
591+
defer nt.mu.RUnlock()
592+
593+
// Return a copy to avoid leaks/unexpected edits.
594+
res := make(map[string]int, len(nt.nodeSetBySKU))
595+
for sku, ns := range nt.nodeSetBySKU {
596+
// For those nodes without a SKU, use `undefined` as the SKU name.
597+
if len(sku) == 0 {
598+
sku = ReservedNameForUndefinedSKU
599+
}
600+
res[sku] = len(ns)
601+
}
602+
return res
603+
}

‎pkg/propertyprovider/azure/trackers/trackers_test.go‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,20 @@ var (
152152
corev1.ResourceMemory: resource.MustParse("1.8Gi"),
153153
},
154154
},
155+
nodeSetBySKU: map[string]NodeSet{
156+
nodeSKU1: {
157+
nodeName1: true,
158+
nodeName3: true,
159+
},
160+
nodeSKU2: {
161+
nodeName2: true,
162+
},
163+
},
164+
skuByNode: map[string]string{
165+
nodeName1: nodeSKU1,
166+
nodeName2: nodeSKU2,
167+
nodeName3: nodeSKU1,
168+
},
155169
pricingProvider: &dummyPricingProvider{},
156170
}
157171

@@ -2213,6 +2227,48 @@ func TestNodeTrackerCosts(t *testing.T) {
22132227
}
22142228
}
22152229

2230+
func TestNodeTrackerNodeCountPerSKU(t *testing.T) {
2231+
testCases := []struct {
2232+
name string
2233+
nt *NodeTracker
2234+
wantCounter map[string]int
2235+
}{
2236+
{
2237+
name: "can return the counter (all nodes have SKUs assigned)",
2238+
nt: nodeTrackerWith3Nodes,
2239+
wantCounter: map[string]int{
2240+
nodeSKU1: 2,
2241+
nodeSKU2: 1,
2242+
},
2243+
},
2244+
{
2245+
name: "can return the counter (with undefined SKUs)",
2246+
nt: &NodeTracker{
2247+
nodeSetBySKU: map[string]NodeSet{
2248+
nodeSKU1: {nodeName1: true},
2249+
"": {
2250+
nodeName2: true,
2251+
nodeName3: true,
2252+
},
2253+
},
2254+
},
2255+
wantCounter: map[string]int{
2256+
nodeSKU1: 1,
2257+
ReservedNameForUndefinedSKU: 2,
2258+
},
2259+
},
2260+
}
2261+
2262+
for _, tc := range testCases {
2263+
t.Run(tc.name, func(t *testing.T) {
2264+
counter := tc.nt.NodeCountPerSKU()
2265+
if diff := cmp.Diff(counter, tc.wantCounter, cmpopts.EquateEmpty()); diff != "" {
2266+
t.Fatalf("NodeCountPerSKU() diff (-got, +want):\n%s", diff)
2267+
}
2268+
})
2269+
}
2270+
}
2271+
22162272
// TestNodeTrackerAddOrUpdate tests the AddOrUpdate method of the PodTracker.
22172273
func TestPodTrackerAddOrUpdate(t *testing.T) {
22182274
testCases := []struct {

‎test/e2e/utils_test.go‎

Lines changed: 29 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -351,6 +351,17 @@ func summarizeAKSClusterProperties(memberCluster *framework.Cluster, mcObj *clus
351351
return nil, fmt.Errorf("no nodes are found")
352352
}
353353

354+
nodeCountBySKU := map[string]int{}
355+
for idx := range nodeList.Items {
356+
node := nodeList.Items[idx]
357+
358+
nodeSKU := node.Labels[trackers.AKSClusterNodeSKULabelName]
359+
if len(nodeSKU) == 0 {
360+
nodeSKU = trackers.ReservedNameForUndefinedSKU
361+
}
362+
nodeCountBySKU[nodeSKU]++
363+
}
364+
354365
totalCPUCapacity := resource.Quantity{}
355366
totalMemoryCapacity := resource.Quantity{}
356367
allocatableCPUCapacity := resource.Quantity{}
@@ -405,18 +416,25 @@ func summarizeAKSClusterProperties(memberCluster *framework.Cluster, mcObj *clus
405416
availableMemoryCapacity.Sub(requestedMemoryCapacity)
406417
}
407418

408-
status := clusterv1beta1.MemberClusterStatus{
409-
Properties: map[clusterv1beta1.PropertyName]clusterv1beta1.PropertyValue{
410-
propertyprovider.NodeCountProperty: {
411-
Value: fmt.Sprintf("%d", nodeCount),
412-
},
413-
azure.PerCPUCoreCostProperty: {
414-
Value: fmt.Sprintf(azure.CostPrecisionTemplate, perCPUCoreCost),
415-
},
416-
azure.PerGBMemoryCostProperty: {
417-
Value: fmt.Sprintf(azure.CostPrecisionTemplate, perGBMemoryCost),
418-
},
419+
properties := map[clusterv1beta1.PropertyName]clusterv1beta1.PropertyValue{
420+
propertyprovider.NodeCountProperty: {
421+
Value: fmt.Sprintf("%d", nodeCount),
422+
},
423+
azure.PerCPUCoreCostProperty: {
424+
Value: fmt.Sprintf(azure.CostPrecisionTemplate, perCPUCoreCost),
425+
},
426+
azure.PerGBMemoryCostProperty: {
427+
Value: fmt.Sprintf(azure.CostPrecisionTemplate, perGBMemoryCost),
419428
},
429+
}
430+
for sku, count := range nodeCountBySKU {
431+
pName := clusterv1beta1.PropertyName(fmt.Sprintf(azure.NodeCountPerSKUPropertyTmpl, sku))
432+
properties[pName] = clusterv1beta1.PropertyValue{
433+
Value: fmt.Sprintf("%d", count),
434+
}
435+
}
436+
status := clusterv1beta1.MemberClusterStatus{
437+
Properties: properties,
420438
ResourceUsage: clusterv1beta1.ResourceUsage{
421439
Capacity: corev1.ResourceList{
422440
corev1.ResourceCPU: totalCPUCapacity,

0 commit comments

Comments
 (0)