getumbrel / umbrel-apps

The official app repository of the Umbrel App Store. Submit apps and updates here. Learn how → https://github.com/getumbrel/umbrel-apps#readme
https://apps.umbrel.com
516 stars 381 forks source link

Update LNDg to v1.9.0 #1523

Open cryptosharks131 opened 1 week ago

cryptosharks131 commented 1 week ago

Updates LNDg to v1.9.0 https://github.com/cryptosharks131/lndg/releases/tag/v1.9.0 https://github.com/cryptosharks131/lndg/pkgs/container/lndg/278874650?tag=v1.9.0

nmfretz commented 5 days ago

Thanks very much for this update @cryptosharks131!

With the change in the bind mounts for lndg, I'm using this opportunity to modify the data directory structure so that it aligns with most of the other apps on the app store, and also future proofs us by making it easier for us to keep adding additional data directories for lndg if they are needed in the future. For example, if you were to add some other container or bind another directory from the existing lndg container, it could just be bound to: ${APP_DATA_DIR}/data/cool-new-direcotry:/cool/new/directory

The changes I made mean that the structure of the lndg bind mounts on the host will look like this:

lndg
|-- data
|   |-- lndg-data
|   |   `-- db.sqlite3
|   `-- logs
|       `-- lndg-controller.log
|-- docker-compose.yml
|-- exports.sh
`-- umbrel-app.yml

I added a pre-start script that migrates existing lndg user's data to these new directories (https://github.com/getumbrel/umbrel-apps/pull/1523/commits/301a10d4164da5558c1d5399c313f6725963d73e)

I have tested both a fresh install and an app update and everything is working well.

Can you please let me know if you are okay with this change?

cryptosharks131 commented 5 days ago

It is probably better to map just the lndg-controller.log file instead of the entire /var/log/ folder inside the container which has many system logs as well. Also was thinking the new folder name may be more descriptive as db.

nmfretz commented 1 day ago

Also was thinking the new folder name may be more descriptive as db

Great idea, I have changed lndg-data --> db (including the appropriate changes to the pre-start script) https://github.com/getumbrel/umbrel-apps/pull/1523/commits/ee3ae6ce0ad1677dd06c506ee96c3e8cb8483e3a

It is probably better to map just the lndg-controller.log file instead of the entire /var/log/ folder inside the container which has many system logs as well.

That's a good point. I had checked /var/log in 1.9.0 when testing the bind mount and didn't see anything else in the directory, but it might be better to not bind the entire directory as you say.

$ sudo docker exec -it lndg_web_1 ls -l /var/log
total 232
-rw-r--r--    1 1000     1000        233279 Sep 30 08:41 lndg-controller.log

In order to do this we need to commit an empty lndg-controller.log file to this repo. This is to get around a weird Docker quirk where if a bind mount doesn't exist on the host, Docker will create it as a directory ,not a file. So for a fresh install of lndg, ${APP_DATA_DIR}/data/logs/lndg-controller.log won't exist on the host yet and so Docker will create it... but as a directory called lndg-controller.log, which would likely cause lndg to error out.

You may not have run across this in testing when updating lndg, because lndg-controller.log would have already existed from the initial install and was mounted from it's old location:

${APP_DATA_DIR}/lndg-controller.log:/var/log/lndg-controller.log from https://github.com/getumbrel/umbrel-apps/pull/1523/commits/5e8cf3811df3401a4219cfdb3fdb6f0f6e002385

But new installs would error out.

I have committed the empty log file here https://github.com/getumbrel/umbrel-apps/pull/1523/commits/2a2ab3401042626daf803c6f6aa0228bd562ff32

If you're all good with this, we can go live.