Skip to content

Commit 4da751b

Browse files
Added VirtualCapacity field to MachineClass NodeTemplate (#1024)
* Added VirtualCapacity field to MachineClass NodeTemplate * Added more unit tests for SyncVirtualCapacity * enqueue machine when machineclass is updated * Update pkg/util/provider/machinecontroller/machine_util.go Co-authored-by: Prashant Tak <prashant.tak@sap.com> * fixed typo dependecies->dependencies * remove setup struct from SyncVirtualCapacity test * remov LastAppliedVirtualCapacityAnnotation if empty * resolved with changes from PR 1015 --------- Co-authored-by: Prashant Tak <prashant.tak@sap.com>
1 parent 817afda commit 4da751b

File tree

13 files changed

+427
-24
lines changed

13 files changed

+427
-24
lines changed

docs/documents/apis.md

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2435,6 +2435,22 @@ Kubernetes core/v1.ResourceList
24352435
</tr>
24362436
<tr>
24372437
<td>
2438+
<code>virtualCapacity</code>
2439+
</td>
2440+
<td>
2441+
<em>
2442+
<a href="https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.29/#resourcelist-v1-core">
2443+
Kubernetes core/v1.ResourceList
2444+
</a>
2445+
</em>
2446+
</td>
2447+
<td>
2448+
<em>(Optional)</em>
2449+
<p>VirtualCapacity represents the expected Node &lsquo;virtual&rsquo; capacity ie comprising virtual extended resources.</p>
2450+
</td>
2451+
</tr>
2452+
<tr>
2453+
<td>
24382454
<code>instanceType</code>
24392455
</td>
24402456
<td>

kubernetes/crds/machine.sapcloud.io_machineclasses.yaml

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,16 @@ spec:
7878
region:
7979
description: Region of the expected node belonging to nodeGroup
8080
type: string
81+
virtualCapacity:
82+
additionalProperties:
83+
anyOf:
84+
- type: integer
85+
- type: string
86+
pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
87+
x-kubernetes-int-or-string: true
88+
description: VirtualCapacity represents the expected Node 'virtual'
89+
capacity ie comprising virtual extended resources.
90+
type: object
8191
zone:
8292
description: Zone of the expected node belonging to nodeGroup
8393
type: string

pkg/apis/machine/types.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -743,6 +743,10 @@ type NodeTemplate struct {
743743
// Capacity contains subfields to track all node resources required to scale nodegroup from zero
744744
Capacity corev1.ResourceList
745745

746+
// VirtualCapacity represents the expected Node 'virtual' capacity ie comprising virtual extended resources.
747+
// +optional
748+
VirtualCapacity corev1.ResourceList
749+
746750
// Instance type of the node belonging to nodeGroup
747751
InstanceType string
748752

pkg/apis/machine/v1alpha1/machineclass_types.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,10 @@ type NodeTemplate struct {
6666
// Capacity contains subfields to track all node resources required to scale nodegroup from zero
6767
Capacity corev1.ResourceList `json:"capacity"`
6868

69+
// VirtualCapacity represents the expected Node 'virtual' capacity ie comprising virtual extended resources.
70+
// +optional
71+
VirtualCapacity corev1.ResourceList `json:"virtualCapacity,omitempty"`
72+
6973
// Instance type of the node belonging to nodeGroup
7074
InstanceType string `json:"instanceType"`
7175

pkg/apis/machine/v1alpha1/zz_generated.conversion.go

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/apis/machine/v1alpha1/zz_generated.deepcopy.go

Lines changed: 7 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/apis/machine/zz_generated.deepcopy.go

Lines changed: 7 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/openapi/openapi_generated.go

Lines changed: 14 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/util/provider/machinecontroller/machine.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ func (c *controller) reconcileClusterMachine(ctx context.Context, machine *v1alp
220220
return retry, err
221221
}
222222

223-
retry, err = c.syncMachineNodeTemplates(ctx, machine)
223+
retry, err = c.syncNodeTemplates(ctx, machine, machineClass)
224224
if err != nil {
225225
return retry, err
226226
}

pkg/util/provider/machinecontroller/machine_util.go

Lines changed: 91 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -486,14 +486,17 @@ func (c *controller) updateMachineStatusAndNodeCondition(ctx context.Context, ma
486486
return machineutils.ShortRetry, err
487487
}
488488

489-
// syncMachineNodeTemplate syncs nodeTemplates between machine and corresponding node-object.
490-
// It ensures, that any nodeTemplate element available on Machine should be available on node-object.
489+
// syncNodeTemplates syncs nodeTemplates between machine, machineClass and corresponding node-object.
490+
// It ensures that any nodeTemplate element available on Machine should be available on node-object.
491+
// It ensures that MachineClass.NodeTemplate.VirtualCapacity is synced to the Node's Capacity.
491492
// Although there could be more elements already available on node-object which will not be touched.
492-
func (c *controller) syncMachineNodeTemplates(ctx context.Context, machine *v1alpha1.Machine) (machineutils.RetryPeriod, error) {
493+
func (c *controller) syncNodeTemplates(ctx context.Context, machine *v1alpha1.Machine, machineClass *v1alpha1.MachineClass) (machineutils.RetryPeriod, error) {
493494
var (
494-
initializedNodeAnnotation bool
495-
currentlyAppliedALTJSONByte []byte
496-
lastAppliedALT v1alpha1.NodeTemplateSpec
495+
initializedNodeAnnotation bool
496+
currentlyAppliedALTJSONByte []byte
497+
lastAppliedALT v1alpha1.NodeTemplateSpec
498+
currentlyAppliedVirtualCapacityJSONByte []byte
499+
lastAppliedVirtualCapacity v1.ResourceList
497500
)
498501

499502
node, err := c.nodeLister.Get(getNodeName(machine))
@@ -524,10 +527,30 @@ func (c *controller) syncMachineNodeTemplates(ctx context.Context, machine *v1al
524527
}
525528
}
526529

530+
lastAppliedVirtualCapacityJSONString, exists := node.Annotations[machineutils.LastAppliedVirtualCapacityAnnotation]
531+
if exists {
532+
err = json.Unmarshal([]byte(lastAppliedVirtualCapacityJSONString), &lastAppliedVirtualCapacity)
533+
if err != nil {
534+
klog.Errorf("Error occurred while syncing node virtual capacity: %s", err)
535+
return machineutils.ShortRetry, err
536+
}
537+
}
538+
527539
annotationsChanged := SyncMachineAnnotations(machine, nodeCopy, lastAppliedALT.Annotations)
528540
labelsChanged := SyncMachineLabels(machine, nodeCopy, lastAppliedALT.Labels)
529541
taintsChanged := SyncMachineTaints(machine, nodeCopy, lastAppliedALT.Spec.Taints)
530542

543+
var virtualCapacityChanged bool
544+
var desiredVirtualCapacity v1.ResourceList
545+
if machineClass != nil && machineClass.NodeTemplate != nil {
546+
desiredVirtualCapacity = machineClass.NodeTemplate.VirtualCapacity
547+
virtualCapacityChanged = SyncVirtualCapacity(desiredVirtualCapacity, nodeCopy, lastAppliedVirtualCapacity)
548+
}
549+
550+
if !initializedNodeAnnotation && !annotationsChanged && !labelsChanged && !taintsChanged && !virtualCapacityChanged {
551+
return machineutils.LongRetry, nil
552+
}
553+
531554
// Update node-object with latest nodeTemplate elements if elements have changed.
532555
if initializedNodeAnnotation || labelsChanged || annotationsChanged || taintsChanged {
533556

@@ -548,23 +571,44 @@ func (c *controller) syncMachineNodeTemplates(ctx context.Context, machine *v1al
548571
return machineutils.ShortRetry, err
549572
}
550573
nodeCopy.Annotations[machineutils.LastAppliedALTAnnotation] = string(currentlyAppliedALTJSONByte)
574+
}
551575

552-
_, err := c.targetCoreClient.CoreV1().Nodes().Update(ctx, nodeCopy, metav1.UpdateOptions{})
576+
if virtualCapacityChanged {
577+
klog.V(3).Infof("virtualCapacity changed, attempting UpdateStatus for node.Status.Capacity of node %q to %v", nodeCopy.Name, nodeCopy.Status.Capacity)
578+
// must patch the Node’s status subresource, because capacity lives under status
579+
nodeUpdated, err := c.targetCoreClient.CoreV1().Nodes().UpdateStatus(ctx, nodeCopy, metav1.UpdateOptions{})
553580
if err != nil {
554-
// Keep retrying until update goes through
555-
klog.Errorf("Updated failed for node object of machine %q. Retrying, error: %q", machine.Name, err)
581+
klog.Errorf("UpdateStatus failed for node %q of machine %q. error: %q", node.Name, machine.Name, err)
582+
return machineutils.ShortRetry, err
583+
}
584+
klog.V(3).Infof("node.Status.Capacity of node %q updated to: %v", node.Name, nodeUpdated.Status.Capacity)
585+
currentlyAppliedVirtualCapacityJSONByte, err = json.Marshal(desiredVirtualCapacity)
586+
if err != nil {
587+
klog.Errorf("Error occurred while syncing node virtual capacity of node %q: %v", node.Name, err)
588+
return machineutils.ShortRetry, err
589+
}
590+
nodeCopy = nodeUpdated.DeepCopy()
591+
if len(desiredVirtualCapacity) == 0 {
592+
delete(nodeCopy.Annotations, machineutils.LastAppliedVirtualCapacityAnnotation)
556593
} else {
557-
// Return error to continue in next reconcile
558-
err = errSuccessfulALTsync
594+
nodeCopy.Annotations[machineutils.LastAppliedVirtualCapacityAnnotation] = string(currentlyAppliedVirtualCapacityJSONByte)
559595
}
596+
}
560597

561-
if apierrors.IsConflict(err) {
562-
return machineutils.ConflictRetry, err
563-
}
564-
return machineutils.ShortRetry, err
598+
_, err = c.targetCoreClient.CoreV1().Nodes().Update(ctx, nodeCopy, metav1.UpdateOptions{})
599+
if err != nil {
600+
// Keep retrying until update goes through
601+
klog.Errorf("Updated failed for node object of machine %q. Retrying, error: %q", machine.Name, err)
602+
} else {
603+
// Return error to continue in next reconcile
604+
err = errSuccessfulALTsync
565605
}
566606

567-
return machineutils.LongRetry, nil
607+
if apierrors.IsConflict(err) {
608+
return machineutils.ConflictRetry, err
609+
}
610+
return machineutils.ShortRetry, err
611+
568612
}
569613

570614
// SyncMachineAnnotations syncs the annotations of the machine with node-objects.
@@ -719,6 +763,37 @@ func SyncMachineTaints(
719763
return toBeUpdated
720764
}
721765

766+
// SyncVirtualCapacity syncs the MachineClass.NodeTemplate.VirtualCapacity with the Node.Status.Capacity
767+
// It returns true if update is needed else false.
768+
func SyncVirtualCapacity(desiredVirtualCapacity v1.ResourceList, node *v1.Node, lastAppliedVirtualCapacity v1.ResourceList) bool {
769+
toBeUpdated := false
770+
771+
if node.Status.Capacity == nil {
772+
node.Status.Capacity = v1.ResourceList{}
773+
}
774+
if desiredVirtualCapacity == nil {
775+
desiredVirtualCapacity = v1.ResourceList{}
776+
}
777+
778+
// Delete any keys that existed in the past but has been deleted now
779+
for prevKey := range lastAppliedVirtualCapacity {
780+
if _, exists := desiredVirtualCapacity[prevKey]; !exists {
781+
delete(node.Status.Capacity, prevKey)
782+
toBeUpdated = true
783+
}
784+
}
785+
786+
// Add/Update any key that doesn't exist or whose value as changed
787+
for targKey, targQuant := range desiredVirtualCapacity {
788+
if nodeQuant, exists := node.Status.Capacity[targKey]; !exists || !nodeQuant.Equal(targQuant) {
789+
node.Status.Capacity[targKey] = targQuant
790+
toBeUpdated = true
791+
}
792+
}
793+
794+
return toBeUpdated
795+
}
796+
722797
// machineCreateErrorHandler updates the machine status based on
723798
// CreateMachineResponse and the error during the machine creation
724799
func (c *controller) machineCreateErrorHandler(ctx context.Context, machine *v1alpha1.Machine, createMachineResponse *driver.CreateMachineResponse, err error) (machineutils.RetryPeriod, error) {

0 commit comments

Comments
 (0)