nfs-server-alpine icon indicating copy to clipboard operation
nfs-server-alpine copied to clipboard

Add more flexible options

Open colearendt opened this issue 7 years ago • 11 comments

  • 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

colearendt avatar Nov 12 '18 19:11 colearendt

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

sjiveson avatar Nov 19 '18 20:11 sjiveson

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 avatar Nov 20 '18 13:11 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.

colearendt avatar Nov 20 '18 13:11 colearendt

Thanks!

sjiveson avatar Nov 22 '18 13:11 sjiveson

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

colearendt avatar Feb 24 '19 18:02 colearendt

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 avatar Feb 24 '19 18:02 sjiveson

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

sjiveson avatar Feb 24 '19 18:02 sjiveson

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

colearendt avatar Feb 24 '19 20:02 colearendt

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 ;-].)

KenjiTakahashi avatar Aug 17 '19 21:08 KenjiTakahashi