feat: Add more MachinePool flexibility (#94)

This will now:

- keep track of machine pool instances in status
- variablize allowing public ips for the machine pool instances

Author: joekr

cloud/scope/cluster.go

@@ -168,21 +168,13 @@ func (s *ClusterScope) setFailureDomains(ctx context.Context) error {
 			})
 		}
 	} else {
-		req := identity.ListFaultDomainsRequest{
-			CompartmentId:      common.String(s.GetCompartmentId()),
-			AvailabilityDomain: respAd.Items[0].Name,
-		}
-		resp, err := s.IdentityClient.ListFaultDomains(ctx, req)
-		if err != nil {
-			s.Logger.Error(err, "failed to list fault domains")
-			return err
-		}
-		for i, fd := range resp.Items {
+		adName := *respAd.Items[0].Name
+		for i, fd := range s.OCICluster.Status.AvailabilityDomains[adName].FaultDomains {
 			s.SetFailureDomain(strconv.Itoa(i+1), clusterv1.FailureDomainSpec{
 				ControlPlane: true,
 				Attributes: map[string]string{
-					AvailabilityDomain: *fd.AvailabilityDomain,
-					FaultDomain:        *fd.Name,
+					AvailabilityDomain: adName,
+					FaultDomain:        fd,
 				},
 			})
 		}

cloud/scope/cluster_test.go

@@ -151,24 +151,6 @@ func TestClusterScope_ReconcileFailureDomains(t *testing.T) {
 		},
 	}}, nil)
 
