umbrel-apps icon indicating copy to clipboard operation
umbrel-apps copied to clipboard

Update LNDg to v1.9.0

Open cryptosharks131 opened this issue 1 year ago • 2 comments

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

cryptosharks131 avatar Sep 24 '24 13:09 cryptosharks131

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?

nmfretz avatar Sep 26 '24 05:09 nmfretz

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.

cryptosharks131 avatar Sep 26 '24 11:09 cryptosharks131

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.

nmfretz avatar Sep 30 '24 12:09 nmfretz

@cryptosharks131 does the above sound okay to you? Sorry for the hold up.

nmfretz avatar Oct 02 '24 00:10 nmfretz

Thanks for the updates this looks good to me!

cryptosharks131 avatar Oct 02 '24 01:10 cryptosharks131

Excellent, thanks again @cryptosharks131!

Re-tested and now going live.

nmfretz avatar Oct 02 '24 11:10 nmfretz