Add more flexible options

[colearendt]

• Allows arbitrary number of SHARED_DIRECTORY inputs by using and parsing the docker CMD
• Allows arbitrary NFS options being set by using NFS_OPTS environment variable
• Tries valiantly to remain backwards compatible by:
  • Appending SHARED_DIRECTORY to the mount inputs
  • Using old patterns / defaults for READ_ONLY and SYNC if NFS_OPTS is not set

Would love to discuss if this is workable from your perspective and how committed you are to backwards compatibility. I think if backwards compatibility is not necessary, these deprecation warnings could be more forceful or you can simply bump the version and simplify the pattern by setting default NFS_OPTS and give users free reign to set their own options by overriding the environment variable.

NOTE: I submitted the PR to the byebyeconfd branch, since those changes looked desirable and these changes are related.

Related to #20 Related to #22

[sjiveson]

Hey Cole, thank you for this, it looks great. I wasn't aware of the ability to use SET_OPTS et al.

I'd like to both understand more about these env vars and get the removal of confd done before I consider merging this. I can't say when that will be but hopefully in a week r s.

Backwards compatibility is definitely important btw.

Also can you explain why you think this helps with https://github.com/sjiveson/nfs-server-alpine/issues/22

[colearendt]

Thanks for the response! So a bit of an overview how all of this works, in case it helps:

Previously: you use options to swap out a template using sed, and create a separate template for SHARED_DIRECTORY_2 Current approach: use an environment variable (NFS_OPTS or SET_OPTS) to set all NFS options for all shares (since you can have an arbitrary number). These shares are all added in sequence to the /etc/exports file

How backwards compatibility is maintained:

• If SHARED_DIRECTORY is set, include it in the list of shares to export
• If NFS_OPTS is not set, use the READ_ONLY AND SYNC env vars to append the defaults into the SET_OPTS environment variable, which will be used in place of NFS_OPTS for all mounts.
• Note that SHARED_DIRECTORY_2 is not accounted for, as I had hoped that SHARED_DIRECTORY_2 would not need backwards compatibility :smile:

Specifically, #22 is related because NFS_OPTS makes it very natural to change all NFS parameters at runtime, rather than requiring a rebuild of the docker image.

What are your thoughts on:

• naming of env vars (i.e. should SET_OPTS be LEGACY_OPTS or something?)
• should the warning messages be more explicit to point people to using NFS_OPTS?

[colearendt]

Oh, also. ${PERMITTED} is used verbatim, since it is kinda independent of NFS_OPTS. It still defines PERMITTED=* if PERMITTED is not defined, again for backwards compatibility.

# In order to run a `READ_ONLY`, `SYNC` version in the new paradigm:
docker run \
  -e "PERMITTED=1.2.3.4" \
  -e "NFS_OPTS=ro,sync,fsid=0,no_subtree_check,no_auth_nlm,insecure,no_root_squash" \
  dockerimage /var/share

# In order to solve #22 , along with READ_WRITE and ASYNC
docker run \
  -e "PERMITTED=*" \
  -e "NFS_OPTS=rw,async,fsid=0,no_subtree_check,no_auth_nlm,insecure" \
  dockerimage /var/share /var/share2

If you like this approach, I am happy to do some work updating the README.

[sjiveson]

Thanks!

[colearendt]

@sjiveson Do you mind clarifying a bit about why you closed? Did you decide that this functionality is not desirable?

[sjiveson]

Ah sorry, its been that long I forgot you did this PR against the byebyeconfd branch, which I just merged into the arm branch (months after originally merging to master) and then deleted.

[sjiveson]

Just recreated the branch and managed to reopen, phew. Hopefully I'll actually find the time to give this the attention it deserves.

[colearendt]

Thanks!! No worries! I’ll take some time to pull in the latest from master too. 😄

[KenjiTakahashi]

Hey guys, what is the status on this? I could use some of these changes, too. Thanks! (Yes, I know I can build my own image, but it is a bit troublesome ;-].)

[jforest]

I would also love it if this could get merged.. it's a very useful update

[saymonaraujo]

This PR is mentioned multiple times in multiple issues. Is there any intention in create a version using this ?