-	identityClient.EXPECT().ListFaultDomains(gomock.Any(), gomock.Eq(identity.ListFaultDomainsRequest{
-		CompartmentId:      common.String("1ad"),
-		AvailabilityDomain: common.String("ad1"),
-	})).Return(identity.ListFaultDomainsResponse{Items: []identity.FaultDomain{
-		{
-			Name:               common.String("fd1"),
-			AvailabilityDomain: common.String("ad1"),
-		},
-		{
-			Name:               common.String("fd2"),
-			AvailabilityDomain: common.String("ad1"),
-		},
-		{
-			Name:               common.String("fd3"),
-			AvailabilityDomain: common.String("ad1"),
-		},
-	}}, nil)
-
 	identityClient.EXPECT().ListFaultDomains(gomock.Any(), gomock.Eq(identity.ListFaultDomainsRequest{
 		CompartmentId:      common.String("2ad"),
 		AvailabilityDomain: common.String("ad1"),

cloud/scope/machine_pool.go

@@ -140,6 +140,10 @@ func (m *MachinePoolScope) SetReady() {
 	m.OCIMachinePool.Status.Ready = true
 }
 
+func (m *MachinePoolScope) SetReplicaCount(count int32) {
+	m.OCIMachinePool.Status.Replicas = count
+}
+
 // GetWorkerMachineSubnet returns the WorkerRole core.Subnet id for the cluster
 func (m *MachinePoolScope) GetWorkerMachineSubnet() *string {
 	for _, subnet := range m.OCICluster.Spec.NetworkSpec.Vcn.Subnets {
@@ -150,6 +154,42 @@ func (m *MachinePoolScope) GetWorkerMachineSubnet() *string {
 	return nil
 }
 
+// ListMachinePoolInstances returns the WorkerRole core.Subnet id for the cluster
+func (m *MachinePoolScope) ListMachinePoolInstances(ctx context.Context) ([]core.InstanceSummary, error) {
+	poolOid := m.OCIMachinePool.Spec.OCID
+	if len(poolOid) <= 0 {
+		return nil, errors.New("OCIMachinePool OCID can't be empty")
+	}
+
+	req := core.ListInstancePoolInstancesRequest{
+		CompartmentId:  common.String(m.OCICluster.Spec.CompartmentId),
+		InstancePoolId: common.String(poolOid),
+	}
+	resp, err := m.ComputeManagementClient.ListInstancePoolInstances(ctx, req)
+	if err != nil {
+		return nil, err
+	}
+
+	return resp.Items, nil
+}
+
+// SetListandSetMachinePoolInstances retrieves a machine pools instances and sets them in the ProviderIDList
+func (m *MachinePoolScope) SetListandSetMachinePoolInstances(ctx context.Context) (int32, error) {
+	poolInstanceSummaries, err := m.ListMachinePoolInstances(ctx)
+	if err != nil {
+		return 0, errors.New("Unable to list machine pool's instances")
+	}
+	providerIDList := make([]string, len(poolInstanceSummaries))
+
+	for i, instance := range poolInstanceSummaries {
+		providerIDList[i] = fmt.Sprintf("oci://%s", *instance.Id)
+	}
+
+	m.OCIMachinePool.Spec.ProviderIDList = providerIDList
+
+	return int32(len(providerIDList)), nil
+}
+
 // GetBootstrapData returns the bootstrap data from the secret in the Machine's bootstrap.dataSecretName.
 func (m *MachinePoolScope) GetBootstrapData() (string, error) {
 	if m.MachinePool.Spec.Template.Spec.Bootstrap.DataSecretName == nil {
@@ -330,9 +370,8 @@ func (m *MachinePoolScope) ReconcileInstanceConfiguration(ctx context.Context) e
 				},
 				Metadata: metadata,
 				CreateVnicDetails: &core.InstanceConfigurationCreateVnicDetails{
-					SubnetId: subnetId,
-					// TODO variablize AssignPublicIp in the future
-					AssignPublicIp: common.Bool(false),
+					SubnetId:       subnetId,
+					AssignPublicIp: common.Bool(m.OCIMachinePool.Spec.VNICAssignPublicIp),
 					NsgIds:         []string{*nsgId},
 				},
 			},
@@ -472,7 +511,9 @@ func (m *MachinePoolScope) UpdatePool(ctx context.Context, instancePool *core.In
 		}
 
 		req := core.UpdateInstancePoolRequest{InstancePoolId: instancePool.Id,
-			UpdateInstancePoolDetails: core.UpdateInstancePoolDetails{Size: common.Int(replicas)}}
+			UpdateInstancePoolDetails: core.UpdateInstancePoolDetails{
+				Size: common.Int(replicas),
+			}}
 
 		if _, err := m.ComputeManagementClient.UpdateInstancePool(ctx, req); err != nil {
 			return errors.Wrap(err, "unable to update instance pool")
@@ -492,7 +533,7 @@ func (m *MachinePoolScope) TerminateInstancePool(ctx context.Context, instancePo
 	return nil
 }
 
-// instancePoolNeedsUpdates compares incoming OCIMachinePool and compares against existing ASG.
+// instancePoolNeedsUpdates compares incoming OCIMachinePool and compares against existing instance pool.
 func instancePoolNeedsUpdates(machinePoolScope *MachinePoolScope, instancePool *core.InstancePool) bool {
 
 	// Allow pool resize

cloud/services/computemanagement/client.go

@@ -13,6 +13,7 @@ type Client interface {
 	TerminateInstancePool(ctx context.Context, request core.TerminateInstancePoolRequest) (response core.TerminateInstancePoolResponse, err error)
 	UpdateInstancePool(ctx context.Context, request core.UpdateInstancePoolRequest) (response core.UpdateInstancePoolResponse, err error)
 	ListInstancePools(ctx context.Context, request core.ListInstancePoolsRequest) (response core.ListInstancePoolsResponse, err error)
+	ListInstancePoolInstances(ctx context.Context, request core.ListInstancePoolInstancesRequest) (response core.ListInstancePoolInstancesResponse, err error)
 
 	// Instance Configuration
 	CreateInstanceConfiguration(ctx context.Context, request core.CreateInstanceConfigurationRequest) (response core.CreateInstanceConfigurationResponse, err error)

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

@@ -54,6 +54,9 @@ spec:
                 description: Custom metadata key/value pairs that you provide, such
                   as the SSH public key required to connect to the instance.
                 type: object
+              ocid:
+                description: OCID is the OCID of the associated InstancePool
+                type: string
               placementDetails:
                 items:
                   properties:
@@ -65,8 +68,17 @@ spec:
                   type: object
                 type: array
               providerID:
-                description: ProviderID is the ARN of the associated InstancePool
+                description: ProviderID is the OCID of the associated InstancePool
+                  in a provider format
                 type: string
+              providerIDList:
+                description: ProviderIDList are the identification IDs of machine
+                  instances provided by the provider. This field must match the provider
+                  IDs as seen on the node objects corresponding to a machine pool's
+                  machine instances.
+                items:
+                  type: string
+                type: array
               shapeConfig:
                 description: The shape configuration of the instance, applicable for
                   flex instances.
@@ -88,6 +100,10 @@ spec:
                     description: The total number of OCPUs available to the instance.
                     type: string
                 type: object
+              vnicAssignPublicIp:
+                default: false
+                description: Whether the VNIC should be assigned a public IP address.
+                type: boolean
             type: object
           status:
             description: OCIMachinePoolStatus defines the observed state of OCIMachinePool

exp/api/v1beta1/ocimachinepool_types.go

@@ -33,10 +33,14 @@ const (
 
 // OCIMachinePoolSpec defines the desired state of OCIMachinePool
 type OCIMachinePoolSpec struct {
-	// ProviderID is the ARN of the associated InstancePool
+	// ProviderID is the OCID of the associated InstancePool in a provider format
 	// +optional
 	ProviderID *string `json:"providerID,omitempty"`
 
+	// OCID is the OCID of the associated InstancePool
+	// +optional
+	OCID string `json:"ocid,omitempty"`
+
 	// OCID of the image to be used to launch the instance.
 	ImageId string `json:"imageId,omitempty"`
 
@@ -47,9 +51,19 @@ type OCIMachinePoolSpec struct {
 	// The shape configuration of the instance, applicable for flex instances.
 	ShapeConfig ShapeConfig `json:"shapeConfig,omitempty"`
 
+	// Whether the VNIC should be assigned a public IP address.
+	// +kubebuilder:default=false
+	// +optional
+	VNICAssignPublicIp bool `json:"vnicAssignPublicIp,omitempty"`
+
 	PlacementDetails []PlacementDetails `json:"placementDetails,omitempty"`
 
 	InstanceConfiguration InstanceConfiguration `json:"instanceConfiguration,omitempty"`
+
+	// ProviderIDList are the identification IDs of machine instances provided by the provider.
+	// This field must match the provider IDs as seen on the node objects corresponding to a machine pool's machine instances.
+	// +optional
+	ProviderIDList []string `json:"providerIDList,omitempty"`
 }
 
 type InstanceConfiguration struct {

exp/api/v1beta1/zz_generated.deepcopy.go

@@ -301,19 +301,31 @@ func (r *OCIMachinePoolReconciler) reconcileNormal(ctx context.Context, logger l
 
 	machinePoolScope.Info("OCI Compute Instance Pool found", "InstancePoolId", *instancePool.Id)
 	machinePoolScope.OCIMachinePool.Spec.ProviderID = common.String(fmt.Sprintf("oci://%s", *instancePool.Id))
+	machinePoolScope.OCIMachinePool.Spec.OCID = *instancePool.Id
 
 	switch instancePool.LifecycleState {
 	case core.InstancePoolLifecycleStateProvisioning, core.InstancePoolLifecycleStateStarting:
 		machinePoolScope.Info("Instance Pool is pending")
 		conditions.MarkFalse(machinePoolScope.OCIMachinePool, infrav1exp.InstancePoolReadyCondition, infrav1exp.InstancePoolNotReadyReason, clusterv1.ConditionSeverityInfo, "")
 		return reconcile.Result{RequeueAfter: 10 * time.Second}, nil
+	case core.InstancePoolLifecycleStateScaling:
+		machinePoolScope.Info("Instance Pool is scaling")
+		conditions.MarkFalse(machinePoolScope.OCIMachinePool, infrav1exp.InstancePoolReadyCondition, infrav1exp.InstancePoolNotReadyReason, clusterv1.ConditionSeverityInfo, "")
+		return reconcile.Result{RequeueAfter: 10 * time.Second}, nil
 	case core.InstancePoolLifecycleStateRunning:
 		machinePoolScope.Info("Instance pool is active")
 
 		// record the event only when pool goes from not ready to ready state
 		r.Recorder.Eventf(machinePoolScope.OCIMachinePool, corev1.EventTypeNormal, "InstancePoolReady",
 			"Instance pool is in ready state")
 		conditions.MarkTrue(machinePoolScope.OCIMachinePool, infrav1exp.InstancePoolReadyCondition)
+
+		instanceCount, err := machinePoolScope.SetListandSetMachinePoolInstances(ctx)
+		if err != nil {
+			return reconcile.Result{}, err
+		}
+
+		machinePoolScope.SetReplicaCount(instanceCount)
 		machinePoolScope.SetReady()
 	default:
 		conditions.MarkFalse(machinePoolScope.OCIMachinePool, infrav1exp.InstancePoolReadyCondition, infrav1exp.InstancePoolProvisionFailedReason, clusterv1.ConditionSeverityError, "")

exp/controllers/ocimachinepool_controller.go

@@ -123,6 +123,8 @@ spec:
   instanceConfiguration:
     instanceDetails:
       shape: "${OCI_NODE_MACHINE_TYPE=VM.Standard.E4.Flex}"
+# uncomment to assign a public IP address once you setup the VCN to allow public ips
+#  vnicAssignPublicIp: true
 #  uncomment if you want to set a specific availability domain
 #  placementDetails:
 #      - availabilityDomain: 1