src: dump logs on build failure and experiment with LLB #43
Conversation
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
👍 Looks good to me! Reviewed everything up to 8233c57 in 20 seconds
More details
- Looked at
32lines of code in1files - Skipped
2files when reviewing. - Skipped posting
2drafted comments based on config settings.
1. src/main.ts:423
- Draft comment:
UsingexecAsyncwith unsanitized input can lead to command injection vulnerabilities. Ensure that variables likellbDumpDirare properly sanitized or use a safer method to handle directory creation. - Reason this comment was not posted:
Comment did not seem useful.
2. src/main.ts:756
- Draft comment:
UsingexecAsyncwith unsanitized input can lead to command injection vulnerabilities. Ensure that variables likestateHelper.buildRefare properly sanitized or use a safer method to handle command execution. - Reason this comment was not posted:
Comment did not seem useful.
Workflow ID: wflow_WENPqP2WnxdHy5Hw
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
⌛ 2 days left in your free trial, upgrade for $20/seat/month or contact us.
adityamaru
force-pushed
the
dump-buildctl-logs
branch
from
8233c57 to
df30dab
Compare
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on df30dab in 28 seconds
More details
- Looked at
39lines of code in1files - Skipped
0files when reviewing. - Skipped posting
3drafted comments based on config settings.
1. src/main.ts:289
- Draft comment:
Usingsudofor executing commands can be a security risk. Ensure that the commands are safe and the environment is secure. - Reason this comment was not posted:
Comment did not seem useful.
2. src/main.ts:423
- Draft comment:
Ensure that the command executed withexecAsyncis safe and properly handles potential errors. - Reason this comment was not posted:
Marked as duplicate.
3. src/main.ts:754
- Draft comment:
Ensure that the command executed withexecAsyncis safe and properly handles potential errors. - Reason this comment was not posted:
Comment did not seem useful.
Workflow ID: wflow_Qgu1IovuE91eqZYf
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
⌛ 2 days left in your free trial, upgrade for $20/seat/month or contact us.
adityamaru
force-pushed
the
dump-buildctl-logs
branch
from
df30dab to
7309a49
Compare
There was a problem hiding this comment.
❌ Changes requested. Incremental review on 7309a49 in 59 seconds
More details
- Looked at
39lines of code in1files - Skipped
0files when reviewing. - Skipped posting
2drafted comments based on config settings.
1. src/main.ts:286
- Draft comment:
Consider adding error handling for theexecAsynccall that creates the/run/buildkitdirectory to ensure robustness. - Reason this comment was not posted:
Comment was not on a valid diff hunk.
2. src/main.ts:287
- Draft comment:
Consider adding error handling for theexecAsynccall that changes permissions of the/run/buildkitdirectory to ensure robustness. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_3UwUkibLP2I8gmXX
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
⌛ 2 days left in your free trial, upgrade for $20/seat/month or contact us.
| @@ -418,6 +418,9 @@ async function getBuilderAddr(inputs: context.Inputs, dockerfilePath: string): P | |||
| core.debug('Successfully obtained sticky disk, proceeding to start buildkitd'); | |||
|
|
|||
| // Start buildkitd. | |||
| // Create a directory for LLB dumps | |||
| const llbDumpDir = '/tmp/buildkit-llb-dumps'; | |||
| await execAsync(`mkdir -p ${llbDumpDir}`); | |||
There was a problem hiding this comment.
Consider adding error handling for the execAsync call that creates the LLB dump directory to ensure robustness.
adityamaru
force-pushed
the
dump-buildctl-logs
branch
from
7309a49 to
92bff7f
Compare
There was a problem hiding this comment.
❌ Changes requested. Incremental review on 92bff7f in 1 minute and 1 seconds
More details
- Looked at
39lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. src/main.ts:292
- Draft comment:
Consider checking the exit code of theexecAsynccommand instead of relying solely onstderrto determine if the command failed. This will provide a more reliable error handling mechanism. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_wprbRHvh3A8JEFz8
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
⌛ 2 days left in your free trial, upgrade for $20/seat/month or contact us.
| @@ -418,6 +418,9 @@ async function getBuilderAddr(inputs: context.Inputs, dockerfilePath: string): P | |||
| core.debug('Successfully obtained sticky disk, proceeding to start buildkitd'); | |||
|
|
|||
| // Start buildkitd. | |||
| // Create a directory for LLB dumps | |||
| const llbDumpDir = '/tmp/buildkit-llb-dumps'; | |||
| await execAsync(`mkdir -p ${llbDumpDir}`); | |||
There was a problem hiding this comment.
Add error handling for the mkdir command to ensure that any issues during directory creation are properly managed. This applies to the mkdir command in getBuilderAddr as well.
| @@ -746,6 +749,15 @@ actionsToolkit.run( | |||
| } | |||
| if (stateHelper.dockerBuildStatus != '') { | |||
| try { | |||
| if (stateHelper.dockerBuildStatus !== 'success' && stateHelper.buildRef) { | |||
| try { | |||
| const {stdout} = await execAsync(`sudo buildctl debug logs ${stateHelper.buildRef}`); | |||
There was a problem hiding this comment.
Consider adding more context or retry logic for the execAsync command when fetching buildctl debug logs to handle potential errors more gracefully.
Important
Enhances logging on build failure by capturing debug logs and setting up LLB dump directory in
main.ts.buildctl debug logson build failure ifstateHelper.dockerBuildStatusis not 'success' inmain.ts./tmp/buildkit-llb-dumpsdirectory and setsBUILDKIT_DEBUG_DUMPLLBandBUILDKIT_DUMP_LLB_DIRingetBuilderAddr().buildctl debug logscommand inmain.ts.This description was created by
for 92bff7f. It will automatically update as commits are pushed.