chore: improve logging with get cluster

internal/cluster/cluster.go

@@ -18,27 +18,29 @@ const (
 	roleAll             = "all"
 )
 
+// Nodes returns the list of nodes in the cluster.
 func Nodes(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
 }
 
+// Template returns the cluster template name.
 func Template(c *capi.Cluster) string {
 	if c == nil || c.Spec.Topology == nil {
 		return ""
@@ -47,7 +49,7 @@ func Template(c *capi.Cluster) string {
 	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 +79,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/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/getv2clusters.go

@@ -4,6 +4,7 @@ package rest
 
 import (
 	"context"
+	"fmt"
 	"log/slog"
 	"math"
 
@@ -23,33 +24,40 @@ const MaxClusters = math.MaxInt32 // Maximum value for int32
 func (s *Server) GetV2Clusters(ctx context.Context, request api.GetV2ClustersRequestObject) (api.GetV2ClustersResponseObject, error) {
 	pageSize, offset, orderBy, filter, err := ValidateParams(request.Params)
 	if err != nil {
+		slog.Error("failed to validate parameters", "pageSize", pageSize, "offset", offset, "orderBy", orderBy, "filter", filter, "error", err)
 		return badRequestGetClustersResponse(err.Error()), nil
 	}
+
 	namespace := request.Params.Activeprojectid.String()
-	allClusters := s.getClusters(ctx, namespace, orderBy, filter)
-	if allClusters == nil {
+	clusters, err := s.getClusters(ctx, namespace, orderBy, filter)
+	if err != nil {
+		slog.Error("failed to get clusters", "namespace", namespace, "filter", filter, "order", orderBy, "error", err)
 		return internalServerErrorGetClustersResponse("failed to retrieve clusters"), nil
-	} else if len(*allClusters) == 0 {
+	}
+
+	if len(*clusters) == 0 {
 		return api.GetV2Clusters200JSONResponse{
-			Clusters:      allClusters,
+			Clusters:      clusters,
 			TotalElements: 0,
 		}, nil
 	}
 
-	paginatedClustersList, err := PaginateItems(*allClusters, *pageSize, *offset)
+	paginatedClusters, err := PaginateItems(*clusters, *pageSize, *offset)
 	if err != nil {
+		slog.Error("failed to paginate clusters", "namespace", namespace, "pageSize", pageSize, "offset", offset, "error", err)
 		return internalServerErrorGetClustersResponse(err.Error()), nil
 	}
 
-	if len(*paginatedClustersList) > MaxClusters {
+	if len(*paginatedClusters) > MaxClusters {
+		slog.Error("number of clusters exceeds the maximum allowed", "namespace", namespace, "count", len(*paginatedClusters))
 		return badRequestGetClustersResponse("number of clusters exceeds the maximum allowed"), nil
 	}
 
-	slog.Info("Clusters state read", "namespace", namespace, "count", len(*allClusters))
+	slog.Info("Clusters state read", "namespace", namespace, "count", len(*clusters))
 
 	return api.GetV2Clusters200JSONResponse{
-		Clusters:      paginatedClustersList,
-		TotalElements: int32(len(*allClusters)),
+		Clusters:      paginatedClusters,
+		TotalElements: int32(len(*clusters)),
 	}, nil
 }
 
@@ -71,36 +79,33 @@ func internalServerErrorGetClustersResponse(message string) api.GetV2Clusters500
 
 // getClusters retrieves and processes the list of clusters in the specified namespace by calling
 // the Cluster API (CAPI), returning a slice of ClusterInfo pointers or nil if an error occurs.
-func (s *Server) getClusters(ctx context.Context, namespace string, orderBy, filter *string) *[]api.ClusterInfo {
+func (s *Server) getClusters(ctx context.Context, namespace string, orderBy, filter *string) (*[]api.ClusterInfo, error) {
 	if namespace == "" {
-		slog.Warn("failed to get clusters project id")
-		return nil
+		return nil, fmt.Errorf("no namespace provided")
 	}
 
 	clusters, err := fetchClustersList(ctx, s, namespace)
 	if err != nil {
-		slog.Error("failed to fetch clusters", "namespace", namespace, "error", err)
-		return nil
+		return nil, fmt.Errorf("failed to fetch clusters: %w", err)
 	}
 
 	convertedClusters := s.convertClusters(ctx, namespace, clusters)
 
 	if filter != nil {
 		convertedClusters, err = FilterItems(convertedClusters, *filter, filterClusters)
 		if err != nil {
-			slog.Error("failed to apply filters", "error", err)
-			return nil
+			return nil, fmt.Errorf("failed to apply filters: %w", err)
 		}
 	}
 
 	if orderBy != nil {
 		convertedClusters, err = OrderItems(convertedClusters, *orderBy, orderClustersBy)
 		if err != nil {
-			slog.Error("failed to apply order by", "error", err)
-			return nil
+			return nil, fmt.Errorf("failed to apply order by: %w", err)
 		}
 	}
-	return &convertedClusters
+
+	return &convertedClusters, nil
 }
 
 func (s *Server) convertClusters(ctx context.Context, namespace string, unstructuredClusters []unstructured.Unstructured) []api.ClusterInfo {
@@ -201,4 +206,3 @@ func orderClustersBy(cluster1, cluster2 api.ClusterInfo, orderBy *OrderBy) bool
 		return false
 	}
 }
-

internal/rest/getv2clustersname.go

@@ -5,6 +5,7 @@ package rest
 import (
 	"context"
 	"errors"
+	"fmt"
 	"log/slog"
 	"regexp"
 
@@ -52,19 +53,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 errors.Unwrap(err) == k8s.ErrClusterNotFound {
 			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,22 +77,23 @@ 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)
 	}
 
@@ -102,14 +102,16 @@ func (s *Server) getCluster(ctx context.Context, activeProjectID, name string) (
 
 	nodes, err := cluster.Nodes(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)
 	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 +126,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")
 	})
 }