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

Ordering of resources and removal of some defaults in class lvm #100

Merged
merged 3 commits into from
Apr 21, 2015
Merged

Ordering of resources and removal of some defaults in class lvm #100

merged 3 commits into from
Apr 21, 2015

Conversation

TorLdre
Copy link
Contributor

@TorLdre TorLdre commented Mar 10, 2015

Hi, thank you all for this fine module! I have a couple of issues though (and a proposal for solution).

  1. When using the lvm class I discovered that the resources are created in random order, so what happens is, the code fails because the vg can't be created on non-existent pv, mkfs can't create file systems on none-existent logvols and so on. This happens and is reproducable when provision several devices. Another example, vgcreate will fail if the drive has an msdos label (in lack of --yes parameter), which happens often, I guess (given eg clearpart --all in anaconda). However, if those drives are pvcreated first, it works. These problems are addressed in manifests/volume_group.pp line 13 and manifests/logical_volume.pp line 58.

  2. The other issue is with the creation of logical volumes using the lvm class. By the current design one has to provide size parameter (while extents should suffice), and it's assumed that the user want's a filesystem and mountpoint, and the device mounted afterwards. However, there are several use cases where this is not what the user want's (me). What I want is a logical volume without file system, and therefore no mountpoints or mounting either. I have tried to address these issues here without removing any functionality. I do, however, see a caveat with these changes, depending on whether users are using default values for fs_type and mountpath. There may very well be better ways to do this.

I do apologize if dragging two differing issues into one request is bad karma. My experience in these matters are rather limited for now.

@tphoney
Copy link
Contributor

tphoney commented Mar 11, 2015

Thanks for effort you have put in, and the explanation of the work you have carried out. Could you please look into the TravisCI failure.

@TorLdre
Copy link
Contributor Author

TorLdre commented Mar 11, 2015

tphoney: Thank you for your feedback. I will redesign my proposal (I found the error).

@TorLdre
Copy link
Contributor Author

TorLdre commented Mar 12, 2015

tphoney: I have done some more work. I have restored the default values (so the caveat described above is no longer a concern). In sum, my proposed changes will provide:

  1. Ordering physical volume -> volume group
  2. $size is no longer mandatory - $extents can be used instead
  3. A new parameter $createfs is provided (defaults to true). If true, the module will behave like before. If false, no creation of file system of mounting will be done.

I have tested creating, deleting, with or without createfs, extents and size. It seems to work pretty well.

tphoney added a commit that referenced this pull request Apr 21, 2015
Ordering of resources and removal of some defaults in class lvm
@tphoney tphoney merged commit 6624b24 into puppetlabs:master Apr 21, 2015
cegeka-jenkins pushed a commit to cegeka/puppet-lvm that referenced this pull request Oct 23, 2017
Ordering of resources and removal of some defaults in class lvm
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants