diff --git a/controlplane/kubeadm/controllers/controller.go b/controlplane/kubeadm/controllers/controller.go index 6ddf82952744..bb9609cff38c 100644 --- a/controlplane/kubeadm/controllers/controller.go +++ b/controlplane/kubeadm/controllers/controller.go @@ -19,6 +19,7 @@ package controllers import ( "context" "fmt" + "sigs.k8s.io/cluster-api/util/collections" "time" "github.com/blang/semver" @@ -36,7 +37,6 @@ import ( "sigs.k8s.io/cluster-api/controllers/remote" controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1alpha4" "sigs.k8s.io/cluster-api/controlplane/kubeadm/internal" - "sigs.k8s.io/cluster-api/controlplane/kubeadm/internal/machinefilters" "sigs.k8s.io/cluster-api/util" "sigs.k8s.io/cluster-api/util/annotations" "sigs.k8s.io/cluster-api/util/conditions" @@ -270,20 +270,20 @@ func (r *KubeadmControlPlaneReconciler) reconcile(ctx context.Context, cluster * return result, err } - controlPlaneMachines, err := r.managementClusterUncached.GetMachinesForCluster(ctx, util.ObjectKey(cluster), machinefilters.ControlPlaneMachines(cluster.Name)) + controlPlaneMachines, err := r.managementClusterUncached.GetMachinesForCluster(ctx, util.ObjectKey(cluster), collections.ControlPlaneMachines(cluster.Name)) if err != nil { log.Error(err, "failed to retrieve control plane machines for cluster") return ctrl.Result{}, err } - adoptableMachines := controlPlaneMachines.Filter(machinefilters.AdoptableControlPlaneMachines(cluster.Name)) + adoptableMachines := controlPlaneMachines.Filter(collections.AdoptableControlPlaneMachines(cluster.Name)) if len(adoptableMachines) > 0 { // We adopt the Machines and then wait for the update event for the ownership reference to re-queue them so the cache is up-to-date err = r.adoptMachines(ctx, kcp, adoptableMachines, cluster) return ctrl.Result{}, err } - ownedMachines := controlPlaneMachines.Filter(machinefilters.OwnedMachines(kcp)) + ownedMachines := controlPlaneMachines.Filter(collections.OwnedMachines(kcp)) if len(ownedMachines) != len(controlPlaneMachines) { log.Info("Not all control plane machines are owned by this KubeadmControlPlane, refusing to operate in mixed management mode") return ctrl.Result{}, nil @@ -353,7 +353,7 @@ func (r *KubeadmControlPlaneReconciler) reconcile(ctx context.Context, cluster * case numMachines > desiredReplicas: log.Info("Scaling down control plane", "Desired", desiredReplicas, "Existing", numMachines) // The last parameter (i.e. machines needing to be rolled out) should always be empty here. - return r.scaleDownControlPlane(ctx, cluster, kcp, controlPlane, internal.FilterableMachineCollection{}) + return r.scaleDownControlPlane(ctx, cluster, kcp, controlPlane, collections.Machines{}) } // Get the workload cluster client. @@ -394,7 +394,7 @@ func (r *KubeadmControlPlaneReconciler) reconcileDelete(ctx context.Context, clu if err != nil { return ctrl.Result{}, err } - ownedMachines := allMachines.Filter(machinefilters.OwnedMachines(kcp)) + ownedMachines := allMachines.Filter(collections.OwnedMachines(kcp)) // If no control plane machines remain, remove the finalizer if len(ownedMachines) == 0 { @@ -428,7 +428,7 @@ func (r *KubeadmControlPlaneReconciler) reconcileDelete(ctx context.Context, clu } // Delete control plane machines in parallel - machinesToDelete := ownedMachines.Filter(machinefilters.Not(machinefilters.HasDeletionTimestamp)) + machinesToDelete := ownedMachines.Filter(collections.Not(collections.HasDeletionTimestamp)) var errs []error for i := range machinesToDelete { m := machinesToDelete[i] @@ -542,7 +542,7 @@ func (r *KubeadmControlPlaneReconciler) reconcileEtcdMembers(ctx context.Context return ctrl.Result{}, nil } -func (r *KubeadmControlPlaneReconciler) adoptMachines(ctx context.Context, kcp *controlplanev1.KubeadmControlPlane, machines internal.FilterableMachineCollection, cluster *clusterv1.Cluster) error { +func (r *KubeadmControlPlaneReconciler) adoptMachines(ctx context.Context, kcp *controlplanev1.KubeadmControlPlane, machines collections.Machines, cluster *clusterv1.Cluster) error { // We do an uncached full quorum read against the KCP to avoid re-adopting Machines the garbage collector just intentionally orphaned // See https://github.com/kubernetes/kubernetes/issues/42639 uncached := controlplanev1.KubeadmControlPlane{} diff --git a/controlplane/kubeadm/controllers/controller_test.go b/controlplane/kubeadm/controllers/controller_test.go index 673f780c4dd9..2f20ace550db 100644 --- a/controlplane/kubeadm/controllers/controller_test.go +++ b/controlplane/kubeadm/controllers/controller_test.go @@ -19,6 +19,7 @@ package controllers import ( "context" "fmt" + "sigs.k8s.io/cluster-api/util/collections" "sync" "testing" "time" @@ -360,7 +361,7 @@ func TestKubeadmControlPlaneReconciler_adoption(t *testing.T) { kcp.Spec.Version = version fmc := &fakeManagementCluster{ - Machines: internal.FilterableMachineCollection{}, + Machines: collections.Machines{}, Workload: fakeWorkloadCluster{}, } objs := []client.Object{cluster.DeepCopy(), kcp.DeepCopy(), tmpl.DeepCopy()} @@ -424,7 +425,7 @@ func TestKubeadmControlPlaneReconciler_adoption(t *testing.T) { kcp.Spec.Version = version fmc := &fakeManagementCluster{ - Machines: internal.FilterableMachineCollection{}, + Machines: collections.Machines{}, Workload: fakeWorkloadCluster{}, } objs := []client.Object{cluster.DeepCopy(), kcp.DeepCopy(), tmpl.DeepCopy()} @@ -534,7 +535,7 @@ func TestKubeadmControlPlaneReconciler_adoption(t *testing.T) { kcp.DeletionTimestamp = &now fmc := &fakeManagementCluster{ - Machines: internal.FilterableMachineCollection{}, + Machines: collections.Machines{}, Workload: fakeWorkloadCluster{}, } objs := []client.Object{cluster.DeepCopy(), kcp.DeepCopy(), tmpl.DeepCopy()} @@ -596,7 +597,7 @@ func TestKubeadmControlPlaneReconciler_adoption(t *testing.T) { kcp.Spec.Version = "v1.17.0" fmc := &fakeManagementCluster{ - Machines: internal.FilterableMachineCollection{ + Machines: collections.Machines{ "test0": &clusterv1.Machine{ ObjectMeta: metav1.ObjectMeta{ Namespace: cluster.Namespace, diff --git a/controlplane/kubeadm/controllers/fakes_test.go b/controlplane/kubeadm/controllers/fakes_test.go index 945f50e97f60..74592865f513 100644 --- a/controlplane/kubeadm/controllers/fakes_test.go +++ b/controlplane/kubeadm/controllers/fakes_test.go @@ -18,18 +18,17 @@ package controllers import ( "context" - "github.com/blang/semver" clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha4" "sigs.k8s.io/cluster-api/controlplane/kubeadm/internal" - "sigs.k8s.io/cluster-api/controlplane/kubeadm/internal/machinefilters" + "sigs.k8s.io/cluster-api/util/collections" "sigs.k8s.io/controller-runtime/pkg/client" ) type fakeManagementCluster struct { // TODO: once all client interactions are moved to the Management cluster this can go away Management *internal.Management - Machines internal.FilterableMachineCollection + Machines collections.Machines Workload fakeWorkloadCluster Reader client.Reader } @@ -46,7 +45,7 @@ func (f *fakeManagementCluster) GetWorkloadCluster(_ context.Context, _ client.O return f.Workload, nil } -func (f *fakeManagementCluster) GetMachinesForCluster(c context.Context, n client.ObjectKey, filters ...machinefilters.Func) (internal.FilterableMachineCollection, error) { +func (f *fakeManagementCluster) GetMachinesForCluster(c context.Context, n client.ObjectKey, filters ...collections.Func) (collections.Machines, error) { if f.Management != nil { return f.Management.GetMachinesForCluster(c, n, filters...) } diff --git a/controlplane/kubeadm/controllers/remediation_test.go b/controlplane/kubeadm/controllers/remediation_test.go index 5cc1e3841bcc..3e0fbdfda380 100644 --- a/controlplane/kubeadm/controllers/remediation_test.go +++ b/controlplane/kubeadm/controllers/remediation_test.go @@ -18,6 +18,7 @@ package controllers import ( "context" + "sigs.k8s.io/cluster-api/util/collections" "testing" . "github.com/onsi/gomega" @@ -50,7 +51,7 @@ func TestReconcileUnhealthyMachines(t *testing.T) { controlPlane := &internal.ControlPlane{ KCP: &controlplanev1.KubeadmControlPlane{}, Cluster: &clusterv1.Cluster{}, - Machines: internal.NewFilterableMachineCollection(), + Machines: collections.New(), } ret, err := r.reconcileUnhealthyMachines(context.TODO(), controlPlane) @@ -66,7 +67,7 @@ func TestReconcileUnhealthyMachines(t *testing.T) { controlPlane := &internal.ControlPlane{ KCP: &controlplanev1.KubeadmControlPlane{}, Cluster: &clusterv1.Cluster{}, - Machines: internal.NewFilterableMachineCollection(m), + Machines: collections.FromMachines(m), } ret, err := r.reconcileUnhealthyMachines(context.TODO(), controlPlane) @@ -82,7 +83,7 @@ func TestReconcileUnhealthyMachines(t *testing.T) { Replicas: utilpointer.Int32Ptr(1), }}, Cluster: &clusterv1.Cluster{}, - Machines: internal.NewFilterableMachineCollection(m), + Machines: collections.FromMachines(m), } ret, err := r.reconcileUnhealthyMachines(context.TODO(), controlPlane) @@ -101,7 +102,7 @@ func TestReconcileUnhealthyMachines(t *testing.T) { Replicas: utilpointer.Int32Ptr(3), }}, Cluster: &clusterv1.Cluster{}, - Machines: internal.NewFilterableMachineCollection(m), + Machines: collections.FromMachines(m), } ret, err := r.reconcileUnhealthyMachines(context.TODO(), controlPlane) @@ -122,7 +123,7 @@ func TestReconcileUnhealthyMachines(t *testing.T) { Replicas: utilpointer.Int32Ptr(3), }}, Cluster: &clusterv1.Cluster{}, - Machines: internal.NewFilterableMachineCollection(m1, m2, m3), + Machines: collections.FromMachines(m1, m2, m3), } ret, err := r.reconcileUnhealthyMachines(context.TODO(), controlPlane) @@ -144,7 +145,7 @@ func TestReconcileUnhealthyMachines(t *testing.T) { Replicas: utilpointer.Int32Ptr(3), }}, Cluster: &clusterv1.Cluster{}, - Machines: internal.NewFilterableMachineCollection(m1, m2, m3), + Machines: collections.FromMachines(m1, m2, m3), } r := &KubeadmControlPlaneReconciler{ @@ -179,7 +180,7 @@ func TestReconcileUnhealthyMachines(t *testing.T) { Replicas: utilpointer.Int32Ptr(5), }}, Cluster: &clusterv1.Cluster{}, - Machines: internal.NewFilterableMachineCollection(m1, m2, m3, m4, m5), + Machines: collections.FromMachines(m1, m2, m3, m4, m5), } r := &KubeadmControlPlaneReconciler{ @@ -217,7 +218,7 @@ func TestReconcileUnhealthyMachines(t *testing.T) { Replicas: utilpointer.Int32Ptr(3), }}, Cluster: &clusterv1.Cluster{}, - Machines: internal.NewFilterableMachineCollection(m1, m2, m3), + Machines: collections.FromMachines(m1, m2, m3), } r := &KubeadmControlPlaneReconciler{ @@ -269,7 +270,7 @@ func TestCanSafelyRemoveEtcdMember(t *testing.T) { Replicas: utilpointer.Int32Ptr(1), }}, Cluster: &clusterv1.Cluster{}, - Machines: internal.NewFilterableMachineCollection(m1), + Machines: collections.FromMachines(m1), } r := &KubeadmControlPlaneReconciler{ @@ -299,7 +300,7 @@ func TestCanSafelyRemoveEtcdMember(t *testing.T) { Replicas: utilpointer.Int32Ptr(2), }}, Cluster: &clusterv1.Cluster{}, - Machines: internal.NewFilterableMachineCollection(m1, m2), + Machines: collections.FromMachines(m1, m2), } r := &KubeadmControlPlaneReconciler{ @@ -330,7 +331,7 @@ func TestCanSafelyRemoveEtcdMember(t *testing.T) { Replicas: utilpointer.Int32Ptr(5), }}, Cluster: &clusterv1.Cluster{}, - Machines: internal.NewFilterableMachineCollection(m1, m2, m3), + Machines: collections.FromMachines(m1, m2, m3), } r := &KubeadmControlPlaneReconciler{ @@ -361,7 +362,7 @@ func TestCanSafelyRemoveEtcdMember(t *testing.T) { Replicas: utilpointer.Int32Ptr(5), }}, Cluster: &clusterv1.Cluster{}, - Machines: internal.NewFilterableMachineCollection(m1, m2, m3), + Machines: collections.FromMachines(m1, m2, m3), } r := &KubeadmControlPlaneReconciler{ @@ -394,7 +395,7 @@ func TestCanSafelyRemoveEtcdMember(t *testing.T) { Replicas: utilpointer.Int32Ptr(5), }}, Cluster: &clusterv1.Cluster{}, - Machines: internal.NewFilterableMachineCollection(m1, m2, m3, m4, m5), + Machines: collections.FromMachines(m1, m2, m3, m4, m5), } r := &KubeadmControlPlaneReconciler{ @@ -427,7 +428,7 @@ func TestCanSafelyRemoveEtcdMember(t *testing.T) { Replicas: utilpointer.Int32Ptr(5), }}, Cluster: &clusterv1.Cluster{}, - Machines: internal.NewFilterableMachineCollection(m1, m2, m3, m4, m5), + Machines: collections.FromMachines(m1, m2, m3, m4, m5), } r := &KubeadmControlPlaneReconciler{ @@ -462,7 +463,7 @@ func TestCanSafelyRemoveEtcdMember(t *testing.T) { Replicas: utilpointer.Int32Ptr(5), }}, Cluster: &clusterv1.Cluster{}, - Machines: internal.NewFilterableMachineCollection(m1, m2, m3, m4, m5, m6, m7), + Machines: collections.FromMachines(m1, m2, m3, m4, m5, m6, m7), } r := &KubeadmControlPlaneReconciler{ @@ -497,7 +498,7 @@ func TestCanSafelyRemoveEtcdMember(t *testing.T) { Replicas: utilpointer.Int32Ptr(5), }}, Cluster: &clusterv1.Cluster{}, - Machines: internal.NewFilterableMachineCollection(m1, m2, m3, m4, m5, m6, m7), + Machines: collections.FromMachines(m1, m2, m3, m4, m5, m6, m7), } r := &KubeadmControlPlaneReconciler{ diff --git a/controlplane/kubeadm/controllers/scale.go b/controlplane/kubeadm/controllers/scale.go index a291bc780343..890002654f0a 100644 --- a/controlplane/kubeadm/controllers/scale.go +++ b/controlplane/kubeadm/controllers/scale.go @@ -18,6 +18,7 @@ package controllers import ( "context" + "sigs.k8s.io/cluster-api/util/collections" "strings" "github.com/pkg/errors" @@ -27,7 +28,6 @@ import ( clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha4" controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1alpha4" "sigs.k8s.io/cluster-api/controlplane/kubeadm/internal" - "sigs.k8s.io/cluster-api/controlplane/kubeadm/internal/machinefilters" "sigs.k8s.io/cluster-api/util" "sigs.k8s.io/cluster-api/util/conditions" ctrl "sigs.k8s.io/controller-runtime" @@ -38,7 +38,7 @@ func (r *KubeadmControlPlaneReconciler) initializeControlPlane(ctx context.Conte // Perform an uncached read of all the owned machines. This check is in place to make sure // that the controller cache is not misbehaving and we end up initializing the cluster more than once. - ownedMachines, err := r.managementClusterUncached.GetMachinesForCluster(ctx, util.ObjectKey(cluster), machinefilters.OwnedMachines(kcp)) + ownedMachines, err := r.managementClusterUncached.GetMachinesForCluster(ctx, util.ObjectKey(cluster), collections.OwnedMachines(kcp)) if err != nil { logger.Error(err, "failed to perform an uncached read of control plane machines for cluster") return ctrl.Result{}, err @@ -88,7 +88,7 @@ func (r *KubeadmControlPlaneReconciler) scaleDownControlPlane( cluster *clusterv1.Cluster, kcp *controlplanev1.KubeadmControlPlane, controlPlane *internal.ControlPlane, - outdatedMachines internal.FilterableMachineCollection, + outdatedMachines collections.Machines, ) (ctrl.Result, error) { logger := controlPlane.Logger() @@ -164,7 +164,7 @@ func (r *KubeadmControlPlaneReconciler) preflightChecks(_ context.Context, contr // If there are deleting machines, wait for the operation to complete. if controlPlane.HasDeletingMachine() { - logger.Info("Waiting for machines to be deleted", "Machines", strings.Join(controlPlane.Machines.Filter(machinefilters.HasDeletionTimestamp).Names(), ", ")) + logger.Info("Waiting for machines to be deleted", "Machines", strings.Join(controlPlane.Machines.Filter(collections.HasDeletionTimestamp).Names(), ", ")) return ctrl.Result{RequeueAfter: deleteRequeueAfter}, nil } @@ -225,7 +225,7 @@ func preflightCheckCondition(kind string, obj conditions.Getter, condition clust return nil } -func selectMachineForScaleDown(controlPlane *internal.ControlPlane, outdatedMachines internal.FilterableMachineCollection) (*clusterv1.Machine, error) { +func selectMachineForScaleDown(controlPlane *internal.ControlPlane, outdatedMachines collections.Machines) (*clusterv1.Machine, error) { machines := controlPlane.Machines switch { case controlPlane.MachineWithDeleteAnnotation(outdatedMachines).Len() > 0: diff --git a/controlplane/kubeadm/controllers/scale_test.go b/controlplane/kubeadm/controllers/scale_test.go index 20bbe6a9b420..394cb952d0ba 100644 --- a/controlplane/kubeadm/controllers/scale_test.go +++ b/controlplane/kubeadm/controllers/scale_test.go @@ -19,6 +19,7 @@ package controllers import ( "context" "fmt" + "sigs.k8s.io/cluster-api/util/collections" "testing" "time" @@ -87,7 +88,7 @@ func TestKubeadmControlPlaneReconciler_scaleUpControlPlane(t *testing.T) { initObjs := []client.Object{cluster.DeepCopy(), kcp.DeepCopy(), genericMachineTemplate.DeepCopy()} fmc := &fakeManagementCluster{ - Machines: internal.NewFilterableMachineCollection(), + Machines: collections.New(), Workload: fakeWorkloadCluster{}, } @@ -126,7 +127,7 @@ func TestKubeadmControlPlaneReconciler_scaleUpControlPlane(t *testing.T) { cluster.Spec.ControlPlaneEndpoint.Host = "nodomain.example.com" cluster.Spec.ControlPlaneEndpoint.Port = 6443 - beforeMachines := internal.NewFilterableMachineCollection() + beforeMachines := collections.New() for i := 0; i < 2; i++ { m, _ := createMachineNodePair(fmt.Sprintf("test-%d", i), cluster.DeepCopy(), kcp.DeepCopy(), true) beforeMachines.Insert(m) @@ -157,7 +158,7 @@ func TestKubeadmControlPlaneReconciler_scaleUpControlPlane(t *testing.T) { g.Expect(fakeClient.List(context.Background(), controlPlaneMachines)).To(Succeed()) g.Expect(controlPlaneMachines.Items).To(HaveLen(len(beforeMachines))) - endMachines := internal.NewFilterableMachineCollectionFromMachineList(controlPlaneMachines) + endMachines := collections.FromMachineList(controlPlaneMachines) for _, m := range endMachines { bm, ok := beforeMachines[m.Name] bm.SetResourceVersion("1") @@ -290,8 +291,8 @@ func TestSelectMachineForScaleDown(t *testing.T) { m7 := machine("machine-7", withFailureDomain("two"), withTimestamp(startDate.Add(-5*time.Hour)), withAnnotation("cluster.x-k8s.io/delete-machine")) m8 := machine("machine-8", withFailureDomain("two"), withTimestamp(startDate.Add(-6*time.Hour)), withAnnotation("cluster.x-k8s.io/delete-machine")) - mc3 := internal.NewFilterableMachineCollection(m1, m2, m3, m4, m5) - mc6 := internal.NewFilterableMachineCollection(m6, m7, m8) + mc3 := collections.FromMachines(m1, m2, m3, m4, m5) + mc6 := collections.FromMachines(m6, m7, m8) fd := clusterv1.FailureDomains{ "one": failureDomain(true), "two": failureDomain(true), @@ -318,56 +319,56 @@ func TestSelectMachineForScaleDown(t *testing.T) { testCases := []struct { name string cp *internal.ControlPlane - outDatedMachines internal.FilterableMachineCollection + outDatedMachines collections.Machines expectErr bool expectedMachine clusterv1.Machine }{ { name: "when there are machines needing upgrade, it returns the oldest machine in the failure domain with the most machines needing upgrade", cp: needsUpgradeControlPlane, - outDatedMachines: internal.NewFilterableMachineCollection(m5), + outDatedMachines: collections.FromMachines(m5), expectErr: false, expectedMachine: clusterv1.Machine{ObjectMeta: metav1.ObjectMeta{Name: "machine-5"}}, }, { name: "when there are no outdated machines, it returns the oldest machine in the largest failure domain", cp: upToDateControlPlane, - outDatedMachines: internal.NewFilterableMachineCollection(), + outDatedMachines: collections.New(), expectErr: false, expectedMachine: clusterv1.Machine{ObjectMeta: metav1.ObjectMeta{Name: "machine-3"}}, }, { name: "when there is a single machine marked with delete annotation key in machine collection, it returns only that marked machine", cp: annotatedControlPlane, - outDatedMachines: internal.NewFilterableMachineCollection(m6, m7), + outDatedMachines: collections.FromMachines(m6, m7), expectErr: false, expectedMachine: clusterv1.Machine{ObjectMeta: metav1.ObjectMeta{Name: "machine-7"}}, }, { name: "when there are machines marked with delete annotation key in machine collection, it returns the oldest marked machine first", cp: annotatedControlPlane, - outDatedMachines: internal.NewFilterableMachineCollection(m7, m8), + outDatedMachines: collections.FromMachines(m7, m8), expectErr: false, expectedMachine: clusterv1.Machine{ObjectMeta: metav1.ObjectMeta{Name: "machine-8"}}, }, { name: "when there are annotated machines which are part of the annotatedControlPlane but not in outdatedMachines, it returns the oldest marked machine first", cp: annotatedControlPlane, - outDatedMachines: internal.NewFilterableMachineCollection(), + outDatedMachines: collections.New(), expectErr: false, expectedMachine: clusterv1.Machine{ObjectMeta: metav1.ObjectMeta{Name: "machine-8"}}, }, { name: "when there are machines needing upgrade, it returns the oldest machine in the failure domain with the most machines needing upgrade", cp: needsUpgradeControlPlane, - outDatedMachines: internal.NewFilterableMachineCollection(m7, m3), + outDatedMachines: collections.FromMachines(m7, m3), expectErr: false, expectedMachine: clusterv1.Machine{ObjectMeta: metav1.ObjectMeta{Name: "machine-7"}}, }, { name: "when there is an up to date machine with delete annotation, while there are any outdated machines without annotatio that still exist, it returns oldest marked machine first", cp: upToDateControlPlane, - outDatedMachines: internal.NewFilterableMachineCollection(m5, m3, m8, m7, m6, m1, m2), + outDatedMachines: collections.FromMachines(m5, m3, m8, m7, m6, m1, m2), expectErr: false, expectedMachine: clusterv1.Machine{ObjectMeta: metav1.ObjectMeta{Name: "machine-8"}}, }, @@ -471,7 +472,7 @@ func TestPreflightChecks(t *testing.T) { controlPlane := &internal.ControlPlane{ Cluster: &clusterv1.Cluster{}, KCP: tt.kcp, - Machines: internal.NewFilterableMachineCollection(tt.machines...), + Machines: collections.FromMachines(tt.machines...), } result, err := r.preflightChecks(context.TODO(), controlPlane) g.Expect(err).NotTo(HaveOccurred()) diff --git a/controlplane/kubeadm/controllers/status.go b/controlplane/kubeadm/controllers/status.go index 0ff057b09925..6945226ed19d 100644 --- a/controlplane/kubeadm/controllers/status.go +++ b/controlplane/kubeadm/controllers/status.go @@ -18,12 +18,12 @@ package controllers import ( "context" + "sigs.k8s.io/cluster-api/util/collections" "github.com/pkg/errors" clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha4" controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1alpha4" "sigs.k8s.io/cluster-api/controlplane/kubeadm/internal" - "sigs.k8s.io/cluster-api/controlplane/kubeadm/internal/machinefilters" "sigs.k8s.io/cluster-api/util" "sigs.k8s.io/cluster-api/util/conditions" ctrl "sigs.k8s.io/controller-runtime" @@ -34,12 +34,12 @@ import ( func (r *KubeadmControlPlaneReconciler) updateStatus(ctx context.Context, kcp *controlplanev1.KubeadmControlPlane, cluster *clusterv1.Cluster) error { log := ctrl.LoggerFrom(ctx, "cluster", cluster.Name) - selector := machinefilters.ControlPlaneSelectorForCluster(cluster.Name) + selector := collections.ControlPlaneSelectorForCluster(cluster.Name) // Copy label selector to its status counterpart in string format. // This is necessary for CRDs including scale subresources. kcp.Status.Selector = selector.String() - ownedMachines, err := r.managementCluster.GetMachinesForCluster(ctx, util.ObjectKey(cluster), machinefilters.OwnedMachines(kcp)) + ownedMachines, err := r.managementCluster.GetMachinesForCluster(ctx, util.ObjectKey(cluster), collections.OwnedMachines(kcp)) if err != nil { return errors.Wrap(err, "failed to get list of owned machines") } @@ -79,7 +79,7 @@ func (r *KubeadmControlPlaneReconciler) updateStatus(ctx context.Context, kcp *c // make sure last resize operation is marked as completed. // NOTE: we are checking the number of machines ready so we report resize completed only when the machines // are actually provisioned (vs reporting completed immediately after the last machine object is created). - readyMachines := ownedMachines.Filter(machinefilters.IsReady()) + readyMachines := ownedMachines.Filter(collections.IsReady()) if int32(len(readyMachines)) == replicas { conditions.MarkTrue(kcp, controlplanev1.ResizedCondition) } diff --git a/controlplane/kubeadm/controllers/upgrade.go b/controlplane/kubeadm/controllers/upgrade.go index 250ef378f5ec..910e5c80f27d 100644 --- a/controlplane/kubeadm/controllers/upgrade.go +++ b/controlplane/kubeadm/controllers/upgrade.go @@ -18,13 +18,13 @@ package controllers import ( "context" - "github.com/blang/semver" "github.com/pkg/errors" clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha4" controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1alpha4" "sigs.k8s.io/cluster-api/controlplane/kubeadm/internal" "sigs.k8s.io/cluster-api/util" + "sigs.k8s.io/cluster-api/util/collections" ctrl "sigs.k8s.io/controller-runtime" ) @@ -33,7 +33,7 @@ func (r *KubeadmControlPlaneReconciler) upgradeControlPlane( cluster *clusterv1.Cluster, kcp *controlplanev1.KubeadmControlPlane, controlPlane *internal.ControlPlane, - machinesRequireUpgrade internal.FilterableMachineCollection, + machinesRequireUpgrade collections.Machines, ) (ctrl.Result, error) { logger := controlPlane.Logger() diff --git a/controlplane/kubeadm/controllers/upgrade_test.go b/controlplane/kubeadm/controllers/upgrade_test.go index c0310d67545b..b073e3529c94 100644 --- a/controlplane/kubeadm/controllers/upgrade_test.go +++ b/controlplane/kubeadm/controllers/upgrade_test.go @@ -18,6 +18,7 @@ package controllers import ( "context" + "sigs.k8s.io/cluster-api/util/collections" "testing" . "github.com/onsi/gomega" @@ -82,7 +83,7 @@ func TestKubeadmControlPlaneReconciler_upgradeControlPlane(t *testing.T) { kcp.Spec.Version = "v1.17.4" // run upgrade the first time, expect we scale up - needingUpgrade := internal.NewFilterableMachineCollectionFromMachineList(initialMachine) + needingUpgrade := collections.FromMachineList(initialMachine) controlPlane.Machines = needingUpgrade result, err = r.upgradeControlPlane(ctx, cluster, kcp, controlPlane, needingUpgrade) g.Expect(result).To(Equal(ctrl.Result{Requeue: true})) @@ -105,7 +106,7 @@ func TestKubeadmControlPlaneReconciler_upgradeControlPlane(t *testing.T) { for i := range bothMachines.Items { setMachineHealthy(&bothMachines.Items[i]) } - controlPlane.Machines = internal.NewFilterableMachineCollectionFromMachineList(bothMachines) + controlPlane.Machines = collections.FromMachineList(bothMachines) // run upgrade the second time, expect we scale down result, err = r.upgradeControlPlane(ctx, cluster, kcp, controlPlane, controlPlane.Machines) diff --git a/controlplane/kubeadm/internal/cluster.go b/controlplane/kubeadm/internal/cluster.go index 8ff147cd9cba..b34fd128e421 100644 --- a/controlplane/kubeadm/internal/cluster.go +++ b/controlplane/kubeadm/internal/cluster.go @@ -21,13 +21,13 @@ import ( "crypto/tls" "crypto/x509" "fmt" + "sigs.k8s.io/cluster-api/util/collections" "time" "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha4" "sigs.k8s.io/cluster-api/controllers/remote" - "sigs.k8s.io/cluster-api/controlplane/kubeadm/internal/machinefilters" "sigs.k8s.io/cluster-api/util/secret" "sigs.k8s.io/controller-runtime/pkg/client" ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" @@ -42,7 +42,7 @@ const ( type ManagementCluster interface { ctrlclient.Reader - GetMachinesForCluster(ctx context.Context, cluster client.ObjectKey, filters ...machinefilters.Func) (FilterableMachineCollection, error) + GetMachinesForCluster(ctx context.Context, cluster client.ObjectKey, filters ...collections.Func) (collections.Machines, error) GetWorkloadCluster(ctx context.Context, clusterKey client.ObjectKey) (WorkloadCluster, error) } @@ -73,7 +73,7 @@ func (m *Management) List(ctx context.Context, list client.ObjectList, opts ...c // GetMachinesForCluster returns a list of machines that can be filtered or not. // If no filter is supplied then all machines associated with the target cluster are returned. -func (m *Management) GetMachinesForCluster(ctx context.Context, cluster client.ObjectKey, filters ...machinefilters.Func) (FilterableMachineCollection, error) { +func (m *Management) GetMachinesForCluster(ctx context.Context, cluster client.ObjectKey, filters ...collections.Func) (collections.Machines, error) { selector := map[string]string{ clusterv1.ClusterLabelName: cluster.Name, } @@ -82,7 +82,7 @@ func (m *Management) GetMachinesForCluster(ctx context.Context, cluster client.O return nil, errors.Wrap(err, "failed to list machines") } - machines := NewFilterableMachineCollectionFromMachineList(ml) + machines := collections.FromMachineList(ml) return machines.Filter(filters...), nil } diff --git a/controlplane/kubeadm/internal/cluster_test.go b/controlplane/kubeadm/internal/cluster_test.go index 86c7afaf8422..16ef5b0da44f 100644 --- a/controlplane/kubeadm/internal/cluster_test.go +++ b/controlplane/kubeadm/internal/cluster_test.go @@ -24,6 +24,7 @@ import ( "crypto/x509/pkix" "fmt" "math/big" + "sigs.k8s.io/cluster-api/util/collections" "testing" "time" @@ -38,7 +39,6 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha4" "sigs.k8s.io/cluster-api/controllers/remote" - "sigs.k8s.io/cluster-api/controlplane/kubeadm/internal/machinefilters" "sigs.k8s.io/cluster-api/util/certs" "sigs.k8s.io/cluster-api/util/kubeconfig" "sigs.k8s.io/cluster-api/util/secret" @@ -60,7 +60,7 @@ func TestGetMachinesForCluster(t *testing.T) { g.Expect(machines).To(HaveLen(3)) // Test the ControlPlaneMachines works - machines, err = m.GetMachinesForCluster(ctx, clusterKey, machinefilters.ControlPlaneMachines("my-cluster")) + machines, err = m.GetMachinesForCluster(ctx, clusterKey, collections.ControlPlaneMachines("my-cluster")) g.Expect(err).NotTo(HaveOccurred()) g.Expect(machines).To(HaveLen(1)) @@ -68,7 +68,7 @@ func TestGetMachinesForCluster(t *testing.T) { nameFilter := func(cluster *clusterv1.Machine) bool { return cluster.Name == "first-machine" } - machines, err = m.GetMachinesForCluster(ctx, clusterKey, machinefilters.ControlPlaneMachines("my-cluster"), nameFilter) + machines, err = m.GetMachinesForCluster(ctx, clusterKey, collections.ControlPlaneMachines("my-cluster"), nameFilter) g.Expect(err).NotTo(HaveOccurred()) g.Expect(machines).To(HaveLen(1)) } diff --git a/controlplane/kubeadm/internal/control_plane.go b/controlplane/kubeadm/internal/control_plane.go index 2f03d74d6a0c..9341a6e9ceaa 100644 --- a/controlplane/kubeadm/internal/control_plane.go +++ b/controlplane/kubeadm/internal/control_plane.go @@ -18,7 +18,6 @@ package internal import ( "context" - "github.com/go-logr/logr" "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" @@ -27,22 +26,27 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" kerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apiserver/pkg/storage/names" + "k8s.io/klog/klogr" clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha4" bootstrapv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1alpha4" "sigs.k8s.io/cluster-api/controllers/external" controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1alpha4" - "sigs.k8s.io/cluster-api/controlplane/kubeadm/internal/machinefilters" + "sigs.k8s.io/cluster-api/util/collections" + "sigs.k8s.io/cluster-api/util/failuredomains" "sigs.k8s.io/cluster-api/util/patch" "sigs.k8s.io/controller-runtime/pkg/client" ) +// Log is the global logger for the internal package. +var Log = klogr.New() + // ControlPlane holds business logic around control planes. // It should never need to connect to a service, that responsibility lies outside of this struct. // Going forward we should be trying to add more logic to here and reduce the amount of logic in the reconciler. type ControlPlane struct { KCP *controlplanev1.KubeadmControlPlane Cluster *clusterv1.Cluster - Machines FilterableMachineCollection + Machines collections.Machines machinesPatchHelpers map[string]*patch.Helper // reconciliationTime is the time of the current reconciliation, and should be used for all "now" calculations @@ -55,7 +59,7 @@ type ControlPlane struct { } // NewControlPlane returns an instantiated ControlPlane. -func NewControlPlane(ctx context.Context, client client.Client, cluster *clusterv1.Cluster, kcp *controlplanev1.KubeadmControlPlane, ownedMachines FilterableMachineCollection) (*ControlPlane, error) { +func NewControlPlane(ctx context.Context, client client.Client, cluster *clusterv1.Cluster, kcp *controlplanev1.KubeadmControlPlane, ownedMachines collections.Machines) (*ControlPlane, error) { infraObjects, err := getInfraResources(ctx, client, ownedMachines) if err != nil { return nil, err @@ -127,9 +131,9 @@ func (c *ControlPlane) EtcdImageData() (string, string) { } // MachineInFailureDomainWithMostMachines returns the first matching failure domain with machines that has the most control-plane machines on it. -func (c *ControlPlane) MachineInFailureDomainWithMostMachines(machines FilterableMachineCollection) (*clusterv1.Machine, error) { +func (c *ControlPlane) MachineInFailureDomainWithMostMachines(machines collections.Machines) (*clusterv1.Machine, error) { fd := c.FailureDomainWithMostMachines(machines) - machinesInFailureDomain := machines.Filter(machinefilters.InFailureDomains(fd)) + machinesInFailureDomain := machines.Filter(collections.InFailureDomains(fd)) machineToMark := machinesInFailureDomain.Oldest() if machineToMark == nil { return nil, errors.New("failed to pick control plane Machine to mark for deletion") @@ -138,19 +142,19 @@ func (c *ControlPlane) MachineInFailureDomainWithMostMachines(machines Filterabl } // MachineWithDeleteAnnotation returns a machine that has been annotated with DeleteMachineAnnotation key. -func (c *ControlPlane) MachineWithDeleteAnnotation(machines FilterableMachineCollection) FilterableMachineCollection { +func (c *ControlPlane) MachineWithDeleteAnnotation(machines collections.Machines) collections.Machines { // See if there are any machines with DeleteMachineAnnotation key. - annotatedMachines := machines.Filter(machinefilters.HasAnnotationKey(clusterv1.DeleteMachineAnnotation)) + annotatedMachines := machines.Filter(collections.HasAnnotationKey(clusterv1.DeleteMachineAnnotation)) // If there are, return list of annotated machines. return annotatedMachines } // FailureDomainWithMostMachines returns a fd which exists both in machines and control-plane machines and has the most // control-plane machines on it. -func (c *ControlPlane) FailureDomainWithMostMachines(machines FilterableMachineCollection) *string { +func (c *ControlPlane) FailureDomainWithMostMachines(machines collections.Machines) *string { // See if there are any Machines that are not in currently defined failure domains first. notInFailureDomains := machines.Filter( - machinefilters.Not(machinefilters.InFailureDomains(c.FailureDomains().FilterControlPlane().GetIDs()...)), + collections.Not(collections.InFailureDomains(c.FailureDomains().FilterControlPlane().GetIDs()...)), ) if len(notInFailureDomains) > 0 { // return the failure domain for the oldest Machine not in the current list of failure domains @@ -158,7 +162,7 @@ func (c *ControlPlane) FailureDomainWithMostMachines(machines FilterableMachineC // in the cluster status. return notInFailureDomains.Oldest().Spec.FailureDomain } - return PickMost(c, machines) + return failuredomains.PickMost(c.Cluster.Status.FailureDomains.FilterControlPlane(), c.Machines, machines) } // NextFailureDomainForScaleUp returns the failure domain with the fewest number of up-to-date machines. @@ -166,7 +170,7 @@ func (c *ControlPlane) NextFailureDomainForScaleUp() *string { if len(c.Cluster.Status.FailureDomains.FilterControlPlane()) == 0 { return nil } - return PickFewest(c.FailureDomains().FilterControlPlane(), c.UpToDateMachines()) + return failuredomains.PickFewest(c.FailureDomains().FilterControlPlane(), c.UpToDateMachines()) } // InitialControlPlaneConfig returns a new KubeadmConfigSpec that is to be used for an initializing control plane. @@ -243,31 +247,31 @@ func (c *ControlPlane) NeedsReplacementNode() bool { // HasDeletingMachine returns true if any machine in the control plane is in the process of being deleted. func (c *ControlPlane) HasDeletingMachine() bool { - return len(c.Machines.Filter(machinefilters.HasDeletionTimestamp)) > 0 + return len(c.Machines.Filter(collections.HasDeletionTimestamp)) > 0 } // MachinesNeedingRollout return a list of machines that need to be rolled out. -func (c *ControlPlane) MachinesNeedingRollout() FilterableMachineCollection { +func (c *ControlPlane) MachinesNeedingRollout() collections.Machines { // Ignore machines to be deleted. - machines := c.Machines.Filter(machinefilters.Not(machinefilters.HasDeletionTimestamp)) + machines := c.Machines.Filter(collections.Not(collections.HasDeletionTimestamp)) // Return machines if they are scheduled for rollout or if with an outdated configuration. return machines.AnyFilter( // Machines that are scheduled for rollout (KCP.Spec.UpgradeAfter set, the UpgradeAfter deadline is expired, and the machine was created before the deadline). - machinefilters.ShouldRolloutAfter(&c.reconciliationTime, c.KCP.Spec.UpgradeAfter), + collections.ShouldRolloutAfter(&c.reconciliationTime, c.KCP.Spec.UpgradeAfter), // Machines that do not match with KCP config. - machinefilters.Not(machinefilters.MatchesKCPConfiguration(c.infraResources, c.kubeadmConfigs, c.KCP)), + collections.Not(MatchesKCPConfiguration(c.infraResources, c.kubeadmConfigs, c.KCP)), ) } // UpToDateMachines returns the machines that are up to date with the control // plane's configuration and therefore do not require rollout. -func (c *ControlPlane) UpToDateMachines() FilterableMachineCollection { +func (c *ControlPlane) UpToDateMachines() collections.Machines { return c.Machines.Difference(c.MachinesNeedingRollout()) } // getInfraResources fetches the external infrastructure resource for each machine in the collection and returns a map of machine.Name -> infraResource. -func getInfraResources(ctx context.Context, cl client.Client, machines FilterableMachineCollection) (map[string]*unstructured.Unstructured, error) { +func getInfraResources(ctx context.Context, cl client.Client, machines collections.Machines) (map[string]*unstructured.Unstructured, error) { result := map[string]*unstructured.Unstructured{} for _, m := range machines { infraObj, err := external.Get(ctx, cl, &m.Spec.InfrastructureRef, m.Namespace) @@ -283,7 +287,7 @@ func getInfraResources(ctx context.Context, cl client.Client, machines Filterabl } // getKubeadmConfigs fetches the kubeadm config for each machine in the collection and returns a map of machine.Name -> KubeadmConfig. -func getKubeadmConfigs(ctx context.Context, cl client.Client, machines FilterableMachineCollection) (map[string]*bootstrapv1.KubeadmConfig, error) { +func getKubeadmConfigs(ctx context.Context, cl client.Client, machines collections.Machines) (map[string]*bootstrapv1.KubeadmConfig, error) { result := map[string]*bootstrapv1.KubeadmConfig{} for _, m := range machines { bootstrapRef := m.Spec.Bootstrap.ConfigRef @@ -308,13 +312,13 @@ func (c *ControlPlane) IsEtcdManaged() bool { } // UnhealthyMachines returns the list of control plane machines marked as unhealthy by MHC. -func (c *ControlPlane) UnhealthyMachines() FilterableMachineCollection { - return c.Machines.Filter(machinefilters.HasUnhealthyCondition) +func (c *ControlPlane) UnhealthyMachines() collections.Machines { + return c.Machines.Filter(collections.HasUnhealthyCondition) } // HealthyMachines returns the list of control plane machines not marked as unhealthy by MHC. -func (c *ControlPlane) HealthyMachines() FilterableMachineCollection { - return c.Machines.Filter(machinefilters.Not(machinefilters.HasUnhealthyCondition)) +func (c *ControlPlane) HealthyMachines() collections.Machines { + return c.Machines.Filter(collections.Not(collections.HasUnhealthyCondition)) } // HasUnhealthyMachine returns true if any machine in the control plane is marked as unhealthy by MHC. diff --git a/controlplane/kubeadm/internal/control_plane_test.go b/controlplane/kubeadm/internal/control_plane_test.go index 82c3512d47ec..167f0d7f5609 100644 --- a/controlplane/kubeadm/internal/control_plane_test.go +++ b/controlplane/kubeadm/internal/control_plane_test.go @@ -17,6 +17,7 @@ limitations under the License. package internal import ( + "sigs.k8s.io/cluster-api/util/collections" "testing" . "github.com/onsi/gomega" @@ -47,7 +48,7 @@ func TestControlPlane(t *testing.T) { }, }, }, - Machines: FilterableMachineCollection{ + Machines: collections.Machines{ "machine-1": machine("machine-1", withFailureDomain("one")), "machine-2": machine("machine-2", withFailureDomain("two")), "machine-3": machine("machine-3", withFailureDomain("two")), @@ -93,18 +94,6 @@ func TestControlPlane(t *testing.T) { }) } -func failureDomain(controlPlane bool) clusterv1.FailureDomainSpec { - return clusterv1.FailureDomainSpec{ - ControlPlane: controlPlane, - } -} - -func withFailureDomain(fd string) machineOpt { - return func(m *clusterv1.Machine) { - m.Spec.FailureDomain = &fd - } -} - func TestHasUnhealthyMachine(t *testing.T) { // healthy machine (without MachineHealthCheckSucceded condition) healthyMachine1 := &clusterv1.Machine{} @@ -120,7 +109,7 @@ func TestHasUnhealthyMachine(t *testing.T) { conditions.MarkFalse(unhealthyMachineOwnerRemediated, clusterv1.MachineOwnerRemediatedCondition, clusterv1.WaitingForRemediationReason, clusterv1.ConditionSeverityWarning, "") c := ControlPlane{ - Machines: NewFilterableMachineCollection( + Machines: collections.FromMachines( healthyMachine1, healthyMachine2, unhealthyMachineNOTOwnerRemediated, @@ -131,3 +120,29 @@ func TestHasUnhealthyMachine(t *testing.T) { g := NewWithT(t) g.Expect(c.HasUnhealthyMachine()).To(BeTrue()) } + +type machineOpt func(*clusterv1.Machine) + +func failureDomain(controlPlane bool) clusterv1.FailureDomainSpec { + return clusterv1.FailureDomainSpec{ + ControlPlane: controlPlane, + } +} + +func withFailureDomain(fd string) machineOpt { + return func(m *clusterv1.Machine) { + m.Spec.FailureDomain = &fd + } +} + +func machine(name string, opts ...machineOpt) *clusterv1.Machine { + m := &clusterv1.Machine{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + }, + } + for _, opt := range opts { + opt(m) + } + return m +} diff --git a/controlplane/kubeadm/internal/machinefilters/machine_filters.go b/controlplane/kubeadm/internal/filters.go similarity index 63% rename from controlplane/kubeadm/internal/machinefilters/machine_filters.go rename to controlplane/kubeadm/internal/filters.go index ae0937b4fd6c..89126661844e 100644 --- a/controlplane/kubeadm/internal/machinefilters/machine_filters.go +++ b/controlplane/kubeadm/internal/filters.go @@ -1,5 +1,5 @@ /* -Copyright 2020 The Kubernetes Authors. +Copyright 2021 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. @@ -14,203 +14,31 @@ See the License for the specific language governing permissions and limitations under the License. */ -package machinefilters +package internal import ( "encoding/json" - "reflect" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/labels" - "k8s.io/apimachinery/pkg/selection" + "reflect" clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha4" bootstrapv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1alpha4" kubeadmv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/types/v1beta1" controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1alpha4" - "sigs.k8s.io/cluster-api/util" - "sigs.k8s.io/cluster-api/util/conditions" - "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/cluster-api/util/collections" ) -type Func func(machine *clusterv1.Machine) bool - -// And returns a filter that returns true if all of the given filters returns true. -func And(filters ...Func) Func { - return func(machine *clusterv1.Machine) bool { - for _, f := range filters { - if !f(machine) { - return false - } - } - return true - } -} - -// Or returns a filter that returns true if any of the given filters returns true. -func Or(filters ...Func) Func { - return func(machine *clusterv1.Machine) bool { - for _, f := range filters { - if f(machine) { - return true - } - } - return false - } -} - -// Not returns a filter that returns the opposite of the given filter. -func Not(mf Func) Func { - return func(machine *clusterv1.Machine) bool { - return !mf(machine) - } -} - -// HasControllerRef is a filter that returns true if the machine has a controller ref -func HasControllerRef(machine *clusterv1.Machine) bool { - if machine == nil { - return false - } - return metav1.GetControllerOf(machine) != nil -} - -// InFailureDomains returns a filter to find all machines -// in any of the given failure domains -func InFailureDomains(failureDomains ...*string) Func { - return func(machine *clusterv1.Machine) bool { - if machine == nil { - return false - } - for i := range failureDomains { - fd := failureDomains[i] - if fd == nil { - if fd == machine.Spec.FailureDomain { - return true - } - continue - } - if machine.Spec.FailureDomain == nil { - continue - } - if *fd == *machine.Spec.FailureDomain { - return true - } - } - return false - } -} - -// OwnedMachines returns a filter to find all owned control plane machines. -// Usage: managementCluster.GetMachinesForCluster(ctx, cluster, machinefilters.OwnedMachines(controlPlane)) -func OwnedMachines(owner client.Object) func(machine *clusterv1.Machine) bool { - return func(machine *clusterv1.Machine) bool { - if machine == nil { - return false - } - return util.IsOwnedByObject(machine, owner) - } -} - -// ControlPlaneMachines returns a filter to find all control plane machines for a cluster, regardless of ownership. -// Usage: managementCluster.GetMachinesForCluster(ctx, cluster, machinefilters.ControlPlaneMachines(cluster.Name)) -func ControlPlaneMachines(clusterName string) func(machine *clusterv1.Machine) bool { - selector := ControlPlaneSelectorForCluster(clusterName) - return func(machine *clusterv1.Machine) bool { - if machine == nil { - return false - } - return selector.Matches(labels.Set(machine.Labels)) - } -} - -// AdoptableControlPlaneMachines returns a filter to find all un-controlled control plane machines. -// Usage: managementCluster.GetMachinesForCluster(ctx, cluster, AdoptableControlPlaneMachines(cluster.Name, controlPlane)) -func AdoptableControlPlaneMachines(clusterName string) func(machine *clusterv1.Machine) bool { - return And( - ControlPlaneMachines(clusterName), - Not(HasControllerRef), - ) -} - -// HasDeletionTimestamp returns a filter to find all machines that have a deletion timestamp. -func HasDeletionTimestamp(machine *clusterv1.Machine) bool { - if machine == nil { - return false - } - return !machine.DeletionTimestamp.IsZero() -} - -// HasUnhealthyCondition returns a filter to find all machines that have a MachineHealthCheckSucceeded condition set to False, -// indicating a problem was detected on the machine, and the MachineOwnerRemediated condition set, indicating that KCP is -// responsible of performing remediation as owner of the machine. -func HasUnhealthyCondition(machine *clusterv1.Machine) bool { - if machine == nil { - return false - } - return conditions.IsFalse(machine, clusterv1.MachineHealthCheckSuccededCondition) && conditions.IsFalse(machine, clusterv1.MachineOwnerRemediatedCondition) -} - -// IsReady returns a filter to find all machines with the ReadyCondition equals to True. -func IsReady() Func { - return func(machine *clusterv1.Machine) bool { - if machine == nil { - return false - } - return conditions.IsTrue(machine, clusterv1.ReadyCondition) - } -} - -// ShouldRolloutAfter returns a filter to find all machines where -// CreationTimestamp < rolloutAfter < reconciliationTIme -func ShouldRolloutAfter(reconciliationTime, rolloutAfter *metav1.Time) Func { - return func(machine *clusterv1.Machine) bool { - if machine == nil { - return false - } - return machine.CreationTimestamp.Before(rolloutAfter) && rolloutAfter.Before(reconciliationTime) - } -} - -// HasAnnotationKey returns a filter to find all machines that have the -// specified Annotation key present -func HasAnnotationKey(key string) Func { - return func(machine *clusterv1.Machine) bool { - if machine == nil || machine.Annotations == nil { - return false - } - if _, ok := machine.Annotations[key]; ok { - return true - } - return false - } -} - -// ControlPlaneSelectorForCluster returns the label selector necessary to get control plane machines for a given cluster. -func ControlPlaneSelectorForCluster(clusterName string) labels.Selector { - must := func(r *labels.Requirement, err error) labels.Requirement { - if err != nil { - panic(err) - } - return *r - } - return labels.NewSelector().Add( - must(labels.NewRequirement(clusterv1.ClusterLabelName, selection.Equals, []string{clusterName})), - must(labels.NewRequirement(clusterv1.MachineControlPlaneLabelName, selection.Exists, []string{})), - ) -} - // MatchesKCPConfiguration returns a filter to find all machines that matches with KCP config and do not require any rollout. // Kubernetes version, infrastructure template, and KubeadmConfig field need to be equivalent. func MatchesKCPConfiguration(infraConfigs map[string]*unstructured.Unstructured, machineConfigs map[string]*bootstrapv1.KubeadmConfig, kcp *controlplanev1.KubeadmControlPlane) func(machine *clusterv1.Machine) bool { - return And( - MatchesKubernetesVersion(kcp.Spec.Version), + return collections.And( + collections.MatchesKubernetesVersion(kcp.Spec.Version), MatchesKubeadmBootstrapConfig(machineConfigs, kcp), MatchesTemplateClonedFrom(infraConfigs, kcp), ) } // MatchesTemplateClonedFrom returns a filter to find all machines that match a given KCP infra template. -func MatchesTemplateClonedFrom(infraConfigs map[string]*unstructured.Unstructured, kcp *controlplanev1.KubeadmControlPlane) Func { +func MatchesTemplateClonedFrom(infraConfigs map[string]*unstructured.Unstructured, kcp *controlplanev1.KubeadmControlPlane) collections.Func { return func(machine *clusterv1.Machine) bool { if machine == nil { return false @@ -239,21 +67,8 @@ func MatchesTemplateClonedFrom(infraConfigs map[string]*unstructured.Unstructure } } -// MatchesKubernetesVersion returns a filter to find all machines that match a given Kubernetes version. -func MatchesKubernetesVersion(kubernetesVersion string) Func { - return func(machine *clusterv1.Machine) bool { - if machine == nil { - return false - } - if machine.Spec.Version == nil { - return false - } - return *machine.Spec.Version == kubernetesVersion - } -} - // MatchesKubeadmBootstrapConfig checks if machine's KubeadmConfigSpec is equivalent with KCP's KubeadmConfigSpec. -func MatchesKubeadmBootstrapConfig(machineConfigs map[string]*bootstrapv1.KubeadmConfig, kcp *controlplanev1.KubeadmControlPlane) Func { +func MatchesKubeadmBootstrapConfig(machineConfigs map[string]*bootstrapv1.KubeadmConfig, kcp *controlplanev1.KubeadmControlPlane) collections.Func { return func(machine *clusterv1.Machine) bool { if machine == nil { return false diff --git a/controlplane/kubeadm/internal/machinefilters/util_test.go b/controlplane/kubeadm/internal/filters_test.go similarity index 81% rename from controlplane/kubeadm/internal/machinefilters/util_test.go rename to controlplane/kubeadm/internal/filters_test.go index a5f63c8a8416..1d5ebd7d272e 100644 --- a/controlplane/kubeadm/internal/machinefilters/util_test.go +++ b/controlplane/kubeadm/internal/filters_test.go @@ -14,12 +14,13 @@ See the License for the specific language governing permissions and limitations under the License. */ -package machinefilters +package internal import ( + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "testing" - "github.com/onsi/gomega" + . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha4" @@ -30,13 +31,13 @@ import ( func TestMatchClusterConfiguration(t *testing.T) { t.Run("machine without the ClusterConfiguration annotation should match (not enough information to make a decision)", func(t *testing.T) { - g := gomega.NewWithT(t) + g := NewWithT(t) kcp := &controlplanev1.KubeadmControlPlane{} m := &clusterv1.Machine{} - g.Expect(matchClusterConfiguration(kcp, m)).To(gomega.BeTrue()) + g.Expect(matchClusterConfiguration(kcp, m)).To(BeTrue()) }) t.Run("machine without an invalid ClusterConfiguration annotation should not match (only solution is to rollout)", func(t *testing.T) { - g := gomega.NewWithT(t) + g := NewWithT(t) kcp := &controlplanev1.KubeadmControlPlane{} m := &clusterv1.Machine{ ObjectMeta: metav1.ObjectMeta{ @@ -45,10 +46,10 @@ func TestMatchClusterConfiguration(t *testing.T) { }, }, } - g.Expect(matchClusterConfiguration(kcp, m)).To(gomega.BeFalse()) + g.Expect(matchClusterConfiguration(kcp, m)).To(BeFalse()) }) t.Run("Return true if cluster configuration matches", func(t *testing.T) { - g := gomega.NewWithT(t) + g := NewWithT(t) kcp := &controlplanev1.KubeadmControlPlane{ Spec: controlplanev1.KubeadmControlPlaneSpec{ KubeadmConfigSpec: bootstrapv1.KubeadmConfigSpec{ @@ -65,10 +66,10 @@ func TestMatchClusterConfiguration(t *testing.T) { }, }, } - g.Expect(matchClusterConfiguration(kcp, m)).To(gomega.BeTrue()) + g.Expect(matchClusterConfiguration(kcp, m)).To(BeTrue()) }) t.Run("Return false if cluster configuration does not match", func(t *testing.T) { - g := gomega.NewWithT(t) + g := NewWithT(t) kcp := &controlplanev1.KubeadmControlPlane{ Spec: controlplanev1.KubeadmControlPlaneSpec{ KubeadmConfigSpec: bootstrapv1.KubeadmConfigSpec{ @@ -85,10 +86,10 @@ func TestMatchClusterConfiguration(t *testing.T) { }, }, } - g.Expect(matchClusterConfiguration(kcp, m)).To(gomega.BeFalse()) + g.Expect(matchClusterConfiguration(kcp, m)).To(BeFalse()) }) t.Run("Return true if cluster configuration is nil (special case)", func(t *testing.T) { - g := gomega.NewWithT(t) + g := NewWithT(t) kcp := &controlplanev1.KubeadmControlPlane{ Spec: controlplanev1.KubeadmControlPlaneSpec{ KubeadmConfigSpec: bootstrapv1.KubeadmConfigSpec{}, @@ -101,13 +102,13 @@ func TestMatchClusterConfiguration(t *testing.T) { }, }, } - g.Expect(matchClusterConfiguration(kcp, m)).To(gomega.BeTrue()) + g.Expect(matchClusterConfiguration(kcp, m)).To(BeTrue()) }) } func TestGetAdjustedKcpConfig(t *testing.T) { t.Run("if the machine is the first control plane, kcp config should get InitConfiguration", func(t *testing.T) { - g := gomega.NewWithT(t) + g := NewWithT(t) kcp := &controlplanev1.KubeadmControlPlane{ Spec: controlplanev1.KubeadmControlPlaneSpec{ KubeadmConfigSpec: bootstrapv1.KubeadmConfigSpec{ @@ -122,11 +123,11 @@ func TestGetAdjustedKcpConfig(t *testing.T) { }, } kcpConfig := getAdjustedKcpConfig(kcp, machineConfig) - g.Expect(kcpConfig.InitConfiguration).ToNot(gomega.BeNil()) - g.Expect(kcpConfig.JoinConfiguration).To(gomega.BeNil()) + g.Expect(kcpConfig.InitConfiguration).ToNot(BeNil()) + g.Expect(kcpConfig.JoinConfiguration).To(BeNil()) }) t.Run("if the machine is a joining control plane, kcp config should get JoinConfiguration", func(t *testing.T) { - g := gomega.NewWithT(t) + g := NewWithT(t) kcp := &controlplanev1.KubeadmControlPlane{ Spec: controlplanev1.KubeadmControlPlaneSpec{ KubeadmConfigSpec: bootstrapv1.KubeadmConfigSpec{ @@ -141,14 +142,14 @@ func TestGetAdjustedKcpConfig(t *testing.T) { }, } kcpConfig := getAdjustedKcpConfig(kcp, machineConfig) - g.Expect(kcpConfig.InitConfiguration).To(gomega.BeNil()) - g.Expect(kcpConfig.JoinConfiguration).ToNot(gomega.BeNil()) + g.Expect(kcpConfig.InitConfiguration).To(BeNil()) + g.Expect(kcpConfig.JoinConfiguration).ToNot(BeNil()) }) } func TestCleanupConfigFields(t *testing.T) { t.Run("ClusterConfiguration gets removed from KcpConfig and MachineConfig", func(t *testing.T) { - g := gomega.NewWithT(t) + g := NewWithT(t) kcpConfig := &bootstrapv1.KubeadmConfigSpec{ ClusterConfiguration: &kubeadmv1beta1.ClusterConfiguration{}, } @@ -158,11 +159,11 @@ func TestCleanupConfigFields(t *testing.T) { }, } cleanupConfigFields(kcpConfig, machineConfig) - g.Expect(kcpConfig.ClusterConfiguration).To(gomega.BeNil()) - g.Expect(machineConfig.Spec.ClusterConfiguration).To(gomega.BeNil()) + g.Expect(kcpConfig.ClusterConfiguration).To(BeNil()) + g.Expect(machineConfig.Spec.ClusterConfiguration).To(BeNil()) }) t.Run("JoinConfiguration gets removed from MachineConfig if it was not derived by KCPConfig", func(t *testing.T) { - g := gomega.NewWithT(t) + g := NewWithT(t) kcpConfig := &bootstrapv1.KubeadmConfigSpec{ JoinConfiguration: nil, // KCP not providing a JoinConfiguration } @@ -172,11 +173,11 @@ func TestCleanupConfigFields(t *testing.T) { }, } cleanupConfigFields(kcpConfig, machineConfig) - g.Expect(kcpConfig.JoinConfiguration).To(gomega.BeNil()) - g.Expect(machineConfig.Spec.JoinConfiguration).To(gomega.BeNil()) + g.Expect(kcpConfig.JoinConfiguration).To(BeNil()) + g.Expect(machineConfig.Spec.JoinConfiguration).To(BeNil()) }) t.Run("JoinConfiguration.Discovery gets removed because it is not relevant for compare", func(t *testing.T) { - g := gomega.NewWithT(t) + g := NewWithT(t) kcpConfig := &bootstrapv1.KubeadmConfigSpec{ JoinConfiguration: &kubeadmv1beta1.JoinConfiguration{ Discovery: kubeadmv1beta1.Discovery{TLSBootstrapToken: "aaa"}, @@ -190,11 +191,11 @@ func TestCleanupConfigFields(t *testing.T) { }, } cleanupConfigFields(kcpConfig, machineConfig) - g.Expect(kcpConfig.JoinConfiguration.Discovery).To(gomega.Equal(kubeadmv1beta1.Discovery{})) - g.Expect(machineConfig.Spec.JoinConfiguration.Discovery).To(gomega.Equal(kubeadmv1beta1.Discovery{})) + g.Expect(kcpConfig.JoinConfiguration.Discovery).To(Equal(kubeadmv1beta1.Discovery{})) + g.Expect(machineConfig.Spec.JoinConfiguration.Discovery).To(Equal(kubeadmv1beta1.Discovery{})) }) t.Run("JoinConfiguration.ControlPlane gets removed from MachineConfig if it was not derived by KCPConfig", func(t *testing.T) { - g := gomega.NewWithT(t) + g := NewWithT(t) kcpConfig := &bootstrapv1.KubeadmConfigSpec{ JoinConfiguration: &kubeadmv1beta1.JoinConfiguration{ ControlPlane: nil, // Control plane configuration missing in KCP @@ -208,11 +209,11 @@ func TestCleanupConfigFields(t *testing.T) { }, } cleanupConfigFields(kcpConfig, machineConfig) - g.Expect(kcpConfig.JoinConfiguration).ToNot(gomega.BeNil()) - g.Expect(machineConfig.Spec.JoinConfiguration.ControlPlane).To(gomega.BeNil()) + g.Expect(kcpConfig.JoinConfiguration).ToNot(BeNil()) + g.Expect(machineConfig.Spec.JoinConfiguration.ControlPlane).To(BeNil()) }) t.Run("JoinConfiguration.NodeRegistrationOptions gets removed from MachineConfig if it was not derived by KCPConfig", func(t *testing.T) { - g := gomega.NewWithT(t) + g := NewWithT(t) kcpConfig := &bootstrapv1.KubeadmConfigSpec{ JoinConfiguration: &kubeadmv1beta1.JoinConfiguration{ NodeRegistration: kubeadmv1beta1.NodeRegistrationOptions{}, // NodeRegistrationOptions configuration missing in KCP @@ -226,11 +227,11 @@ func TestCleanupConfigFields(t *testing.T) { }, } cleanupConfigFields(kcpConfig, machineConfig) - g.Expect(kcpConfig.JoinConfiguration).ToNot(gomega.BeNil()) - g.Expect(machineConfig.Spec.JoinConfiguration.NodeRegistration).To(gomega.Equal(kubeadmv1beta1.NodeRegistrationOptions{})) + g.Expect(kcpConfig.JoinConfiguration).ToNot(BeNil()) + g.Expect(machineConfig.Spec.JoinConfiguration.NodeRegistration).To(Equal(kubeadmv1beta1.NodeRegistrationOptions{})) }) t.Run("InitConfiguration.TypeMeta gets removed from MachineConfig", func(t *testing.T) { - g := gomega.NewWithT(t) + g := NewWithT(t) kcpConfig := &bootstrapv1.KubeadmConfigSpec{ InitConfiguration: &kubeadmv1beta1.InitConfiguration{}, } @@ -245,11 +246,11 @@ func TestCleanupConfigFields(t *testing.T) { }, } cleanupConfigFields(kcpConfig, machineConfig) - g.Expect(kcpConfig.InitConfiguration).ToNot(gomega.BeNil()) - g.Expect(machineConfig.Spec.InitConfiguration.TypeMeta).To(gomega.Equal(metav1.TypeMeta{})) + g.Expect(kcpConfig.InitConfiguration).ToNot(BeNil()) + g.Expect(machineConfig.Spec.InitConfiguration.TypeMeta).To(Equal(metav1.TypeMeta{})) }) t.Run("JoinConfiguration.TypeMeta gets removed from MachineConfig", func(t *testing.T) { - g := gomega.NewWithT(t) + g := NewWithT(t) kcpConfig := &bootstrapv1.KubeadmConfigSpec{ JoinConfiguration: &kubeadmv1beta1.JoinConfiguration{}, } @@ -264,20 +265,20 @@ func TestCleanupConfigFields(t *testing.T) { }, } cleanupConfigFields(kcpConfig, machineConfig) - g.Expect(kcpConfig.JoinConfiguration).ToNot(gomega.BeNil()) - g.Expect(machineConfig.Spec.JoinConfiguration.TypeMeta).To(gomega.Equal(metav1.TypeMeta{})) + g.Expect(kcpConfig.JoinConfiguration).ToNot(BeNil()) + g.Expect(machineConfig.Spec.JoinConfiguration.TypeMeta).To(Equal(metav1.TypeMeta{})) }) } func TestMatchInitOrJoinConfiguration(t *testing.T) { t.Run("returns true if the machine does not have a bootstrap config", func(t *testing.T) { - g := gomega.NewWithT(t) + g := NewWithT(t) kcp := &controlplanev1.KubeadmControlPlane{} m := &clusterv1.Machine{} - g.Expect(matchInitOrJoinConfiguration(nil, kcp, m)).To(gomega.BeTrue()) + g.Expect(matchInitOrJoinConfiguration(nil, kcp, m)).To(BeTrue()) }) t.Run("returns true if the there are problems reading the bootstrap config", func(t *testing.T) { - g := gomega.NewWithT(t) + g := NewWithT(t) kcp := &controlplanev1.KubeadmControlPlane{} m := &clusterv1.Machine{ Spec: clusterv1.MachineSpec{ @@ -286,10 +287,10 @@ func TestMatchInitOrJoinConfiguration(t *testing.T) { }, }, } - g.Expect(matchInitOrJoinConfiguration(nil, kcp, m)).To(gomega.BeTrue()) + g.Expect(matchInitOrJoinConfiguration(nil, kcp, m)).To(BeTrue()) }) t.Run("returns true if InitConfiguration is equal", func(t *testing.T) { - g := gomega.NewWithT(t) + g := NewWithT(t) kcp := &controlplanev1.KubeadmControlPlane{ Spec: controlplanev1.KubeadmControlPlaneSpec{ KubeadmConfigSpec: bootstrapv1.KubeadmConfigSpec{ @@ -334,10 +335,10 @@ func TestMatchInitOrJoinConfiguration(t *testing.T) { }, }, } - g.Expect(matchInitOrJoinConfiguration(machineConfigs, kcp, m)).To(gomega.BeTrue()) + g.Expect(matchInitOrJoinConfiguration(machineConfigs, kcp, m)).To(BeTrue()) }) t.Run("returns false if InitConfiguration is NOT equal", func(t *testing.T) { - g := gomega.NewWithT(t) + g := NewWithT(t) kcp := &controlplanev1.KubeadmControlPlane{ Spec: controlplanev1.KubeadmControlPlaneSpec{ KubeadmConfigSpec: bootstrapv1.KubeadmConfigSpec{ @@ -386,10 +387,10 @@ func TestMatchInitOrJoinConfiguration(t *testing.T) { }, }, } - g.Expect(matchInitOrJoinConfiguration(machineConfigs, kcp, m)).To(gomega.BeFalse()) + g.Expect(matchInitOrJoinConfiguration(machineConfigs, kcp, m)).To(BeFalse()) }) t.Run("returns true if JoinConfiguration is equal", func(t *testing.T) { - g := gomega.NewWithT(t) + g := NewWithT(t) kcp := &controlplanev1.KubeadmControlPlane{ Spec: controlplanev1.KubeadmControlPlaneSpec{ KubeadmConfigSpec: bootstrapv1.KubeadmConfigSpec{ @@ -434,10 +435,10 @@ func TestMatchInitOrJoinConfiguration(t *testing.T) { }, }, } - g.Expect(matchInitOrJoinConfiguration(machineConfigs, kcp, m)).To(gomega.BeTrue()) + g.Expect(matchInitOrJoinConfiguration(machineConfigs, kcp, m)).To(BeTrue()) }) t.Run("returns false if JoinConfiguration is NOT equal", func(t *testing.T) { - g := gomega.NewWithT(t) + g := NewWithT(t) kcp := &controlplanev1.KubeadmControlPlane{ Spec: controlplanev1.KubeadmControlPlaneSpec{ KubeadmConfigSpec: bootstrapv1.KubeadmConfigSpec{ @@ -486,10 +487,10 @@ func TestMatchInitOrJoinConfiguration(t *testing.T) { }, }, } - g.Expect(matchInitOrJoinConfiguration(machineConfigs, kcp, m)).To(gomega.BeFalse()) + g.Expect(matchInitOrJoinConfiguration(machineConfigs, kcp, m)).To(BeFalse()) }) t.Run("returns false if some other configurations are not equal", func(t *testing.T) { - g := gomega.NewWithT(t) + g := NewWithT(t) kcp := &controlplanev1.KubeadmControlPlane{ Spec: controlplanev1.KubeadmControlPlaneSpec{ KubeadmConfigSpec: bootstrapv1.KubeadmConfigSpec{ @@ -535,13 +536,13 @@ func TestMatchInitOrJoinConfiguration(t *testing.T) { }, }, } - g.Expect(matchInitOrJoinConfiguration(machineConfigs, kcp, m)).To(gomega.BeFalse()) + g.Expect(matchInitOrJoinConfiguration(machineConfigs, kcp, m)).To(BeFalse()) }) } func TestMatchesKubeadmBootstrapConfig(t *testing.T) { t.Run("returns true if ClusterConfiguration is equal", func(t *testing.T) { - g := gomega.NewWithT(t) + g := NewWithT(t) kcp := &controlplanev1.KubeadmControlPlane{ Spec: controlplanev1.KubeadmControlPlaneSpec{ KubeadmConfigSpec: bootstrapv1.KubeadmConfigSpec{ @@ -562,10 +563,10 @@ func TestMatchesKubeadmBootstrapConfig(t *testing.T) { m.Name: {}, } f := MatchesKubeadmBootstrapConfig(machineConfigs, kcp) - g.Expect(f(m)).To(gomega.BeTrue()) + g.Expect(f(m)).To(BeTrue()) }) t.Run("returns false if ClusterConfiguration is NOT equal", func(t *testing.T) { - g := gomega.NewWithT(t) + g := NewWithT(t) kcp := &controlplanev1.KubeadmControlPlane{ Spec: controlplanev1.KubeadmControlPlaneSpec{ KubeadmConfigSpec: bootstrapv1.KubeadmConfigSpec{ @@ -586,10 +587,10 @@ func TestMatchesKubeadmBootstrapConfig(t *testing.T) { m.Name: {}, } f := MatchesKubeadmBootstrapConfig(machineConfigs, kcp) - g.Expect(f(m)).To(gomega.BeFalse()) + g.Expect(f(m)).To(BeFalse()) }) t.Run("returns true if InitConfiguration is equal", func(t *testing.T) { - g := gomega.NewWithT(t) + g := NewWithT(t) kcp := &controlplanev1.KubeadmControlPlane{ Spec: controlplanev1.KubeadmControlPlaneSpec{ KubeadmConfigSpec: bootstrapv1.KubeadmConfigSpec{ @@ -635,10 +636,10 @@ func TestMatchesKubeadmBootstrapConfig(t *testing.T) { }, } f := MatchesKubeadmBootstrapConfig(machineConfigs, kcp) - g.Expect(f(m)).To(gomega.BeTrue()) + g.Expect(f(m)).To(BeTrue()) }) t.Run("returns false if InitConfiguration is NOT equal", func(t *testing.T) { - g := gomega.NewWithT(t) + g := NewWithT(t) kcp := &controlplanev1.KubeadmControlPlane{ Spec: controlplanev1.KubeadmControlPlaneSpec{ KubeadmConfigSpec: bootstrapv1.KubeadmConfigSpec{ @@ -688,10 +689,10 @@ func TestMatchesKubeadmBootstrapConfig(t *testing.T) { }, } f := MatchesKubeadmBootstrapConfig(machineConfigs, kcp) - g.Expect(f(m)).To(gomega.BeFalse()) + g.Expect(f(m)).To(BeFalse()) }) t.Run("returns true if JoinConfiguration is equal", func(t *testing.T) { - g := gomega.NewWithT(t) + g := NewWithT(t) kcp := &controlplanev1.KubeadmControlPlane{ Spec: controlplanev1.KubeadmControlPlaneSpec{ KubeadmConfigSpec: bootstrapv1.KubeadmConfigSpec{ @@ -737,10 +738,10 @@ func TestMatchesKubeadmBootstrapConfig(t *testing.T) { }, } f := MatchesKubeadmBootstrapConfig(machineConfigs, kcp) - g.Expect(f(m)).To(gomega.BeTrue()) + g.Expect(f(m)).To(BeTrue()) }) t.Run("returns false if JoinConfiguration is NOT equal", func(t *testing.T) { - g := gomega.NewWithT(t) + g := NewWithT(t) kcp := &controlplanev1.KubeadmControlPlane{ Spec: controlplanev1.KubeadmControlPlaneSpec{ KubeadmConfigSpec: bootstrapv1.KubeadmConfigSpec{ @@ -790,10 +791,10 @@ func TestMatchesKubeadmBootstrapConfig(t *testing.T) { }, } f := MatchesKubeadmBootstrapConfig(machineConfigs, kcp) - g.Expect(f(m)).To(gomega.BeFalse()) + g.Expect(f(m)).To(BeFalse()) }) t.Run("returns false if some other configurations are not equal", func(t *testing.T) { - g := gomega.NewWithT(t) + g := NewWithT(t) kcp := &controlplanev1.KubeadmControlPlane{ Spec: controlplanev1.KubeadmControlPlaneSpec{ KubeadmConfigSpec: bootstrapv1.KubeadmConfigSpec{ @@ -840,6 +841,116 @@ func TestMatchesKubeadmBootstrapConfig(t *testing.T) { }, } f := MatchesKubeadmBootstrapConfig(machineConfigs, kcp) - g.Expect(f(m)).To(gomega.BeFalse()) + g.Expect(f(m)).To(BeFalse()) }) } + +func TestMatchesTemplateClonedFrom(t *testing.T) { + t.Run("nil machine returns false", func(t *testing.T) { + g := NewWithT(t) + g.Expect( + MatchesTemplateClonedFrom(nil, nil)(nil), + ).To(BeFalse()) + }) + + t.Run("returns true if machine not found", func(t *testing.T) { + g := NewWithT(t) + kcp := &controlplanev1.KubeadmControlPlane{} + machine := &clusterv1.Machine{ + Spec: clusterv1.MachineSpec{ + InfrastructureRef: corev1.ObjectReference{ + Kind: "KubeadmConfig", + Namespace: "default", + Name: "test", + APIVersion: bootstrapv1.GroupVersion.String(), + }, + }, + } + g.Expect( + MatchesTemplateClonedFrom(map[string]*unstructured.Unstructured{}, kcp)(machine), + ).To(BeTrue()) + }) +} + +func TestMatchesTemplateClonedFrom_WithClonedFromAnnotations(t *testing.T) { + kcp := &controlplanev1.KubeadmControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + }, + Spec: controlplanev1.KubeadmControlPlaneSpec{ + InfrastructureTemplate: corev1.ObjectReference{ + Kind: "GenericMachineTemplate", + Namespace: "default", + Name: "infra-foo", + APIVersion: "generic.io/v1", + }, + }, + } + machine := &clusterv1.Machine{ + Spec: clusterv1.MachineSpec{ + InfrastructureRef: corev1.ObjectReference{ + APIVersion: "infrastructure.cluster.x-k8s.io/v1alpha4", + Kind: "InfrastructureMachine", + Name: "infra-config1", + Namespace: "default", + }, + }, + } + tests := []struct { + name string + annotations map[string]interface{} + expectMatch bool + }{ + { + name: "returns true if annotations don't exist", + annotations: map[string]interface{}{}, + expectMatch: true, + }, + { + name: "returns false if annotations don't match anything", + annotations: map[string]interface{}{ + clusterv1.TemplateClonedFromNameAnnotation: "barfoo1", + clusterv1.TemplateClonedFromGroupKindAnnotation: "barfoo2", + }, + expectMatch: false, + }, + { + name: "returns false if TemplateClonedFromNameAnnotation matches but TemplateClonedFromGroupKindAnnotation doesn't", + annotations: map[string]interface{}{ + clusterv1.TemplateClonedFromNameAnnotation: "infra-foo", + clusterv1.TemplateClonedFromGroupKindAnnotation: "barfoo2", + }, + expectMatch: false, + }, + { + name: "returns true if both annotations match", + annotations: map[string]interface{}{ + clusterv1.TemplateClonedFromNameAnnotation: "infra-foo", + clusterv1.TemplateClonedFromGroupKindAnnotation: "GenericMachineTemplate.generic.io", + }, + expectMatch: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + infraConfigs := map[string]*unstructured.Unstructured{ + machine.Name: { + Object: map[string]interface{}{ + "kind": "InfrastructureMachine", + "apiVersion": "infrastructure.cluster.x-k8s.io/v1alpha4", + "metadata": map[string]interface{}{ + "name": "infra-config1", + "namespace": "default", + "annotations": tt.annotations, + }, + }, + }, + } + g.Expect( + MatchesTemplateClonedFrom(infraConfigs, kcp)(machine), + ).To(Equal(tt.expectMatch)) + }) + } +} diff --git a/controlplane/kubeadm/internal/workload_cluster_conditions.go b/controlplane/kubeadm/internal/workload_cluster_conditions.go index 38f1e4bd53a6..4e75d5f77310 100644 --- a/controlplane/kubeadm/internal/workload_cluster_conditions.go +++ b/controlplane/kubeadm/internal/workload_cluster_conditions.go @@ -19,6 +19,7 @@ package internal import ( "context" "fmt" + "sigs.k8s.io/cluster-api/util/collections" "strings" corev1 "k8s.io/api/core/v1" @@ -333,7 +334,7 @@ func (w *Workload) UpdateStaticPodConditions(ctx context.Context, controlPlane * }) } -func hasProvisioningMachine(machines FilterableMachineCollection) bool { +func hasProvisioningMachine(machines collections.Machines) bool { for _, machine := range machines { if machine.Status.NodeRef == nil { return true diff --git a/controlplane/kubeadm/internal/workload_cluster_conditions_test.go b/controlplane/kubeadm/internal/workload_cluster_conditions_test.go index 7384e33a3ba2..ed0ba4edc665 100644 --- a/controlplane/kubeadm/internal/workload_cluster_conditions_test.go +++ b/controlplane/kubeadm/internal/workload_cluster_conditions_test.go @@ -17,6 +17,7 @@ limitations under the License. package internal import ( + "sigs.k8s.io/cluster-api/util/collections" "testing" . "github.com/onsi/gomega" @@ -474,7 +475,7 @@ func TestUpdateEtcdConditions(t *testing.T) { } controlPane := &ControlPlane{ KCP: tt.kcp, - Machines: NewFilterableMachineCollection(tt.machines...), + Machines: collections.FromMachines(tt.machines...), } w.UpdateEtcdConditions(ctx, controlPane) @@ -746,7 +747,7 @@ func TestUpdateStaticPodConditions(t *testing.T) { } controlPane := &ControlPlane{ KCP: tt.kcp, - Machines: NewFilterableMachineCollection(tt.machines...), + Machines: collections.FromMachines(tt.machines...), } w.UpdateStaticPodConditions(ctx, controlPane) diff --git a/controlplane/kubeadm/internal/machine_collection.go b/util/collections/machine_collection.go similarity index 54% rename from controlplane/kubeadm/internal/machine_collection.go rename to util/collections/machine_collection.go index 3fb55b0de6fe..84c4b0cd4b28 100644 --- a/controlplane/kubeadm/internal/machine_collection.go +++ b/util/collections/machine_collection.go @@ -21,34 +21,38 @@ limitations under the License. // - Sortable data type is removed in favor of util.MachinesByCreationTimestamp // - nil checks added to account for the pointer // - Added Filter, AnyFilter, and Oldest methods -// - Added NewFilterableMachineCollectionFromMachineList initializer +// - Added FromMachineList initializer // - Updated Has to also check for equality of Machines // - Removed unused methods -package internal +package collections import ( "sort" clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha4" - "sigs.k8s.io/cluster-api/controlplane/kubeadm/internal/machinefilters" "sigs.k8s.io/cluster-api/util" "sigs.k8s.io/cluster-api/util/conditions" ) -// FilterableMachineCollection is a set of Machines -type FilterableMachineCollection map[string]*clusterv1.Machine +// Machines is a set of Machines +type Machines map[string]*clusterv1.Machine -// NewFilterableMachineCollection creates a FilterableMachineCollection from a list of values. -func NewFilterableMachineCollection(machines ...*clusterv1.Machine) FilterableMachineCollection { - ss := make(FilterableMachineCollection, len(machines)) +// New creates an empty Machines. +func New() Machines { + return make(Machines, 0) +} + +// FromMachines creates a Machines from a list of values. +func FromMachines(machines ...*clusterv1.Machine) Machines { + ss := make(Machines, len(machines)) ss.Insert(machines...) return ss } -// NewFilterableMachineCollectionFromMachineList creates a FilterableMachineCollection from the given MachineList -func NewFilterableMachineCollectionFromMachineList(machineList *clusterv1.MachineList) FilterableMachineCollection { - ss := make(FilterableMachineCollection, len(machineList.Items)) +// FromMachineList creates a Machines from the given MachineList +func FromMachineList(machineList *clusterv1.MachineList) Machines { + ss := make(Machines, len(machineList.Items)) for i := range machineList.Items { ss.Insert(&machineList.Items[i]) } @@ -56,7 +60,7 @@ func NewFilterableMachineCollectionFromMachineList(machineList *clusterv1.Machin } // Insert adds items to the set. -func (s FilterableMachineCollection) Insert(machines ...*clusterv1.Machine) { +func (s Machines) Insert(machines ...*clusterv1.Machine) { for i := range machines { if machines[i] != nil { m := machines[i] @@ -66,7 +70,7 @@ func (s FilterableMachineCollection) Insert(machines ...*clusterv1.Machine) { } // Difference returns a copy without machines that are in the given collection -func (s FilterableMachineCollection) Difference(machines FilterableMachineCollection) FilterableMachineCollection { +func (s Machines) Difference(machines Machines) Machines { return s.Filter(func(m *clusterv1.Machine) bool { _, found := machines[m.Name] return !found @@ -74,7 +78,7 @@ func (s FilterableMachineCollection) Difference(machines FilterableMachineCollec } // SortedByCreationTimestamp returns the machines sorted by creation timestamp -func (s FilterableMachineCollection) SortedByCreationTimestamp() []*clusterv1.Machine { +func (s Machines) SortedByCreationTimestamp() []*clusterv1.Machine { res := make(util.MachinesByCreationTimestamp, 0, len(s)) for _, value := range s { res = append(res, value) @@ -84,7 +88,7 @@ func (s FilterableMachineCollection) SortedByCreationTimestamp() []*clusterv1.Ma } // UnsortedList returns the slice with contents in random order. -func (s FilterableMachineCollection) UnsortedList() []*clusterv1.Machine { +func (s Machines) UnsortedList() []*clusterv1.Machine { res := make([]*clusterv1.Machine, 0, len(s)) for _, value := range s { res = append(res, value) @@ -93,13 +97,13 @@ func (s FilterableMachineCollection) UnsortedList() []*clusterv1.Machine { } // Len returns the size of the set. -func (s FilterableMachineCollection) Len() int { +func (s Machines) Len() int { return len(s) } -// newFilteredMachineCollection creates a FilterableMachineCollection from a filtered list of values. -func newFilteredMachineCollection(filter machinefilters.Func, machines ...*clusterv1.Machine) FilterableMachineCollection { - ss := make(FilterableMachineCollection, len(machines)) +// newFilteredMachineCollection creates a Machines from a filtered list of values. +func newFilteredMachineCollection(filter Func, machines ...*clusterv1.Machine) Machines { + ss := make(Machines, len(machines)) for i := range machines { m := machines[i] if filter(m) { @@ -109,18 +113,18 @@ func newFilteredMachineCollection(filter machinefilters.Func, machines ...*clust return ss } -// Filter returns a FilterableMachineCollection containing only the Machines that match all of the given MachineFilters -func (s FilterableMachineCollection) Filter(filters ...machinefilters.Func) FilterableMachineCollection { - return newFilteredMachineCollection(machinefilters.And(filters...), s.UnsortedList()...) +// Filter returns a Machines containing only the Machines that match all of the given MachineFilters +func (s Machines) Filter(filters ...Func) Machines { + return newFilteredMachineCollection(And(filters...), s.UnsortedList()...) } -// AnyFilter returns a FilterableMachineCollection containing only the Machines that match any of the given MachineFilters -func (s FilterableMachineCollection) AnyFilter(filters ...machinefilters.Func) FilterableMachineCollection { - return newFilteredMachineCollection(machinefilters.Or(filters...), s.UnsortedList()...) +// AnyFilter returns a Machines containing only the Machines that match any of the given MachineFilters +func (s Machines) AnyFilter(filters ...Func) Machines { + return newFilteredMachineCollection(Or(filters...), s.UnsortedList()...) } // Oldest returns the Machine with the oldest CreationTimestamp -func (s FilterableMachineCollection) Oldest() *clusterv1.Machine { +func (s Machines) Oldest() *clusterv1.Machine { if len(s) == 0 { return nil } @@ -128,7 +132,7 @@ func (s FilterableMachineCollection) Oldest() *clusterv1.Machine { } // Newest returns the Machine with the most recent CreationTimestamp -func (s FilterableMachineCollection) Newest() *clusterv1.Machine { +func (s Machines) Newest() *clusterv1.Machine { if len(s) == 0 { return nil } @@ -136,8 +140,8 @@ func (s FilterableMachineCollection) Newest() *clusterv1.Machine { } // DeepCopy returns a deep copy -func (s FilterableMachineCollection) DeepCopy() FilterableMachineCollection { - result := make(FilterableMachineCollection, len(s)) +func (s Machines) DeepCopy() Machines { + result := make(Machines, len(s)) for _, m := range s { result.Insert(m.DeepCopy()) } @@ -145,7 +149,7 @@ func (s FilterableMachineCollection) DeepCopy() FilterableMachineCollection { } // ConditionGetters returns the slice with machines converted into conditions.Getter. -func (s FilterableMachineCollection) ConditionGetters() []conditions.Getter { +func (s Machines) ConditionGetters() []conditions.Getter { res := make([]conditions.Getter, 0, len(s)) for _, v := range s { value := *v @@ -156,7 +160,7 @@ func (s FilterableMachineCollection) ConditionGetters() []conditions.Getter { // Names returns a slice of the names of each machine in the collection. // Useful for logging and test assertions. -func (s FilterableMachineCollection) Names() []string { +func (s Machines) Names() []string { names := make([]string, 0, s.Len()) for _, m := range s { names = append(names, m.Name) diff --git a/controlplane/kubeadm/internal/machine_collection_test.go b/util/collections/machine_collection_test.go similarity index 90% rename from controlplane/kubeadm/internal/machine_collection_test.go rename to util/collections/machine_collection_test.go index 375938e0dc60..e996d1c272e2 100644 --- a/controlplane/kubeadm/internal/machine_collection_test.go +++ b/util/collections/machine_collection_test.go @@ -14,9 +14,10 @@ See the License for the specific language governing permissions and limitations under the License. */ -package internal +package collections_test import ( + "sigs.k8s.io/cluster-api/util/collections" "testing" "time" @@ -32,10 +33,10 @@ func TestMachineCollection(t *testing.T) { } var _ = Describe("Machine Collection", func() { - Describe("FilterableMachineCollection", func() { - var collection FilterableMachineCollection + Describe("Machines", func() { + var collection collections.Machines BeforeEach(func() { - collection = FilterableMachineCollection{ + collection = collections.Machines{ "machine-4": machine("machine-4", withCreationTimestamp(metav1.Time{Time: time.Date(2018, 04, 02, 03, 04, 05, 06, time.UTC)})), "machine-5": machine("machine-5", withCreationTimestamp(metav1.Time{Time: time.Date(2018, 05, 02, 03, 04, 05, 06, time.UTC)})), "machine-2": machine("machine-2", withCreationTimestamp(metav1.Time{Time: time.Date(2018, 02, 02, 03, 04, 05, 06, time.UTC)})), @@ -66,8 +67,8 @@ var _ = Describe("Machine Collection", func() { Describe("Names", func() { It("returns a slice of names of each machine in the collection", func() { - Expect(NewFilterableMachineCollection().Names()).To(BeEmpty()) - Expect(NewFilterableMachineCollection(machine("1"), machine("2")).Names()).To(ConsistOf("1", "2")) + Expect(collections.New().Names()).To(BeEmpty()) + Expect(collections.FromMachines(machine("1"), machine("2")).Names()).To(ConsistOf("1", "2")) }) }) }) diff --git a/util/collections/machine_filters.go b/util/collections/machine_filters.go new file mode 100644 index 000000000000..6c42152a53f1 --- /dev/null +++ b/util/collections/machine_filters.go @@ -0,0 +1,206 @@ +/* +Copyright 2020 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 collections + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/selection" + clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha4" + "sigs.k8s.io/cluster-api/util" + "sigs.k8s.io/cluster-api/util/conditions" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +type Func func(machine *clusterv1.Machine) bool + +// And returns a filter that returns true if all of the given filters returns true. +func And(filters ...Func) Func { + return func(machine *clusterv1.Machine) bool { + for _, f := range filters { + if !f(machine) { + return false + } + } + return true + } +} + +// Or returns a filter that returns true if any of the given filters returns true. +func Or(filters ...Func) Func { + return func(machine *clusterv1.Machine) bool { + for _, f := range filters { + if f(machine) { + return true + } + } + return false + } +} + +// Not returns a filter that returns the opposite of the given filter. +func Not(mf Func) Func { + return func(machine *clusterv1.Machine) bool { + return !mf(machine) + } +} + +// HasControllerRef is a filter that returns true if the machine has a controller ref +func HasControllerRef(machine *clusterv1.Machine) bool { + if machine == nil { + return false + } + return metav1.GetControllerOf(machine) != nil +} + +// InFailureDomains returns a filter to find all machines +// in any of the given failure domains +func InFailureDomains(failureDomains ...*string) Func { + return func(machine *clusterv1.Machine) bool { + if machine == nil { + return false + } + for i := range failureDomains { + fd := failureDomains[i] + if fd == nil { + if fd == machine.Spec.FailureDomain { + return true + } + continue + } + if machine.Spec.FailureDomain == nil { + continue + } + if *fd == *machine.Spec.FailureDomain { + return true + } + } + return false + } +} + +// OwnedMachines returns a filter to find all owned control plane machines. +// Usage: managementCluster.GetMachinesForCluster(ctx, cluster, machinefilters.OwnedMachines(controlPlane)) +func OwnedMachines(owner client.Object) func(machine *clusterv1.Machine) bool { + return func(machine *clusterv1.Machine) bool { + if machine == nil { + return false + } + return util.IsOwnedByObject(machine, owner) + } +} + +// ControlPlaneMachines returns a filter to find all control plane machines for a cluster, regardless of ownership. +// Usage: managementCluster.GetMachinesForCluster(ctx, cluster, machinefilters.ControlPlaneMachines(cluster.Name)) +func ControlPlaneMachines(clusterName string) func(machine *clusterv1.Machine) bool { + selector := ControlPlaneSelectorForCluster(clusterName) + return func(machine *clusterv1.Machine) bool { + if machine == nil { + return false + } + return selector.Matches(labels.Set(machine.Labels)) + } +} + +// AdoptableControlPlaneMachines returns a filter to find all un-controlled control plane machines. +// Usage: managementCluster.GetMachinesForCluster(ctx, cluster, AdoptableControlPlaneMachines(cluster.Name, controlPlane)) +func AdoptableControlPlaneMachines(clusterName string) func(machine *clusterv1.Machine) bool { + return And( + ControlPlaneMachines(clusterName), + Not(HasControllerRef), + ) +} + +// HasDeletionTimestamp returns a filter to find all machines that have a deletion timestamp. +func HasDeletionTimestamp(machine *clusterv1.Machine) bool { + if machine == nil { + return false + } + return !machine.DeletionTimestamp.IsZero() +} + +// HasUnhealthyCondition returns a filter to find all machines that have a MachineHealthCheckSucceeded condition set to False, +// indicating a problem was detected on the machine, and the MachineOwnerRemediated condition set, indicating that KCP is +// responsible of performing remediation as owner of the machine. +func HasUnhealthyCondition(machine *clusterv1.Machine) bool { + if machine == nil { + return false + } + return conditions.IsFalse(machine, clusterv1.MachineHealthCheckSuccededCondition) && conditions.IsFalse(machine, clusterv1.MachineOwnerRemediatedCondition) +} + +// IsReady returns a filter to find all machines with the ReadyCondition equals to True. +func IsReady() Func { + return func(machine *clusterv1.Machine) bool { + if machine == nil { + return false + } + return conditions.IsTrue(machine, clusterv1.ReadyCondition) + } +} + +// ShouldRolloutAfter returns a filter to find all machines where +// CreationTimestamp < rolloutAfter < reconciliationTIme +func ShouldRolloutAfter(reconciliationTime, rolloutAfter *metav1.Time) Func { + return func(machine *clusterv1.Machine) bool { + if machine == nil { + return false + } + return machine.CreationTimestamp.Before(rolloutAfter) && rolloutAfter.Before(reconciliationTime) + } +} + +// HasAnnotationKey returns a filter to find all machines that have the +// specified Annotation key present +func HasAnnotationKey(key string) Func { + return func(machine *clusterv1.Machine) bool { + if machine == nil || machine.Annotations == nil { + return false + } + if _, ok := machine.Annotations[key]; ok { + return true + } + return false + } +} + +// ControlPlaneSelectorForCluster returns the label selector necessary to get control plane machines for a given cluster. +func ControlPlaneSelectorForCluster(clusterName string) labels.Selector { + must := func(r *labels.Requirement, err error) labels.Requirement { + if err != nil { + panic(err) + } + return *r + } + return labels.NewSelector().Add( + must(labels.NewRequirement(clusterv1.ClusterLabelName, selection.Equals, []string{clusterName})), + must(labels.NewRequirement(clusterv1.MachineControlPlaneLabelName, selection.Exists, []string{})), + ) +} + +// MatchesKubernetesVersion returns a filter to find all machines that match a given Kubernetes version. +func MatchesKubernetesVersion(kubernetesVersion string) Func { + return func(machine *clusterv1.Machine) bool { + if machine == nil { + return false + } + if machine.Spec.Version == nil { + return false + } + return *machine.Spec.Version == kubernetesVersion + } +} diff --git a/controlplane/kubeadm/internal/machinefilters/machine_filters_test.go b/util/collections/machine_filters_test.go similarity index 56% rename from controlplane/kubeadm/internal/machinefilters/machine_filters_test.go rename to util/collections/machine_filters_test.go index c3019629b4d1..623752d99462 100644 --- a/controlplane/kubeadm/internal/machinefilters/machine_filters_test.go +++ b/util/collections/machine_filters_test.go @@ -14,23 +14,19 @@ See the License for the specific language governing permissions and limitations under the License. */ -package machinefilters_test +package collections_test import ( + "sigs.k8s.io/cluster-api/util/collections" "testing" "time" . "github.com/onsi/gomega" "sigs.k8s.io/cluster-api/util/conditions" - corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/utils/pointer" clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha4" - bootstrapv1 "sigs.k8s.io/cluster-api/bootstrap/kubeadm/api/v1alpha4" - controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1alpha4" - "sigs.k8s.io/cluster-api/controlplane/kubeadm/internal/machinefilters" ) func falseFilter(_ *clusterv1.Machine) bool { @@ -45,12 +41,12 @@ func TestNot(t *testing.T) { t.Run("returns false given a machine filter that returns true", func(t *testing.T) { g := NewWithT(t) m := &clusterv1.Machine{} - g.Expect(machinefilters.Not(trueFilter)(m)).To(BeFalse()) + g.Expect(collections.Not(trueFilter)(m)).To(BeFalse()) }) t.Run("returns true given a machine filter that returns false", func(t *testing.T) { g := NewWithT(t) m := &clusterv1.Machine{} - g.Expect(machinefilters.Not(falseFilter)(m)).To(BeTrue()) + g.Expect(collections.Not(falseFilter)(m)).To(BeTrue()) }) } @@ -58,12 +54,12 @@ func TestAnd(t *testing.T) { t.Run("returns true if both given machine filters return true", func(t *testing.T) { g := NewWithT(t) m := &clusterv1.Machine{} - g.Expect(machinefilters.And(trueFilter, trueFilter)(m)).To(BeTrue()) + g.Expect(collections.And(trueFilter, trueFilter)(m)).To(BeTrue()) }) t.Run("returns false if either given machine filter returns false", func(t *testing.T) { g := NewWithT(t) m := &clusterv1.Machine{} - g.Expect(machinefilters.And(trueFilter, falseFilter)(m)).To(BeFalse()) + g.Expect(collections.And(trueFilter, falseFilter)(m)).To(BeFalse()) }) } @@ -71,12 +67,12 @@ func TestOr(t *testing.T) { t.Run("returns true if either given machine filters return true", func(t *testing.T) { g := NewWithT(t) m := &clusterv1.Machine{} - g.Expect(machinefilters.Or(trueFilter, falseFilter)(m)).To(BeTrue()) + g.Expect(collections.Or(trueFilter, falseFilter)(m)).To(BeTrue()) }) t.Run("returns false if both given machine filter returns false", func(t *testing.T) { g := NewWithT(t) m := &clusterv1.Machine{} - g.Expect(machinefilters.Or(falseFilter, falseFilter)(m)).To(BeFalse()) + g.Expect(collections.Or(falseFilter, falseFilter)(m)).To(BeFalse()) }) } @@ -84,26 +80,26 @@ func TestHasUnhealthyCondition(t *testing.T) { t.Run("healthy machine (without HealthCheckSucceeded condition) should return false", func(t *testing.T) { g := NewWithT(t) m := &clusterv1.Machine{} - g.Expect(machinefilters.HasUnhealthyCondition(m)).To(BeFalse()) + g.Expect(collections.HasUnhealthyCondition(m)).To(BeFalse()) }) t.Run("healthy machine (with HealthCheckSucceeded condition == True) should return false", func(t *testing.T) { g := NewWithT(t) m := &clusterv1.Machine{} conditions.MarkTrue(m, clusterv1.MachineHealthCheckSuccededCondition) - g.Expect(machinefilters.HasUnhealthyCondition(m)).To(BeFalse()) + g.Expect(collections.HasUnhealthyCondition(m)).To(BeFalse()) }) t.Run("unhealthy machine NOT eligible for KCP remediation (with withHealthCheckSucceeded condition == False but without OwnerRemediated) should return false", func(t *testing.T) { g := NewWithT(t) m := &clusterv1.Machine{} conditions.MarkFalse(m, clusterv1.MachineHealthCheckSuccededCondition, clusterv1.MachineHasFailureReason, clusterv1.ConditionSeverityWarning, "") - g.Expect(machinefilters.HasUnhealthyCondition(m)).To(BeFalse()) + g.Expect(collections.HasUnhealthyCondition(m)).To(BeFalse()) }) t.Run("unhealthy machine eligible for KCP (with HealthCheckSucceeded condition == False and with OwnerRemediated) should return true", func(t *testing.T) { g := NewWithT(t) m := &clusterv1.Machine{} conditions.MarkFalse(m, clusterv1.MachineHealthCheckSuccededCondition, clusterv1.MachineHasFailureReason, clusterv1.ConditionSeverityWarning, "") conditions.MarkFalse(m, clusterv1.MachineOwnerRemediatedCondition, clusterv1.WaitingForRemediationReason, clusterv1.ConditionSeverityWarning, "") - g.Expect(machinefilters.HasUnhealthyCondition(m)).To(BeTrue()) + g.Expect(collections.HasUnhealthyCondition(m)).To(BeTrue()) }) } @@ -113,19 +109,19 @@ func TestHasDeletionTimestamp(t *testing.T) { m := &clusterv1.Machine{} now := metav1.Now() m.SetDeletionTimestamp(&now) - g.Expect(machinefilters.HasDeletionTimestamp(m)).To(BeTrue()) + g.Expect(collections.HasDeletionTimestamp(m)).To(BeTrue()) }) t.Run("machine with nil deletion timestamp returns false", func(t *testing.T) { g := NewWithT(t) m := &clusterv1.Machine{} - g.Expect(machinefilters.HasDeletionTimestamp(m)).To(BeFalse()) + g.Expect(collections.HasDeletionTimestamp(m)).To(BeFalse()) }) t.Run("machine with zero deletion timestamp returns false", func(t *testing.T) { g := NewWithT(t) m := &clusterv1.Machine{} zero := metav1.NewTime(time.Time{}) m.SetDeletionTimestamp(&zero) - g.Expect(machinefilters.HasDeletionTimestamp(m)).To(BeFalse()) + g.Expect(collections.HasDeletionTimestamp(m)).To(BeFalse()) }) } @@ -133,37 +129,37 @@ func TestShouldRolloutAfter(t *testing.T) { reconciliationTime := metav1.Date(2009, time.November, 10, 23, 0, 0, 0, time.UTC) t.Run("if the machine is nil it returns false", func(t *testing.T) { g := NewWithT(t) - g.Expect(machinefilters.ShouldRolloutAfter(&reconciliationTime, &reconciliationTime)(nil)).To(BeFalse()) + g.Expect(collections.ShouldRolloutAfter(&reconciliationTime, &reconciliationTime)(nil)).To(BeFalse()) }) t.Run("if the reconciliationTime is nil it returns false", func(t *testing.T) { g := NewWithT(t) m := &clusterv1.Machine{} - g.Expect(machinefilters.ShouldRolloutAfter(nil, &reconciliationTime)(m)).To(BeFalse()) + g.Expect(collections.ShouldRolloutAfter(nil, &reconciliationTime)(m)).To(BeFalse()) }) t.Run("if the rolloutAfter is nil it returns false", func(t *testing.T) { g := NewWithT(t) m := &clusterv1.Machine{} - g.Expect(machinefilters.ShouldRolloutAfter(&reconciliationTime, nil)(m)).To(BeFalse()) + g.Expect(collections.ShouldRolloutAfter(&reconciliationTime, nil)(m)).To(BeFalse()) }) t.Run("if rolloutAfter is after the reconciliation time, return false", func(t *testing.T) { g := NewWithT(t) m := &clusterv1.Machine{} rolloutAfter := metav1.NewTime(reconciliationTime.Add(+1 * time.Hour)) - g.Expect(machinefilters.ShouldRolloutAfter(&reconciliationTime, &rolloutAfter)(m)).To(BeFalse()) + g.Expect(collections.ShouldRolloutAfter(&reconciliationTime, &rolloutAfter)(m)).To(BeFalse()) }) t.Run("if rolloutAfter is before the reconciliation time and the machine was created before rolloutAfter, return true", func(t *testing.T) { g := NewWithT(t) m := &clusterv1.Machine{} m.SetCreationTimestamp(metav1.NewTime(reconciliationTime.Add(-2 * time.Hour))) rolloutAfter := metav1.NewTime(reconciliationTime.Add(-1 * time.Hour)) - g.Expect(machinefilters.ShouldRolloutAfter(&reconciliationTime, &rolloutAfter)(m)).To(BeTrue()) + g.Expect(collections.ShouldRolloutAfter(&reconciliationTime, &rolloutAfter)(m)).To(BeTrue()) }) t.Run("if rolloutAfter is before the reconciliation time and the machine was created after rolloutAfter, return false", func(t *testing.T) { g := NewWithT(t) m := &clusterv1.Machine{} m.SetCreationTimestamp(metav1.NewTime(reconciliationTime.Add(+1 * time.Hour))) rolloutAfter := metav1.NewTime(reconciliationTime.Add(-1 * time.Hour)) - g.Expect(machinefilters.ShouldRolloutAfter(&reconciliationTime, &rolloutAfter)(m)).To(BeFalse()) + g.Expect(collections.ShouldRolloutAfter(&reconciliationTime, &rolloutAfter)(m)).To(BeFalse()) }) } @@ -172,35 +168,35 @@ func TestHashAnnotationKey(t *testing.T) { g := NewWithT(t) m := &clusterv1.Machine{} m.SetAnnotations(map[string]string{"test": ""}) - g.Expect(machinefilters.HasAnnotationKey("test")(m)).To(BeTrue()) + g.Expect(collections.HasAnnotationKey("test")(m)).To(BeTrue()) }) t.Run("machine with specified annotation with non-empty value returns true", func(t *testing.T) { g := NewWithT(t) m := &clusterv1.Machine{} m.SetAnnotations(map[string]string{"test": "blue"}) - g.Expect(machinefilters.HasAnnotationKey("test")(m)).To(BeTrue()) + g.Expect(collections.HasAnnotationKey("test")(m)).To(BeTrue()) }) t.Run("machine without specified annotation returns false", func(t *testing.T) { g := NewWithT(t) m := &clusterv1.Machine{} - g.Expect(machinefilters.HasAnnotationKey("foo")(m)).To(BeFalse()) + g.Expect(collections.HasAnnotationKey("foo")(m)).To(BeFalse()) }) } func TestInFailureDomain(t *testing.T) { t.Run("nil machine returns false", func(t *testing.T) { g := NewWithT(t) - g.Expect(machinefilters.InFailureDomains(pointer.StringPtr("test"))(nil)).To(BeFalse()) + g.Expect(collections.InFailureDomains(pointer.StringPtr("test"))(nil)).To(BeFalse()) }) t.Run("machine with given failure domain returns true", func(t *testing.T) { g := NewWithT(t) m := &clusterv1.Machine{Spec: clusterv1.MachineSpec{FailureDomain: pointer.StringPtr("test")}} - g.Expect(machinefilters.InFailureDomains(pointer.StringPtr("test"))(m)).To(BeTrue()) + g.Expect(collections.InFailureDomains(pointer.StringPtr("test"))(m)).To(BeTrue()) }) t.Run("machine with a different failure domain returns false", func(t *testing.T) { g := NewWithT(t) m := &clusterv1.Machine{Spec: clusterv1.MachineSpec{FailureDomain: pointer.StringPtr("notTest")}} - g.Expect(machinefilters.InFailureDomains( + g.Expect(collections.InFailureDomains( pointer.StringPtr("test"), pointer.StringPtr("test2"), pointer.StringPtr("test3"), @@ -210,24 +206,24 @@ func TestInFailureDomain(t *testing.T) { t.Run("machine without failure domain returns false", func(t *testing.T) { g := NewWithT(t) m := &clusterv1.Machine{} - g.Expect(machinefilters.InFailureDomains(pointer.StringPtr("test"))(m)).To(BeFalse()) + g.Expect(collections.InFailureDomains(pointer.StringPtr("test"))(m)).To(BeFalse()) }) t.Run("machine without failure domain returns true, when nil used for failure domain", func(t *testing.T) { g := NewWithT(t) m := &clusterv1.Machine{} - g.Expect(machinefilters.InFailureDomains(nil)(m)).To(BeTrue()) + g.Expect(collections.InFailureDomains(nil)(m)).To(BeTrue()) }) t.Run("machine with failure domain returns true, when one of multiple failure domains match", func(t *testing.T) { g := NewWithT(t) m := &clusterv1.Machine{Spec: clusterv1.MachineSpec{FailureDomain: pointer.StringPtr("test")}} - g.Expect(machinefilters.InFailureDomains(pointer.StringPtr("foo"), pointer.StringPtr("test"))(m)).To(BeTrue()) + g.Expect(collections.InFailureDomains(pointer.StringPtr("foo"), pointer.StringPtr("test"))(m)).To(BeTrue()) }) } func TestMatchesKubernetesVersion(t *testing.T) { t.Run("nil machine returns false", func(t *testing.T) { g := NewWithT(t) - g.Expect(machinefilters.MatchesKubernetesVersion("some_ver")(nil)).To(BeFalse()) + g.Expect(collections.MatchesKubernetesVersion("some_ver")(nil)).To(BeFalse()) }) t.Run("nil machine.Spec.Version returns false", func(t *testing.T) { @@ -237,7 +233,7 @@ func TestMatchesKubernetesVersion(t *testing.T) { Version: nil, }, } - g.Expect(machinefilters.MatchesKubernetesVersion("some_ver")(machine)).To(BeFalse()) + g.Expect(collections.MatchesKubernetesVersion("some_ver")(machine)).To(BeFalse()) }) t.Run("machine.Spec.Version returns true if matches", func(t *testing.T) { @@ -248,7 +244,7 @@ func TestMatchesKubernetesVersion(t *testing.T) { Version: &kversion, }, } - g.Expect(machinefilters.MatchesKubernetesVersion("some_ver")(machine)).To(BeTrue()) + g.Expect(collections.MatchesKubernetesVersion("some_ver")(machine)).To(BeTrue()) }) t.Run("machine.Spec.Version returns false if does not match", func(t *testing.T) { @@ -259,116 +255,6 @@ func TestMatchesKubernetesVersion(t *testing.T) { Version: &kversion, }, } - g.Expect(machinefilters.MatchesKubernetesVersion("some_ver")(machine)).To(BeFalse()) + g.Expect(collections.MatchesKubernetesVersion("some_ver")(machine)).To(BeFalse()) }) } - -func TestMatchesTemplateClonedFrom(t *testing.T) { - t.Run("nil machine returns false", func(t *testing.T) { - g := NewWithT(t) - g.Expect( - machinefilters.MatchesTemplateClonedFrom(nil, nil)(nil), - ).To(BeFalse()) - }) - - t.Run("returns true if machine not found", func(t *testing.T) { - g := NewWithT(t) - kcp := &controlplanev1.KubeadmControlPlane{} - machine := &clusterv1.Machine{ - Spec: clusterv1.MachineSpec{ - InfrastructureRef: corev1.ObjectReference{ - Kind: "KubeadmConfig", - Namespace: "default", - Name: "test", - APIVersion: bootstrapv1.GroupVersion.String(), - }, - }, - } - g.Expect( - machinefilters.MatchesTemplateClonedFrom(map[string]*unstructured.Unstructured{}, kcp)(machine), - ).To(BeTrue()) - }) -} - -func TestMatchesTemplateClonedFrom_WithClonedFromAnnotations(t *testing.T) { - kcp := &controlplanev1.KubeadmControlPlane{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: "default", - }, - Spec: controlplanev1.KubeadmControlPlaneSpec{ - InfrastructureTemplate: corev1.ObjectReference{ - Kind: "GenericMachineTemplate", - Namespace: "default", - Name: "infra-foo", - APIVersion: "generic.io/v1", - }, - }, - } - machine := &clusterv1.Machine{ - Spec: clusterv1.MachineSpec{ - InfrastructureRef: corev1.ObjectReference{ - APIVersion: "infrastructure.cluster.x-k8s.io/v1alpha4", - Kind: "InfrastructureMachine", - Name: "infra-config1", - Namespace: "default", - }, - }, - } - tests := []struct { - name string - annotations map[string]interface{} - expectMatch bool - }{ - { - name: "returns true if annotations don't exist", - annotations: map[string]interface{}{}, - expectMatch: true, - }, - { - name: "returns false if annotations don't match anything", - annotations: map[string]interface{}{ - clusterv1.TemplateClonedFromNameAnnotation: "barfoo1", - clusterv1.TemplateClonedFromGroupKindAnnotation: "barfoo2", - }, - expectMatch: false, - }, - { - name: "returns false if TemplateClonedFromNameAnnotation matches but TemplateClonedFromGroupKindAnnotation doesn't", - annotations: map[string]interface{}{ - clusterv1.TemplateClonedFromNameAnnotation: "infra-foo", - clusterv1.TemplateClonedFromGroupKindAnnotation: "barfoo2", - }, - expectMatch: false, - }, - { - name: "returns true if both annotations match", - annotations: map[string]interface{}{ - clusterv1.TemplateClonedFromNameAnnotation: "infra-foo", - clusterv1.TemplateClonedFromGroupKindAnnotation: "GenericMachineTemplate.generic.io", - }, - expectMatch: true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - g := NewWithT(t) - infraConfigs := map[string]*unstructured.Unstructured{ - machine.Name: { - Object: map[string]interface{}{ - "kind": "InfrastructureMachine", - "apiVersion": "infrastructure.cluster.x-k8s.io/v1alpha4", - "metadata": map[string]interface{}{ - "name": "infra-config1", - "namespace": "default", - "annotations": tt.annotations, - }, - }, - }, - } - g.Expect( - machinefilters.MatchesTemplateClonedFrom(infraConfigs, kcp)(machine), - ).To(Equal(tt.expectMatch)) - }) - } -} diff --git a/controlplane/kubeadm/internal/failure_domain.go b/util/failuredomains/failure_domains.go similarity index 79% rename from controlplane/kubeadm/internal/failure_domain.go rename to util/failuredomains/failure_domains.go index 0a745981de25..46b5ad412927 100644 --- a/controlplane/kubeadm/internal/failure_domain.go +++ b/util/failuredomains/failure_domains.go @@ -14,20 +14,18 @@ See the License for the specific language governing permissions and limitations under the License. */ -package internal +package failuredomains import ( + "k8s.io/klog/klogr" "sort" - "k8s.io/klog/klogr" "k8s.io/utils/pointer" clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha4" + "sigs.k8s.io/cluster-api/util/collections" ) -// Log is the global logger for the internal package. -var Log = klogr.New() - type failureDomainAggregation struct { id string count int @@ -50,10 +48,10 @@ func (f failureDomainAggregations) Swap(i, j int) { f[i], f[j] = f[j], f[i] } -// PickMost returns a failure domain that is in machines and has most control-plane machines on. -func PickMost(c *ControlPlane, machines FilterableMachineCollection) *string { - // orderDescending sorts failure domains according to all control plane machines - fds := orderDescending(c.Cluster.Status.FailureDomains.FilterControlPlane(), c.Machines) +// PickMost returns a failure domain that is in machines and has most of the group of machines on. +func PickMost(failureDomains clusterv1.FailureDomains, groupMachines, machines collections.Machines) *string { + // orderDescending sorts failure domains according to all machines belonging to the group. + fds := orderDescending(failureDomains, groupMachines) for _, fd := range fds { for _, m := range machines { if m.Spec.FailureDomain == nil { @@ -68,7 +66,7 @@ func PickMost(c *ControlPlane, machines FilterableMachineCollection) *string { } // orderDescending returns the sorted failure domains in decreasing order. -func orderDescending(failureDomains clusterv1.FailureDomains, machines FilterableMachineCollection) failureDomainAggregations { +func orderDescending(failureDomains clusterv1.FailureDomains, machines collections.Machines) failureDomainAggregations { aggregations := pick(failureDomains, machines) if len(aggregations) == 0 { return nil @@ -78,7 +76,7 @@ func orderDescending(failureDomains clusterv1.FailureDomains, machines Filterabl } // PickFewest returns the failure domain with the fewest number of machines. -func PickFewest(failureDomains clusterv1.FailureDomains, machines FilterableMachineCollection) *string { +func PickFewest(failureDomains clusterv1.FailureDomains, machines collections.Machines) *string { aggregations := pick(failureDomains, machines) if len(aggregations) == 0 { return nil @@ -87,7 +85,7 @@ func PickFewest(failureDomains clusterv1.FailureDomains, machines FilterableMach return pointer.StringPtr(aggregations[0].id) } -func pick(failureDomains clusterv1.FailureDomains, machines FilterableMachineCollection) failureDomainAggregations { +func pick(failureDomains clusterv1.FailureDomains, machines collections.Machines) failureDomainAggregations { if len(failureDomains) == 0 { return failureDomainAggregations{} } @@ -106,7 +104,7 @@ func pick(failureDomains clusterv1.FailureDomains, machines FilterableMachineCol } id := *m.Spec.FailureDomain if _, ok := failureDomains[id]; !ok { - Log.Info("unknown failure domain", "machine-name", m.GetName(), "failure-domain-id", id, "known-failure-domains", failureDomains) + klogr.New().Info("unknown failure domain", "machine-name", m.GetName(), "failure-domain-id", id, "known-failure-domains", failureDomains) continue } counters[id]++ diff --git a/controlplane/kubeadm/internal/failure_domain_test.go b/util/failuredomains/failure_domains_test.go similarity index 85% rename from controlplane/kubeadm/internal/failure_domain_test.go rename to util/failuredomains/failure_domains_test.go index 3fe4aba896ce..e0bce8cac728 100644 --- a/controlplane/kubeadm/internal/failure_domain_test.go +++ b/util/failuredomains/failure_domains_test.go @@ -14,9 +14,10 @@ See the License for the specific language governing permissions and limitations under the License. */ -package internal +package failuredomains import ( + "sigs.k8s.io/cluster-api/util/collections" "testing" . "github.com/onsi/gomega" @@ -41,7 +42,7 @@ func TestNewFailureDomainPicker(t *testing.T) { testcases := []struct { name string fds clusterv1.FailureDomains - machines FilterableMachineCollection + machines collections.Machines expected []*string }{ { @@ -58,7 +59,7 @@ func TestNewFailureDomainPicker(t *testing.T) { { name: "one machine in a failure domain", fds: fds, - machines: NewFilterableMachineCollection(machinea.DeepCopy()), + machines: collections.FromMachines(machinea.DeepCopy()), expected: []*string{b}, }, { @@ -66,7 +67,7 @@ func TestNewFailureDomainPicker(t *testing.T) { fds: clusterv1.FailureDomains{ *a: clusterv1.FailureDomainSpec{}, }, - machines: NewFilterableMachineCollection(machinenil.DeepCopy()), + machines: collections.FromMachines(machinenil.DeepCopy()), expected: []*string{a}, }, { @@ -74,7 +75,7 @@ func TestNewFailureDomainPicker(t *testing.T) { fds: clusterv1.FailureDomains{ *a: clusterv1.FailureDomainSpec{}, }, - machines: NewFilterableMachineCollection(machineb.DeepCopy()), + machines: collections.FromMachines(machineb.DeepCopy()), expected: []*string{a}, }, { @@ -112,7 +113,7 @@ func TestNewFailureDomainPickMost(t *testing.T) { testcases := []struct { name string fds clusterv1.FailureDomains - machines FilterableMachineCollection + machines collections.Machines expected []*string }{ { @@ -129,7 +130,7 @@ func TestNewFailureDomainPickMost(t *testing.T) { { name: "one machine in a failure domain", fds: fds, - machines: NewFilterableMachineCollection(machinea.DeepCopy()), + machines: collections.FromMachines(machinea.DeepCopy()), expected: []*string{a}, }, { @@ -137,7 +138,7 @@ func TestNewFailureDomainPickMost(t *testing.T) { fds: clusterv1.FailureDomains{ *a: clusterv1.FailureDomainSpec{ControlPlane: true}, }, - machines: NewFilterableMachineCollection(machinenil.DeepCopy()), + machines: collections.FromMachines(machinenil.DeepCopy()), expected: nil, }, { @@ -145,7 +146,7 @@ func TestNewFailureDomainPickMost(t *testing.T) { fds: clusterv1.FailureDomains{ *a: clusterv1.FailureDomainSpec{ControlPlane: true}, }, - machines: NewFilterableMachineCollection(machineb.DeepCopy()), + machines: collections.FromMachines(machineb.DeepCopy()), expected: nil, }, { @@ -155,7 +156,7 @@ func TestNewFailureDomainPickMost(t *testing.T) { }, { name: "nil failure domains with machines", - machines: NewFilterableMachineCollection(machineb.DeepCopy()), + machines: collections.FromMachines(machineb.DeepCopy()), expected: nil, }, { @@ -167,8 +168,7 @@ func TestNewFailureDomainPickMost(t *testing.T) { t.Run(tc.name, func(t *testing.T) { g := NewWithT(t) - fd := PickMost(&ControlPlane{Machines: tc.machines, - Cluster: &clusterv1.Cluster{Status: clusterv1.ClusterStatus{FailureDomains: tc.fds}}}, tc.machines) + fd := PickMost(tc.fds, tc.machines, tc.machines) if tc.expected == nil { g.Expect(fd).To(BeNil()) } else {