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

nfs_export LWRP not truly idempotent - creates duplicate entries if things change #48

Open
StFS opened this issue Feb 12, 2015 · 9 comments
Labels
Bug Something isn't working
Milestone

Comments

@StFS
Copy link

StFS commented Feb 12, 2015

If I run a recipe that creates an export for /mnt/foo everything works. However, if I then change something about that export (for instance, add or remove an option), re-running the recipe will create a second entry in /etc/exports and the run will fail since the NFS server will complain that it can't handle duplicate entries.

I would think that the LWRP should look for lines based on the exported directory and update the line if it exists already.

@atomic-penguin
Copy link
Contributor

The LWRP is good about inserting things, but there is no mechanism for removing things. Its an append-only operation as it stands now.

By changing something about the composed line after its been inserted, then you are effectively creating a different share as far as the LWRP is concerned. As far as exportfs is concerned, its really not too picky about the uniqueness of a share. There is no magic primary key in an export tab to key off of for replacement.

I am not opposed to changing it to a replacement model. But, it has potential to be a breaking change.

@StFS
Copy link
Author

StFS commented Feb 13, 2015

Okay... well as it stands now, if you add a duplicate share (that is, you export the same directory twice) the NFS service doesn't start at all. It fails with an error. I don't know enough about NFS to say for sure that this is always the way it is or whether there is any way to export the same directory twice... but I'd think that having the LWRP behave in such a way that it at least tries to makes sure that the exports file is valid and that the service will start after modifications would be desirable.

@atomic-penguin
Copy link
Contributor

I am able to start nfs-kernel-server just fine with the same share exported to two distinct networks, or two sets of distinct options. I can repro the problem if a duplicate shares have the same CIDR ACL. A duplicate share exported to a different CIDR ACL is still a valid declaration in exports.

# This is valid, because the share is not a unique primary key and has nothing to do with why its failing.
/mnt/foo 127.0.0.0/8(ro,sync,no_root_squash)
/mnt/foo 127.0.0.2(rw,sync,no_root_squash)
# So is this.
/mnt/foo 127.0.0.3(ro,sync,no_root_squash) 127.0.0.4(rw,sync,no_root_squash)
#  It can get even more complicated if there is a large set of CIDRs which each need to be checked for uniqueness.
/mnt/foo 127.0.0.5(ro,sync,no_root_squash) 127.0.0.0/8(rw,sync,no_root_squash) 127.0.0.6(ro,sync,no_root_squash)
# This fails because the share, in combination with the CIDR, is a unique primary key.
/mnt/foo 127.0.0.0/8(ro,sync,no_root_squash)
/mnt/foo 127.0.0.0/8(rw,sync,no_root_squash)

Let me reiterate, that I am not opposed to changing the behavior.

@atomic-penguin atomic-penguin added the Bug Something isn't working label Feb 13, 2015
@atomic-penguin atomic-penguin added this to the 3.0.0 milestone Feb 13, 2015
@StFS
Copy link
Author

StFS commented Feb 13, 2015

well... I agree that since exporting the same directory twice is legal then my suggestion of using that as a primary key of sorts is silly. Using a combination of the directory and network might be an option but it would probably be a bit complicated since the network can be specified in a number of different ways.

I don't suppose it would be possible to store the "before modifications" version of the /etc/exports file and then see if a restart of the NFS service actually works... if it doesn't then reverting back to the old version, starting the service and failing the chef run? That way, a converge at least wouldn't bring down a live NFS server.

@atomic-penguin
Copy link
Contributor

That is exactly the point I was trying to make. I acknowledge that it is a problem. But its not a simple fix that I can just whip out in a few minutes.

I am leaning towards, the replace_or_add resource here. Which could be implemented so that the last thing declared that matches the pattern of share+CIDR wins.

@glenjamin
Copy link

What about adding a "name" field to the LWRP, and including this as a comment on the end of the line?

This could default to path+type, but allows users to override if needed.

@yoshiwaan
Copy link
Contributor

I agree that a resource name is the best way to identify an entry in this case (via a comment), as the use case scenario where this crops up is most likely that a user is modifying an existing share and is thus editing the resource in Chef code.

@nkadel-skyhook
Copy link

nkadel-skyhook commented Oct 21, 2016

What about providing a template to simply deploy a managed /etc/exports, and not using an LWRP? I'm sure some people would want to continue using the LWRP for tuning local configurations with individual entries. LWRP's have their uses for such configuratons, but managing /etc/exports could avoid overlaps and double entries by working from a template, topped with a "# Chef managed" warning. It would also allow a much more graceful activation via roles, and attributes, without every nfs cookbook user having to write their own wrapper cookbook.

That approach could also provide a much more effective rollback mechanism, if needed. I've worked with just such rollbacks for /etc/sudoers and /etc/named changes, to avoid chef deployments taking out critical managed services due to mismatched merges. If there's any buy-in, I'd be willing to try a pull request for this.

@nickkeyzer
Copy link

managing /etc/exports could avoid overlaps and double entries by working from a template, topped with a "# Chef managed" warning. It would also allow a much more graceful activation via roles, and attributes, without every nfs cookbook user having to write their own wrapper cookbook.

This is a perfect example. I'm working with this exact scenario now. My NFS server exports to different networks, each with unique options, depending on the Environment of the node. This is very tricky to do with the LWRP whereas a template file would be much easier to manage via node attributes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

6 participants