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.

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 MachinePool Machine implementation to CAPD components #8842

Merged
merged 1 commit into from
Nov 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 4 additions & 5 deletions exp/internal/controllers/machinepool_controller_phases.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import (
"sigs.k8s.io/cluster-api/util/annotations"
"sigs.k8s.io/cluster-api/util/conditions"
utilconversion "sigs.k8s.io/cluster-api/util/conversion"
"sigs.k8s.io/cluster-api/util/labels"
"sigs.k8s.io/cluster-api/util/labels/format"
"sigs.k8s.io/cluster-api/util/patch"
)
Expand Down Expand Up @@ -485,12 +486,10 @@ func computeDesiredMachine(mp *expv1.MachinePool, infraMachine *unstructured.Uns
func (r *MachinePoolReconciler) infraMachineToMachinePoolMapper(ctx context.Context, o client.Object) []ctrl.Request {
log := ctrl.LoggerFrom(ctx)

labels := o.GetLabels()
_, machinePoolOwned := labels[clusterv1.MachinePoolNameLabel]
if machinePoolOwned {
machinePool, err := utilexp.GetMachinePoolByLabels(ctx, r.Client, o.GetNamespace(), labels)
if labels.IsMachinePoolOwned(o) {
machinePool, err := utilexp.GetMachinePoolByLabels(ctx, r.Client, o.GetNamespace(), o.GetLabels())
if err != nil {
log.Error(err, "failed to get MachinePool for InfraMachine", "infraMachine", klog.KObj(o), "labels", labels)
log.Error(err, "failed to get MachinePool for InfraMachine", "infraMachine", klog.KObj(o), "labels", o.GetLabels())
return nil
}
if machinePool != nil {
Expand Down
19 changes: 2 additions & 17 deletions internal/webhooks/machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"

clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
"sigs.k8s.io/cluster-api/util/labels"
"sigs.k8s.io/cluster-api/util/version"
)

Expand Down Expand Up @@ -120,7 +121,7 @@ func (webhook *Machine) validate(oldM, newM *clusterv1.Machine) error {
specPath := field.NewPath("spec")
if newM.Spec.Bootstrap.ConfigRef == nil && newM.Spec.Bootstrap.DataSecretName == nil {
// MachinePool Machines don't have a bootstrap configRef, so don't require it. The bootstrap config is instead owned by the MachinePool.
if !isMachinePoolMachine(newM) {
if !labels.IsMachinePoolOwned(newM) {
allErrs = append(
allErrs,
field.Required(
Expand Down Expand Up @@ -171,19 +172,3 @@ func (webhook *Machine) validate(oldM, newM *clusterv1.Machine) error {
}
return apierrors.NewInvalid(clusterv1.GroupVersion.WithKind("Machine").GroupKind(), newM.Name, allErrs)
}

func isMachinePoolMachine(m *clusterv1.Machine) bool {
if m.Labels != nil {
if _, ok := m.Labels[clusterv1.MachinePoolNameLabel]; ok {
return true
}
}

for _, owner := range m.OwnerReferences {
if owner.Kind == "MachinePool" {
return true
}
}

return false
}
54 changes: 0 additions & 54 deletions internal/webhooks/machine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -220,60 +220,6 @@ func TestMachineClusterNameImmutable(t *testing.T) {
}
}

func TestIsMachinePoolMachine(t *testing.T) {
tests := []struct {
name string
machine clusterv1.Machine
isMPM bool
}{
{
name: "machine is a MachinePoolMachine",
machine: clusterv1.Machine{
ObjectMeta: metav1.ObjectMeta{
OwnerReferences: []metav1.OwnerReference{
{
Kind: "MachinePool",
},
},
},
},
isMPM: true,
},
{
name: "machine is not a MachinePoolMachine",
machine: clusterv1.Machine{
ObjectMeta: metav1.ObjectMeta{
OwnerReferences: []metav1.OwnerReference{
{
Kind: "NotMachinePool",
},
},
},
},
isMPM: false,
},
{
name: "machine is not a MachinePoolMachine, no owner references",
machine: clusterv1.Machine{
ObjectMeta: metav1.ObjectMeta{
OwnerReferences: nil,
},
},
isMPM: false,
},
}

for i := range tests {
tt := tests[i]
t.Run(tt.name, func(t *testing.T) {
g := NewWithT(t)

result := isMachinePoolMachine(&tt.machine)
g.Expect(result).To(Equal(tt.isMPM))
})
}
}

func TestMachineVersionValidation(t *testing.T) {
tests := []struct {
name string
Expand Down
13 changes: 10 additions & 3 deletions test/e2e/clusterclass_rollout.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import (
"sigs.k8s.io/cluster-api/test/framework"
"sigs.k8s.io/cluster-api/test/framework/clusterctl"
"sigs.k8s.io/cluster-api/util"
"sigs.k8s.io/cluster-api/util/labels"
"sigs.k8s.io/cluster-api/util/patch"
)

Expand Down Expand Up @@ -246,6 +247,8 @@ func ClusterClassRolloutSpec(ctx context.Context, inputGetter func() ClusterClas
})
By("Verifying all Machines are replaced through rollout")
Eventually(func(g Gomega) {
// Note: This excludes MachinePool Machines as they are not replaced by rollout yet.
// This is tracked by https://github.com/kubernetes-sigs/cluster-api/issues/8858.
Jont828 marked this conversation as resolved.
Show resolved Hide resolved
machinesAfterUpgrade := getMachinesByCluster(ctx, input.BootstrapClusterProxy.GetClient(), clusterResources.Cluster)
g.Expect(machinesAfterUpgrade.HasAny(machinesBeforeUpgrade.UnsortedList()...)).To(BeFalse(), "All Machines must be replaced through rollout")
}, input.E2EConfig.GetIntervals(specName, "wait-control-plane")...).Should(Succeed())
Expand Down Expand Up @@ -898,15 +901,19 @@ func mustMetadata(metadata *clusterv1.ObjectMeta, err error) *clusterv1.ObjectMe
}

// getMachinesByCluster gets the Machines of a Cluster and returns them as a Set of Machine names.
// Note: This excludes MachinePool Machines as they are not replaced by rollout yet.
func getMachinesByCluster(ctx context.Context, client client.Client, cluster *clusterv1.Cluster) sets.Set[string] {
machines := sets.Set[string]{}
machinesByCluster := framework.GetMachinesByCluster(ctx, framework.GetMachinesByClusterInput{
Lister: client,
ClusterName: cluster.Name,
Namespace: cluster.Namespace,
})
for _, m := range machinesByCluster {
machines.Insert(m.Name)
for i := range machinesByCluster {
m := machinesByCluster[i]
if !labels.IsMachinePoolOwned(&m) {
machines.Insert(m.Name)
}
}
return machines
}
Expand Down Expand Up @@ -935,7 +942,7 @@ func getMDTopology(cluster *clusterv1.Cluster, md *clusterv1.MachineDeployment)
return nil
}

// getMPClass looks up the MachinePoolClass for a md in the ClusterClass.
// getMPClass looks up the MachinePoolClass for a MachinePool in the ClusterClass.
func getMPClass(cluster *clusterv1.Cluster, clusterClass *clusterv1.ClusterClass, mp *expv1.MachinePool) *clusterv1.MachinePoolClass {
mpTopology := getMPTopology(cluster, mp)

Expand Down
8 changes: 6 additions & 2 deletions test/framework/ownerreference_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import (
controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1beta1"
addonsv1 "sigs.k8s.io/cluster-api/exp/addons/api/v1beta1"
expv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1"
infraexpv1 "sigs.k8s.io/cluster-api/test/infrastructure/docker/exp/api/v1beta1"
"sigs.k8s.io/cluster-api/util/patch"
)

Expand Down Expand Up @@ -288,6 +289,8 @@ var (
dockerMachinePoolTemplateKind = "DockerMachinePoolTemplate"
dockerClusterKind = "DockerCluster"
dockerClusterTemplateKind = "DockerClusterTemplate"

dockerMachinePoolController = metav1.OwnerReference{Kind: dockerMachinePoolKind, APIVersion: infraexpv1.GroupVersion.String()}
)

// DockerInfraOwnerReferenceAssertions maps Docker Infrastructure types to functions which return an error if the passed
Expand All @@ -296,8 +299,9 @@ var (
// That document should be updated if these references change.
var DockerInfraOwnerReferenceAssertions = map[string]func([]metav1.OwnerReference) error{
dockerMachineKind: func(owners []metav1.OwnerReference) error {
// The DockerMachine must be owned and controlled by a Machine.
return HasExactOwners(owners, machineController)
// The DockerMachine must be owned and controlled by a Machine or a DockerMachinePool.
return HasOneOfExactOwners(owners, []metav1.OwnerReference{machineController}, []metav1.OwnerReference{machineController, dockerMachinePoolController})

},
dockerMachineTemplateKind: func(owners []metav1.OwnerReference) error {
// Base DockerMachineTemplates referenced in a ClusterClass must be owned by the ClusterClass.
Expand Down

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

9 changes: 9 additions & 0 deletions test/infrastructure/docker/config/rbac/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,15 @@ rules:
- get
- list
- watch
- apiGroups:
- cluster.x-k8s.io
resources:
- machines
verbs:
- delete
- get
- list
- watch
- apiGroups:
- infrastructure.cluster.x-k8s.io
resources:
Expand Down
28 changes: 26 additions & 2 deletions test/infrastructure/docker/exp/api/v1alpha4/conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,40 @@ limitations under the License.
package v1alpha4

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

infraexpv1 "sigs.k8s.io/cluster-api/test/infrastructure/docker/exp/api/v1beta1"
utilconversion "sigs.k8s.io/cluster-api/util/conversion"
)

func (src *DockerMachinePool) ConvertTo(dstRaw conversion.Hub) error {
dst := dstRaw.(*infraexpv1.DockerMachinePool)

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

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

dst.Status.InfrastructureMachineKind = restored.Status.InfrastructureMachineKind

return nil
}

func (dst *DockerMachinePool) ConvertFrom(srcRaw conversion.Hub) error {
src := srcRaw.(*infraexpv1.DockerMachinePool)

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

// Preserve Hub data on down-conversion except for metadata
return utilconversion.MarshalData(src, dst)
}

func (src *DockerMachinePoolList) ConvertTo(dstRaw conversion.Hub) error {
Expand All @@ -45,3 +64,8 @@ func (dst *DockerMachinePoolList) ConvertFrom(srcRaw conversion.Hub) error {

return Convert_v1beta1_DockerMachinePoolList_To_v1alpha4_DockerMachinePoolList(src, dst, nil)
}

func Convert_v1beta1_DockerMachinePoolStatus_To_v1alpha4_DockerMachinePoolStatus(in *infraexpv1.DockerMachinePoolStatus, out *DockerMachinePoolStatus, s apiconversion.Scope) error {
// NOTE: custom conversion func is required because Status.InfrastructureMachineKind has been added in v1beta1.
return autoConvert_v1beta1_DockerMachinePoolStatus_To_v1alpha4_DockerMachinePoolStatus(in, out, s)
}

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

Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,10 @@ type DockerMachinePoolStatus struct {
// Conditions defines current service state of the DockerMachinePool.
// +optional
Conditions clusterv1.Conditions `json:"conditions,omitempty"`

// InfrastructureMachineKind is the kind of the infrastructure resources behind MachinePool Machines.
// +optional
InfrastructureMachineKind string `json:"infrastructureMachineKind,omitempty"`
}

// DockerMachinePoolInstanceStatus contains status information about a DockerMachinePool.
Expand Down Expand Up @@ -130,13 +134,13 @@ type DockerMachinePool struct {
}

// GetConditions returns the set of conditions for this object.
func (c *DockerMachinePool) GetConditions() clusterv1.Conditions {
return c.Status.Conditions
func (d *DockerMachinePool) GetConditions() clusterv1.Conditions {
return d.Status.Conditions
}

// SetConditions sets the conditions on this object.
func (c *DockerMachinePool) SetConditions(conditions clusterv1.Conditions) {
c.Status.Conditions = conditions
func (d *DockerMachinePool) SetConditions(conditions clusterv1.Conditions) {
d.Status.Conditions = conditions
}

// +kubebuilder:object:root=true
Expand Down
Loading
Loading