Skip to content

Commit d86ee63

Browse files
authored
optimize getv2clusters (#26)
* feat: improve list function * chore: address comments
1 parent 5b22ebb commit d86ee63

File tree

6 files changed

+55
-17
lines changed

6 files changed

+55
-17
lines changed

VERSION

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
2.0.6
1+
2.0.7

deployment/charts/cluster-manager/Chart.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,6 @@ type: application
1616
# This is the chart version. This version number should be incremented each time you make changes
1717
# to the chart and its templates, including the app version.
1818
# Versions are expected to follow Semantic Versioning (https://semver.org/)
19-
version: 2.0.6
20-
appVersion: 2.0.6
19+
version: 2.0.7
20+
appVersion: 2.0.7
2121
annotations: {}

deployment/charts/cluster-template-crd/Chart.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,6 @@ apiVersion: v2
66
name: cluster-template-crd
77
description: A Helm chart for the ClusterTemplate CRD
88
type: application
9-
version: 2.0.6
10-
appVersion: 2.0.6
9+
version: 2.0.7
10+
appVersion: 2.0.7
1111
annotations: {}

internal/rest/getv2clusters.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,11 @@ func (s *Server) getClusters(ctx context.Context, namespace string, orderBy, fil
105105

106106
func (s *Server) convertClusters(ctx context.Context, namespace string, unstructuredClusters []unstructured.Unstructured) []api.ClusterInfo {
107107
clusters := make([]api.ClusterInfo, 0, len(unstructuredClusters))
108+
allMachines, err := fetchAllMachinesList(ctx, s, namespace)
109+
if err != nil {
110+
slog.Error("failed to fetch machines", "namespace", namespace, "error", err)
111+
return nil
112+
}
108113
for _, item := range unstructuredClusters {
109114
capiCluster := capi.Cluster{}
110115
if err := runtime.DefaultUnstructuredConverter.FromUnstructured(item.Object, &capiCluster); err != nil {
@@ -114,11 +119,7 @@ func (s *Server) convertClusters(ctx context.Context, namespace string, unstruct
114119

115120
slog.Debug("Processing cluster", "name", capiCluster.Name, "labels", capiCluster.Labels)
116121
// get machines associated with the cluster
117-
machines, err := fetchMachinesList(ctx, s, namespace, capiCluster.Name)
118-
if err != nil {
119-
slog.Error("failed to fetch machines for cluster", "cluster", capiCluster.Name, "error", err)
120-
continue
121-
}
122+
machines := getClusterMachines(allMachines, capiCluster.Name)
122123

123124
labels := labels.Filter(capiCluster.Labels)
124125
unstructuredLabels := convert.MapStringToAny(labels)
@@ -200,3 +201,4 @@ func orderClustersBy(cluster1, cluster2 api.ClusterInfo, orderBy *OrderBy) bool
200201
return false
201202
}
202203
}
204+

internal/rest/getv2clusters_test.go

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,14 +63,26 @@ var clusterStatusInProgressControlPlane = capi.ClusterStatus{
6363

6464
func createMockServer(t *testing.T, clusters []capi.Cluster, projectID string, options ...bool) *Server {
6565
unstructuredClusters := make([]unstructured.Unstructured, len(clusters))
66+
machinesList := make([]unstructured.Unstructured, len(clusters))
6667
for i, cluster := range clusters {
6768
unstructuredCluster, err := convert.ToUnstructured(cluster)
6869
require.NoError(t, err, "convertClusterToUnstructured() error = %v, want nil")
6970
unstructuredClusters[i] = *unstructuredCluster
71+
machine := capi.Machine{
72+
ObjectMeta: metav1.ObjectMeta{Name: cluster.Name},
73+
Spec: capi.MachineSpec{ClusterName: cluster.Name},
74+
Status: capi.MachineStatus{Phase: string(capi.MachinePhaseRunning)},
75+
}
76+
unstructuredMachine, err := convert.ToUnstructured(machine)
77+
require.NoError(t, err, "convertClusterToUnstructured() error = %v, want nil")
78+
machinesList[i] = *unstructuredMachine
7079
}
7180
unstructuredClusterList := &unstructured.UnstructuredList{
7281
Items: unstructuredClusters,
7382
}
83+
unstructuredMachineList := &unstructured.UnstructuredList{
84+
Items: machinesList,
85+
}
7486
// default is to set up k8s client and machineResource mocks
7587
setupK8sMocks := true
7688
mockMachineResource := true
@@ -88,12 +100,16 @@ func createMockServer(t *testing.T, clusters []capi.Cluster, projectID string, o
88100
mockedk8sclient = k8s.NewMockInterface(t)
89101
mockedk8sclient.EXPECT().Resource(core.ClusterResourceSchema).Return(nsResource)
90102
if mockMachineResource {
103+
machineResource := k8s.NewMockResourceInterface(t)
104+
machineResource.EXPECT().List(mock.Anything, metav1.ListOptions{}).Return(unstructuredMachineList, nil)
105+
namespacedMachineResource := k8s.NewMockNamespaceableResourceInterface(t)
91106
for _, cluster := range clusters {
92-
resource.EXPECT().List(mock.Anything, metav1.ListOptions{
107+
machineResource.EXPECT().List(mock.Anything, metav1.ListOptions{
93108
LabelSelector: "cluster.x-k8s.io/cluster-name=" + cluster.Name,
94109
}).Return(&unstructured.UnstructuredList{Items: []unstructured.Unstructured{}}, nil).Maybe()
95110
}
96-
mockedk8sclient.EXPECT().Resource(core.MachineResourceSchema).Return(nsResource).Maybe()
111+
namespacedMachineResource.EXPECT().Namespace(projectID).Return(machineResource)
112+
mockedk8sclient.EXPECT().Resource(core.MachineResourceSchema).Return(namespacedMachineResource).Maybe()
97113
}
98114
}
99115
return NewServer(mockedk8sclient)
@@ -158,7 +174,7 @@ var expectedActiveProjectID = "655a6892-4280-4c37-97b1-31161ac0b99e"
158174
func TestGetV2Clusters200(t *testing.T) {
159175
t.Run("No Clusters", func(t *testing.T) {
160176
clusters := []capi.Cluster{}
161-
server := createMockServer(t, clusters, expectedActiveProjectID, true, false)
177+
server := createMockServer(t, clusters, expectedActiveProjectID, true, true)
162178
require.NotNil(t, server, "NewServer() returned nil, want not nil")
163179

164180
// Create a new request & response recorder

internal/rest/utils.go

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ type ContextKey string
2525
const (
2626
ActiveProjectIdHeaderKey = "Activeprojectid"
2727
ActiveProjectIdContextKey ContextKey = ActiveProjectIdHeaderKey
28+
ClusterNameSelectorKey = "cluster.x-k8s.io/cluster-name"
2829
)
2930

3031
func validateClusterDetail(clusterDetail api.ClusterDetailInfo) error {
@@ -293,6 +294,17 @@ func getNodeHealth(cluster *capi.Cluster, machines []unstructured.Unstructured)
293294
return status
294295
}
295296

297+
// getClusterMachines filters the machines by cluster name
298+
func getClusterMachines(machines []unstructured.Unstructured, name string ) ([]unstructured.Unstructured) {
299+
var filteredMachines []unstructured.Unstructured
300+
for _, machine := range machines {
301+
if machine.GetLabels()[ClusterNameSelectorKey] == name {
302+
filteredMachines = append(filteredMachines, machine)
303+
}
304+
}
305+
return filteredMachines
306+
}
307+
296308
func ptr[T any](v T) *T {
297309
return &v
298310
}
@@ -341,7 +353,7 @@ func fetchMachineFromCluster(ctx context.Context, s *Server, namespace string, c
341353
func fetchIntelMachineBindingFromCluster(ctx context.Context, s *Server, namespace string, clusterName string, nodeID string) (*intelv1alpha1.IntelMachineBinding, error) {
342354
// first get the machine using the nodeID
343355
unstructuredMachines, err := s.k8sclient.Resource(core.MachineResourceSchema).Namespace(namespace).List(ctx, v1.ListOptions{
344-
LabelSelector: "cluster.x-k8s.io/cluster-name=" + clusterName,
356+
LabelSelector: ClusterNameSelectorKey + "=" + clusterName,
345357
})
346358
if unstructuredMachines == nil || len(unstructuredMachines.Items) == 0 {
347359
return nil, fmt.Errorf("machine not found for node ID %s", nodeID)
@@ -366,7 +378,7 @@ func fetchIntelMachineBindingFromCluster(ctx context.Context, s *Server, namespa
366378
}
367379
// get the machine bindings for the cluster
368380
unstructuredMachineList, err := s.k8sclient.Resource(core.BindingsResourceSchema).Namespace(namespace).List(ctx, v1.ListOptions{
369-
LabelSelector: "cluster.x-k8s.io/cluster-name=" + clusterName,
381+
LabelSelector: ClusterNameSelectorKey + "=" + clusterName,
370382
})
371383
if err != nil {
372384
return nil, err
@@ -388,7 +400,7 @@ func fetchIntelMachineBindingFromCluster(ctx context.Context, s *Server, namespa
388400

389401
func fetchMachine(ctx context.Context, s *Server, namespace string, clusterName string, nodeID string) (*capi.Machine, error) {
390402
unstructuredMachines, err := s.k8sclient.Resource(core.MachineResourceSchema).Namespace(namespace).List(ctx, v1.ListOptions{
391-
LabelSelector: "cluster.x-k8s.io/cluster-name=" + clusterName,
403+
LabelSelector: ClusterNameSelectorKey + "=" + clusterName,
392404
})
393405
if unstructuredMachines == nil || len(unstructuredMachines.Items) == 0 {
394406
return nil, fmt.Errorf("machine not found for node ID %s", nodeID)
@@ -411,10 +423,18 @@ func fetchMachine(ctx context.Context, s *Server, namespace string, clusterName
411423

412424
func fetchMachinesList(ctx context.Context, s *Server, namespace string, clusterName string) ([]unstructured.Unstructured, error) {
413425
unstructuredMachineList, err := s.k8sclient.Resource(core.MachineResourceSchema).Namespace(namespace).List(ctx, v1.ListOptions{
414-
LabelSelector: "cluster.x-k8s.io/cluster-name=" + clusterName,
426+
LabelSelector: ClusterNameSelectorKey + "=" + clusterName,
415427
})
416428
if err != nil {
417429
return nil, err
418430
}
419431
return unstructuredMachineList.Items, nil
420432
}
433+
434+
func fetchAllMachinesList(ctx context.Context, s *Server, namespace string) ([]unstructured.Unstructured, error) {
435+
unstructuredMachineList, err := s.k8sclient.Resource(core.MachineResourceSchema).Namespace(namespace).List(ctx, v1.ListOptions{})
436+
if err != nil {
437+
return nil, err
438+
}
439+
return unstructuredMachineList.Items, nil
440+
}

0 commit comments

Comments
 (0)