Skip to content

build image log fix#1375

Merged
giurgiur99 merged 1 commit into
mainfrom
fix/buildimage-log-path
May 19, 2026
Merged

build image log fix#1375
giurgiur99 merged 1 commit into
mainfrom
fix/buildimage-log-path

Conversation

@giurgiur99
Copy link
Copy Markdown
Contributor

@giurgiur99 giurgiur99 commented May 19, 2026

Fixes # .

Changes proposed in this PR:

BuildImage builds its log file path from tempFolder instead of getStoragePath() (which is tempFolder + clusterHash). The job folder is created under getStoragePath(), so writes target a non-existent directory.

Fix

  • buildImage: use getStoragePath() for imageLogFile. Aligns with pullImage.
  • buildImage.test.ts: mkdirSync updated to match.

@giurgiur99 giurgiur99 marked this pull request as ready for review May 19, 2026 06:50
@giurgiur99
Copy link
Copy Markdown
Contributor Author

test with a job stopped mid-build

image

@giurgiur99
Copy link
Copy Markdown
Contributor Author

/run-security-scan

Copy link
Copy Markdown
Member

@alexcos20 alexcos20 left a comment

Choose a reason for hiding this comment

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

AI automated code review (Gemini 3).

Overall risk: low

Summary:
This PR updates the C2D Docker engine to correctly resolve the image.log file path using getStoragePath() rather than a hardcoded reference to tempFolder. The unit tests have been updated accordingly to mirror this logic. Overall, the logic is sound and correctly addresses path resolution inconsistencies. LGTM!

Comments:
• [INFO][style] Consider using path.join instead of string concatenation for constructing file paths. This ensures better cross-platform compatibility and avoids subtle bugs with missing or duplicate directory separators.

-    const imageLogFile = this.getStoragePath() + '/' + job.jobId + '/data/logs/image.log'
+    const imageLogFile = path.join(this.getStoragePath(), job.jobId, 'data', 'logs', 'image.log')

(Ensure path is imported at the top of the file if you apply this change).
• [INFO][other] Good job dynamically fetching the expected storage path from the engine instance rather than hardcoding the temp folder. This keeps the test robust against future underlying path logic changes.

@giurgiur99 giurgiur99 merged commit f81d8a7 into main May 19, 2026
11 checks passed
@giurgiur99 giurgiur99 deleted the fix/buildimage-log-path branch May 19, 2026 07:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants