diff --git a/exp/internal/controllers/machinepool_controller_phases.go b/exp/internal/controllers/machinepool_controller_phases.go index 396edd251160..9449b692dc7a 100644 --- a/exp/internal/controllers/machinepool_controller_phases.go +++ b/exp/internal/controllers/machinepool_controller_phases.go @@ -42,6 +42,7 @@ import ( "sigs.k8s.io/cluster-api/controllers/external" capierrors "sigs.k8s.io/cluster-api/errors" expv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1" + utilexp "sigs.k8s.io/cluster-api/exp/util" "sigs.k8s.io/cluster-api/util" "sigs.k8s.io/cluster-api/util/annotations" "sigs.k8s.io/cluster-api/util/conditions" @@ -459,6 +460,8 @@ func (r *MachinePoolReconciler) ensureInfraMachineOnwerRefs(ctx context.Context, if err := patchHelper.Patch(ctx, infraMachine); err != nil { return errors.Wrapf(err, "failed to patch %s", klog.KObj(infraMachine)) } + + log.V(4).Info("Successfully set ownerRef on infraMachine", "infraMachine", infraMachine.GetName(), "namespace", infraMachine.GetNamespace(), "machine", machine.GetName()) } } @@ -514,29 +517,22 @@ func getNewMachine(mp *expv1.MachinePool, infraMachine *unstructured.Unstructure func (r *MachinePoolReconciler) infraMachineToMachinePoolMapper(ctx context.Context, o client.Object) []ctrl.Request { log := ctrl.LoggerFrom(ctx) - machinePoolList := &expv1.MachinePoolList{} labels := o.GetLabels() - selector := map[string]string{} - if clusterName, ok := labels[clusterv1.ClusterNameLabel]; ok { - selector = map[string]string{clusterv1.ClusterNameLabel: clusterName} - } - - if poolNameHash, ok := labels[clusterv1.MachinePoolNameLabel]; ok { - if err := r.Client.List(ctx, machinePoolList, client.InNamespace(o.GetNamespace()), client.MatchingLabels(selector)); err != nil { - log.Error(err, "failed to get MachinePool for InfraMachine") + _, machinePoolOwned := labels[clusterv1.MachinePoolNameLabel] + if machinePoolOwned { + machinePool, err := utilexp.GetMachinePoolByLabels(ctx, r.Client, o.GetNamespace(), labels) + if err != nil { + log.Error(err, "failed to get MachinePool for InfraMachine", "infraMachine", klog.KObj(o), "labels", labels) return nil } - - for _, mp := range machinePoolList.Items { - if format.MustFormatValue(mp.Name) == poolNameHash { - return []ctrl.Request{ - { - NamespacedName: client.ObjectKey{ - Namespace: mp.Namespace, - Name: mp.Name, - }, + if machinePool != nil { + return []ctrl.Request{ + { + NamespacedName: client.ObjectKey{ + Namespace: machinePool.Namespace, + Name: machinePool.Name, }, - } + }, } } } diff --git a/exp/util/suite_test.go b/exp/util/suite_test.go new file mode 100644 index 000000000000..04180607a019 --- /dev/null +++ b/exp/util/suite_test.go @@ -0,0 +1,33 @@ +/* +Copyright 2023 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package util + +import ( + "k8s.io/apimachinery/pkg/runtime" + ctrl "sigs.k8s.io/controller-runtime" + + expv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1" +) + +var ( + ctx = ctrl.SetupSignalHandler() + fakeScheme = runtime.NewScheme() +) + +func init() { + _ = expv1.AddToScheme(fakeScheme) +} diff --git a/exp/util/util.go b/exp/util/util.go index d2904603e0ff..cef656984454 100644 --- a/exp/util/util.go +++ b/exp/util/util.go @@ -29,7 +29,9 @@ import ( "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/reconcile" + clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" expv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1" + "sigs.k8s.io/cluster-api/util/labels/format" ) // GetOwnerMachinePool returns the MachinePool objects owning the current resource. @@ -49,7 +51,7 @@ func GetOwnerMachinePool(ctx context.Context, c client.Client, obj metav1.Object return nil, nil } -// GetMachinePoolByName finds and returns a MachinePool object usting the specified params. +// GetMachinePoolByName finds and returns a MachinePool object using the specified params. func GetMachinePoolByName(ctx context.Context, c client.Client, namespace, name string) (*expv1.MachinePool, error) { m := &expv1.MachinePool{} key := client.ObjectKey{Name: name, Namespace: namespace} @@ -59,6 +61,32 @@ func GetMachinePoolByName(ctx context.Context, c client.Client, namespace, name return m, nil } +// GetMachinePoolByLabels finds and returns a MachinePool object using the value of clusterv1.MachinePoolNameLabel. +// This differs from GetMachinePoolByName as the label value can be a hash. +func GetMachinePoolByLabels(ctx context.Context, c client.Client, namespace string, labels map[string]string) (*expv1.MachinePool, error) { + selector := map[string]string{} + if clusterName, ok := labels[clusterv1.ClusterNameLabel]; ok { + selector = map[string]string{clusterv1.ClusterNameLabel: clusterName} + } + + if poolNameHash, ok := labels[clusterv1.MachinePoolNameLabel]; ok { + machinePoolList := &expv1.MachinePoolList{} + if err := c.List(ctx, machinePoolList, client.InNamespace(namespace), client.MatchingLabels(selector)); err != nil { + return nil, errors.Wrapf(err, "failed to list MachinePools using labels %v", selector) + } + + for _, mp := range machinePoolList.Items { + if format.MustFormatValue(mp.Name) == poolNameHash { + return &mp, nil + } + } + } else { + return nil, errors.Errorf("labels missing required key `%s`", clusterv1.MachinePoolNameLabel) + } + + return nil, nil +} + // MachinePoolToInfrastructureMapFunc returns a handler.MapFunc that watches for // MachinePool events and returns reconciliation requests for an infrastructure provider object. func MachinePoolToInfrastructureMapFunc(gvk schema.GroupVersionKind, log logr.Logger) handler.MapFunc { diff --git a/exp/util/util_test.go b/exp/util/util_test.go new file mode 100644 index 000000000000..d33949f6d47e --- /dev/null +++ b/exp/util/util_test.go @@ -0,0 +1,155 @@ +/* +Copyright 2023 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Package util implements utility functions. +package util + +import ( + "fmt" + "testing" + + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + expv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1" + "sigs.k8s.io/cluster-api/util/labels/format" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" +) + +func TestGetMachinePoolByLabels(t *testing.T) { + g := NewWithT(t) + + longMachinePoolName := "this-is-a-very-long-machinepool-name-that-will-turned-into-a-hash-because-it-is-longer-than-63-characters" + namespace := "default" + + testcases := []struct { + name string + labels map[string]string + machinePools []client.Object + expectedMachinePoolName string + expectedError string + }{ + { + name: "returns a MachinePool with matching labels", + labels: map[string]string{ + clusterv1.MachinePoolNameLabel: "test-pool", + }, + machinePools: []client.Object{ + &expv1.MachinePool{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pool", + Namespace: "default", + }, + }, + &expv1.MachinePool{ + ObjectMeta: metav1.ObjectMeta{ + Name: "other-pool", + Namespace: "default", + }, + }, + }, + expectedMachinePoolName: "test-pool", + }, + { + name: "returns a MachinePool with matching labels and cluster name is included", + labels: map[string]string{ + clusterv1.MachinePoolNameLabel: "test-pool", + clusterv1.ClusterNameLabel: "test-cluster", + }, + machinePools: []client.Object{ + &expv1.MachinePool{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pool", + Namespace: "default", + Labels: map[string]string{ + clusterv1.ClusterNameLabel: "test-cluster", + }, + }, + }, + &expv1.MachinePool{ + ObjectMeta: metav1.ObjectMeta{ + Name: "other-pool", + Namespace: "default", + Labels: map[string]string{ + clusterv1.ClusterNameLabel: "test-cluster", + }, + }, + }, + }, + expectedMachinePoolName: "test-pool", + }, + { + name: "returns a MachinePool where label is a hash", + labels: map[string]string{ + clusterv1.MachinePoolNameLabel: format.MustFormatValue(longMachinePoolName), + }, + machinePools: []client.Object{ + &expv1.MachinePool{ + ObjectMeta: metav1.ObjectMeta{ + Name: longMachinePoolName, + Namespace: "default", + }, + }, + &expv1.MachinePool{ + ObjectMeta: metav1.ObjectMeta{ + Name: "other-pool", + Namespace: "default", + }, + }, + }, + expectedMachinePoolName: longMachinePoolName, + }, + { + name: "missing required key", + labels: map[string]string{}, + expectedError: fmt.Sprintf("labels missing required key `%s`", clusterv1.MachinePoolNameLabel), + }, + { + name: "returns nil when no machine pool matches", + labels: map[string]string{ + clusterv1.MachinePoolNameLabel: "test-pool", + }, + machinePools: []client.Object{}, + expectedMachinePoolName: "", + }, + } + + for _, tc := range testcases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + + clientFake := fake.NewClientBuilder(). + WithScheme(fakeScheme). + WithObjects( + tc.machinePools..., + ).Build() + + mp, err := GetMachinePoolByLabels(ctx, clientFake, namespace, tc.labels) + if tc.expectedError != "" { + g.Expect(err).To(MatchError(tc.expectedError)) + } else { + g.Expect(err).NotTo(HaveOccurred()) + if tc.expectedMachinePoolName != "" { + g.Expect(mp).ToNot(BeNil()) + g.Expect(mp.Name).To(Equal(tc.expectedMachinePoolName)) + } else { + g.Expect(mp).To(BeNil()) + } + } + }) + } +}