Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ Add externallyManagedReplicaCount field to MachinePool #5685

Closed
wants to merge 2 commits into from
Closed

✨ Add externallyManagedReplicaCount field to MachinePool #5685

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2288064
🌱 Fix comment for MachinePool noderefs
dthorsen Nov 16, 2021
b5889a6
✨ Add externallyManagedReplicaCount field to MachinePool
dthorsen Nov 16, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions config/crd/bases/cluster.x-k8s.io_machinepools.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1011,6 +1011,14 @@ spec:
to.
minLength: 1
type: string
externallyManagedReplicaCount:
description: Whether the Replicas value is externally managed. This
is useful when the infrastructure is a managed machine pool, or
if the cluster-autoscaler is scaling the underlying cloud infrastructure
directly outside of cluster-api. Set this to true if you wish the
replica count to be managed outside of cluster-api. Defaults to
false.
type: boolean
failureDomains:
description: FailureDomains is the list of failure domains this MachinePool
should be attached to.
Expand Down
32 changes: 30 additions & 2 deletions exp/api/v1alpha3/conversion.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/conversion"

expv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1"
utilconversion "sigs.k8s.io/cluster-api/util/conversion"
)

// Convert_v1alpha3_MachinePoolSpec_To_v1beta1_MachinePoolSpec is an autogenerated conversion function.
Expand Down Expand Up @@ -60,13 +61,36 @@ func Convert_v1beta1_MachinePool_To_v1alpha3_MachinePool(in *expv1.MachinePool,
func (src *MachinePool) ConvertTo(dstRaw conversion.Hub) error {
dst := dstRaw.(*expv1.MachinePool)

return Convert_v1alpha3_MachinePool_To_v1beta1_MachinePool(src, dst, nil)
if err := Convert_v1alpha3_MachinePool_To_v1beta1_MachinePool(src, dst, nil); err != nil {
return err
}

// Manually restore data.
restored := &expv1.MachinePool{}
if ok, err := utilconversion.UnmarshalData(src, restored); err != nil || !ok {
return err
}

if restored.Spec.ExternallyManagedReplicaCount != nil {
dst.Spec.ExternallyManagedReplicaCount = restored.Spec.ExternallyManagedReplicaCount
}

return nil
}

func (dst *MachinePool) ConvertFrom(srcRaw conversion.Hub) error {
src := srcRaw.(*expv1.MachinePool)

return Convert_v1beta1_MachinePool_To_v1alpha3_MachinePool(src, dst, nil)
if err := Convert_v1beta1_MachinePool_To_v1alpha3_MachinePool(src, dst, nil); err != nil {
return err
}

// Preserve Hub data on down-conversion except for metadata
if err := utilconversion.MarshalData(src, dst); err != nil {
return err
}

return nil
}

func (src *MachinePoolList) ConvertTo(dstRaw conversion.Hub) error {
Expand All @@ -80,3 +104,7 @@ func (dst *MachinePoolList) ConvertFrom(srcRaw conversion.Hub) error {

return Convert_v1beta1_MachinePoolList_To_v1alpha3_MachinePoolList(src, dst, nil)
}

func Convert_v1beta1_MachinePoolSpec_To_v1alpha3_MachinePoolSpec(in *expv1.MachinePoolSpec, out *MachinePoolSpec, s apimachineryconversion.Scope) error {
return autoConvert_v1beta1_MachinePoolSpec_To_v1alpha3_MachinePoolSpec(in, out, s)
}
16 changes: 6 additions & 10 deletions exp/api/v1alpha3/zz_generated.conversion.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

33 changes: 31 additions & 2 deletions exp/api/v1alpha4/conversion.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,46 @@ limitations under the License.
package v1alpha4

import (
apimachineryconversion "k8s.io/apimachinery/pkg/conversion"
"sigs.k8s.io/controller-runtime/pkg/conversion"

expv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1"
utilconversion "sigs.k8s.io/cluster-api/util/conversion"
)

func (src *MachinePool) ConvertTo(dstRaw conversion.Hub) error {
dst := dstRaw.(*expv1.MachinePool)

return Convert_v1alpha4_MachinePool_To_v1beta1_MachinePool(src, dst, nil)
if err := Convert_v1alpha4_MachinePool_To_v1beta1_MachinePool(src, dst, nil); err != nil {
return err
}

// Manually restore data.
restored := &expv1.MachinePool{}
if ok, err := utilconversion.UnmarshalData(src, restored); err != nil || !ok {
return err
}

if restored.Spec.ExternallyManagedReplicaCount != nil {
dst.Spec.ExternallyManagedReplicaCount = restored.Spec.ExternallyManagedReplicaCount
}

return nil
}

func (dst *MachinePool) ConvertFrom(srcRaw conversion.Hub) error {
src := srcRaw.(*expv1.MachinePool)

return Convert_v1beta1_MachinePool_To_v1alpha4_MachinePool(src, dst, nil)
if err := Convert_v1beta1_MachinePool_To_v1alpha4_MachinePool(src, dst, nil); err != nil {
return err
}

// Preserve Hub data on down-conversion except for metadata
if err := utilconversion.MarshalData(src, dst); err != nil {
return err
}

return nil
}

func (src *MachinePoolList) ConvertTo(dstRaw conversion.Hub) error {
Expand All @@ -45,3 +70,7 @@ func (dst *MachinePoolList) ConvertFrom(srcRaw conversion.Hub) error {

return Convert_v1beta1_MachinePoolList_To_v1alpha4_MachinePoolList(src, dst, nil)
}

func Convert_v1beta1_MachinePoolSpec_To_v1alpha4_MachinePoolSpec(in *expv1.MachinePoolSpec, out *MachinePoolSpec, s apimachineryconversion.Scope) error {
return autoConvert_v1beta1_MachinePoolSpec_To_v1alpha4_MachinePoolSpec(in, out, s)
}
16 changes: 6 additions & 10 deletions exp/api/v1alpha4/zz_generated.conversion.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions exp/api/v1beta1/machinepool_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,13 @@ type MachinePoolSpec struct {
// FailureDomains is the list of failure domains this MachinePool should be attached to.
// +optional
FailureDomains []string `json:"failureDomains,omitempty"`

// Whether the Replicas value is externally managed. This is useful when
// the infrastructure is a managed machine pool, or if the cluster-autoscaler is scaling
// the underlying cloud infrastructure directly outside of cluster-api. Set this to true
// if you wish the replica count to be managed outside of cluster-api. Defaults to false.
// +optional
Copy link
Member

@sbueringer sbueringer Jan 7, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we document this property and the (expected) behavior of infra providers in the book (https://cluster-api.sigs.k8s.io/developer/architecture/controllers/machine-pool.html#infrastructure-provider)?

I assume this logic in the core MachinePool controller won't change when ExternallyManagedReplicaCount is set?

After the machine pool controller sets the OwnerReferences on the associated objects, it waits for the bootstrap and infrastructure objects referenced by the machine to have the Status.Ready field set to true. Then the infrastructure object is ready, the machine pool controller will attempt to read its spec.ProviderIDList and copy it into MachinePool.Spec.ProviderIDList.

The machine pool controller uses the kubeconfig for the new workload cluster to watch new nodes coming up. When a node appears with a Node.Spec.ProviderID in MachinePool.Spec.ProviderIDList, the machine pool controller increments the number of ready replicas. When all replicas are ready and the infrastructure ref is also Ready, the machine pool controller marks the machine pool as Running.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I will definitely update the book. And also yes, the logic you mention does not change.

ExternallyManagedReplicaCount *bool `json:"externallyManagedReplicaCount,omitempty"`
Comment on lines +64 to +69
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this field used anywhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will be used to signal to the infrastructure provider that it is responsible for managing the spec.replicas and relevant status fields on the MachinePool. So there is no implementation code in the MachinePool itself yet. That may change a little as the MachinePool Machines implementation begins though.

}

// ANCHOR_END: MachinePoolSpec
Expand Down
5 changes: 5 additions & 0 deletions exp/api/v1beta1/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion exp/internal/controllers/machinepool_controller_noderef.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func (r *MachinePoolReconciler) reconcileNodeRefs(ctx context.Context, cluster *
return ctrl.Result{}, nil
}

// Check that the Machine doesn't already have a NodeRefs.
// Check that the MachinePool doesn't already have NodeRefs.
if mp.Status.Replicas == mp.Status.ReadyReplicas && len(mp.Status.NodeRefs) == int(mp.Status.ReadyReplicas) {
conditions.MarkTrue(mp, expv1.ReplicasReadyCondition)
return ctrl.Result{}, nil
Expand Down