chore: improve logging with get cluster

Signed-off-by: Eoghan Lawless <[email protected]>

Author: eoghanlawless

internal/cluster/cluster.go

@@ -18,36 +18,36 @@ const (
 	roleAll             = "all"
 )
 
-func Nodes(ctx context.Context, cli *k8s.Client, cluster *capi.Cluster) ([]api.NodeInfo, error) {
+func GetNodes(ctx context.Context, cli *k8s.Client, cluster *capi.Cluster) ([]api.NodeInfo, error) {
 	nodes := []api.NodeInfo{}
 
-	machines, err := cli.Machines(ctx, cluster.Namespace, cluster.Name)
+	machines, err := cli.GetMachines(ctx, cluster.Namespace, cluster.Name)
 	if err != nil {
 		return nodes, err
 	}
 
 	for _, m := range machines {
-		id, err := nodeId(ctx, cli, m)
+		id, err := getNodeId(ctx, cli, m)
 		if err != nil {
 			return nodes, err
 		}
 		role := nodeRole(m)
-		status := nodeStatus(m)
+		status := getNodeStatus(m)
 		nodes = append(nodes, api.NodeInfo{Id: &id, Role: &role, Status: &status})
 	}
 
 	return nodes, nil
 }
 
-func Template(c *capi.Cluster) string {
+func GetTemplate(c *capi.Cluster) string {
 	if c == nil || c.Spec.Topology == nil {
 		return ""
 	}
 
 	return c.Spec.Topology.Class
 }
 
-func nodeStatus(machine capi.Machine) api.StatusInfo {
+func getNodeStatus(machine capi.Machine) api.StatusInfo {
 	translate := map[capi.MachinePhase]api.StatusInfoCondition{
 		// MachinePhasePending is the first state a Machine is assigned by Cluster API Machine controller after being created.
 		capi.MachinePhasePending: api.STATUSCONDITIONPROVISIONING,
@@ -77,7 +77,7 @@ func nodeStatus(machine capi.Machine) api.StatusInfo {
 	return status
 }
 
-func nodeId(ctx context.Context, cli *k8s.Client, machine capi.Machine) (string, error) {
+func getNodeId(ctx context.Context, cli *k8s.Client, machine capi.Machine) (string, error) {
 	providerMachineName := machine.Spec.InfrastructureRef.Name
 	providerMachineKind := machine.Spec.InfrastructureRef.Kind
 	switch providerMachineKind {

internal/cluster/cluster_test.go

@@ -93,7 +93,7 @@ func TestNodes(t *testing.T) {
 	assert.NoError(t, err)
 
 	// test
-	nodes, err := cluster.Nodes(context.Background(), cli, c)
+	nodes, err := cluster.GetNodes(context.Background(), cli, c)
 	assert.NoError(t, err)
 	if diff := cmp.Diff(nodes, expectedNode); diff != "" {
 		t.Errorf("Nodes() mismatch (-want +got):\n%s", diff)
@@ -104,7 +104,7 @@ func TestTemplate(t *testing.T) {
 	expectedTemplate := "test-template"
 	c := &capi.Cluster{Spec: capi.ClusterSpec{Topology: &capi.Topology{Class: expectedTemplate}}}
 
-	template := cluster.Template(c)
+	template := cluster.GetTemplate(c)
 	if template != expectedTemplate {
 		t.Errorf("Expected %s, got %s", expectedTemplate, template)
 	}

internal/k8s/client.go

@@ -351,8 +351,8 @@ func (cli *Client) Template(ctx context.Context, namespace, name string) (ct.Clu
 	return template, nil
 }
 
-// Cluster returns the cluster with the given name in the given namespace
-func (cli *Client) Cluster(ctx context.Context, namespace, name string) (*capi.Cluster, error) {
+// GetCluster returns the cluster with the given name in the given namespace
+func (cli *Client) GetCluster(ctx context.Context, namespace, name string) (*capi.Cluster, error) {
 	var cluster capi.Cluster
 
 	unstructuredCluster, err := cli.Dyn.Resource(clusterResourceSchema).Namespace(namespace).Get(ctx, name, metav1.GetOptions{})
@@ -371,8 +371,8 @@ func (cli *Client) Cluster(ctx context.Context, namespace, name string) (*capi.C
 	return &cluster, nil
 }
 
-// Machines returns the machine with the given name in the given namespace for the given cluster
-func (cli *Client) Machines(ctx context.Context, namespace, clusterName string) ([]capi.Machine, error) {
+// GetMachines returns the machine with the given name in the given namespace for the given cluster
+func (cli *Client) GetMachines(ctx context.Context, namespace, clusterName string) ([]capi.Machine, error) {
 	var machines []capi.Machine
 
 	opts := metav1.ListOptions{LabelSelector: fmt.Sprintf("cluster.x-k8s.io/cluster-name=%v", clusterName)}

internal/rest/getv2clustersname.go

@@ -5,8 +5,10 @@ package rest
 import (
 	"context"
 	"errors"
+	"fmt"
 	"log/slog"
 	"regexp"
+	"strings"
 
 	"github.com/open-edge-platform/cluster-manager/v2/internal/cluster"
 	"github.com/open-edge-platform/cluster-manager/v2/internal/convert"
@@ -52,19 +54,17 @@ func (s *Server) GetV2ClustersName(ctx context.Context, request api.GetV2Cluster
 
 	cluster, err := s.getCluster(ctx, activeProjectID, name)
 	if err != nil {
-		errMsg := err.Error()
-		if err.Error() == "cluster not found" {
-			slog.Warn("cluster not found", "name", name)
+		if strings.Contains(err.Error(), k8s.ErrClusterNotFound.Error()) {
 			return api.GetV2ClustersName404JSONResponse{
 				N404NotFoundJSONResponse: api.N404NotFoundJSONResponse{
-					Message: &errMsg,
+					Message: ptr(err.Error()),
 				},
 			}, nil
 		}
 		slog.Error("failed to get cluster", "name", name, "error", err)
 		return api.GetV2ClustersName500JSONResponse{
 			N500InternalServerErrorJSONResponse: api.N500InternalServerErrorJSONResponse{
-				Message: &errMsg,
+				Message: ptr(err.Error()),
 			},
 		}, nil
 	}
@@ -78,38 +78,41 @@ func (s *Server) getCluster(ctx context.Context, activeProjectID, name string) (
 
 	cli, err := k8s.New(k8s.WithDynamicClient(s.k8sclient))
 	if err != nil {
-		return api.ClusterDetailInfo{}, errors.New("internal server error")
+		slog.Error("failed to create k8s client", "error", err)
+		return api.ClusterDetailInfo{}, fmt.Errorf("failed to create k8s client, err: %w", err)
 	}
 
-	capiCluster, err := cli.Cluster(ctx, namespace, name)
+	capiCluster, err := cli.GetCluster(ctx, namespace, name)
 	if err != nil {
-		if err == k8s.ErrClusterNotFound {
-			return api.ClusterDetailInfo{}, errors.New("cluster not found")
-		}
-		return api.ClusterDetailInfo{}, errors.New("internal server error")
+		slog.Error("failed to get cluster", "name", name, "error", err)
+		return api.ClusterDetailInfo{}, fmt.Errorf("failed to get cluster, err: %w", err)
 	}
 	if capiCluster.Name == "" {
 		return api.ClusterDetailInfo{}, errors.New("missing cluster name")
 	}
+
 	// get machines associated with the cluster
 	machines, err := fetchMachinesList(ctx, s, namespace, capiCluster.Name)
 	if err != nil {
+		// do we need to return error here?
 		slog.Error("failed to fetch machines for cluster", "cluster", capiCluster.Name, "error", err)
 	}
 
 	labels := labels.Filter(capiCluster.Labels)
 	unstrucutreLabels := convert.MapStringToAny(labels)
 
-	nodes, err := cluster.Nodes(ctx, cli, capiCluster)
+	nodes, err := cluster.GetNodes(ctx, cli, capiCluster)
 	if err != nil {
-		return api.ClusterDetailInfo{}, err
+		slog.Error("failed to get nodes", "cluster", capiCluster.Name, "error", err)
+		return api.ClusterDetailInfo{}, fmt.Errorf("failed to get nodes, err: %w", err)
 	}
 
-	template := cluster.Template(capiCluster)
+	template := cluster.GetTemplate(capiCluster)
 	lp, errs := getClusterLifecyclePhase(capiCluster)
 	if len(errs) > 0 {
 		slog.Debug("errors while building cluster lifecycle phase", "cluster", capiCluster.Name, "errors", errs)
 	}
+
 	clusterDetailInfo := api.ClusterDetailInfo{
 		Name:                &capiCluster.Name,
 		ProviderStatus:      getProviderStatus(capiCluster),
@@ -124,7 +127,8 @@ func (s *Server) getCluster(ctx context.Context, activeProjectID, name string) (
 	}
 
 	if err := validateClusterDetail(clusterDetailInfo); err != nil {
-		return api.ClusterDetailInfo{}, errors.New("internal server error")
+		slog.Error("failed to validate cluster detail", "cluster", capiCluster.Name, "error", err)
+		return api.ClusterDetailInfo{}, fmt.Errorf("failed to validate cluster detail, err: %w", err)
 	}
 
 	return clusterDetailInfo, nil

internal/rest/getv2clustersname_test.go

@@ -379,7 +379,7 @@ func TestGetV2ClustersName404(t *testing.T) {
 		// check the response type and message
 		resp, ok := response.(api.GetV2ClustersName404JSONResponse)
 		require.True(t, ok, "GetV2ClustersName() response type = %T, want api.GetV2ClustersName404JSONResponse", response)
-		require.Equal(t, "cluster not found", *resp.N404NotFoundJSONResponse.Message, "GetV2ClustersName() message = %v, want %v", *resp.N404NotFoundJSONResponse.Message, "cluster not found")
+		require.Equal(t, "failed to get cluster, err: cluster not found", *resp.N404NotFoundJSONResponse.Message, "GetV2ClustersName() message = %v, want %v", *resp.N404NotFoundJSONResponse.Message, "cluster not found")
 	})
 }