workflows: Windows docker insufficient disk space fix#11671
workflows: Windows docker insufficient disk space fix#11671mabrarov wants to merge 3 commits intofluent:masterfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a Windows-only pre-build PowerShell step that conditionally relocates the machine Changes
Sequence Diagram(s)sequenceDiagram
participant Runner as GitHub Actions Runner
participant PS as PowerShell step
participant FS as Host Filesystem
participant Docker as Docker service
participant Build as Windows image build
Runner->>PS: run Windows-only pre-build step
PS->>FS: check for D:\ existence
alt D:\ exists
PS->>FS: create D:\SystemTemp and apply SDDL ACL (rgba(0,128,0,0.5))
PS->>Runner: set machine env SYSTEMTEMP = D:\SystemTemp
PS->>Docker: restart service (rgba(0,0,255,0.5))
else D:\ missing
PS-->>Runner: emit warning and skip relocation (rgba(255,165,0,0.5))
end
Runner->>Build: proceed with Windows image build
Build->>Build: install Ninja into C:\ninja (rgba(0,0,255,0.5))
Build->>Build: run CMake with generator Ninja and build with Ninja (rgba(0,128,0,0.5))
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
mabrarov
requested review from
celalettin1286,
niedbalski and
patrick-stephens
as code owners
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1b278f99f2
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
mabrarov
force-pushed
the
feature/docker_windows_disk_space
branch
from
1b278f9 to
44cd950
Compare
mabrarov
force-pushed
the
feature/docker_windows_disk_space
branch
from
44cd950 to
2e9ad2b
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/pr-image-tests.yaml:
- Around line 114-127: The call-build-images.yaml workflow is missing the
Windows SYSTEMTEMP relocation workaround present in the pr-image-tests workflow;
add a step mirroring the relocate-system-temp-dir step (same name/id if desired)
into call-build-images.yaml before the Docker build of
dockerfiles/Dockerfile.windows so that when running on windows-2022/windows-2025
runners it checks for D:\, creates D:\SystemTemp, sets the directory ACL, sets
the SYSTEMTEMP machine environment variable and restarts the docker service
(same shell: pwsh) to avoid disk-space failures during the Docker build.
In `@dockerfiles/Dockerfile.windows`:
- Around line 97-115: The Dockerfile downloads and extracts ninja-win.zip
without integrity checks; add checksum verification by introducing an ARG/ENV
like NINJA_SHA256 alongside NINJA_VERSION/NINJA_URL, then after
Invoke-WebRequest compute the file's SHA256 (e.g., via PowerShell Get-FileHash)
and compare to NINJA_SHA256, aborting the RUN step (exit non‑zero) if it does
not match before calling Expand-Archive; if no trusted SHA256 is available,
document that you should either pin a computed checksum for this build, request
official checksums from the Ninja project, or use an alternative verification
(signed artifacts or VirusTotal) and fail the build when verification cannot be
performed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ee76632d-559c-47bd-9f36-f96b2008d7b7
📒 Files selected for processing (2)
.github/workflows/pr-image-tests.yamldockerfiles/Dockerfile.windows
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/pr-image-tests.yaml:
- Around line 116-125: The PowerShell relocation block can fail silently because
commands like New-Item, Set-Acl and Restart-Service do not stop on errors;
update the block to enable fail-fast by setting $ErrorActionPreference = "Stop"
at the start of the script (or just before the D:\ check) and add -ErrorAction
Stop to the calls to New-Item, Set-Acl and Restart-Service so any failure will
throw and fail the job immediately; ensure you reference the existing commands
New-Item, Set-Acl and Restart-Service when making these changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 87d02538-cb5d-4243-ac9d-5b302c8dcc36
📒 Files selected for processing (2)
.github/workflows/pr-image-tests.yamldockerfiles/Dockerfile.windows
🚧 Files skipped from review as they are similar to previous changes (1)
- dockerfiles/Dockerfile.windows
mabrarov
force-pushed
the
feature/docker_windows_disk_space
branch
from
2e9ad2b to
5f50a68
Compare
mabrarov
changed the title
Signed-off-by: Marat Abrarov <abrarov@gmail.com>
mabrarov
force-pushed
the
feature/docker_windows_disk_space
branch
from
5f50a68 to
da03039
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
dockerfiles/Dockerfile.windows (1)
204-214:⚠️ Potential issue | 🟠 MajorResolve Windows+Ninja CMake path before flipping the generator.
Line 204 switches to
Ninja, butCMakeLists.txtlines 1265–1269 resolveEXTERNAL_BUILD_TOOLto$(MAKE)for Windows (the else branch). This is makefile-variable syntax incompatible with Ninja. WhenEXTERNAL_BUILD_TOOLis consumed incmake/libbacktrace.cmakeandCMakeLists.txtfor jemalloc builds, Ninja will not understand the makefile syntax and the build will fail.Add an explicit Windows+Ninja branch in
CMakeLists.txtthat setsEXTERNAL_BUILD_TOOLto Ninja-compatible syntax before merging this Dockerfile change, or revert toNMake Makefileshere until the CMake fix exists.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dockerfiles/Dockerfile.windows` around lines 204 - 214, The Dockerfile switches CMake to the Ninja generator but CMakeLists.txt currently sets EXTERNAL_BUILD_TOOL to makefile-style $(MAKE) on Windows which Ninja cannot consume; update CMakeLists.txt (and cmake/libbacktrace.cmake if it also references it) to add a Windows+Ninja branch that sets EXTERNAL_BUILD_TOOL to a Ninja-compatible invocation (e.g., "ninja" or the appropriate ${CMAKE_MAKE_PROGRAM} value) before the logic that uses EXTERNAL_BUILD_TOOL (search for EXTERNAL_BUILD_TOOL and the jemalloc build sections), or revert this Dockerfile change to use "NMake Makefiles" until that CMake branch is added.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@dockerfiles/Dockerfile.windows`:
- Around line 204-214: The Dockerfile switches CMake to the Ninja generator but
CMakeLists.txt currently sets EXTERNAL_BUILD_TOOL to makefile-style $(MAKE) on
Windows which Ninja cannot consume; update CMakeLists.txt (and
cmake/libbacktrace.cmake if it also references it) to add a Windows+Ninja branch
that sets EXTERNAL_BUILD_TOOL to a Ninja-compatible invocation (e.g., "ninja" or
the appropriate ${CMAKE_MAKE_PROGRAM} value) before the logic that uses
EXTERNAL_BUILD_TOOL (search for EXTERNAL_BUILD_TOOL and the jemalloc build
sections), or revert this Dockerfile change to use "NMake Makefiles" until that
CMake branch is added.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c0d5486d-97c3-4664-8273-e0019d8c95b0
📒 Files selected for processing (3)
.github/workflows/call-build-images.yaml.github/workflows/pr-image-tests.yamldockerfiles/Dockerfile.windows
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/call-build-images.yaml
- .github/workflows/pr-image-tests.yaml
Signed-off-by: Marat Abrarov <abrarov@gmail.com>
Signed-off-by: Marat Abrarov <abrarov@gmail.com>
mabrarov
force-pushed
the
feature/docker_windows_disk_space
branch
from
da03039 to
0d6bbac
Compare
1 participant
Summary
-joption, NMake doesn't support parallel builds).Testing
ok-package-testlabel to test for all targets (requires maintainer to do).Documentation
Backporting
Notes
Documentation about workloads and components of Visual Studio Build Tools 2022: https://github.com/MicrosoftDocs/visualstudio-docs/blob/main/docs/install/includes/vs-2022/workload-component-id-vs-build-tools.md#desktop-development-with-c
Documentation about workloads and components of Visual Studio Build Tools 2019: https://github.com/MicrosoftDocs/visualstudio-docs/blob/main/docs/install/includes/vs-2019/workload-component-id-vs-build-tools.md#desktop-development-with-c
The 0d6bbac commit is optional. It is added into this PR to ensure build of docker images for Windows containers is triggered on CI.
This PR was integrated into #11397 for additional testing of building docker images for Windows containers.
Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.
Summary by CodeRabbit