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

Support for nested stacks directory #687

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

Conversation

mkoo21
Copy link

@mkoo21 mkoo21 commented Dec 13, 2024

⚠️⚠️⚠️ Since we do not accept all types of pull requests and do not want to waste your time. Please be sure that you have read pull request rules:
https://github.com/louislam/dockge/blob/master/CONTRIBUTING.md

Tick the checkbox if you understand [x]:

  • I have read and understand the pull request rules.

Description

This PR accompanies the discussion in #214.

Currently, dockge assumes all stacks are exactly one level deep in the stacks directory, and the "name" of a project equals the name of the folder containing it. This assumption is not quite right, and leads to weird behavior where dockge can be "tricked" into thinking a random folder is a docker compose project, or will simply not distinguish between two separate projects.

Some examples of where this assumption breaks:

  • you have multiple projects living in the same folder, perhaps with more than one config file (e.g. $stacksDir/project/compose-dev.yml and $stacksDir/project/compose-prod.yml). If there is also a compose.yml dockge will assign both projects the name "project" and not distinguish between them.
  • you have multiple instances of one project (e.g. $stacksDir/application1/authentik and $stacksDir/application2/authentik). Dockge will assign both projects the name authentik and will not distinguish them.
  • All of these also apply in case of a naming conflict with a project that is run from outside the configured stacks directory (e.g. $notStacksDir/application3/authentik will produce yet another naming conflict).
  • If the config file name (like compose-dev.yml is not in the acceptedComposeFileNames) dockge will simply not work with this project. This behavior is undocumented.

In these cases the user will tell docker compose to tag the project with different names. So docker compose will be aware of the difference but dockge will behave strangely. Plus, people with many (say, 20+) projects running on one machine will most certainly want to use some nesting to organize their stacks directory. For these reasons, I think letting docker compose should be the main source of truth.

A real example I had while testing this is I had a server directory I want to use as my stacksDir that contains a folder called authentik. This folder is actually a volume mount for my authentik project, and does not contain a config file. The actual authentik stack is running from a different location on my machine. Even though dockge tries to skip folders that don't contain an acceptedComposeFilename, the name of the folder matches the name of a project that appears in the output of docker compose ls, so dockge thinks it is a project managed by dockge and then says it cannot find the configuration file:

Screenshot From 2024-12-14 19-14-32

This PR treats docker compose ls as the main source of truth, and puts the folder scan afterwards as a "nice to have" for discovering unknown projects. And it fixes the issue described above!

Screenshot From 2024-12-14 22-04-04

There are two commits in this PR that cover the two pieces of functionality changed. They can be reviewed separately, and the first half can be merged independently if you don't like the second. This changes some existing functionality, but everything that used to work should work the same as before, if not slightly better.

The first part focuses on the stacks.ts logic that assumes a stack "managed by dockge" means it lives in the folder $stacksDir + $projectName. It asks docker compose for a list of projects and treats all the projects where the config file starts with stacksDir as being managed by dockge (current master does a folder search then asks docker compose for services not managed by dockge, but they all go into the string map which can cause conflicts).

The second part restores dockge's ability to "discover" projects that had never been run through the CLI, by adding the folder search after the call to docker compose. But it makes that search recursive using node FS. In case of a naming conflict between a "known" project and a "discovered" project, it keeps the "known" project but adds some extra logging and makes dockge's behavior extra transparent. Naming conflicts between projects "known" to docker compose will still happen if the user makes an error using docker compose (e.g.. not tagging projects with names), but at least dockge's behavior will match docker compose's.

Fixes #(issue)
#214

Type of change

Please delete any options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Checklist

  • My code follows the style guidelines of this project
  • I ran ESLint and other linters for modified files
  • I have performed a self-review of my own code and tested it
  • I have commented my code, particularly in hard-to-understand areas
    (including JSDoc for methods)
  • My changes generate no new warnings
  • My code needed automated testing. I have added them (this is optional task)

Screenshots (if any)

Please do not use any external image service. Instead, just paste in or drag and drop the image here, and it will be uploaded automatically.

@mkoo21 mkoo21 force-pushed the master branch 4 times, most recently from 33de00a to 1f594d5 Compare December 15, 2024 01:03
@mkoo21 mkoo21 changed the title draft support for nested stacks directory Support for nested stacks directory Dec 15, 2024
@mkoo21 mkoo21 force-pushed the master branch 3 times, most recently from 67d710b to e17ede3 Compare December 15, 2024 02:25
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.

1 participant