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

/var/www/data is now a volume #296

Merged
merged 1 commit into from
Feb 26, 2025
Merged

Conversation

ne20002
Copy link
Collaborator

@ne20002 ne20002 commented Feb 24, 2025

Fixes: #290

/var/www/data is now a dedicated volume by default. This prevents writes to the container's image volume.

Co-authored-by: @m33m33

Copy link
Collaborator

@MrPetovan MrPetovan left a comment

Choose a reason for hiding this comment

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

This looks good in the absolute, but what happens for people who were running the Docker image and upgrade it? Won't they be losing access to the filesystem content which reference is now different?

@ne20002
Copy link
Collaborator Author

ne20002 commented Feb 24, 2025

Isn't the file storage setting been set in the database after creation?
I'll recheck that.

@m33m33
Copy link

m33m33 commented Feb 24, 2025

Isn't the file storage setting been set in the database after creation?

I'll recheck that.

After a fresh install on 2024.12 the storage is Database. The admin have to change it manually and choose a www-data writable path

@m33m33
Copy link

m33m33 commented Feb 24, 2025

This looks good in the absolute, but what happens for people who were running the Docker image and upgrade it? Won't they be losing access to the filesystem content which reference is now different?

Pulling an updated app image will not change:

  • the docker-compose.yml file
  • host directory mappings to the storage path (if the admin had chosen this option, like me for instance)
  • host directory content
  • the storage backend decided by the instance admin and the Filesystem target directory

To my understanding of docker layered approach of filesystem in containers and volumes a friendica app container upgrade or rm will delete data. This is already a risk -probably underestimated- whatever we decide here.

Maybe it is an invitation to be explicit about it in the next update release notes and/or warning on the admins forum.

@ne20002
Copy link
Collaborator Author

ne20002 commented Feb 24, 2025

It is as I remembered and @m33m33 described.

The database is the default storage. When the user changes this to file system the setting is saved to the database which has priority over the config file.

So existing instances shall not face any issue with the chnage.

@ne20002
Copy link
Collaborator Author

ne20002 commented Feb 24, 2025

Pulling a new image version for a container will replace its image volume and all layers put ontop.
Whereas other / mounted volumes are unaffected.
If anyone really used the /var/www/html/storage path this has already been part of the /var/www/html volume.
And as the change will not affect settings of existing instances I believe we're save with the change.

@m33m33
Copy link

m33m33 commented Feb 24, 2025

And as the change will not affect settings of existing instances I believe we're save with the change.

The change by itself is safe.

As @MrPetovan asked about possible side effects I think it would be fair to point in release notes that an instance with Filesystem backend storage pointing inside the app image will result in data loss during upgrade, rm, rebuild. And that risk is avoided from now on with new safer settings (dedicated volume) on new deployments.

@ne20002
Copy link
Collaborator Author

ne20002 commented Feb 24, 2025

I don't see the data loss.
As written, for any instance that already is using the file system storage its configuration will not change. It will continue to use the folder as already configured.

The change is only for new instances. No need for documentation.

Until now the default has been /var/www/html/storage which is inside the html volume. And that's a bad idea as pointed out on the setting's page.
The container's image volume is not used to store data.

With the change for new instances the file storage will default to /var/www/data in its own volume.

@m33m33
Copy link

m33m33 commented Feb 24, 2025

When you look at https://docs.friendi.ca/develop/admin/settings/?h=filesystem#file-storage-backend you are instructed to use a directory outside web server tree.

In the app image there is a /var/www/data directory patiently waiting for someone to use that as the Filestorage target directory.

It I am not mistaken, /var/www/data is inside the app image, not the html volume, right ?
You see what could go wrong in this scenario ? That's what incited me to map a host volume for Filesystem target directory instead.

I remember a chat about it I will look at matrix logs later: Others may have found /var/www/data very handy and pointed the target there.

@ne20002
Copy link
Collaborator Author

ne20002 commented Feb 24, 2025

I see what could go wrong in the scenario and that was the reason why I propose this change to move /var/www/data into its own volume.

But /var/www/data is not the default for the setting. /var/www/html/storage is the derfault shown in the settings page.
It's unlikely that one would choose /var/www/data if not pointed to it.

So, if anyone chose /var/www/data before (and did not mount that directory) he will for sure already faced data loss.

Saving data into the web app directory is a bad idea. So this PR changes this by providing a separate volume and the related default.

No existing instance will face any issue and will work as before with its settings.

@ne20002 ne20002 requested a review from nupplaphil February 24, 2025 20:18
@m33m33
Copy link

m33m33 commented Feb 24, 2025

That's fine by me, merging this PR is a good thing anyway.

@MrPetovan MrPetovan merged commit bfeb118 into friendica:stable Feb 26, 2025
19 checks passed
@MrPetovan MrPetovan added the Enhancement New feature or request label Feb 26, 2025
@ne20002 ne20002 deleted the feat/data-volume branch February 26, 2025 10:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

avatar/profile icon missing when using fpm-alpine variant
3 participants