Skip to content

feat: add download action for pod logs#299

Open
A69SHUBHAM wants to merge 1 commit into
volcano-sh:mainfrom
A69SHUBHAM:download-pod-logs
Open

feat: add download action for pod logs#299
A69SHUBHAM wants to merge 1 commit into
volcano-sh:mainfrom
A69SHUBHAM:download-pod-logs

Conversation

@A69SHUBHAM

Copy link
Copy Markdown

Summary

This PR adds a download action for pod logs.

Users can now download the currently displayed log output as a .log file directly from the pod details view.

Changes

  • Added a download button to the pod log interface.
  • Implemented browser-based log file generation using native APIs.
  • Used the pod name as the default filename when available.
  • Preserved existing log viewing functionality.

Verification

  • Verified logs can be downloaded successfully.
  • Verified empty log content is handled gracefully.
  • Verified existing pod log functionality remains unchanged.
  • Verified TypeScript and build checks pass.

Closes #298

Signed-off-by: A69SHUBHAM <spacekrai0@gmail.com>
@volcano-sh-bot

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign kevin-wangzefeng for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces the ability to fetch, view, and download Kubernetes pod logs from the dashboard. It adds a new tRPC procedure getPodLogs and updates the frontend to fetch and display these logs alongside the YAML configuration. However, two critical issues were identified: first, refetching queries in pod-management.tsx relies on a potentially stale selectedPod state, which should be resolved by fetching directly using tRPC utils; second, returning the raw response from readNamespacedPodLog in the tRPC router will cause serialization errors, so only response.body should be returned.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +222 to +243
const [yamlResponse, logsResponse] = await Promise.all([
podYamlQuery.refetch(),
podLogsQuery.refetch()
]);
const yaml = yamlResponse.data || pod.yaml || "";
// Ensure logs is a string
let logsText = "";
if (typeof logsResponse.data === 'string') {
logsText = logsResponse.data;
} else if (logsResponse.data && typeof logsResponse.data === 'object') {
logsText = JSON.stringify(logsResponse.data, null, 2);
} else {
logsText = pod.logs || "No logs available.";
}

setSelectedPod({ ...pod, yaml, logs: logsText });
setShowPodDetails(true);
} catch (err) {
setError("Failed to fetch pod YAML");
setError("Failed to fetch pod details");
console.error(err)
}
}, [podYamlQuery])
}, [podYamlQuery, podLogsQuery])

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

There is a critical bug here: when handlePodClick is called, selectedPod is still null (or holds the previously selected pod). Since podYamlQuery and podLogsQuery are defined using selectedPod?.namespace and selectedPod?.name, calling .refetch() on them will execute the queries with stale or empty parameters from the previous render cycle, rather than the parameters of the newly clicked pod.

To fix this, use utils.podRouter.getPodYaml.fetch and utils.podRouter.getPodLogs.fetch directly with the clicked pod's properties. This guarantees that the correct pod's details are fetched immediately and avoids any race conditions or hook dependency issues. Note that once this is applied, both podYamlQuery and podLogsQuery hooks become unused and can be cleaned up.

            const [yaml, logs] = await Promise.all([
                utils.podRouter.getPodYaml.fetch({
                    namespace: pod.namespace,
                    name: pod.name
                }),
                utils.podRouter.getPodLogs.fetch({
                    namespace: pod.namespace,
                    name: pod.name
                })
            ]);

            setSelectedPod({ ...pod, yaml: yaml || "", logs: logs || "No logs available." });
            setShowPodDetails(true);
        } catch (err) {
            setError("Failed to fetch pod details");
            console.error(err)
        }
    }, [utils])

Comment on lines +43 to +47
const response = await k8sCoreApi.readNamespacedPodLog({
name,
namespace,
});
return response;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

In @kubernetes/client-node, readNamespacedPodLog returns an object of type { response: http.IncomingMessage; body: string; }. Returning the entire response object directly will attempt to serialize the http.IncomingMessage object (which contains circular references and sockets) over tRPC, causing a serialization error.

Instead, you should return response.body to only send the actual log string.

Suggested change
const response = await k8sCoreApi.readNamespacedPodLog({
name,
namespace,
});
return response;
const response = await k8sCoreApi.readNamespacedPodLog({
name,
namespace,
});
return response.body;

Comment on lines +85 to +97
const podLogsQuery = trpc.podRouter.getPodLogs.useQuery(
{
namespace: selectedPod?.namespace || "",
name: selectedPod?.name || ""
},
{
enabled: false,
onError: (err) => {
console.error("Error fetching pod logs:", err);
setError(`Pod Logs API error: ${err.message}`);
},
},
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Since we are fetching the pod logs directly using utils.podRouter.getPodLogs.fetch in handlePodClick, this podLogsQuery hook is no longer needed and can be safely removed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add download option for pod logs

2 participants