Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Separate system services in directories #38

Open
wants to merge 19 commits into
base: master
Choose a base branch
from

Conversation

rugbymauri
Copy link
Contributor

Instead of listing all Services in the docker-compose.yml, each services as his own docker-compose.yml file.

Services can be enabled and disabled, individually.

Allows to add new Services in the DDE-System.

@tuxes3
Copy link
Member

tuxes3 commented Jan 17, 2023

good idea, thanks! please improve the update guide before we can merge this!

@tuxes3
Copy link
Member

tuxes3 commented Jan 17, 2023

@rugbymauri can be merged!

@s4mpl3d
Copy link
Member

s4mpl3d commented Mar 15, 2023

Nice work @rugbymauri !

Only thing that's missing for me, is functionality to define default services that should be started on a project basis.
If you e.g need to use memcached, redis, postgres and mailhog for a project, you would have to run 6 commands before the system and application are up and running.
Maybe a global user-configuration would be sufficient, to at least start the most used services 🤔 .

@tuxes3 what's your opinion?

@tuxes3
Copy link
Member

tuxes3 commented Mar 15, 2023

What about introducing a ddefile which when present lists all needed services? While executing up dde checks for these services. This prerequisites that all services are defined in the system directory.

Most project will use the current default. Therefore if the ddefile is not implemented the 6 commands will only be executed once. Even if we implement the ddefile we will all need to execute this 6 commands. As you also first need to update each project.

@s4mpl3d
Copy link
Member

s4mpl3d commented Mar 15, 2023

@tuxes3 so on a project basis?

Even if we implement the ddefile we will all need to execute this 6 commands. As you also first need to update each project.

We should automate that too. We could introduce a system:update command, which checks the current folder for this file and starts all services defined (also extend system:up). We should also include an option to avoid stopping other containers in the case we are jumping between projects with different setups

_ddeCheckNetwork

system:services:update dnsmasq
system:services:enable dnsmasq
Copy link
Member

Choose a reason for hiding this comment

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

@rugbymauri the update command should only execute :update, not :enable. :enable should be handled by the up command.

Also we should get the list of services dynamically, in case a new service gets added

for f in *; do
if [ -f ${ROOT_DIR}/services/${f}/docker-compose.yml ]; then
_logGreen "Starting service ${f}"
cd ${ROOT_DIR}/services/${f}
Copy link
Member

Choose a reason for hiding this comment

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

This can be handled better

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should be resolved by the commit 879864e


cd services

for f in *; do
Copy link
Member

Choose a reason for hiding this comment

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

Can't we get the list of running services dynamically?

Comment on lines 39 to 45
system:services:update mailhog
system:services:enable mailhog
Copy link
Member

Choose a reason for hiding this comment

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

@s4mpl3d @rugbymauri probably mark as optional?

Copy link
Member

Choose a reason for hiding this comment

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

@xarem we should get a list of currently running services and only update those 👍

@sbaerlocher
Copy link
Contributor

It's fundamentally a sound idea to structure services in such a way that the DDE system starts only those needed, rather than launching everything simultaneously. However, splitting them into multiple Docker-Compose files, each containing just one service, seems overly complex. A more effective approach would be to use Docker Compose's "Profile" feature (refer to Docker Compose Profiles). This feature allows assigning specific profiles to individual services, and only selected profiles are started. Consequently, a docker-compose up without specifying a service doesn't automatically start all containers. Additionally, I suggest creating a .dde.yml file in the respective project directories, which contains information about the required services. When executing dde up, this file would be read, and the relevant services would be initiated. This approach would also simplify documentation, as it eliminates the need to list individually which DDE services need to be started for optimal project development. In Pull Request #64, I've started implementing such a .dde.yml file.

Copy link

gitguardian bot commented Feb 28, 2024

️✅ There are no secrets present in this pull request anymore.

If these secrets were true positive and are still valid, we highly recommend you to revoke them.
Once a secret has been leaked into a git repository, you should consider it compromised, even if it was deleted immediately.
Find here more information about risks.


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

@rugbymauri
Copy link
Contributor Author

i refactored the PR. now, instead of creating/removing a file in the services/conf.d directory the services enabled are stored in the data/dde.yml file

enjoy

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.

5 participants