Feat: Support Externally Managed Autoscaler for MachinePools (#515)

Adding the check for annotation "cluster.x-k8s.io/replicas-managed-by"

Author: joekr

cloud/scope/machine_pool.go

@@ -44,6 +44,7 @@ import (
 	"k8s.io/utils/pointer"
 	clusterv1beta1 "sigs.k8s.io/cluster-api/api/core/v1beta1"
 	clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2"
+	"sigs.k8s.io/cluster-api/util/annotations"
 	v1beta1conditions "sigs.k8s.io/cluster-api/util/deprecated/v1beta1/conditions"
 	v1beta1patch "sigs.k8s.io/cluster-api/util/deprecated/v1beta1/patch"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -182,6 +183,36 @@ func (m *MachinePoolScope) SetReplicaCount(count int32) {
 	m.OCIMachinePool.Status.Replicas = count
 }
 
+// SyncReplicasFromInstancePool updates the owner MachinePool spec to match the
+// observed OCI instance pool size when replicas are externally managed.
+func (m *MachinePoolScope) SyncReplicasFromInstancePool(ctx context.Context, instancePool *core.InstancePool) error {
+	if !annotations.ReplicasManagedByExternalAutoscaler(m.MachinePool) {
+		return nil
+	}
+	if instancePool == nil || instancePool.Size == nil {
+		m.Info("Synced MachinePool instancePool or size is nil.")
+		return nil
+	}
+
+	observedReplicas := int32(*instancePool.Size)
+	if m.MachinePool.Spec.Replicas != nil && *m.MachinePool.Spec.Replicas == observedReplicas {
+		return nil
+	}
+
+	helper, err := v1beta1patch.NewHelper(m.MachinePool, m.Client)
+	if err != nil {
+		return errors.Wrap(err, "failed to init machinepool patch helper")
+	}
+
+	m.MachinePool.Spec.Replicas = pointer.Int32(observedReplicas)
+	if err := helper.Patch(ctx, m.MachinePool); err != nil {
+		return errors.Wrap(err, "failed to patch machinepool replicas from observed instance pool size")
+	}
+
+	m.Info("Synced MachinePool replicas from observed instance pool size", "replicas", observedReplicas)
+	return nil
+}
+
 // GetWorkerMachineSubnet returns the WorkerRole core.Subnet id for the cluster
 func (m *MachinePoolScope) GetWorkerMachineSubnet() *string {
 	for _, subnet := range ptr.ToSubnetSlice(m.OCIClusterAccesor.GetNetworkSpec().Vcn.Subnets) {
@@ -826,7 +857,7 @@ func instancePoolNeedsUpdates(machinePoolScope *MachinePoolScope, instancePool *
 	if instancePool.Size != nil {
 		instanePoolSize = *instancePool.Size
 	}
-	if machinePoolReplicas != instanePoolSize {
+	if !annotations.ReplicasManagedByExternalAutoscaler(machinePoolScope.MachinePool) && machinePoolReplicas != instanePoolSize {
 		return true
 	} else if !(reflect.DeepEqual(machinePoolScope.OCIMachinePool.Spec.InstanceConfiguration.InstanceConfigurationId, instancePool.InstanceConfigurationId)) {
 		return true

cloud/scope/machine_pool_test.go

@@ -33,6 +33,7 @@ import (
 	"github.com/oracle/oci-go-sdk/v65/core"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -1876,6 +1877,22 @@ func TestInstancePoolUpdate(t *testing.T) {
 					}, nil)
 			},
 		},
+		{
+			name:          "no update due to change in replica size as annotation is set",
+			errorExpected: false,
+			instancepool: &core.InstancePool{
+				Size:                    common.Int(3),
+				InstanceConfigurationId: common.String("config_id"),
+			},
+			testSpecificSetup: func(ms *MachinePoolScope) {
+				ms.MachinePool.Annotations = map[string]string{
+					clusterv1.ReplicasManagedByAnnotation: "", // empty value counts as true (= externally managed)
+				}
+				newReplicas := int32(4)
+				ms.MachinePool.Spec.Replicas = &newReplicas
+				ms.OCIMachinePool.Spec.InstanceConfiguration.InstanceConfigurationId = common.String("config_id")
+			},
+		},
 	}
 
 	for _, tc := range tests {
@@ -1893,3 +1910,120 @@ func TestInstancePoolUpdate(t *testing.T) {
 		})
 	}
 }
+
+func TestSyncReplicasFromInstancePool(t *testing.T) {
+	var (
+		ms       *MachinePoolScope
+		mockCtrl *gomock.Controller
+	)
+
+	setup := func(t *testing.T, g *WithT) {
+		var err error
+		mockCtrl = gomock.NewController(t)
+		secret := &corev1.Secret{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "bootstrap",
+				Namespace: "default",
+			},
+			Data: map[string][]byte{
+				"value": []byte("test"),
+			},
+		}
+		ociCluster := &infrastructurev1beta2.OCICluster{
+			ObjectMeta: metav1.ObjectMeta{
+				UID: "cluster_uid",
+			},
+			Spec: infrastructurev1beta2.OCIClusterSpec{
+				CompartmentId:         "test-compartment",
+				OCIResourceIdentifier: "resource_uid",
+			},
+		}
+		replicas := int32(3)
+		infraMachinePool := &infrav2exp.OCIMachinePool{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "test",
+				Namespace: "default",
+			},
+		}
+		machinePool := &clusterv1.MachinePool{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "test",
+				Namespace: "default",
+			},
+			Spec: clusterv1.MachinePoolSpec{
+				Replicas: &replicas,
+				Template: clusterv1.MachineTemplateSpec{
+					Spec: clusterv1.MachineSpec{
+						Bootstrap: clusterv1.Bootstrap{
+							DataSecretName: common.String("bootstrap"),
+						},
+					},
+				},
+			},
+		}
+		scheme := runtime.NewScheme()
+		g.Expect(corev1.AddToScheme(scheme)).To(Succeed())
+		g.Expect(clusterv1.AddToScheme(scheme)).To(Succeed())
+		g.Expect(infrav2exp.AddToScheme(scheme)).To(Succeed())
+
+		client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(secret, infraMachinePool, machinePool).Build()
+		ms, err = NewMachinePoolScope(MachinePoolScopeParams{
+			ComputeManagementClient: mock_computemanagement.NewMockClient(mockCtrl),
+			OCIMachinePool:          infraMachinePool,
+			OCIClusterAccessor: OCISelfManagedCluster{
+				OCICluster: ociCluster,
+			},
+			Cluster:     &clusterv1.Cluster{},
+			MachinePool: machinePool,
+			Client:      client,
+		})
+		g.Expect(err).To(BeNil())
+	}
+	teardown := func(t *testing.T, g *WithT) {
+		mockCtrl.Finish()
+	}
+
+	tests := []struct {
+		name             string
+		setup            func(ms *MachinePoolScope)
+		instancePool     *core.InstancePool
+		expectedReplicas int32
+	}{
+		{
+			name: "does not patch replicas when annotation is not set",
+			setup: func(ms *MachinePoolScope) {
+				ms.MachinePool.Annotations = nil
+			},
+			instancePool:     &core.InstancePool{Size: common.Int(4)},
+			expectedReplicas: 3,
+		},
+		{
+			name: "patches replicas from observed size when annotation is set",
+			setup: func(ms *MachinePoolScope) {
+				ms.MachinePool.Annotations = map[string]string{
+					clusterv1.ReplicasManagedByAnnotation: "", // empty value counts as true (= externally managed)
+				}
+			},
+			instancePool:     &core.InstancePool{Size: common.Int(4)},
+			expectedReplicas: 4,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			g := NewWithT(t)
+			defer teardown(t, g)
+			setup(t, g)
+			tc.setup(ms)
+
+			err := ms.SyncReplicasFromInstancePool(context.Background(), tc.instancePool)
+			g.Expect(err).To(BeNil())
+
+			updatedMachinePool := &clusterv1.MachinePool{}
+			err = ms.Client.Get(context.Background(), client.ObjectKey{Name: ms.MachinePool.Name, Namespace: ms.MachinePool.Namespace}, updatedMachinePool)
+			g.Expect(err).To(BeNil())
+			g.Expect(updatedMachinePool.Spec.Replicas).ToNot(BeNil())
+			g.Expect(*updatedMachinePool.Spec.Replicas).To(Equal(tc.expectedReplicas))
+		})
+	}
+}

