Skip to content

Commit 6a30ad7

Browse files
shyamradhakrishnanshyamradhakrishnan
authored
Ignore node pool size update by default (#174)
1 parent cd3d7f8 commit 6a30ad7

5 files changed

+198
-12
lines changed

‎cloud/scope/managed_machine_pool.go‎

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -498,10 +498,15 @@ func (m *ManagedMachinePoolScope) getWorkerMachineSubnet(name *string) *string {
498498
func (m *ManagedMachinePoolScope) UpdateNodePool(ctx context.Context, pool *oke.NodePool) (bool, error) {
499499
spec := m.OCIManagedMachinePool.Spec.DeepCopy()
500500
setMachinePoolSpecDefaults(spec)
501-
501+
nodePoolSizeUpdateRequired := false
502+
// if node pool size update flags is set and if the number of nodes in the spec is not equal to number set in the node pool
503+
// update the node pool
504+
if m.OCIManagedMachinePool.Spec.NodePoolNodeConfig.UpdateNodePoolSize && (*m.MachinePool.Spec.Replicas != int32(*pool.NodeConfigDetails.Size)) {
505+
nodePoolSizeUpdateRequired = true
506+
}
502507
actual := m.getSpecFromAPIObject(pool)
503508
if !reflect.DeepEqual(spec, actual) ||
504-
m.OCIManagedMachinePool.Name != *pool.Name {
509+
m.OCIManagedMachinePool.Name != *pool.Name || nodePoolSizeUpdateRequired {
505510
m.Logger.Info("Updating node pool")
506511
// printing json specs will help debug problems when there are spurious/unwanted updates
507512
jsonSpec, err := json.Marshal(*spec)
@@ -518,12 +523,14 @@ func (m *ManagedMachinePoolScope) UpdateNodePool(ctx context.Context, pool *oke.
518523
return false, err
519524
}
520525
nodeConfigDetails := oke.UpdateNodePoolNodeConfigDetails{
521-
Size: common.Int(int(*m.MachinePool.Spec.Replicas)),
522526
NsgIds: m.getWorkerMachineNSGs(),
523527
PlacementConfigs: placementConfig,
524528
IsPvEncryptionInTransitEnabled: spec.NodePoolNodeConfig.IsPvEncryptionInTransitEnabled,
525529
KmsKeyId: spec.NodePoolNodeConfig.KmsKeyId,
526530
}
531+
if m.OCIManagedMachinePool.Spec.NodePoolNodeConfig.UpdateNodePoolSize {
532+
nodeConfigDetails.Size = common.Int(int(*m.MachinePool.Spec.Replicas))
533+
}
527534
nodeShapeConfig := oke.UpdateNodeShapeConfigDetails{}
528535
if spec.NodeShapeConfig != nil {
529536
ocpuString := spec.NodeShapeConfig.Ocpus
@@ -605,11 +612,15 @@ func (m *ManagedMachinePoolScope) UpdateNodePool(ctx context.Context, pool *oke.
605612
func setMachinePoolSpecDefaults(spec *infrav1exp.OCIManagedMachinePoolSpec) {
606613
spec.ProviderIDList = nil
607614
spec.ProviderID = nil
608-
if spec.NodePoolNodeConfig != nil && spec.NodePoolNodeConfig.PlacementConfigs != nil {
609-
configs := spec.NodePoolNodeConfig.PlacementConfigs
610-
sort.Slice(configs, func(i, j int) bool {
611-
return *configs[i].AvailabilityDomain < *configs[j].AvailabilityDomain
612-
})
615+
if spec.NodePoolNodeConfig != nil {
616+
if spec.NodePoolNodeConfig.PlacementConfigs != nil {
617+
configs := spec.NodePoolNodeConfig.PlacementConfigs
618+
sort.Slice(configs, func(i, j int) bool {
619+
return *configs[i].AvailabilityDomain < *configs[j].AvailabilityDomain
620+
})
621+
}
622+
// set to default value as we should not compare this field
623+
spec.NodePoolNodeConfig.UpdateNodePoolSize = false
613624
}
614625
podNetworkOptions := spec.NodePoolNodeConfig.NodePoolPodNetworkOptionDetails
615626
if podNetworkOptions != nil {

‎cloud/scope/managed_machine_pool_test.go‎

Lines changed: 148 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -692,6 +692,154 @@ func TestManagedMachinePoolUpdate(t *testing.T) {
692692
},
693693
},
694694
},
695+
{
696+
name: "update due to change in replica size",
697+
errorExpected: false,
698+
testSpecificSetup: func(cs *ManagedMachinePoolScope, okeClient *mock_containerengine.MockClient) {
699+
ms.OCIManagedCluster.Spec.OCIResourceIdentifier = "resource_uid"
700+
newReplicas := int32(4)
701+
ms.MachinePool.Spec.Replicas = &newReplicas
702+
ms.OCIManagedMachinePool.Spec = infrav1exp.OCIManagedMachinePoolSpec{
703+
Version: common.String("v1.24.5"),
704+
ID: common.String("node-pool-id"),
705+
NodeMetadata: map[string]string{"key1": "value1"},
706+
InitialNodeLabels: []infrav1exp.KeyValue{{
707+
Key: common.String("key"),
708+
Value: common.String("value"),
709+
}},
710+
NodeShape: "test-shape",
711+
NodeShapeConfig: &infrav1exp.NodeShapeConfig{
712+
Ocpus: common.String("2"),
713+
MemoryInGBs: common.String("16"),
714+
},
715+
NodeSourceViaImage: &infrav1exp.NodeSourceViaImage{
716+
ImageId: common.String("test-image-id"),
717+
},
718+
SshPublicKey: "test-ssh-public-key",
719+
NodePoolNodeConfig: &infrav1exp.NodePoolNodeConfig{
720+
UpdateNodePoolSize: true,
721+
PlacementConfigs: []infrav1exp.PlacementConfig{
722+
{
723+
AvailabilityDomain: common.String("test-ad"),
724+
SubnetName: common.String("worker-subnet"),
725+
CapacityReservationId: common.String("cap-id"),
726+
FaultDomains: []string{"fd-1", "fd-2"},
727+
},
728+
},
729+
NsgNames: []string{"worker-nsg"},
730+
KmsKeyId: common.String("kms-key-id"),
731+
IsPvEncryptionInTransitEnabled: common.Bool(true),
732+
NodePoolPodNetworkOptionDetails: &infrav1exp.NodePoolPodNetworkOptionDetails{
733+
CniType: infrav1exp.VCNNativeCNI,
734+
VcnIpNativePodNetworkOptions: infrav1exp.VcnIpNativePodNetworkOptions{
735+
SubnetNames: []string{"pod-subnet"},
736+
MaxPodsPerNode: common.Int(31),
737+
NSGNames: []string{"pod-nsg"},
738+
},
739+
},
740+
},
741+
NodeEvictionNodePoolSettings: &infrav1exp.NodeEvictionNodePoolSettings{
742+
EvictionGraceDuration: common.String("PT30M"),
743+
IsForceDeleteAfterGraceDuration: common.Bool(true),
744+
},
745+
}
746+
okeClient.EXPECT().UpdateNodePool(gomock.Any(), gomock.Eq(oke.UpdateNodePoolRequest{
747+
NodePoolId: common.String("node-pool-id"),
748+
UpdateNodePoolDetails: oke.UpdateNodePoolDetails{
749+
Name: common.String("test"),
750+
KubernetesVersion: common.String("v1.24.5"),
751+
NodeMetadata: map[string]string{"key1": "value1"},
752+
InitialNodeLabels: []oke.KeyValue{{
753+
Key: common.String("key"),
754+
Value: common.String("value"),
755+
}},
756+
NodeShape: common.String("test-shape"),
757+
NodeShapeConfig: &oke.UpdateNodeShapeConfigDetails{
758+
Ocpus: common.Float32(2),
759+
MemoryInGBs: common.Float32(16),
760+
},
761+
NodeSourceDetails: &oke.NodeSourceViaImageDetails{
762+
ImageId: common.String("test-image-id"),
763+
},
764+
SshPublicKey: common.String("test-ssh-public-key"),
765+
NodeConfigDetails: &oke.UpdateNodePoolNodeConfigDetails{
766+
Size: common.Int(4),
767+
PlacementConfigs: []oke.NodePoolPlacementConfigDetails{
768+
{
769+
AvailabilityDomain: common.String("test-ad"),
770+
CapacityReservationId: common.String("cap-id"),
771+
SubnetId: common.String("subnet-id"),
772+
FaultDomains: []string{"fd-1", "fd-2"},
773+
},
774+
},
775+
NsgIds: []string{"nsg-id"},
776+
KmsKeyId: common.String("kms-key-id"),
777+
IsPvEncryptionInTransitEnabled: common.Bool(true),
778+
NodePoolPodNetworkOptionDetails: oke.OciVcnIpNativeNodePoolPodNetworkOptionDetails{
779+
PodSubnetIds: []string{"pod-subnet-id"},
780+
MaxPodsPerNode: common.Int(31),
781+
PodNsgIds: []string{"pod-nsg-id"},
782+
},
783+
},
784+
NodeEvictionNodePoolSettings: &oke.NodeEvictionNodePoolSettings{
785+
EvictionGraceDuration: common.String("PT30M"),
786+
IsForceDeleteAfterGraceDuration: common.Bool(true),
787+
},
788+
},
789+
})).
790+
Return(oke.UpdateNodePoolResponse{
791+
OpcWorkRequestId: common.String("opc-work-request-id"),
792+
}, nil)
793+
},
794+
nodePool: oke.NodePool{
795+
ClusterId: common.String("cluster-id"),
796+
Id: common.String("node-pool-id"),
797+
Name: common.String("test"),
798+
CompartmentId: common.String("test-compartment"),
799+
KubernetesVersion: common.String("v1.24.5"),
800+
NodeMetadata: map[string]string{"key1": "value1"},
801+
InitialNodeLabels: []oke.KeyValue{{
802+
Key: common.String("key"),
803+
Value: common.String("value"),
804+
}},
805+
NodeShape: common.String("test-shape"),
806+
NodeShapeConfig: &oke.NodeShapeConfig{
807+
Ocpus: common.Float32(2),
808+
MemoryInGBs: common.Float32(16),
809+
},
810+
NodeSourceDetails: oke.NodeSourceViaImageDetails{
811+
ImageId: common.String("test-image-id"),
812+
},
813+
FreeformTags: tags,
814+
DefinedTags: definedTagsInterface,
815+
SshPublicKey: common.String("test-ssh-public-key"),
816+
NodeConfigDetails: &oke.NodePoolNodeConfigDetails{
817+
Size: common.Int(3),
818+
PlacementConfigs: []oke.NodePoolPlacementConfigDetails{
819+
{
820+
AvailabilityDomain: common.String("test-ad"),
821+
SubnetId: common.String("subnet-id"),
822+
CapacityReservationId: common.String("cap-id"),
823+
FaultDomains: []string{"fd-1", "fd-2"},
824+
},
825+
},
826+
NsgIds: []string{"nsg-id"},
827+
KmsKeyId: common.String("kms-key-id"),
828+
IsPvEncryptionInTransitEnabled: common.Bool(true),
829+
FreeformTags: tags,
830+
DefinedTags: definedTagsInterface,
831+
NodePoolPodNetworkOptionDetails: oke.OciVcnIpNativeNodePoolPodNetworkOptionDetails{
832+
PodSubnetIds: []string{"pod-subnet-id"},
833+
MaxPodsPerNode: common.Int(31),
834+
PodNsgIds: []string{"pod-nsg-id"},
835+
},
836+
},
837+
NodeEvictionNodePoolSettings: &oke.NodeEvictionNodePoolSettings{
838+
EvictionGraceDuration: common.String("PT30M"),
839+
IsForceDeleteAfterGraceDuration: common.Bool(true),
840+
},
841+
},
842+
},
695843
{
696844
name: "update due to change in k8s version",
697845
errorExpected: false,
@@ -760,7 +908,6 @@ func TestManagedMachinePoolUpdate(t *testing.T) {
760908
},
761909
SshPublicKey: common.String("test-ssh-public-key"),
762910
NodeConfigDetails: &oke.UpdateNodePoolNodeConfigDetails{
763-
Size: common.Int(3),
764911
PlacementConfigs: []oke.NodePoolPlacementConfigDetails{
765912
{
766913
AvailabilityDomain: common.String("test-ad"),
@@ -905,7 +1052,6 @@ func TestManagedMachinePoolUpdate(t *testing.T) {
9051052
},
9061053
SshPublicKey: common.String("test-ssh-public-key"),
9071054
NodeConfigDetails: &oke.UpdateNodePoolNodeConfigDetails{
908-
Size: common.Int(3),
9091055
PlacementConfigs: []oke.NodePoolPlacementConfigDetails{
9101056
{
9111057
AvailabilityDomain: common.String("test-ad"),
@@ -1051,7 +1197,6 @@ func TestManagedMachinePoolUpdate(t *testing.T) {
10511197
},
10521198
SshPublicKey: common.String("test-ssh-public-key"),
10531199
NodeConfigDetails: &oke.UpdateNodePoolNodeConfigDetails{
1054-
Size: common.Int(3),
10551200
PlacementConfigs: []oke.NodePoolPlacementConfigDetails{
10561201
{
10571202
AvailabilityDomain: common.String("test-ad"),

‎config/crd/bases/infrastructure.cluster.x-k8s.io_ocimanagedmachinepools.yaml‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,17 @@ spec:
157157
type: string
158158
type: object
159159
type: array
160+
updateNodePoolSize:
161+
description: UpdateNodePoolSize defines whether the node pool
162+
size should be updated when there is a spec change. By default
163+
the value is false, which means that on machine pool update,
164+
the node pool size is not updated. This is to make sure the
165+
provider reconciliation of node pool does not overwrite any
166+
changes done by an external system, most probably Kubernetes
167+
Cluster Autoscaler. This property can be set ot true if CAPI
168+
based cluster auto scaler is used rather than the cloud provider
169+
specific one.
170+
type: boolean
160171
type: object
161172
nodeShape:
162173
description: NodeShape defines the name of the node shape of the nodes

‎config/crd/bases/infrastructure.cluster.x-k8s.io_ocimanagedmachinepooltemplates.yaml‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,18 @@ spec:
174174
type: string
175175
type: object
176176
type: array
177+
updateNodePoolSize:
178+
description: UpdateNodePoolSize defines whether the node
179+
pool size should be updated when there is a spec change.
180+
By default the value is false, which means that on machine
181+
pool update, the node pool size is not updated. This
182+
is to make sure the provider reconciliation of node
183+
pool does not overwrite any changes done by an external
184+
system, most probably Kubernetes Cluster Autoscaler.
185+
This property can be set ot true if CAPI based cluster
186+
auto scaler is used rather than the cloud provider specific
187+
one.
188+
type: boolean
177189
type: object
178190
nodeShape:
179191
description: NodeShape defines the name of the node shape

‎exp/api/v1beta1/ocimanagedmachinepool_types.go‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,6 @@ type OCIManagedMachinePoolSpec struct {
6868

6969
// NodePoolNodeConfig describes the configuration of nodes in the node pool.
7070
type NodePoolNodeConfig struct {
71-
7271
// IsPvEncryptionInTransitEnabled defines whether in transit encryption should be enabled on the nodes.
7372
// +optional
7473
IsPvEncryptionInTransitEnabled *bool `json:"isPvEncryptionInTransitEnabled,omitempty"`
@@ -89,6 +88,14 @@ type NodePoolNodeConfig struct {
8988
// NodePoolPodNetworkOptionDetails defines the pod networking details of the node pool
9089
// +optional
9190
NodePoolPodNetworkOptionDetails *NodePoolPodNetworkOptionDetails `json:"nodePoolPodNetworkOptionDetails,omitempty"`
91+
92+
// UpdateNodePoolSize defines whether the node pool size should be updated when there is a spec change.
93+
// By default the value is false, which means that on machine pool update, the node pool size is not updated.
94+
// This is to make sure the provider reconciliation of node pool does not overwrite any changes done by an external system,
95+
// most probably Kubernetes Cluster Autoscaler. This property can be set ot true if CAPI based cluster auto scaler is used
96+
// rather than the cloud provider specific one.
97+
// +optional
98+
UpdateNodePoolSize bool `json:"updateNodePoolSize,omitempty"`
9299
}
93100

94101
const (

0 commit comments

Comments
 (0)