search

subspacecommunity / subspace

A fork of the simple WireGuard VPN server GUI community maintained
MIT License
1.8k stars 131 forks source link

FIX: /data creation in entrypoint #171

Open agonbar opened 3 years ago

agonbar commented 3 years ago

to: @subspacecommunity/subspace-maintainers cc: @subspacecommunity/subspace-maintainers related to: resolves:

Error if /data volume is not created via docker, the entrypoint tries to create /data/wireguard and fails.

Background

This would allow to launch the docker image without persistence

Changes

The entrypoint.sh file, add a -p to the mkdir

Testing

Launch the image without creating a volume, it would fail before.

sonarcloud[bot] commented 3 years ago

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

gchamon commented 3 years ago

Why would you want to create subspace without persistence? Is this a feature we want? Could it come back and cause problems? What problem does launching without persistence acutally solves?

If we would accept this pr, maybe we should document it well that, even though subspace could be launch without persistence, that is not a recommended way.

Similarly, even if this pr wouldn't be merged, I think subspace should fail explicitly, and not relying on mkdir failing, showing that failing without persistent volume is an intended behaviour.

What do you think, @agonbar ?

agonbar commented 3 years ago

Hmmmm, i'd like to be able to launch without persistence for easiness in the development. But I agree that we need to warn the user when this happens and discourage people from using it this way, I'll give some thought on how to show this to the user in a friendly manner.

gchamon commented 3 years ago

@agonbar I will stand by and wait for updates on this before reviewing. Thanks for considering my suggestions!

gchamon commented 3 years ago

@agonbar do you need help documenting this change? I think this your suggestion here would be a welcome change

agonbar commented 3 years ago

It's fine, this is low priority and right now I spend all my spare time in testing the PRs, this project needs more automated tests, it would save me a lot of time.

gchamon commented 3 years ago

It's fine, this is low priority and right now I spend all my spare time in testing the PRs, this project needs more automated tests, it would save me a lot of time.

I will get to automating tests ASAP.

I can see unit tests being important, but we could do a whole sort of end to end tests using docker. I will open an issue to track that, or if one is already opened, I'll use it.