config/rbac/role.yaml

@@ -1,4 +1,3 @@
-
 ---
 apiVersion: rbac.authorization.k8s.io/v1
 kind: ClusterRole
@@ -134,6 +133,8 @@ rules:
     - get
     - list
     - watch
+    - update
+    - patch
 - apiGroups:
     - ""
   resources:

exp/controllers/ocimachinepool_controller.go

@@ -387,6 +387,10 @@ func (r *OCIMachinePoolReconciler) reconcileNormal(ctx context.Context, logger l
 			return reconcile.Result{}, err
 		}
 
+		if err := machinePoolScope.SyncReplicasFromInstancePool(ctx, instancePool); err != nil {
+			return reconcile.Result{}, err
+		}
+
 		instancePool, err = machinePoolScope.UpdatePool(ctx, instancePool)
 		if err != nil {
 			r.Recorder.Eventf(machinePoolScope.OCIMachinePool, corev1.EventTypeWarning, "FailedUpdate", "Failed to update instance pool: %v", err)

exp/controllers/ocimachinepool_controller_test.go

@@ -223,7 +223,7 @@ func TestReconciliationFunction(t *testing.T) {
 		computeManagementClient = mock_computemanagement.NewMockClient(mockCtrl)
 		machinePool := getMachinePool()
 		ociMachinePool = getOCIMachinePool()
-		client := fake.NewClientBuilder().WithScheme(testScheme()).WithStatusSubresource(ociMachinePool).WithObjects(getSecret(), ociMachinePool).Build()
+		client := fake.NewClientBuilder().WithScheme(testScheme()).WithStatusSubresource(ociMachinePool).WithObjects(getSecret(), ociMachinePool, machinePool).Build()
 		ociCluster := getOCIClusterWithOwner()
 		ms, err = scope.NewMachinePoolScope(scope.MachinePoolScopeParams{
 			ComputeManagementClient: computeManagementClient,
@@ -383,6 +383,82 @@ func TestReconciliationFunction(t *testing.T) {
 				g.Expect(*machine.Spec.OCID).To(Equal("id-1"))
 			},
 		},
+		{
+			name:               "instance pool running with externally managed replicas",
+			errorExpected:      false,
+			conditionAssertion: []conditionAssertion{{infrav2exp.LaunchTemplateReadyCondition, corev1.ConditionTrue, "", ""}, {infrav2exp.InstancePoolReadyCondition, corev1.ConditionTrue, "", ""}},
+			testSpecificSetup: func(t *test, machinePoolScope *scope.MachinePoolScope, computeManagementClient *mock_computemanagement.MockClient) {
+				ms.OCIMachinePool.Spec.InstanceConfiguration = infrav2exp.InstanceConfiguration{
+					Shape:                   common.String("test-shape"),
+					InstanceConfigurationId: common.String("test"),
+				}
+				ms.MachinePool.Annotations = map[string]string{
+					clusterv1beta1.ReplicasManagedByAnnotation: "",
+				}
+				r.Client = interceptor.NewClient(fake.NewClientBuilder().WithScheme(testScheme()).WithObjects(getSecret(), ociMachinePool, ms.MachinePool).Build(), interceptor.Funcs{
+					Create: func(ctx context.Context, client client.WithWatch, obj client.Object, opts ...client.CreateOption) error {
+						m := obj.(*infrav2exp.OCIMachinePoolMachine)
+						t.createPoolMachines = append(t.createPoolMachines, *m)
+						return nil
+					},
+				})
+				ms.Client = r.Client
+				ms.MachinePool.Spec.Template.Spec.Bootstrap.DataSecretName = common.String("bootstrap")
+				ms.OCIMachinePool.Spec.OCID = common.String("pool-id")
+				computeManagementClient.EXPECT().GetInstanceConfiguration(gomock.Any(), gomock.Eq(core.GetInstanceConfigurationRequest{
+					InstanceConfigurationId: common.String("test"),
+				})).
+					Return(core.GetInstanceConfigurationResponse{
+						InstanceConfiguration: core.InstanceConfiguration{
+							Id: common.String("test"),
+							InstanceDetails: core.ComputeInstanceDetails{
+								LaunchDetails: &core.InstanceConfigurationLaunchInstanceDetails{
+									DefinedTags:   definedTagsInterface,
+									FreeformTags:  tags,
+									CompartmentId: common.String("test-compartment"),
+									Shape:         common.String("test-shape"),
+									CreateVnicDetails: &core.InstanceConfigurationCreateVnicDetails{
+										FreeformTags: tags,
+										NsgIds:       []string{"worker-nsg-id"},
+										SubnetId:     common.String("worker-subnet-id"),
+									},
+									SourceDetails: core.InstanceConfigurationInstanceSourceViaImageDetails{},
+									Metadata:      map[string]string{"user_data": "dGVzdA=="},
+								},
+							},
+						},
+					}, nil)
+
+				computeManagementClient.EXPECT().GetInstancePool(gomock.Any(), gomock.Any()).
+					Return(core.GetInstancePoolResponse{
+						InstancePool: core.InstancePool{
+							LifecycleState:          core.InstancePoolLifecycleStateRunning,
+							Id:                      common.String("pool-id"),
+							InstanceConfigurationId: common.String("test"),
+							Size:                    common.Int(4),
+						},
+					}, nil)
+				computeManagementClient.EXPECT().ListInstancePoolInstances(gomock.Any(), gomock.Any()).
+					Return(core.ListInstancePoolInstancesResponse{
+						Items: []core.InstanceSummary{{
+							Id:          common.String("id-1"),
+							State:       common.String("Running"),
+							DisplayName: common.String("name-1"),
+						}},
+					}, nil)
+				computeManagementClient.EXPECT().ListInstanceConfigurations(gomock.Any(), gomock.Any()).
+					Return(core.ListInstanceConfigurationsResponse{}, nil)
+			},
+			validate: func(g *WithT, t *test) {
+				g.Expect(len(t.createPoolMachines)).To(Equal(1))
+
+				updatedMachinePool := &clusterv1.MachinePool{}
+				err := ms.Client.Get(context.Background(), client.ObjectKey{Name: ms.MachinePool.Name, Namespace: ms.MachinePool.Namespace}, updatedMachinePool)
+				g.Expect(err).To(BeNil())
+				g.Expect(updatedMachinePool.Spec.Replicas).ToNot(BeNil())
+				g.Expect(*updatedMachinePool.Spec.Replicas).To(Equal(int32(4)))
+			},
+		},
 		{
 			name:               "delete unwanted machinepool machine",
 			errorExpected:      false,