diff --git a/cloud/scope/managed_machine_pool.go b/cloud/scope/managed_machine_pool.go index beb186900..c38ef22af 100644 --- a/cloud/scope/managed_machine_pool.go +++ b/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 { diff --git a/cloud/scope/managed_machine_pool_test.go b/cloud/scope/managed_machine_pool_test.go index 8f9ea052f..19d9a622f 100644 --- a/cloud/scope/managed_machine_pool_test.go +++ b/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, diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_ocimanagedmachinepools.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_ocimanagedmachinepools.yaml index e9d1df025..c7d71579b 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_ocimanagedmachinepools.yaml +++ b/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 diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_ocimanagedmachinepooltemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_ocimanagedmachinepooltemplates.yaml index 95e0c1468..3cfd5f30a 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_ocimanagedmachinepooltemplates.yaml +++ b/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 diff --git a/exp/api/v1beta1/ocimanagedmachinepool_types.go b/exp/api/v1beta1/ocimanagedmachinepool_types.go index 946faad0d..7d4709ea7 100644 --- a/exp/api/v1beta1/ocimanagedmachinepool_types.go +++ b/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 ( diff --git a/templates/cluster-template-managed-private.yaml b/templates/cluster-template-managed-private.yaml index 8e5c0a416..b0a189f62 100644 --- a/templates/cluster-template-managed-private.yaml +++ b/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} diff --git a/templates/cluster-template-managed.yaml b/templates/cluster-template-managed.yaml index 95b41f990..31b4ade8d 100644 --- a/templates/cluster-template-managed.yaml +++ b/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} diff --git a/test/e2e/data/infrastructure-oci/v1beta1/cluster-template-managed/machine-pool.yaml b/test/e2e/data/infrastructure-oci/v1beta1/cluster-template-managed/machine-pool.yaml index 9a6a5d9c5..21d7b23bb 100644 --- a/test/e2e/data/infrastructure-oci/v1beta1/cluster-template-managed/machine-pool.yaml +++ b/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 --- \ No newline at end of file