Use externally managed replicas annotation and remove the CAPOCI specific flag which was introduced

cloud/scope/managed_machine_pool.go

@@ -38,6 +38,7 @@ import (
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	capierrors "sigs.k8s.io/cluster-api/errors"
 	expclusterv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1"
+	"sigs.k8s.io/cluster-api/util/annotations"
 	"sigs.k8s.io/cluster-api/util/patch"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
@@ -503,9 +504,9 @@ func (m *ManagedMachinePoolScope) UpdateNodePool(ctx context.Context, pool *oke.
 	spec := m.OCIManagedMachinePool.Spec.DeepCopy()
 	setMachinePoolSpecDefaults(spec)
 	nodePoolSizeUpdateRequired := false
-	// 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
+	// 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
 	// update the node pool
-	if m.OCIManagedMachinePool.Spec.NodePoolNodeConfig.UpdateNodePoolSize && (*m.MachinePool.Spec.Replicas != int32(*pool.NodeConfigDetails.Size)) {
+	if !annotations.ReplicasManagedByExternalAutoscaler(m.MachinePool) && (*m.MachinePool.Spec.Replicas != int32(*pool.NodeConfigDetails.Size)) {
 		nodePoolSizeUpdateRequired = true
 	}
 	actual := m.getSpecFromAPIObject(pool)
@@ -532,7 +533,7 @@ func (m *ManagedMachinePoolScope) UpdateNodePool(ctx context.Context, pool *oke.
 			IsPvEncryptionInTransitEnabled: spec.NodePoolNodeConfig.IsPvEncryptionInTransitEnabled,
 			KmsKeyId:                       spec.NodePoolNodeConfig.KmsKeyId,
 		}
-		if m.OCIManagedMachinePool.Spec.NodePoolNodeConfig.UpdateNodePoolSize {
+		if nodePoolSizeUpdateRequired {
 			nodeConfigDetails.Size = common.Int(int(*m.MachinePool.Spec.Replicas))
 		}
 		nodeShapeConfig := oke.UpdateNodeShapeConfigDetails{}
@@ -623,8 +624,6 @@ func setMachinePoolSpecDefaults(spec *infrav1exp.OCIManagedMachinePoolSpec) {
 				return *configs[i].AvailabilityDomain < *configs[j].AvailabilityDomain
 			})
 		}
-		// set to default value as we should not compare this field
-		spec.NodePoolNodeConfig.UpdateNodePoolSize = false
 	}
 	podNetworkOptions := spec.NodePoolNodeConfig.NodePoolPodNetworkOptionDetails
 	if podNetworkOptions != nil {

cloud/scope/managed_machine_pool_test.go

@@ -717,7 +717,6 @@ func TestManagedMachinePoolUpdate(t *testing.T) {
 					},
 					SshPublicKey: "test-ssh-public-key",
 					NodePoolNodeConfig: &infrav1exp.NodePoolNodeConfig{
-						UpdateNodePoolSize: true,
 						PlacementConfigs: []infrav1exp.PlacementConfig{
 							{
 								AvailabilityDomain:    common.String("test-ad"),
@@ -840,6 +839,108 @@ func TestManagedMachinePoolUpdate(t *testing.T) {
 				},
 			},
 		},
+		{
+			name:          "no update due to change in replica size as annotation is set",
+			errorExpected: false,
+			testSpecificSetup: func(cs *ManagedMachinePoolScope, okeClient *mock_containerengine.MockClient) {
+				ms.OCIManagedCluster.Spec.OCIResourceIdentifier = "resource_uid"
+				ms.MachinePool.Annotations = make(map[string]string)
+				ms.MachinePool.Annotations[clusterv1.ReplicasManagedByAnnotation] = ""
+				newReplicas := int32(4)
+				ms.MachinePool.Spec.Replicas = &newReplicas
+				ms.OCIManagedMachinePool.Spec = infrav1exp.OCIManagedMachinePoolSpec{
+					Version:      common.String("v1.24.5"),
+					ID:           common.String("node-pool-id"),
+					NodeMetadata: map[string]string{"key1": "value1"},
+					InitialNodeLabels: []infrav1exp.KeyValue{{
+						Key:   common.String("key"),
+						Value: common.String("value"),
+					}},
+					NodeShape: "test-shape",
+					NodeShapeConfig: &infrav1exp.NodeShapeConfig{
+						Ocpus:       common.String("2"),
+						MemoryInGBs: common.String("16"),
+					},
+					NodeSourceViaImage: &infrav1exp.NodeSourceViaImage{
+						ImageId: common.String("test-image-id"),
+					},
+					SshPublicKey: "test-ssh-public-key",
+					NodePoolNodeConfig: &infrav1exp.NodePoolNodeConfig{
+						PlacementConfigs: []infrav1exp.PlacementConfig{
+							{
+								AvailabilityDomain:    common.String("test-ad"),
+								SubnetName:            common.String("worker-subnet"),
+								CapacityReservationId: common.String("cap-id"),
+								FaultDomains:          []string{"fd-1", "fd-2"},
+							},
+						},
+						NsgNames:                       []string{"worker-nsg"},
+						KmsKeyId:                       common.String("kms-key-id"),
+						IsPvEncryptionInTransitEnabled: common.Bool(true),
+						NodePoolPodNetworkOptionDetails: &infrav1exp.NodePoolPodNetworkOptionDetails{
+							CniType: infrav1exp.VCNNativeCNI,
+							VcnIpNativePodNetworkOptions: infrav1exp.VcnIpNativePodNetworkOptions{
+								SubnetNames:    []string{"pod-subnet"},
+								MaxPodsPerNode: common.Int(31),
+								NSGNames:       []string{"pod-nsg"},
+							},
+						},
+					},
+					NodeEvictionNodePoolSettings: &infrav1exp.NodeEvictionNodePoolSettings{
+						EvictionGraceDuration:           common.String("PT30M"),
+						IsForceDeleteAfterGraceDuration: common.Bool(true),
+					},
+				}
+			},
+			nodePool: oke.NodePool{
+				ClusterId:         common.String("cluster-id"),
+				Id:                common.String("node-pool-id"),
+				Name:              common.String("test"),
+				CompartmentId:     common.String("test-compartment"),
+				KubernetesVersion: common.String("v1.24.5"),
+				NodeMetadata:      map[string]string{"key1": "value1"},
+				InitialNodeLabels: []oke.KeyValue{{
+					Key:   common.String("key"),
+					Value: common.String("value"),
+				}},
+				NodeShape: common.String("test-shape"),
+				NodeShapeConfig: &oke.NodeShapeConfig{
+					Ocpus:       common.Float32(2),
+					MemoryInGBs: common.Float32(16),
+				},
+				NodeSourceDetails: oke.NodeSourceViaImageDetails{
+					ImageId: common.String("test-image-id"),
+				},
+				FreeformTags: tags,
+				DefinedTags:  definedTagsInterface,
+				SshPublicKey: common.String("test-ssh-public-key"),
+				NodeConfigDetails: &oke.NodePoolNodeConfigDetails{
+					Size: common.Int(3),
+					PlacementConfigs: []oke.NodePoolPlacementConfigDetails{
+						{
+							AvailabilityDomain:    common.String("test-ad"),
+							SubnetId:              common.String("subnet-id"),
+							CapacityReservationId: common.String("cap-id"),
+							FaultDomains:          []string{"fd-1", "fd-2"},
+						},
+					},
+					NsgIds:                         []string{"nsg-id"},
+					KmsKeyId:                       common.String("kms-key-id"),
+					IsPvEncryptionInTransitEnabled: common.Bool(true),
+					FreeformTags:                   tags,
+					DefinedTags:                    definedTagsInterface,
+					NodePoolPodNetworkOptionDetails: oke.OciVcnIpNativeNodePoolPodNetworkOptionDetails{
+						PodSubnetIds:   []string{"pod-subnet-id"},
+						MaxPodsPerNode: common.Int(31),
+						PodNsgIds:      []string{"pod-nsg-id"},
+					},
+				},
+				NodeEvictionNodePoolSettings: &oke.NodeEvictionNodePoolSettings{
+					EvictionGraceDuration:           common.String("PT30M"),
+					IsForceDeleteAfterGraceDuration: common.Bool(true),
+				},
+			},
+		},
 		{
 			name:          "update due to change in k8s version",
 			errorExpected: false,

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

@@ -156,17 +156,6 @@ spec:
                           type: string
                       type: object
                     type: array
-                  updateNodePoolSize:
-                    description: UpdateNodePoolSize defines whether the node pool
-                      size should be updated when there is a spec change. By default
-                      the value is false, which means that on machine pool update,
-                      the node pool size is not updated. This is to make sure the
-                      provider reconciliation of node pool does not overwrite any
-                      changes done by an external system, most probably Kubernetes
-                      Cluster Autoscaler. This property can be set ot true if CAPI
-                      based cluster auto scaler is used rather than the cloud provider
-                      specific one.
-                    type: boolean
                 type: object
               nodeShape:
                 description: NodeShape defines the name of the node shape of the nodes

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

@@ -173,18 +173,6 @@ spec:
                                   type: string
                               type: object
                             type: array
-                          updateNodePoolSize:
-                            description: UpdateNodePoolSize defines whether the node
-                              pool size should be updated when there is a spec change.
-                              By default the value is false, which means that on machine
-                              pool update, the node pool size is not updated. This
-                              is to make sure the provider reconciliation of node
-                              pool does not overwrite any changes done by an external
-                              system, most probably Kubernetes Cluster Autoscaler.
-                              This property can be set ot true if CAPI based cluster
-                              auto scaler is used rather than the cloud provider specific
-                              one.
-                            type: boolean
                         type: object
                       nodeShape:
                         description: NodeShape defines the name of the node shape

exp/api/v1beta1/ocimanagedmachinepool_types.go

@@ -88,14 +88,6 @@ type NodePoolNodeConfig struct {
 	// NodePoolPodNetworkOptionDetails defines the pod networking details of the node pool
 	// +optional
 	NodePoolPodNetworkOptionDetails *NodePoolPodNetworkOptionDetails `json:"nodePoolPodNetworkOptionDetails,omitempty"`
-
-	// UpdateNodePoolSize defines whether the node pool size should be updated when there is a spec change.
-	// By default the value is false, which means that on machine pool update, the node pool size is not updated.
-	// This is to make sure the provider reconciliation of node pool does not overwrite any changes done by an external system,
-	// most probably Kubernetes Cluster Autoscaler. This property can be set ot true if CAPI based cluster auto scaler is used
-	// rather than the cloud provider specific one.
-	// +optional
-	UpdateNodePoolSize bool `json:"updateNodePoolSize,omitempty"`
 }
 
 const (

templates/cluster-template-managed-private.yaml

@@ -58,6 +58,8 @@ kind: MachinePool
 metadata:
   name: ${CLUSTER_NAME}-mp-0
   namespace: default
+  annotations:
+    "cluster.x-k8s.io/replicas-managed-by": ""
 spec:
   clusterName: ${CLUSTER_NAME}
   replicas: ${NODE_MACHINE_COUNT}

templates/cluster-template-managed.yaml

@@ -39,6 +39,8 @@ kind: MachinePool
 metadata:
   name: ${CLUSTER_NAME}-mp-0
   namespace: default
+  annotations:
+    "cluster.x-k8s.io/replicas-managed-by": ""
 spec:
   clusterName: ${CLUSTER_NAME}
   replicas: ${NODE_MACHINE_COUNT}

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

@@ -35,6 +35,4 @@ spec:
   nodeShapeConfig:
     memoryInGBs: "16"
     ocpus: "1"
-  nodePoolNodeConfig:
-    updateNodePoolSize: true
 ---