From 20b85713ced658a329255b15c2a5fcce025a1c49 Mon Sep 17 00:00:00 2001 From: Ludwig Bedacht Date: Thu, 2 May 2024 11:56:51 +0200 Subject: [PATCH 1/6] Improve deletion logic Makes sure to delete resources properly when deleting all resources at once. Also fixes handling for the credential secret which so far was not properly cleaned up. --- api/v1alpha1/ionoscloudcluster_types.go | 4 + .../ionoscloudcluster_controller.go | 16 ++- internal/controller/util.go | 45 +++++++ scope/cluster.go | 22 ++++ scope/cluster_test.go | 114 ++++++++++++++++++ scope/machine.go | 34 ++---- scope/machine_test.go | 4 +- 7 files changed, 210 insertions(+), 29 deletions(-) diff --git a/api/v1alpha1/ionoscloudcluster_types.go b/api/v1alpha1/ionoscloudcluster_types.go index 3709855b..03a2666f 100644 --- a/api/v1alpha1/ionoscloudcluster_types.go +++ b/api/v1alpha1/ionoscloudcluster_types.go @@ -27,6 +27,10 @@ const ( // associated with the IonosCloudCluster before removing it from the API server. ClusterFinalizer = "ionoscloudcluster.infrastructure.cluster.x-k8s.io" + // ClusterCredentialFinalizer allows cleanup of resources, which are + // associated with the IonosCloudCluster credentials before removing it from the API server. + ClusterCredentialFinalizer = ClusterFinalizer + "/credentials" + // IonosCloudClusterReady is the condition for the IonosCloudCluster, which indicates that the cluster is ready. IonosCloudClusterReady clusterv1.ConditionType = "ClusterReady" diff --git a/internal/controller/ionoscloudcluster_controller.go b/internal/controller/ionoscloudcluster_controller.go index 5f47387c..521df8fb 100644 --- a/internal/controller/ionoscloudcluster_controller.go +++ b/internal/controller/ionoscloudcluster_controller.go @@ -171,8 +171,16 @@ func (r *IonosCloudClusterReconciler) reconcileDelete( return ctrl.Result{RequeueAfter: defaultReconcileDuration}, nil } - // TODO(lubedacht): check if there are any more machine CRs existing. - // If there are requeue with an offset. + machines, err := clusterScope.ListMachinesForCluster(ctx, nil) + if err != nil { + log.Error(err, "unable to list machines for cluster") + return ctrl.Result{}, err + } + + if len(machines) > 0 { + log.Info("Waiting for all IonosCloudMachines to be deleted", "remaining", len(machines)) + return ctrl.Result{RequeueAfter: defaultReconcileDuration}, nil + } reconcileSequence := []serviceReconcileStep[scope.Cluster]{ {"ReconcileControlPlaneEndpointDeletion", cloudService.ReconcileControlPlaneEndpointDeletion}, @@ -186,6 +194,10 @@ func (r *IonosCloudClusterReconciler) reconcileDelete( return ctrl.Result{RequeueAfter: defaultReconcileDuration}, err } } + if err := removeCredentialFinalizer(ctx, r.Client, clusterScope.IonosCluster); err != nil { + log.Error(err, "unable to remove finalizer from IonosCloudCluster") + return ctrl.Result{}, err + } controllerutil.RemoveFinalizer(clusterScope.IonosCluster, infrav1.ClusterFinalizer) return ctrl.Result{}, nil } diff --git a/internal/controller/util.go b/internal/controller/util.go index 1c091d37..cf4a3544 100644 --- a/internal/controller/util.go +++ b/internal/controller/util.go @@ -19,6 +19,7 @@ package controller import ( "context" "fmt" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "time" "github.com/go-logr/logr" @@ -80,6 +81,10 @@ func createServiceFromCluster( return nil, err } + if err := ensureSecretControlledByCluster(ctx, c, cluster, &authSecret); err != nil { + return nil, err + } + token := string(authSecret.Data["token"]) apiURL := string(authSecret.Data["apiURL"]) caBundle := authSecret.Data["caBundle"] @@ -91,3 +96,43 @@ func createServiceFromCluster( return cloud.NewService(ionosClient, log) } + +// ensureSecretControlledByCluster ensures that the secrets will contain a finalizer and a controller reference. +// The secret should only be deleted when there are no resources left in the IONOS Cloud environment. +func ensureSecretControlledByCluster( + ctx context.Context, c client.Client, + cluster *infrav1.IonosCloudCluster, + secret *corev1.Secret, +) error { + requireUpdate := controllerutil.AddFinalizer(secret, infrav1.ClusterCredentialFinalizer) + + if !controllerutil.HasControllerReference(secret) { + if err := controllerutil.SetControllerReference(cluster, secret, c.Scheme()); err != nil { + return err + } + requireUpdate = true + } + + if requireUpdate { + return c.Update(ctx, secret) + } + + return nil +} + +// removeCredentialFinalizer removes the finalizer from the credential secret. +func removeCredentialFinalizer(ctx context.Context, c client.Client, cluster *infrav1.IonosCloudCluster) error { + secretKey := client.ObjectKey{ + Namespace: cluster.Namespace, + Name: cluster.Spec.CredentialsRef.Name, + } + var secret corev1.Secret + + if err := c.Get(ctx, secretKey, &secret); err != nil { + // If the secret does not exist anymore, there is nothing we can do. + return client.IgnoreNotFound(err) + } + + controllerutil.RemoveFinalizer(&secret, infrav1.ClusterCredentialFinalizer) + return c.Update(ctx, &secret) +} diff --git a/scope/cluster.go b/scope/cluster.go index ff580fb8..5b76326c 100644 --- a/scope/cluster.go +++ b/scope/cluster.go @@ -34,6 +34,7 @@ import ( // Cluster defines a basic cluster context for primary use in IonosCloudClusterReconciler. type Cluster struct { + client client.Client patchHelper *patch.Helper Cluster *clusterv1.Cluster IonosCluster *infrav1.IonosCloudCluster @@ -67,6 +68,7 @@ func NewCluster(params ClusterParams) (*Cluster, error) { } clusterScope := &Cluster{ + client: params.Client, Cluster: params.Cluster, IonosCluster: params.IonosCluster, patchHelper: helper, @@ -85,6 +87,26 @@ func (c *Cluster) SetControlPlaneEndpointIPBlockID(id string) { c.IonosCluster.Status.ControlPlaneEndpointIPBlockID = id } +// ListMachinesForCluster returns a list of IonosCloudMachines in the same namespace and with the same cluster label. +// With machineLabels, additional search labels can be provided. +func (c *Cluster) ListMachinesForCluster( + ctx context.Context, + machineLabels client.MatchingLabels, +) ([]infrav1.IonosCloudMachine, error) { + if machineLabels == nil { + machineLabels = client.MatchingLabels{} + } + + machineLabels[clusterv1.ClusterNameLabel] = c.Cluster.Name + listOpts := []client.ListOption{client.InNamespace(c.Cluster.Namespace), machineLabels} + + machineList := &infrav1.IonosCloudMachineList{} + if err := c.client.List(ctx, machineList, listOpts...); err != nil { + return nil, err + } + return machineList.Items, nil +} + // Location is a shortcut for getting the location used by the IONOS Cloud cluster IP block. func (c *Cluster) Location() string { return c.IonosCluster.Spec.Location diff --git a/scope/cluster_test.go b/scope/cluster_test.go index d8165dfe..e859c67e 100644 --- a/scope/cluster_test.go +++ b/scope/cluster_test.go @@ -17,11 +17,14 @@ limitations under the License. package scope import ( + "context" "testing" "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" infrav1 "github.com/ionos-cloud/cluster-api-provider-ionoscloud/api/v1alpha1" @@ -85,3 +88,114 @@ func TestNewClusterMissingParams(t *testing.T) { }) } } + +func TestClusterListMachinesForCluster(t *testing.T) { + scheme := runtime.NewScheme() + require.NoError(t, infrav1.AddToScheme(scheme)) + + const clusterName = "test-cluster" + + makeLabels := func(clusterName string, additionalLabels map[string]string) map[string]string { + if additionalLabels == nil { + return map[string]string{clusterv1.ClusterNameLabel: clusterName} + } + + additionalLabels[clusterv1.ClusterNameLabel] = clusterName + return additionalLabels + } + + tests := []struct { + name string + initialObjects []client.Object + searchLabels client.MatchingLabels + expectedNames map[string]struct{} + }{{ + name: "List all machines for a cluster", + initialObjects: []client.Object{ + buildMachineWithLabel("machine-1", makeLabels(clusterName, nil)), + buildMachineWithLabel("machine-2", makeLabels(clusterName, nil)), + buildMachineWithLabel("machine-3", makeLabels(clusterName, nil)), + }, + searchLabels: client.MatchingLabels{}, + expectedNames: map[string]struct{}{ + "machine-1": {}, + "machine-2": {}, + "machine-3": {}, + }, + }, { + name: "List only machines with specific labels", + initialObjects: []client.Object{ + buildMachineWithLabel("machine-1", makeLabels(clusterName, map[string]string{"foo": "bar"})), + buildMachineWithLabel("machine-2", makeLabels(clusterName, map[string]string{"foo": "bar"})), + buildMachineWithLabel("machine-3", makeLabels(clusterName, nil)), + }, + searchLabels: client.MatchingLabels{ + "foo": "bar", + }, + expectedNames: map[string]struct{}{ + "machine-1": {}, + "machine-2": {}, + }, + }, { + name: "List no machines", + initialObjects: []client.Object{ + buildMachineWithLabel("machine-1", makeLabels(clusterName, map[string]string{"foo": "notbar"})), + buildMachineWithLabel("machine-2", makeLabels(clusterName, map[string]string{"foo": "notbar"})), + buildMachineWithLabel("machine-3", makeLabels(clusterName, map[string]string{"foo": "notbar"})), + }, + searchLabels: makeLabels(clusterName, map[string]string{"foo": "bar"}), + expectedNames: map[string]struct{}{}, + }} + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + params := ClusterParams{ + Cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: clusterName, + Namespace: metav1.NamespaceDefault, + Labels: map[string]string{ + clusterv1.ClusterNameLabel: clusterName, + }, + }, + }, + IonosCluster: &infrav1.IonosCloudCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-ionos-cluster", + Namespace: metav1.NamespaceDefault, + Labels: map[string]string{ + clusterv1.ClusterNameLabel: clusterName, + }, + }, + Status: infrav1.IonosCloudClusterStatus{}, + }, + } + + cl := fake.NewClientBuilder().WithScheme(scheme). + WithObjects(test.initialObjects...).Build() + + params.Client = cl + cs, err := NewCluster(params) + require.NoError(t, err) + require.NotNil(t, cs) + + machines, err := cs.ListMachinesForCluster(context.Background(), test.searchLabels) + require.NoError(t, err) + require.Len(t, machines, len(test.expectedNames)) + + for _, m := range machines { + require.Contains(t, test.expectedNames, m.Name) + } + }) + } +} + +func buildMachineWithLabel(name string, labels map[string]string) *infrav1.IonosCloudMachine { + return &infrav1.IonosCloudMachine{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: metav1.NamespaceDefault, + Labels: labels, + }, + } +} diff --git a/scope/machine.go b/scope/machine.go index f714460f..1a93fb40 100644 --- a/scope/machine.go +++ b/scope/machine.go @@ -122,24 +122,12 @@ func (m *Machine) CountMachines(ctx context.Context, machineLabels client.Matchi return len(machines), err } -// ListMachines returns a list of IonosCloudMachines in the same namespace and with the same cluster label. -// With machineLabels, additional search labels can be provided. +// ListMachines is a convenience wrapper function for the Cluster.ListMachinesForCluster function. func (m *Machine) ListMachines( ctx context.Context, machineLabels client.MatchingLabels, ) ([]infrav1.IonosCloudMachine, error) { - if machineLabels == nil { - machineLabels = client.MatchingLabels{} - } - - machineLabels[clusterv1.ClusterNameLabel] = m.ClusterScope.Cluster.Name - listOpts := []client.ListOption{client.InNamespace(m.IonosMachine.Namespace), machineLabels} - - machineList := &infrav1.IonosCloudMachineList{} - if err := m.client.List(ctx, machineList, listOpts...); err != nil { - return nil, err - } - return machineList.Items, nil + return m.ClusterScope.ListMachinesForCluster(ctx, machineLabels) } // FindLatestMachine returns the latest IonosCloudMachine in the same namespace @@ -151,23 +139,17 @@ func (m *Machine) FindLatestMachine( ctx context.Context, matchLabels client.MatchingLabels, ) (*infrav1.IonosCloudMachine, error) { - if matchLabels == nil { - matchLabels = client.MatchingLabels{} - } - - matchLabels[clusterv1.ClusterNameLabel] = m.ClusterScope.Cluster.Name - listOpts := []client.ListOption{client.InNamespace(m.IonosMachine.Namespace), matchLabels} - - machineList := &infrav1.IonosCloudMachineList{} - if err := m.client.List(ctx, machineList, listOpts...); err != nil { + machines, err := m.ClusterScope.ListMachinesForCluster(ctx, matchLabels) + if err != nil { return nil, err } - if len(machineList.Items) <= 1 { + + if len(machines) <= 1 { return nil, nil } - latestMachine := machineList.Items[0] - for _, machine := range machineList.Items { + latestMachine := machines[0] + for _, machine := range machines { if !machine.CreationTimestamp.Before(&latestMachine.CreationTimestamp) && machine.Name != m.IonosMachine.Name { latestMachine = machine } diff --git a/scope/machine_test.go b/scope/machine_test.go index 47d0b2f2..c5c16d9d 100644 --- a/scope/machine_test.go +++ b/scope/machine_test.go @@ -37,10 +37,12 @@ func exampleParams(t *testing.T) MachineParams { if err := infrav1.AddToScheme(scheme.Scheme); err != nil { require.NoError(t, err, "could not construct params") } + cl := fake.NewClientBuilder().WithScheme(scheme.Scheme).Build() return MachineParams{ - Client: fake.NewClientBuilder().WithScheme(scheme.Scheme).Build(), + Client: cl, Machine: &clusterv1.Machine{}, ClusterScope: &Cluster{ + client: cl, Cluster: &clusterv1.Cluster{}, }, IonosMachine: &infrav1.IonosCloudMachine{}, From 510924a3a935fdc6843919b3ed909ca50be5fdb6 Mon Sep 17 00:00:00 2001 From: Ludwig Bedacht Date: Thu, 2 May 2024 12:00:57 +0200 Subject: [PATCH 2/6] lint me bby one more time --- internal/controller/util.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/controller/util.go b/internal/controller/util.go index cf4a3544..0fafd279 100644 --- a/internal/controller/util.go +++ b/internal/controller/util.go @@ -19,13 +19,13 @@ package controller import ( "context" "fmt" - "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "time" "github.com/go-logr/logr" sdk "github.com/ionos-cloud/sdk-go/v6" corev1 "k8s.io/api/core/v1" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" infrav1 "github.com/ionos-cloud/cluster-api-provider-ionoscloud/api/v1alpha1" icc "github.com/ionos-cloud/cluster-api-provider-ionoscloud/internal/ionoscloud/client" From f0aae267eb31b2f93dd85ed558a4fe84317e0676 Mon Sep 17 00:00:00 2001 From: Ludwig Bedacht Date: Thu, 2 May 2024 13:08:59 +0200 Subject: [PATCH 3/6] plural form --- api/v1alpha1/ionoscloudcluster_types.go | 4 ++-- internal/controller/util.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/api/v1alpha1/ionoscloudcluster_types.go b/api/v1alpha1/ionoscloudcluster_types.go index 03a2666f..103118a1 100644 --- a/api/v1alpha1/ionoscloudcluster_types.go +++ b/api/v1alpha1/ionoscloudcluster_types.go @@ -27,9 +27,9 @@ const ( // associated with the IonosCloudCluster before removing it from the API server. ClusterFinalizer = "ionoscloudcluster.infrastructure.cluster.x-k8s.io" - // ClusterCredentialFinalizer allows cleanup of resources, which are + // ClusterCredentialsFinalizer allows cleanup of resources, which are // associated with the IonosCloudCluster credentials before removing it from the API server. - ClusterCredentialFinalizer = ClusterFinalizer + "/credentials" + ClusterCredentialsFinalizer = ClusterFinalizer + "/credentials" // IonosCloudClusterReady is the condition for the IonosCloudCluster, which indicates that the cluster is ready. IonosCloudClusterReady clusterv1.ConditionType = "ClusterReady" diff --git a/internal/controller/util.go b/internal/controller/util.go index 0fafd279..2c137414 100644 --- a/internal/controller/util.go +++ b/internal/controller/util.go @@ -104,7 +104,7 @@ func ensureSecretControlledByCluster( cluster *infrav1.IonosCloudCluster, secret *corev1.Secret, ) error { - requireUpdate := controllerutil.AddFinalizer(secret, infrav1.ClusterCredentialFinalizer) + requireUpdate := controllerutil.AddFinalizer(secret, infrav1.ClusterCredentialsFinalizer) if !controllerutil.HasControllerReference(secret) { if err := controllerutil.SetControllerReference(cluster, secret, c.Scheme()); err != nil { @@ -133,6 +133,6 @@ func removeCredentialFinalizer(ctx context.Context, c client.Client, cluster *in return client.IgnoreNotFound(err) } - controllerutil.RemoveFinalizer(&secret, infrav1.ClusterCredentialFinalizer) + controllerutil.RemoveFinalizer(&secret, infrav1.ClusterCredentialsFinalizer) return c.Update(ctx, &secret) } From 1f9f63bd3a0e8e27933d810f4caff8285027d636 Mon Sep 17 00:00:00 2001 From: Ludwig Bedacht Date: Thu, 2 May 2024 14:44:12 +0200 Subject: [PATCH 4/6] satisfy Jonas --- internal/controller/ionoscloudcluster_controller.go | 2 +- internal/controller/util.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/controller/ionoscloudcluster_controller.go b/internal/controller/ionoscloudcluster_controller.go index 521df8fb..9a82968e 100644 --- a/internal/controller/ionoscloudcluster_controller.go +++ b/internal/controller/ionoscloudcluster_controller.go @@ -194,7 +194,7 @@ func (r *IonosCloudClusterReconciler) reconcileDelete( return ctrl.Result{RequeueAfter: defaultReconcileDuration}, err } } - if err := removeCredentialFinalizer(ctx, r.Client, clusterScope.IonosCluster); err != nil { + if err := removeCredentialsFinalizer(ctx, r.Client, clusterScope.IonosCluster); err != nil { log.Error(err, "unable to remove finalizer from IonosCloudCluster") return ctrl.Result{}, err } diff --git a/internal/controller/util.go b/internal/controller/util.go index 2c137414..7a60a22e 100644 --- a/internal/controller/util.go +++ b/internal/controller/util.go @@ -120,8 +120,8 @@ func ensureSecretControlledByCluster( return nil } -// removeCredentialFinalizer removes the finalizer from the credential secret. -func removeCredentialFinalizer(ctx context.Context, c client.Client, cluster *infrav1.IonosCloudCluster) error { +// removeCredentialsFinalizer removes the finalizer from the credential secret. +func removeCredentialsFinalizer(ctx context.Context, c client.Client, cluster *infrav1.IonosCloudCluster) error { secretKey := client.ObjectKey{ Namespace: cluster.Namespace, Name: cluster.Spec.CredentialsRef.Name, From b8533c33b5401f825127c788791f50820aed170e Mon Sep 17 00:00:00 2001 From: Ludwig Bedacht Date: Fri, 3 May 2024 09:58:05 +0200 Subject: [PATCH 5/6] updates from PR --- internal/controller/ionoscloudcluster_controller.go | 4 +--- internal/controller/ionoscloudmachine_controller.go | 2 +- internal/service/cloud/network.go | 2 +- scope/cluster.go | 4 ++-- scope/cluster_test.go | 13 +++++++------ scope/machine.go | 6 +++--- 6 files changed, 15 insertions(+), 16 deletions(-) diff --git a/internal/controller/ionoscloudcluster_controller.go b/internal/controller/ionoscloudcluster_controller.go index 9a82968e..1d0c317e 100644 --- a/internal/controller/ionoscloudcluster_controller.go +++ b/internal/controller/ionoscloudcluster_controller.go @@ -171,9 +171,8 @@ func (r *IonosCloudClusterReconciler) reconcileDelete( return ctrl.Result{RequeueAfter: defaultReconcileDuration}, nil } - machines, err := clusterScope.ListMachinesForCluster(ctx, nil) + machines, err := clusterScope.ListMachines(ctx, nil) if err != nil { - log.Error(err, "unable to list machines for cluster") return ctrl.Result{}, err } @@ -195,7 +194,6 @@ func (r *IonosCloudClusterReconciler) reconcileDelete( } } if err := removeCredentialsFinalizer(ctx, r.Client, clusterScope.IonosCluster); err != nil { - log.Error(err, "unable to remove finalizer from IonosCloudCluster") return ctrl.Result{}, err } controllerutil.RemoveFinalizer(clusterScope.IonosCluster, infrav1.ClusterFinalizer) diff --git a/internal/controller/ionoscloudmachine_controller.go b/internal/controller/ionoscloudmachine_controller.go index e9dd5580..a3ba2961 100644 --- a/internal/controller/ionoscloudmachine_controller.go +++ b/internal/controller/ionoscloudmachine_controller.go @@ -153,7 +153,7 @@ func (r *IonosCloudMachineReconciler) reconcileNormal( if controllerutil.AddFinalizer(machineScope.IonosMachine, infrav1.MachineFinalizer) { if err := machineScope.PatchObject(); err != nil { - log.Error(err, "unable to update finalizer on object") + err = fmt.Errorf("unable to update finalizer on object: %w", err) return ctrl.Result{}, err } } diff --git a/internal/service/cloud/network.go b/internal/service/cloud/network.go index 726068f8..e2e25ad7 100644 --- a/internal/service/cloud/network.go +++ b/internal/service/cloud/network.go @@ -443,7 +443,7 @@ func (s *Service) getServerNICID(ctx context.Context, ms *scope.Machine) (string log.Info("Server was not found or already deleted.") return "", nil } - log.Error(err, "Unable to retrieve server") + err = fmt.Errorf("unable to retrieve server %w", err) return "", err } diff --git a/scope/cluster.go b/scope/cluster.go index 2c1d6cc8..daf6a16e 100644 --- a/scope/cluster.go +++ b/scope/cluster.go @@ -123,9 +123,9 @@ func (c *Cluster) SetControlPlaneEndpointIPBlockID(id string) { c.IonosCluster.Status.ControlPlaneEndpointIPBlockID = id } -// ListMachinesForCluster returns a list of IonosCloudMachines in the same namespace and with the same cluster label. +// ListMachines returns a list of IonosCloudMachines in the same namespace and with the same cluster label. // With machineLabels, additional search labels can be provided. -func (c *Cluster) ListMachinesForCluster( +func (c *Cluster) ListMachines( ctx context.Context, machineLabels client.MatchingLabels, ) ([]infrav1.IonosCloudMachine, error) { diff --git a/scope/cluster_test.go b/scope/cluster_test.go index ed766dcc..99ccf27f 100644 --- a/scope/cluster_test.go +++ b/scope/cluster_test.go @@ -25,6 +25,7 @@ import ( "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/sets" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -165,7 +166,7 @@ func TestCluster_GetControlPlaneEndpointIP(t *testing.T) { } } -func TestClusterListMachinesForCluster(t *testing.T) { +func TestClusterListMachines(t *testing.T) { scheme := runtime.NewScheme() require.NoError(t, infrav1.AddToScheme(scheme)) @@ -184,7 +185,7 @@ func TestClusterListMachinesForCluster(t *testing.T) { name string initialObjects []client.Object searchLabels client.MatchingLabels - expectedNames map[string]struct{} + expectedNames sets.Set[string] }{{ name: "List all machines for a cluster", initialObjects: []client.Object{ @@ -193,7 +194,7 @@ func TestClusterListMachinesForCluster(t *testing.T) { buildMachineWithLabel("machine-3", makeLabels(clusterName, nil)), }, searchLabels: client.MatchingLabels{}, - expectedNames: map[string]struct{}{ + expectedNames: sets.Set[string]{ "machine-1": {}, "machine-2": {}, "machine-3": {}, @@ -208,7 +209,7 @@ func TestClusterListMachinesForCluster(t *testing.T) { searchLabels: client.MatchingLabels{ "foo": "bar", }, - expectedNames: map[string]struct{}{ + expectedNames: sets.Set[string]{ "machine-1": {}, "machine-2": {}, }, @@ -220,7 +221,7 @@ func TestClusterListMachinesForCluster(t *testing.T) { buildMachineWithLabel("machine-3", makeLabels(clusterName, map[string]string{"foo": "notbar"})), }, searchLabels: makeLabels(clusterName, map[string]string{"foo": "bar"}), - expectedNames: map[string]struct{}{}, + expectedNames: sets.Set[string]{}, }} for _, test := range tests { @@ -255,7 +256,7 @@ func TestClusterListMachinesForCluster(t *testing.T) { require.NoError(t, err) require.NotNil(t, cs) - machines, err := cs.ListMachinesForCluster(context.Background(), test.searchLabels) + machines, err := cs.ListMachines(context.Background(), test.searchLabels) require.NoError(t, err) require.Len(t, machines, len(test.expectedNames)) diff --git a/scope/machine.go b/scope/machine.go index 1a93fb40..6c681295 100644 --- a/scope/machine.go +++ b/scope/machine.go @@ -122,12 +122,12 @@ func (m *Machine) CountMachines(ctx context.Context, machineLabels client.Matchi return len(machines), err } -// ListMachines is a convenience wrapper function for the Cluster.ListMachinesForCluster function. +// ListMachines is a convenience wrapper function for the Cluster.ListMachines function. func (m *Machine) ListMachines( ctx context.Context, machineLabels client.MatchingLabels, ) ([]infrav1.IonosCloudMachine, error) { - return m.ClusterScope.ListMachinesForCluster(ctx, machineLabels) + return m.ClusterScope.ListMachines(ctx, machineLabels) } // FindLatestMachine returns the latest IonosCloudMachine in the same namespace @@ -139,7 +139,7 @@ func (m *Machine) FindLatestMachine( ctx context.Context, matchLabels client.MatchingLabels, ) (*infrav1.IonosCloudMachine, error) { - machines, err := m.ClusterScope.ListMachinesForCluster(ctx, matchLabels) + machines, err := m.ClusterScope.ListMachines(ctx, matchLabels) if err != nil { return nil, err } From 6e6e6663dc73d761aea30e8ca66a03509d63f3db Mon Sep 17 00:00:00 2001 From: Ludwig Bedacht Date: Fri, 3 May 2024 10:02:17 +0200 Subject: [PATCH 6/6] use sets.New --- scope/cluster_test.go | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/scope/cluster_test.go b/scope/cluster_test.go index 99ccf27f..45699c32 100644 --- a/scope/cluster_test.go +++ b/scope/cluster_test.go @@ -193,12 +193,8 @@ func TestClusterListMachines(t *testing.T) { buildMachineWithLabel("machine-2", makeLabels(clusterName, nil)), buildMachineWithLabel("machine-3", makeLabels(clusterName, nil)), }, - searchLabels: client.MatchingLabels{}, - expectedNames: sets.Set[string]{ - "machine-1": {}, - "machine-2": {}, - "machine-3": {}, - }, + searchLabels: client.MatchingLabels{}, + expectedNames: sets.New("machine-1", "machine-2", "machine-3"), }, { name: "List only machines with specific labels", initialObjects: []client.Object{ @@ -209,10 +205,7 @@ func TestClusterListMachines(t *testing.T) { searchLabels: client.MatchingLabels{ "foo": "bar", }, - expectedNames: sets.Set[string]{ - "machine-1": {}, - "machine-2": {}, - }, + expectedNames: sets.New("machine-1", "machine-2"), }, { name: "List no machines", initialObjects: []client.Object{ @@ -221,7 +214,7 @@ func TestClusterListMachines(t *testing.T) { buildMachineWithLabel("machine-3", makeLabels(clusterName, map[string]string{"foo": "notbar"})), }, searchLabels: makeLabels(clusterName, map[string]string{"foo": "bar"}), - expectedNames: sets.Set[string]{}, + expectedNames: sets.New[string](), }} for _, test := range tests {