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

App Submission: Dockge #1106

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

Conversation

FlyinPancake
Copy link

App Submission

Dockge

256x256 SVG icon

https://dockge.kuma.pet/icon.svg

Gallery images

dockge-1
dockge-2
dockge-3
dockge-4

I have tested my app on:

  • umbrelOS on a Raspberry Pi
  • umbrelOS on an Umbrel Home
  • umbrelOS on Linux VM

Copy link
Contributor

@highghlow highghlow left a comment

Choose a reason for hiding this comment

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

Thanks for the submission! I have suggested some changes to it

dockge/umbrel-app.yml Outdated Show resolved Hide resolved
stop_grace_period: 1m
restart: on-failure
environment:
DOCKER_ENSURE_BRIDGE: "dind0:10.32.0.1/16"
Copy link
Contributor

Choose a reason for hiding this comment

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

What's this IP?

Copy link
Author

Choose a reason for hiding this comment

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

This is copied from portainer. That being said, it might be interesting to see if Dockge would clash with Portainer noew

Copy link
Contributor

Choose a reason for hiding this comment

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

Two host bridges with the same IP reservations and different names, portainer and dockge, it probably will crash. You could use dind0:10.33.0.1/16, it is not being used by any other app.

Copy link
Contributor

@joaovictor-local joaovictor-local Jul 1, 2024

Choose a reason for hiding this comment

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

I was wrong about it, it will not crash but I still think it would be good to have a different ip reservation range for each app.

dockge/umbrel-app.yml Outdated Show resolved Hide resolved
dockge/umbrel-app.yml Outdated Show resolved Hide resolved
dockge/umbrel-app.yml Outdated Show resolved Hide resolved
dockge/umbrel-app.yml Outdated Show resolved Hide resolved
@highghlow
Copy link
Contributor

Now we'll have to wait for nmfretz

FlyinPancake and others added 2 commits May 27, 2024 08:51
Co-authored-by: highghlow <132668972+highghlow@users.noreply.github.com>
@nmfretz
Copy link
Contributor

nmfretz commented Jun 27, 2024

Hey, sorry for the delay on this @FlyinPancake. I'll look at this soon. Thanks very much for the submission!

dockge/docker-compose.yml Outdated Show resolved Hide resolved
stop_grace_period: 1m
restart: on-failure
environment:
DOCKER_ENSURE_BRIDGE: "dind0:10.32.0.1/16"
Copy link
Contributor

@joaovictor-local joaovictor-local Jul 1, 2024

Choose a reason for hiding this comment

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

I was wrong about it, it will not crash but I still think it would be good to have a different ip reservation range for each app.

@nmfretz
Copy link
Contributor

nmfretz commented Aug 29, 2024

Hi @FlyinPancake, I have triggered our new github actions linter to start the review process (see above). The relevant changes that need to be made are adding the three missing host volume mount directories to this repo:

image

cc @sharknoon I'm thinking we should change the linter severity on the missing volume mounts to "error" since we want to avoid unintended permissions issues.

@FlyinPancake you can check Immich here to see how we do it (.gitkeep files in empty directories): https://github.com/getumbrel/umbrel-apps/tree/master/immich/data
It is a mistake on our part that we didn't include these in the Portainer app (does not result in errors though because the app services run as root).

In the meantime I will take a look and make sure that our entrypoint.sh and 'dind' service environment variables from the Portainer app can be duplicated here without issue.

FlyinPancake and others added 2 commits September 11, 2024 14:02
Co-authored-by: joaovictor.local <me@joaovictor.email>
@getumbrel getumbrel deleted a comment from github-actions bot Sep 18, 2024
@getumbrel getumbrel deleted a comment from github-actions bot Sep 18, 2024
@getumbrel getumbrel deleted a comment from github-actions bot Sep 18, 2024
@nmfretz
Copy link
Contributor

nmfretz commented Sep 18, 2024

@FlyinPancake I have tested Dockge and it is working well, great work! I have pushed a few commits to clean up the app description, tweak bind mounts, and change the port so that it doesn't clash with other apps and result in a failed install.

We are currently getting Gallery assets prepared and then we will go live!


As a note for our records, there appears to be a bug in Dockge right now that prevents certain valid compose.yml from working like it should. This is out of our hands and is not a deal breaker for this to launch, but I'm noting it here:

As an example, if you add a valid named volume like this:

volumes:
  metube_downloads:

Dockge changes it automatically to

volumes:
  metube_downloads:  null

Which results in Dockge throwing an error and not bringing up the compose project:

service "metube" refers to undefined volume metube_downloads: invalid compose project.

Someone else has documented it here in this Issue: louislam/dockge#595

It's easy enough to get around (by specifying additional options after the volume name), but it's unfortunate because in order to persist data across app restarts, users need to use named volumes and so may run into this exact error.

@sharknoon
Copy link
Contributor

I think in yaml (which is a superset of json) you can do the following

volumes:
  myvolume: {}

That should prevent assigning null.

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