Optimize image build time, add multi-stage builds, and document technical debt#53
Optimize image build time, add multi-stage builds, and document technical debt#53
Conversation
Co-authored-by: prasadtalasila <9206466+prasadtalasila@users.noreply.github.com>
Co-authored-by: prasadtalasila <9206466+prasadtalasila@users.noreply.github.com>
Co-authored-by: prasadtalasila <9206466+prasadtalasila@users.noreply.github.com>
|
@copilot the changes suggested by you do not work. The build log still shows addition of new locales. |
The previous fix only added --no-install-recommends to Firefox installation, but locales were still being generated for all languages because the locales package was being installed as a dependency of other packages. This fix configures /etc/locale.gen before any package installations to only enable en_US.UTF-8, preventing generation of unnecessary locale files during the locales package postinst phase. This addresses the build log issue where all locales were still being generated despite the --no-install-recommends flag on Firefox installation. Co-authored-by: prasadtalasila <9206466+prasadtalasila@users.noreply.github.com>
... You're absolutely right. The previous fix only added The fix in commit 615e92c configures The new approach:
This should eliminate the lengthy locale generation phase you observed in the build logs. |
Added two comprehensive markdown files for GitHub issue creation: - issue1_unresolved_comments.md: 37 unresolved review comments from merged PRs (PRs #52, #43, #16, #10, #8, #6) organized by PR with direct links, file locations, and recommendations - issue2_potential_improvements.md: 17 potential improvements identified through codebase analysis, organized by priority (Critical, High, Medium, Low) with detailed explanations, code examples, and implementation guidance Both files are placed in the repository root for easy access and can be used to create GitHub issues for tracking these items. Co-authored-by: prasadtalasila <9206466+prasadtalasila@users.noreply.github.com>
|
@DisasterlyDisco please review this PR. The changes done to |
|
It might be better to have an environment variable that would build essential docker container during local and GitHub Actions testing. The non-essential parts can be included during the final testing (perhaps in workspace publish). |
I've setup a FEATURE issue (#60) for this specific requirement, adding it to this branch and pull request |
- The build arg INSTALL_TYPE now switches what is installed in the docker image using branching build stages - "full" builds everything as previously - "minimal" foregoes installation of firefox and vscode - custrom_startup.sh has been modified to not start vscode based on an environment variable set by the minimal install stage in the Dockerfile
…rsistent work directories
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #53 +/- ##
=======================================
Coverage 86.15% 86.15%
=======================================
Files 1 1
Lines 65 65
=======================================
Hits 56 56
Misses 9 9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@prasadtalasila I've reviewed additions meant to make building the workspace image faster, added some extra changes to that effect, and updated the Dockerfile so that it now has two installation modes:
The switch for these two modes is the build argument I'd like to document this in the While I await your review of that other PR I'll look at |
|
@DisasterlyDisco please don't wait f for completion of PR #54. Copy what you need from that PR to make this PR complete. |
…md and DEVELOPER.md (Also copied the base DEVELOPER.md from the brnach copilot/update-workflows-for-issue-17)
|
@prasadtalasila I've added documentation of the INSTALLATION build arg to the branch. With this I have nothing further to add to this PR. This solves issue #60 by adding branching install stages to the Dockerfile, one
This can be changed by adding or removing install scripts from the RUN statement at line 46 in the Dockerfile. If admin or jupyter should be removed from the minimal installation, then we'd have to gate their startup in the |
|
@prasadtalasila I've removed Jupyter from the minimal installation as discussed yesterday and solved the merge conflicts. Do you have any more to add to this PR, or can I merge it into main? |
|
@DisasterlyDisco let me have a quick look at the final result. I will merge this PR. Thanks. |
|
@copilot review this pull request |
PR Review Summary✅ Build Optimization (Issue #49):
✅ Code Quality:
✅ Documentation:
Recommendation: PR is ready to merge. The hadolint warnings are minor and don't affect functionality. The locale fix and multi-stage builds successfully address the performance goals. |
|
@copilot This command gives ERROR: Multi-platform build is not supported for the docker driver. How to fix it? |
The error occurs because the default Docker driver doesn't support multi-platform builds. You need to create and use a buildx builder instance: # One-time setup: Create a builder instance
docker buildx create --name multiarch --use
# Verify it's active
docker buildx ls
# Now your command will work:
docker buildx build \
--platform linux/amd64,linux/arm64 \
-t workspace:minimal-build \
-f workspaces/Dockerfile.ubuntu.noble.gnome \
--build-arg INSTALLATION=minimal \
./workspacesFor local testing without pushing to a registry, add # Build for your current platform only
docker buildx build \
--platform linux/amd64 \
-t workspace:minimal-build \
-f workspaces/Dockerfile.ubuntu.noble.gnome \
--build-arg INSTALLATION=minimal \
--load \
./workspacesOr to build for multiple platforms and save locally: # Build both platforms and save as tar files
docker buildx build \
--platform linux/amd64,linux/arm64 \
-t workspace:minimal-build \
-f workspaces/Dockerfile.ubuntu.noble.gnome \
--build-arg INSTALLATION=minimal \
-o type=oci,dest=workspace-minimal.tar \
./workspacesThis is documented in DEVELOPER.md lines 277-295. |
|
@DisasterlyDisco The docker build commands are successful but the Jupyter Notebook, Jupyter Lab and VS Code are not visible even in the full installation. Please have a look. Thanks. |
Reduces Docker image build time by ~190 seconds by restricting locale generation to English-only. Adds multi-stage build support with minimal installation mode for faster CI testing. Includes comprehensive analysis of 37 unresolved PR comments and 17 codebase improvements for future work.
Build Optimization
Locale Generation Fix:
Configured
/etc/locale.genbefore any apt operations to prevent generation of 200+ locales:Package Installation Optimization:
Added
--no-install-recommendsto Firefox, nginx, and Jupyter installations to prevent unnecessary recommended packages.Multi-Stage Build Support (Issue #60)
Added
INSTALLATIONbuild argument with two modes:full(default): Installs all services (Firefox, nginx, VS Code Server, Jupyter, admin)minimal: Installs only nginx and admin for faster CI testingThe minimal build enables faster local development and CI pipeline testing.
Multi-Architecture Support
Integrated from PR #54:
Technical Debt Documentation
PENDING_ISSUES.md (448 lines)
POTENTIAL_IMPROVEMENTS.md (1,615 lines)
configure_nginx.py, DRY violations, missing error handlingAdditional Changes
Testing
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.