Add facts, functions, tasks, plans #215
Conversation
| @@ -287,6 +287,40 @@ parameters documented above also apply to AIX systems. | |||
| * range (Parameter) - Sets the inter-physical volume allocation policy. AIX only | |||
| * type (Parameter) - Configures the logical volume type. AIX only | |||
|
|
|||
| ## Functions | |||
|
|
|||
| `lvm::bytes_to_size`: Converts a number of bytes to a size format that LVM can understand e.g. `lvm::bytes_to_size(214748364800)` would return `200g` | |||
There was a problem hiding this comment.
Generally we do not document functions in the readme, this can be removed
There was a problem hiding this comment.
Why wouldn't we document them? There aren't documented anywhere else are they?
lib/puppet/type/logical_volume.rb
Outdated
| @@ -13,9 +13,19 @@ | |||
| raise ArgumentError, "Volume names must be entirely unqualified" | |||
| end | |||
| end | |||
|
|
|||
| # This is a bit of a hack, drags the volume_group up from the provider if | |||
| # it was specified, allowing `puppet resource` and otrher things that rely | |||
| end | ||
|
|
||
| newparam(:volume_group) do | ||
| newproperty(:volume_group) do |
There was a problem hiding this comment.
Changing this, will cause issues. It may be backwards incompatible
There was a problem hiding this comment.
The problem with this being a param is that it's not possible to pull a param from puppet resource which will then cause all the tasks to fail. By definition this should have always been a property as it is able to be read back from the system and isn't just something that is provided on creation. Can we run the beaker suite or something to verify what issues it might create as this change is critical to this entire PR (except the facts)
There was a problem hiding this comment.
@tphoney Any feedback on this?
Puppet can't currently validate these Also removed redundant task to reduce confusion
|
👍 from me. I am just going to get another review, due to its size |
|
@HelenCampbell @pmcmaw Can either of you have a look at this PR. Nice new functionality, great work. |
|
Hey @dylanratcliffe Something you may want to be aware of is that this module is currently only testing against puppet 4 in travis. I created a PR to enable puppet 5 and puppet6 however I am seeing quite a few failures. Bolt is only supported on ruby 2.3+ (2.3 is shipped with puppet5). Therefore if this PR makes use of bolt in any way the configuration is not supported and will only be supported in puppet5 and puppet6. This is the PR I have mentioned regarding updating the testing in Travis: #216 |
|
@pmcmaw This PR doesn't make use of any of Bolt's libraries at all so we shouldn't (and aren't) seeing any issues with Puppet 4. All changes to the T&Ps, functions and tests are Puppet 4 compatible as we've seen in the tests and should be fine with 5 and 6 also but we'll see once that PR is merged. This PR doesn't been to be blocked by #216 does it? |
|
@dylanratcliffe no doesn't block just something I want to ensure you were aware of. |
|
I didn't find how to post comment to code. Maybe I can't do it as PR already merged. |
|
@Tardja I looked at that and thought the merge had worked correctly but maybe not, do you mean that the the error has now come back? |
|
@dylanratcliffe, yes. I'm not good in coding. Recently updated this module from very old version and got this error. As was explained to me that vgs string not related to confine and executes even if lvm not present on host |
|
@dylanratcliffe , yes, code now fails on systems without LVM binaries installed. |
Add facts, functions, tasks, plans
It's a big one.
New features
puppet resourceto work forlogical_volumesFixes