Feature: Improve Development Setup#795
Conversation
…files - Introduced docker-compose.dev.yml for development services including phpMyAdmin and Mailhog. - Added docker-compose.discourse.yml for Discourse services including PostgreSQL, Redis, and Sidekiq. - Removed existing phpMyAdmin and Discourse configurations from docker-compose.yml to streamline service management. By extracting these services into separate files we can build up the stack on a case by case basis. This will free up host machines resources and make it easier to manage the development environment. I will be creating some development scripts in the future to help with the setup and tearing down of the development environment.
This cleans up the comments in the docker-compose files since I had originally created each docker-compose file by `cp docker-compose.yml`. Some of the comments were removed in some of the files because they referenced services that were not present in the file. Additionally, I refactored some of the comments to be more linear and easier to read.
Taskfile.yml was chosen over Makefile primarily because its YAML syntax is significantly easier to read and understand. This improves maintainability and makes it more approachable to folks who may not be familiar with the syntax of Makefiles. Additionally, Taskfile is also designed as a general task runner, unlike the build-focused nature of Makefiles. This makes it a better fit for managing diverse workflows, automating steps, and helper functions beyond just compilation. For now though, we will only be using it to manage the Docker containers while we work on refactoring the app setup process. That said, we could have gone with bash scripts to manage these tasks, but bash scripts become increasingly difficult to manage and read as the logic grows. Writing intricate functions, handling argument parsing, implementing error checking, and managing dependencies between steps often leads to bash code that is verbose, hard to follow, and challenging to debug and maintain. Taskfile provides a more structured and familiar syntax to orchestrate the tasks.
Taskfiles allow the use of wildcards to pass arguments as part of the task name. This allows us to reduce the number of tasks since we don't need to create a task for each combination of containers we want to start or stop. For example, docker:up:dev, docker:up:discourse, etc. One caveat is that Taskfiles do not currently support lazy variable lookup. As such, we can't save the output of a dependent task to a variable and reuse it in a subsequent task. This is a bit of a pain, but for our use case, we can live with it. The workaround then was to use a map variable to act as a switch statement to determine which stack to start or stop in line. This requires the map experimental feature to be enabled, which we have done via the .env file.
Similar to the prior commits, let's add a task for rebuilding the Docker containers.
By leveraging YAML anchors and variables, we can dedupe a lot of the task configurations for the docker wildcard tasks.
Similar to the prior commits, let's add a task for restarting the Docker containers.
This adds some miscellaneous tasks that will be useful for development, such as following the logs of the main app container, opening a shell into the container, and running specific commands within the container.
Some folks, or machines, may still be using an older version of Docker where Docker Compose has not been fully integrated into Docker. As such, we should check if the docker-compose command exists and use that for all docker compose commands. Otherwise, we will use the docker compose command.
Instead of using multiple docker-compose files, we can use docker compose profiles to build up the stack on a case by case basis. This will simplify things since there will only be one docker-compose file, while we can still retain the ability to configure the different services as needed.
There was a problem hiding this comment.
Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- .env.example: Language not supported
Comments suppressed due to low confidence (1)
docs/local-development.md:25
- The line containing '****' appears extraneous and could lead to formatting issues in the rendered documentation. Consider removing it to improve clarity.
****
edwh
left a comment
edwh
left a comment
There was a problem hiding this comment.
Generally this is looking excellent. But a few comments and I'm not sure it's ready to merge yet.
|
Couple of other things from testing.
Possibly for this reason, when I changed some of front-end code (e.g.
FYI: Often during development I run with |
This updates the docs to exclusively reference Task commands and removes the alternative Docker commands. As such, we are indicating that Task is now mandatory. Following this, I've moved the Task experimental env variable to a dedicated `.taskrc.yml` file. This will be more appropriate and easier to manage then having it in the `.env.example` file.
This addresses TheRestartProject#795 (comment) We will now use `localhost` for the default `APP_URL` in the `.env.example` file. As such, the docs have been updated to reflect this change.
This addresses TheRestartProject#795 (comment): - Replaces all references of "base" with "core" - Consolidates all Docker tasks into their wildcard counterparts. - Instead of having a default `docker:x` task, we will now just use the `docker:x-*` variant to interact with the core services. - Replaces all references of "dev" with "debug". - The "dev" services are in regards to phpMyAdmin and Mailhog, which are actually more debugging tools than services needed for "dev". - Add profiles to the core services in the docker-compose configuration. - The core services will now use the core profile to keep things consistent. That said, we need to include the other profiles as well so other services can automatically start and stop the core services without us having to bloat the Taskfile - i.e. include multiple --profile flags. - Update the documentation to reflect the new terminology and tasks.
This commit removes the deprecated `image: php:7.4` line from the docker-compose configuration since we already declare the image in the Dockfile.
Ignore node_modules folder to help with issues on Windows.
|
@edwh I tested it on my Windows machine but I did not see the That said, I have seen this error before and applied the following patch. 4d1842c This was needed for a Fedora machine we have that is a running an older version of Docker (v24.0.5) As for the changes not being picked up, I found the following that could explain the underlying cause and have confirmed a solution:
Basically, Laravel's Mix, aka Webpack, is not receiving the file change events. Therefore, if we instead use Mix's polling feature, then we can pick up the changes like that. We already have the poll script in the Line 7 in 2922407 npm watch then the Vue changes will be picked up.
I have confirmed this worked on my Windows machine. |
I forgot to update this reference in the docs when I was consolidating the terminology for the docker commands.
I'm running Docker Desktop 4.30.0, which is pretty recent. I tried added that change to the Dockerfile and I still see the error. If you don't see it, then I will need to debug though I'm not sure I can see any consequences of it yet, because...
Me too. Can we make this the default, then? The main issue for me now which would block use of this is speed:
|
@ardelato I tried this and it's much faster for me. So rather than run the
Can you see if that performs well for you? If so then we can document this as the recommended way of running on Windows. |
|
Did you follow similar steps as outlined in this blog post? https://medium.com/@suyashsingh.stem/increase-docker-performance-on-windows-by-20x-6d2318256b9a If so, then I can confirm that the Docker build speed and up speed were a lot faster. However, I came across a different issue which has to do with ownership of the mounted volumes and thus get a permission denied error when trying to run |
Yes. I don't remember seeing a permissions error - I'll come back here next time I try it. |
|
Coming back to it, I remembered that I needed to do an The change to use |
|
@edwh could you share your WSL setup and steps you've taken? I can't seem to properly get the same results you are getting. Even after using |
Include a note about what the 'All' option does and a reference to the docker-compose.yml file for more information.
There are some issues with the watch command on different operating systems. The watch command relies on the File System Events API to detect changes in the file system but in Windows this may be bypassed because of the multiple layers of virtualization. Therefore a polling mechanism is more reliable.
This feels like something which would work better if you ping me on Slack and we do a screen share. But:
|
|
@ardelato I think this is still with you for some final doc tweaks? |
By utilizing the official PHP image, we can remove the need for some PHP extensions to be installed. In addition, by removing the SSH setup, we can remove the telnet dependency and additional sshd configuration. From here, we can further improve the dependency installation process by utilizing the install-php-extensions tool which will install the dependent system packages, i.e. lib*-dev, etc, install the PHP extension, and finally clean up after itself. Utilizing this tool we can install the additional PHP extensions we need that are not included in the official PHP image. One thing to note, we will pin the install-php-extensions tool to the current latest version for security purposes.
By default, Docker uses the root user as such the need to specify the user in the docker-compose.yml file and the usage of sudo in the Dockerfile are unnecessary. We will be adding a non-root user in a later commit to handle the file permissions of the mounted volumes.
The working directory should be set after we've finished installing all the dependencies and will start working with the actual app code. In addition, we can drop the copy composer.lock and composer.json line as they were already copied from the "." copy line.
Instead of installing composer v1 and using the phar file, we will now just use a single instance of composer which will be installed in the Dockerfile.
We were getting an error about the git directory not being safe for the mounted codebase.
Introduces GID mapping between host and container to resolve file permission conflicts when mounting volumes. The host's group ID is automatically retrieved via Taskfile and passed as a build argument to modify the container's www-data group. This ensures both environments share the same group permissions. Key changes: - Add GID build argument to Dockerfile with host group ID detection - Configure www-data group to match host GID for consistent file access - Set proper home directory for www-data to prevent cache pollution in mounted volumes - Enables read/write operations on mounted files from both host and container This resolves permission issues across different Docker environments (Docker CE, Docker Desktop) and operating systems where virtualization and mounting behaviors vary. In addition, it ensures tools like composer and npm do not create cache artifacts in the project directory -- i.e. the mounted directory.
Handle the case where the GID already exists
…sions - Introduced a new Nginx service in docker-compose.yml to serve the application. - Updated Dockerfile to create a non-root user with customizable UID and GID for better permission handling. - Modified docker_run.sh to use php-fpm instead of the built-in server. - Added nginx.conf for Nginx configuration to handle PHP requests and static files. These changes enhance the application's deployment and avoid conflicts with the built-in www-data user and group.
The file permissions may be wrong if we mount the host directory into the container. As such we need to chown the files on startup. This is similar to the example Docker has documented for the Laravel project. https://github.com/dockersamples/laravel-docker-examples/blob/main/docker/development/php-fpm/entrypoint.sh
Instead of chowning all files, let's target only the directories where the content will change between the host and the container.
The current gitignore files are set with 0644 permissions, but we were changing the permissions to 0755. As such, there was git diff from this change.
|
We should be good now. I included the changes for resolving the permission issues we were seeing in Windows and updated the docs accordingly. |
|
@edwh I am not really sure what to do about the Docker errors |
I agree. I've marked those two as safe. |
|
Description
This pull request introduces two major changes to enhance the local development setup for the Restarters project.
The changes include:
Taskfile.ymlfor task managementdocker-compose.ymlfile to support different development profilesIn addition, I've added documentation surrounding these changes.
CR Notes:
Commit 703d8b6 goes over the reason for choosing Task as opposed to a
bashscript orMakefile.By utilizing Docker Compose Profiles we can customize which services we would spin up locally. That way we do not utilize more resources than needed for local development.
As for the documentation, it goes over the prerequisites and setup for local development, as well as examples of how to run frequent actions. That said, I added the document to
/docsbut I am not sure if that is the correct location for it.