Request for Merge#255
Open
kerklangsi wants to merge 14 commits into
Open
Conversation
Introduces the AMP_LICENCE environment variable and a new configure_license routine to reactivate the AMP licence across all instances if provided. The main entrypoint script now calls this routine during startup.
This update installs the Docker CLI and introduces a docker-wrapper script to dynamically rewrite volume and mount arguments, enabling Docker-out-of-Docker scenarios. It also adds logic to detect and export the host AMP data directory, improves AMP user/group creation (including Docker group detection), enhances file permission checks, and refines entrypoint routines for better startup and error handling.
Moved group creation logic to a new create_group_user function and enhanced detection of Docker group and Docker Desktop remote API. Improved AMP user creation to ensure correct group assignment and verification. Removed unused environment variables from Dockerfile.
Changed variable name from APP_GROUP_GID to APP_GID for consistency and clarity in the create_group_user function.
There was a problem hiding this comment.
Pull request overview
This pull request enhances the AMP Dockerized container with Docker-out-of-Docker capabilities, dynamic user/group management, and improved startup routines. However, critical bugs in the code must be fixed before merging, including duplicate function declarations, dead code, and incorrect Docker API usage.
- Added Docker CLI installation and a wrapper script for volume mount path translation between container and host contexts
- Refactored group/user creation logic to support Docker, Docker Desktop, and custom group configurations
- Introduced new lifecycle functions for license reactivation and host home directory detection
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| entrypoint/utils.sh | Added documentation comments for existing utility functions |
| entrypoint/routines.sh | Added group/user creation refactoring, license configuration, and host home detection functions (contains critical bugs) |
| entrypoint/main.sh | Updated startup sequence to call new group/user and license configuration functions |
| entrypoint/docker-wrapper.sh | New wrapper script that intercepts Docker CLI commands and rewrites volume mount paths |
| Dockerfile | Added Docker CLI installation, line ending normalization, and docker-wrapper installation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces significant improvements to the AMP Dockerized container, focusing on robust Docker CLI integration, dynamic user/group management, enhanced volume mount handling, and improved startup routines. The changes make the container more portable and compatible with various Docker environments, including Docker Desktop, and ensure correct file permissions and licensing. Additionally, new utility and lifecycle functions have been added for better maintainability and extensibility.
Docker CLI Integration and Volume Mount Handling
entrypoint/docker-wrapper.sh) rewrites volume and mount arguments to map container paths to host paths, improving compatibility with different host setups. ([[1]](https://github.com/MitchTalmadge/AMP-dockerized/pull/255/files#diff-dd2c0eb6ea5cfc6c4bd4eac30934e2d5746747af48fef6da689e85b752f39557R227-R241),[[2]](https://github.com/MitchTalmadge/AMP-dockerized/pull/255/files#diff-cc2d14f650b5b6dd4880aba3ac9a643e2c43342f34f41ea9fc5bea4a886fbcf7R1-R166))User and Group Management
[[1]](https://github.com/MitchTalmadge/AMP-dockerized/pull/255/files#diff-5a6c28f9c763a2cf3c6842494b13a8042ace99593040059c95e7857b03cebf8aR115-R256),[[2]](https://github.com/MitchTalmadge/AMP-dockerized/pull/255/files#diff-57b215d96aecc27cc31cf53345df84baa814d7108e09b3acf91058a790f7393fR42-R43))Startup and Lifecycle Enhancements
configure_license), host home detection (amp_host_home), and improved startup sequence by running user/group creation and license configuration before AMP initialization. ([[1]](https://github.com/MitchTalmadge/AMP-dockerized/pull/255/files#diff-57b215d96aecc27cc31cf53345df84baa814d7108e09b3acf91058a790f7393fR54-R57),[[2]](https://github.com/MitchTalmadge/AMP-dockerized/pull/255/files#diff-5a6c28f9c763a2cf3c6842494b13a8042ace99593040059c95e7857b03cebf8aR115-R256))File Permissions and Environment Configuration
/home/amp, ensuring the directory is writable for the AMP user/group and providing warnings and corrective actions if not. ([entrypoint/routines.shR49-R64](https://github.com/MitchTalmadge/AMP-dockerized/pull/255/files#diff-5a6c28f9c763a2cf3c6842494b13a8042ace99593040059c95e7857b03cebf8aR49-R64))Utility and Command Functions
[[1]](https://github.com/MitchTalmadge/AMP-dockerized/pull/255/files#diff-3e48cba4cf91540e103717b7da6c12592ea3f78173863be71f94b94c5bc49a9eR22),[[2]](https://github.com/MitchTalmadge/AMP-dockerized/pull/255/files#diff-3e48cba4cf91540e103717b7da6c12592ea3f78173863be71f94b94c5bc49a9eR32-R42))These changes collectively enhance the reliability, flexibility, and usability of the AMP Dockerized container, making it easier to run in diverse environments and simplifying administration and automation.