Skip to content

Commit 942e650

Browse files
shyamradhakrishnanshyamradhakrishnan
authored
Use externally managed replicas annotation and remove the CAPOCI specific flag which was introduced (#202)
1 parent d81354d commit 942e650

File tree

8 files changed

+110
-39
lines changed

8 files changed

+110
-39
lines changed

‎cloud/scope/managed_machine_pool.go‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import (
3838
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
3939
capierrors "sigs.k8s.io/cluster-api/errors"
4040
expclusterv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1"
41+
"sigs.k8s.io/cluster-api/util/annotations"
4142
"sigs.k8s.io/cluster-api/util/patch"
4243
"sigs.k8s.io/controller-runtime/pkg/client"
4344
)
@@ -503,9 +504,9 @@ func (m *ManagedMachinePoolScope) UpdateNodePool(ctx context.Context, pool *oke.
503504
spec := m.OCIManagedMachinePool.Spec.DeepCopy()
504505
setMachinePoolSpecDefaults(spec)
505506
nodePoolSizeUpdateRequired := false
506-
// 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
507+
// if replicas is not managed by cluster autoscaler and if the number of nodes in the spec is not equal to number set in the node pool
507508
// update the node pool
508-
if m.OCIManagedMachinePool.Spec.NodePoolNodeConfig.UpdateNodePoolSize && (*m.MachinePool.Spec.Replicas != int32(*pool.NodeConfigDetails.Size)) {
509+
if !annotations.ReplicasManagedByExternalAutoscaler(m.MachinePool) && (*m.MachinePool.Spec.Replicas != int32(*pool.NodeConfigDetails.Size)) {
509510
nodePoolSizeUpdateRequired = true
510511
}
511512
actual := m.getSpecFromAPIObject(pool)
@@ -532,7 +533,7 @@ func (m *ManagedMachinePoolScope) UpdateNodePool(ctx context.Context, pool *oke.
532533
IsPvEncryptionInTransitEnabled: spec.NodePoolNodeConfig.IsPvEncryptionInTransitEnabled,
533534
KmsKeyId: spec.NodePoolNodeConfig.KmsKeyId,
534535
}
535-
if m.OCIManagedMachinePool.Spec.NodePoolNodeConfig.UpdateNodePoolSize {
536+
if nodePoolSizeUpdateRequired {
536537
nodeConfigDetails.Size = common.Int(int(*m.MachinePool.Spec.Replicas))
537538
}
538539
nodeShapeConfig := oke.UpdateNodeShapeConfigDetails{}
@@ -623,8 +624,6 @@ func setMachinePoolSpecDefaults(spec *infrav1exp.OCIManagedMachinePoolSpec) {
623624
return *configs[i].AvailabilityDomain < *configs[j].AvailabilityDomain
624625
})
625626
}
626-
// set to default value as we should not compare this field
627-
spec.NodePoolNodeConfig.UpdateNodePoolSize = false
628627
}
629628
podNetworkOptions := spec.NodePoolNodeConfig.NodePoolPodNetworkOptionDetails
630629
if podNetworkOptions != nil {

‎cloud/scope/managed_machine_pool_test.go‎

Lines changed: 102 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -717,7 +717,6 @@ func TestManagedMachinePoolUpdate(t *testing.T) {
717717
},
718718
SshPublicKey: "test-ssh-public-key",
719719
NodePoolNodeConfig: &infrav1exp.NodePoolNodeConfig{
720-
UpdateNodePoolSize: true,
721720
PlacementConfigs: []infrav1exp.PlacementConfig{
722721
{
723722
AvailabilityDomain: common.String("test-ad"),
@@ -840,6 +839,108 @@ func TestManagedMachinePoolUpdate(t *testing.T) {
840839
},
841840
},
842841
},
842+
{
843+
name: "no update due to change in replica size as annotation is set",
844+
errorExpected: false,
845+
testSpecificSetup: func(cs *ManagedMachinePoolScope, okeClient *mock_containerengine.MockClient) {
846+
ms.OCIManagedCluster.Spec.OCIResourceIdentifier = "resource_uid"
847+
ms.MachinePool.Annotations = make(map[string]string)
848+
ms.MachinePool.Annotations[clusterv1.ReplicasManagedByAnnotation] = ""
849+
newReplicas := int32(4)
850+
ms.MachinePool.Spec.Replicas = &newReplicas
851+
ms.OCIManagedMachinePool.Spec = infrav1exp.OCIManagedMachinePoolSpec{
852+
Version: common.String("v1.24.5"),
853+
ID: common.String("node-pool-id"),
854+
NodeMetadata: map[string]string{"key1": "value1"},
855+
InitialNodeLabels: []infrav1exp.KeyValue{{
856+
Key: common.String("key"),
857+
Value: common.String("value"),
858+
}},
859+
NodeShape: "test-shape",
860+
NodeShapeConfig: &infrav1exp.NodeShapeConfig{
861+
Ocpus: common.String("2"),
862+
MemoryInGBs: common.String("16"),
863+
},
864+
NodeSourceViaImage: &infrav1exp.NodeSourceViaImage{
865+
ImageId: common.String("test-image-id"),
866+
},
867+
SshPublicKey: "test-ssh-public-key",
868+
NodePoolNodeConfig: &infrav1exp.NodePoolNodeConfig{
869+
PlacementConfigs: []infrav1exp.PlacementConfig{
870+
{
871+
AvailabilityDomain: common.String("test-ad"),
872+
SubnetName: common.String("worker-subnet"),
873+
CapacityReservationId: common.String("cap-id"),
874+
FaultDomains: []string{"fd-1", "fd-2"},
875+
},
876+
},
877+
NsgNames: []string{"worker-nsg"},
878+
KmsKeyId: common.String("kms-key-id"),
879+
IsPvEncryptionInTransitEnabled: common.Bool(true),
880+
NodePoolPodNetworkOptionDetails: &infrav1exp.NodePoolPodNetworkOptionDetails{
881+
CniType: infrav1exp.VCNNativeCNI,
882+
VcnIpNativePodNetworkOptions: infrav1exp.VcnIpNativePodNetworkOptions{
883+
SubnetNames: []string{"pod-subnet"},
884+
MaxPodsPerNode: common.Int(31),
885+
NSGNames: []string{"pod-nsg"},
886+
},
887+
},
888+
},
889+
NodeEvictionNodePoolSettings: &infrav1exp.NodeEvictionNodePoolSettings{
890+
EvictionGraceDuration: common.String("PT30M"),
891+
IsForceDeleteAfterGraceDuration: common.Bool(true),
892+
},
893+
}
894+
},
895+
nodePool: oke.NodePool{
896+
ClusterId: common.String("cluster-id"),
897+
Id: common.String("node-pool-id"),
898+
Name: common.String("test"),
899+
CompartmentId: common.String("test-compartment"),
900+
KubernetesVersion: common.String("v1.24.5"),
901+
NodeMetadata: map[string]string{"key1": "value1"},
902+
InitialNodeLabels: []oke.KeyValue{{
903+
Key: common.String("key"),
904+
Value: common.String("value"),
905+
}},
906+
NodeShape: common.String("test-shape"),
907+
NodeShapeConfig: &oke.NodeShapeConfig{
908+
Ocpus: common.Float32(2),
909+
MemoryInGBs: common.Float32(16),
910+
},
911+
NodeSourceDetails: oke.NodeSourceViaImageDetails{
912+
ImageId: common.String("test-image-id"),
913+
},
914+
FreeformTags: tags,
915+
DefinedTags: definedTagsInterface,
916+
SshPublicKey: common.String("test-ssh-public-key"),
917+
NodeConfigDetails: &oke.NodePoolNodeConfigDetails{
918+
Size: common.Int(3),
919+
PlacementConfigs: []oke.NodePoolPlacementConfigDetails{
920+
{
921+
AvailabilityDomain: common.String("test-ad"),
922+
SubnetId: common.String("subnet-id"),
923+
CapacityReservationId: common.String("cap-id"),
924+
FaultDomains: []string{"fd-1", "fd-2"},
925+
},
926+
},
927+
NsgIds: []string{"nsg-id"},
928+
KmsKeyId: common.String("kms-key-id"),
929+
IsPvEncryptionInTransitEnabled: common.Bool(true),
930+
FreeformTags: tags,
931+
DefinedTags: definedTagsInterface,
932+
NodePoolPodNetworkOptionDetails: oke.OciVcnIpNativeNodePoolPodNetworkOptionDetails{
933+
PodSubnetIds: []string{"pod-subnet-id"},
934+
MaxPodsPerNode: common.Int(31),
935+
PodNsgIds: []string{"pod-nsg-id"},
936+
},
937+
},
938+
NodeEvictionNodePoolSettings: &oke.NodeEvictionNodePoolSettings{
939+
EvictionGraceDuration: common.String("PT30M"),
940+
IsForceDeleteAfterGraceDuration: common.Bool(true),
941+
},
942+
},
943+
},
843944
{
844945
name: "update due to change in k8s version",
845946
errorExpected: false,

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

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

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

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

‎exp/api/v1beta1/ocimanagedmachinepool_types.go‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -88,14 +88,6 @@ type NodePoolNodeConfig struct {
8888
// NodePoolPodNetworkOptionDetails defines the pod networking details of the node pool
8989
// +optional
9090
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"`
9991
}
10092

10193
const (

‎templates/cluster-template-managed-private.yaml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,8 @@ kind: MachinePool
5858
metadata:
5959
name: ${CLUSTER_NAME}-mp-0
6060
namespace: default
61+
annotations:
62+
"cluster.x-k8s.io/replicas-managed-by": ""
6163
spec:
6264
clusterName: ${CLUSTER_NAME}
6365
replicas: ${NODE_MACHINE_COUNT}

‎templates/cluster-template-managed.yaml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ kind: MachinePool
3939
metadata:
4040
name: ${CLUSTER_NAME}-mp-0
4141
namespace: default
42+
annotations:
43+
"cluster.x-k8s.io/replicas-managed-by": ""
4244
spec:
4345
clusterName: ${CLUSTER_NAME}
4446
replicas: ${NODE_MACHINE_COUNT}

‎test/e2e/data/infrastructure-oci/v1beta1/cluster-template-managed/machine-pool.yaml‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,4 @@ spec:
3535
nodeShapeConfig:
3636
memoryInGBs: "16"
3737
ocpus: "1"
38-
nodePoolNodeConfig:
39-
updateNodePoolSize: true
4038
---

0 commit comments

Comments
 (0)