Add node pool recycling (#285)

* Add OKE node pool recycling option
oracle · Jun 22, 2023 · e5676d6 · e5676d6
1 parent 40c3141
commit e5676d6
Show file tree

Hide file tree

Showing 24 changed files with 573 additions and 51 deletions.
diff --git a/Makefile b/Makefile
@@ -289,6 +289,7 @@ generate-e2e-templates: $(KUSTOMIZE)
 	$(KUSTOMIZE) build $(OCI_TEMPLATES)/v1beta2/cluster-template-externally-managed-vcn --load-restrictor LoadRestrictionsNone > $(OCI_TEMPLATES)/v1beta2/cluster-template-externally-managed-vcn.yaml
 	$(KUSTOMIZE) build $(OCI_TEMPLATES)/v1beta2/cluster-template-machine-pool --load-restrictor LoadRestrictionsNone > $(OCI_TEMPLATES)/v1beta2/cluster-template-machine-pool.yaml
 	$(KUSTOMIZE) build $(OCI_TEMPLATES)/v1beta2/cluster-template-managed --load-restrictor LoadRestrictionsNone > $(OCI_TEMPLATES)/v1beta2/cluster-template-managed.yaml
+	$(KUSTOMIZE) build $(OCI_TEMPLATES)/v1beta2/cluster-template-managed-node-recycling --load-restrictor LoadRestrictionsNone > $(OCI_TEMPLATES)/v1beta2/cluster-template-managed-node-recycling.yaml
 	$(KUSTOMIZE) build $(OCI_TEMPLATES)/v1beta2/cluster-template-managed-cluster-identity --load-restrictor LoadRestrictionsNone > $(OCI_TEMPLATES)/v1beta2/cluster-template-managed-cluster-identity.yaml
 	$(KUSTOMIZE) build $(OCI_TEMPLATES)/v1beta2/cluster-template-cluster-identity --load-restrictor LoadRestrictionsNone > $(OCI_TEMPLATES)/v1beta2/cluster-template-cluster-identity.yaml
 	$(KUSTOMIZE) build $(OCI_TEMPLATES)/v1beta2/cluster-template-windows-calico --load-restrictor LoadRestrictionsNone > $(OCI_TEMPLATES)/v1beta2/cluster-template-windows-calico.yaml

diff --git a/cloud/scope/managed_control_plane.go b/cloud/scope/managed_control_plane.go
@@ -588,6 +588,9 @@ func (s *ManagedControlPlaneScope) UpdateControlPlane(ctx context.Context, okeCl
 // there is a chance user will edit the cluster
 func setControlPlaneSpecDefaults(spec *infrav2exp.OCIManagedControlPlaneSpec) {
 	spec.ControlPlaneEndpoint = clusterv1.APIEndpoint{}
+	if spec.ClusterType == "" {
+		spec.ClusterType = infrav2exp.BasicClusterType
+	}
 	if spec.ImagePolicyConfig == nil {
 		spec.ImagePolicyConfig = &infrav2exp.ImagePolicyConfig{
 			IsPolicyEnabled: common.Bool(false),
@@ -663,6 +666,7 @@ func (s *ManagedControlPlaneScope) getSpecFromActual(cluster *oke.Cluster) *infr
 			spec.ClusterType = infrav2exp.EnhancedClusterType
 			break
 		default:
+			spec.ClusterType = infrav2exp.BasicClusterType
 			break
 		}
 	}

diff --git a/cloud/scope/managed_control_plane_test.go b/cloud/scope/managed_control_plane_test.go
@@ -473,6 +473,7 @@ func TestControlPlaneUpdation(t *testing.T) {
 				CompartmentId:     common.String("test-compartment"),
 				VcnId:             common.String("vcn-id"),
 				KubernetesVersion: common.String("v1.24.5"),
+				Type:              oke.ClusterTypeBasicCluster,
 				FreeformTags:      tags,
 				DefinedTags:       definedTagsInterface,
 				EndpointConfig: &oke.ClusterEndpointConfig{

diff --git a/cloud/scope/managed_machine_pool.go b/cloud/scope/managed_machine_pool.go
@@ -313,6 +313,14 @@ func (m *ManagedMachinePoolScope) CreateNodePool(ctx context.Context) (*oke.Node
 			IsForceDeleteAfterGraceDuration: m.OCIManagedMachinePool.Spec.NodeEvictionNodePoolSettings.IsForceDeleteAfterGraceDuration,
 		}
 	}
+	recycleConfig := m.OCIManagedMachinePool.Spec.NodePoolCyclingDetails
+	if recycleConfig != nil {
+		nodePoolDetails.NodePoolCyclingDetails = &oke.NodePoolCyclingDetails{
+			IsNodeCyclingEnabled: recycleConfig.IsNodeCyclingEnabled,
+			MaximumSurge:         recycleConfig.MaximumSurge,
+			MaximumUnavailable:   recycleConfig.MaximumUnavailable,
+		}
+	}
 	nodePoolDetails.InitialNodeLabels = m.getInitialNodeKeyValuePairs()
 
 	req := oke.CreateNodePoolRequest{
@@ -603,16 +611,22 @@ func (m *ManagedMachinePoolScope) UpdateNodePool(ctx context.Context, pool *oke.
 			return false, err
 		}
 		m.Logger.Info("Node pool", "spec", jsonSpec, "actual", jsonActual)
-		placementConfig, err := m.buildPlacementConfig(spec.NodePoolNodeConfig.PlacementConfigs)
-		if err != nil {
-			return false, err
-		}
+
 		nodeConfigDetails := oke.UpdateNodePoolNodeConfigDetails{
 			NsgIds:                         m.getWorkerMachineNSGs(),
-			PlacementConfigs:               placementConfig,
 			IsPvEncryptionInTransitEnabled: spec.NodePoolNodeConfig.IsPvEncryptionInTransitEnabled,
 			KmsKeyId:                       spec.NodePoolNodeConfig.KmsKeyId,
 		}
+		// send placement config only if there is an actual change in placement
+		// placement config and recycle config cannot be sent at the same time, and most use cases will
+		// be to update kubernetes version in which case, placement config is not required to be sent
+		if !reflect.DeepEqual(spec.NodePoolNodeConfig.PlacementConfigs, actual.NodePoolNodeConfig.PlacementConfigs) {
+			placementConfig, err := m.buildPlacementConfig(spec.NodePoolNodeConfig.PlacementConfigs)
+			if err != nil {
+				return false, err
+			}
+			nodeConfigDetails.PlacementConfigs = placementConfig
+		}
 		if nodePoolSizeUpdateRequired {
 			nodeConfigDetails.Size = common.Int(int(*m.MachinePool.Spec.Replicas))
 		}
@@ -643,7 +657,9 @@ func (m *ManagedMachinePoolScope) UpdateNodePool(ctx context.Context, pool *oke.
 			return false, err
 		}
 		sourceDetails := oke.NodeSourceViaImageDetails{
-			ImageId:             spec.NodeSourceViaImage.ImageId,
+			// use image id from machinepool spec itself as the copy will not have the image set in the
+			// setNodepoolImageId method above
+			ImageId:             m.OCIManagedMachinePool.Spec.NodeSourceViaImage.ImageId,
 			BootVolumeSizeInGBs: spec.NodeSourceViaImage.BootVolumeSizeInGBs,
 		}
 
@@ -672,6 +688,19 @@ func (m *ManagedMachinePoolScope) UpdateNodePool(ctx context.Context, pool *oke.
 			NodeConfigDetails: &nodeConfigDetails,
 			NodeMetadata:      spec.NodeMetadata,
 		}
+		recycleConfig := spec.NodePoolCyclingDetails
+		// cannot send recycle config and placement config together
+		if recycleConfig != nil && len(nodeConfigDetails.PlacementConfigs) == 0 {
+			nodePoolDetails.NodePoolCyclingDetails = &oke.NodePoolCyclingDetails{
+				IsNodeCyclingEnabled: recycleConfig.IsNodeCyclingEnabled,
+				MaximumSurge:         recycleConfig.MaximumSurge,
+				MaximumUnavailable:   recycleConfig.MaximumUnavailable,
+			}
+		}
+		if recycleConfig != nil && len(nodeConfigDetails.PlacementConfigs) != 0 {
+			m.Logger.V(LogLevelWarn).Info("Placement configuration has been changed in the update, " +
+				"hence node pool recycling configuration will not be sent with the update request")
+		}
 		if spec.NodeEvictionNodePoolSettings != nil {
 			nodePoolDetails.NodeEvictionNodePoolSettings = &oke.NodeEvictionNodePoolSettings{
 				EvictionGraceDuration:           spec.NodeEvictionNodePoolSettings.EvictionGraceDuration,
@@ -701,6 +730,7 @@ func (m *ManagedMachinePoolScope) UpdateNodePool(ctx context.Context, pool *oke.
 func setMachinePoolSpecDefaults(spec *infrav2exp.OCIManagedMachinePoolSpec) {
 	spec.ProviderIDList = nil
 	spec.ProviderID = nil
+
 	if spec.NodePoolNodeConfig != nil {
 		if spec.NodePoolNodeConfig.PlacementConfigs != nil {
 			configs := spec.NodePoolNodeConfig.PlacementConfigs
@@ -782,6 +812,14 @@ func (m *ManagedMachinePoolScope) getSpecFromAPIObject(pool *oke.NodePool) *expi
 		}
 		spec.NodeShapeConfig = &nodeShapeConfig
 	}
+	if pool.NodePoolCyclingDetails != nil {
+		cyclingDetails := pool.NodePoolCyclingDetails
+		spec.NodePoolCyclingDetails = &expinfra1.NodePoolCyclingDetails{
+			IsNodeCyclingEnabled: cyclingDetails.IsNodeCyclingEnabled,
+			MaximumSurge:         cyclingDetails.MaximumSurge,
+			MaximumUnavailable:   cyclingDetails.MaximumUnavailable,
+		}
+	}
 	return &spec
 }
 

diff --git a/cloud/scope/managed_machine_pool_test.go b/cloud/scope/managed_machine_pool_test.go
@@ -1129,15 +1129,7 @@ func TestManagedMachinePoolUpdate(t *testing.T) {
 						},
 						SshPublicKey: common.String("test-ssh-public-key"),
 						NodeConfigDetails: &oke.UpdateNodePoolNodeConfigDetails{
-							Size: common.Int(4),
-							PlacementConfigs: []oke.NodePoolPlacementConfigDetails{
-								{
-									AvailabilityDomain:    common.String("test-ad"),
-									CapacityReservationId: common.String("cap-id"),
-									SubnetId:              common.String("subnet-id"),
-									FaultDomains:          []string{"fd-1", "fd-2"},
-								},
-							},
+							Size:                           common.Int(4),
 							NsgIds:                         []string{"nsg-id"},
 							KmsKeyId:                       common.String("kms-key-id"),
 							IsPvEncryptionInTransitEnabled: common.Bool(true),
@@ -1376,14 +1368,6 @@ func TestManagedMachinePoolUpdate(t *testing.T) {
 						},
 						SshPublicKey: common.String("test-ssh-public-key"),
 						NodeConfigDetails: &oke.UpdateNodePoolNodeConfigDetails{
-							PlacementConfigs: []oke.NodePoolPlacementConfigDetails{
-								{
-									AvailabilityDomain:    common.String("test-ad"),
-									CapacityReservationId: common.String("cap-id"),
-									SubnetId:              common.String("subnet-id"),
-									FaultDomains:          []string{"fd-1", "fd-2"},
-								},
-							},
 							NsgIds:                         []string{"nsg-id"},
 							KmsKeyId:                       common.String("kms-key-id"),
 							IsPvEncryptionInTransitEnabled: common.Bool(true),
@@ -1618,6 +1602,11 @@ func TestManagedMachinePoolUpdate(t *testing.T) {
 					NodeSourceViaImage: &infrav2exp.NodeSourceViaImage{
 						ImageId: common.String("test-image-id"),
 					},
+					NodePoolCyclingDetails: &infrav2exp.NodePoolCyclingDetails{
+						IsNodeCyclingEnabled: common.Bool(true),
+						MaximumSurge:         common.String("20%"),
+						MaximumUnavailable:   common.String("10%"),
+					},
 					SshPublicKey: "test-ssh-public-key",
 					NodePoolNodeConfig: &infrav2exp.NodePoolNodeConfig{
 						PlacementConfigs: []infrav2exp.PlacementConfig{
@@ -1663,16 +1652,13 @@ func TestManagedMachinePoolUpdate(t *testing.T) {
 						NodeSourceDetails: &oke.NodeSourceViaImageDetails{
 							ImageId: common.String("test-image-id"),
 						},
+						NodePoolCyclingDetails: &oke.NodePoolCyclingDetails{
+							IsNodeCyclingEnabled: common.Bool(true),
+							MaximumSurge:         common.String("20%"),
+							MaximumUnavailable:   common.String("10%"),
+						},
 						SshPublicKey: common.String("test-ssh-public-key"),
 						NodeConfigDetails: &oke.UpdateNodePoolNodeConfigDetails{
-							PlacementConfigs: []oke.NodePoolPlacementConfigDetails{
-								{
-									AvailabilityDomain:    common.String("test-ad"),
-									CapacityReservationId: common.String("cap-id"),
-									SubnetId:              common.String("subnet-id"),
-									FaultDomains:          []string{"fd-1", "fd-2"},
-								},
-							},
 							NsgIds:                         []string{"nsg-id"},
 							KmsKeyId:                       common.String("kms-key-id"),
 							IsPvEncryptionInTransitEnabled: common.Bool(true),

diff --git a/cloud/scope/util.go b/cloud/scope/util.go
@@ -20,6 +20,10 @@ import (
 	infrastructurev1beta2 "github.com/oracle/cluster-api-provider-oci/api/v1beta2"
 )
 
+const (
+	LogLevelWarn = 3
+)
+
 // GetNsgNamesFromId returns the names of the NSGs with the provided IDs
 func GetNsgNamesFromId(ids []string, nsgs []*infrastructurev1beta2.NSG) []string {
 	names := make([]string, 0)

diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_ocimanagedmachinepools.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_ocimanagedmachinepools.yaml
@@ -339,6 +339,28 @@ spec:
                 description: NodeMetadata defines a list of key/value pairs to add
                   to each underlying OCI instance in the node pool on launch.
                 type: object
+              nodePoolCyclingDetails:
+                description: NodePoolCyclingDetails defines the node pool recycling
+                  options.
+                properties:
+                  isNodeCyclingEnabled:
+                    description: IsNodeCyclingEnabled refers if nodes in the nodepool
+                      will be cycled to have new changes.
+                    type: boolean
+                  maximumSurge:
+                    description: MaximumSurge refers to the maximum additional new
+                      compute instances that would be temporarily created and added
+                      to nodepool during the cycling nodepool process. OKE supports
+                      both integer and percentage input. Defaults to 1, Ranges from
+                      0 to Nodepool size or 0% to 100%
+                    type: string
+                  maximumUnavailable:
+                    description: Maximum active nodes that would be terminated from
+                      nodepool during the cycling nodepool process. OKE supports both
+                      integer and percentage input. Defaults to 0, Ranges from 0 to
+                      Nodepool size or 0% to 100%
+                    type: string
+                type: object
               nodePoolNodeConfig:
                 description: NodePoolNodeConfig defines the configuration of nodes
                   in the node pool.

diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_ocimanagedmachinepooltemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_ocimanagedmachinepooltemplates.yaml
@@ -308,6 +308,28 @@ spec:
                           to add to each underlying OCI instance in the node pool
                           on launch.
                         type: object
+                      nodePoolCyclingDetails:
+                        description: NodePoolCyclingDetails defines the node pool
+                          recycling options.
+                        properties:
+                          isNodeCyclingEnabled:
+                            description: IsNodeCyclingEnabled refers if nodes in the
+                              nodepool will be cycled to have new changes.
+                            type: boolean
+                          maximumSurge:
+                            description: MaximumSurge refers to the maximum additional
+                              new compute instances that would be temporarily created
+                              and added to nodepool during the cycling nodepool process.
+                              OKE supports both integer and percentage input. Defaults
+                              to 1, Ranges from 0 to Nodepool size or 0% to 100%
+                            type: string
+                          maximumUnavailable:
+                            description: Maximum active nodes that would be terminated
+                              from nodepool during the cycling nodepool process. OKE
+                              supports both integer and percentage input. Defaults
+                              to 0, Ranges from 0 to Nodepool size or 0% to 100%
+                            type: string
+                        type: object
                       nodePoolNodeConfig:
                         description: NodePoolNodeConfig defines the configuration
                           of nodes in the node pool.

diff --git a/exp/api/v1beta1/conversion.go b/exp/api/v1beta1/conversion.go
@@ -51,3 +51,7 @@ func Convert_v1beta2_NetworkDetails_To_v1beta1_NetworkDetails(in *infrastructure
 func Convert_v1beta2_OCIManagedControlPlaneSpec_To_v1beta1_OCIManagedControlPlaneSpec(in *v1beta2.OCIManagedControlPlaneSpec, out *OCIManagedControlPlaneSpec, s conversion.Scope) error {
 	return autoConvert_v1beta2_OCIManagedControlPlaneSpec_To_v1beta1_OCIManagedControlPlaneSpec(in, out, s)
 }
+
+func Convert_v1beta2_OCIManagedMachinePoolSpec_To_v1beta1_OCIManagedMachinePoolSpec(in *v1beta2.OCIManagedMachinePoolSpec, out *OCIManagedMachinePoolSpec, s conversion.Scope) error {
+	return autoConvert_v1beta2_OCIManagedMachinePoolSpec_To_v1beta1_OCIManagedMachinePoolSpec(in, out, s)
+}
diff --git a/exp/api/v1beta1/ocimanagedmachinepool_conversion.go b/exp/api/v1beta1/ocimanagedmachinepool_conversion.go
@@ -34,6 +34,7 @@ func (src *OCIManagedMachinePool) ConvertTo(dstRaw conversion.Hub) error {
 	if ok, err := utilconversion.UnmarshalData(src, restored); err != nil || !ok {
 		return err
 	}
+	dst.Spec.NodePoolCyclingDetails = restored.Spec.NodePoolCyclingDetails
 
 	return nil
 }