-
Notifications
You must be signed in to change notification settings - Fork 396
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
App Submission: Dockge #1106
Conversation
There was a problem hiding this 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
stop_grace_period: 1m | ||
restart: on-failure | ||
environment: | ||
DOCKER_ENSURE_BRIDGE: "dind0:10.32.0.1/16" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's this IP?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
Now we'll have to wait for nmfretz |
Co-authored-by: highghlow <[email protected]>
Hey, sorry for the delay on this @FlyinPancake. I'll look at this soon. Thanks very much for the submission! |
stop_grace_period: 1m | ||
restart: on-failure | ||
environment: | ||
DOCKER_ENSURE_BRIDGE: "dind0:10.32.0.1/16" |
There was a problem hiding this comment.
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.
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: 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 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. |
Co-authored-by: joaovictor.local <[email protected]>
@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:
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. |
I think in yaml (which is a superset of json) you can do the following volumes:
myvolume: {} That should prevent assigning null. |
Good point @sharknoon. And actually, I just tried my original yaml format again: volumes:
metube_downloads: And it successfully deployed without issue when Dockge automatically changed it to: volumes:
metube_downloads: null So perhaps I did something else wrong in my testing (e.g., indentation) and incorrectly attributed it to problems with the above format. |
Sadly the author is taking a break from Dockge in preparation for Uptime Kuma V2, so this issue persists |
Thanks again for this wonderful app submission @FlyinPancake. We are now ready to go live. Congratulations on getting your first app on the app store! |
App Submission
Dockge
256x256 SVG icon
https://dockge.kuma.pet/icon.svg
Gallery images
I have tested my app on: