Enhance lvm2 support

[noelmcloughlin]

This PR introduces full support for all Use Case supported by LVM2 API. The states documented in pillar.example are verified on Ubuntu 16.04. The code is ready for merge.

Existing Pillar data will not work, but formula has no forks so no user impact.

See the README for full list of states supported.

[dseira]

Let me check it.

[noelmcloughlin]

Thank you @dseira

[xing-yang]

@nmadho can you please review this PR? Thanks!

[nmadhok]

@xing-yang Make sure to add @nmadhok so I get notifications directly (otherwise I typically just scan through notifications from repositories). This one may take me some time as lots of changes and it isn't backwards compatible. Not everybody forks the project so some may have it cloned directly and may be currently using it.

@noelmcloughlin Would it be possible to add backwards compatibility so it doesn't break stuff for people currently using this formula?

[noelmcloughlin]

Yes, it would be easy to support existing pillar data. Mine is the first PR against this repo - so I doubt there are many end users exist - but this request is sensible ... I will update the commit.

@dseira, @nmadhok PR review should validating existing supported Use Case:

• Install LVM
• Add PV
• Add VG
• Add LV

Everything else is new feature enhancement. I have tested many UC (few complex scenarios need further testing, i.e. thin provisioning) but these UC are not currently supported anyway.

[nmadhok]

@dseira I think we can merge this. Do you have any feedback/concerns before I merge it?

[noelmcloughlin]

Weird! Found two issues with config.sls which appear unrelated to my PR.

Legacy pillar data

{u'lv': {u'lv1': {u'name': u'lv_data1', u'vgname': u'vg_data', u'options': {u'size': u'1g', u'stripes': 5, u'stripesize': u'8K', u'thinpool': True, u'thinvolume': True}}, u'lv_data2': {u'enabled': False, u'vgname': u'vg_data2', u'options': {u'size': u'1g'}}}, u'pv': {u'pv1': {u'enabled': True, u'device': u'/dev/sdb', u'options': {u'setphysicalvolumesize': u'2g'}}, u'/dev/sdc': {u'enabled': True, u'options': None, u'whatever_pvcreate_option': u'value'}, u'/dev/sdd': {u'enabled': True}}, 'pkg': 'lvm2', u'vg': {u'vg1': {u'name': u'vg_data', u'enabled': True, u'devices': [u'/dev/sdb', u'/dev/sdc'], u'options': {u'physicalextentsize': u'1g'}, u'vg_data2': {u'devices': [u'/dev/sdd']}}}}

Issue 1: 'None' is not iterable - Fixed in latest commit.

Issue 2: TypeError: argument of type 'bool' is not iterable Not sure whats happening here

; line 28
---
[...]
{% endfor %}
{% endif %}

{% if 'vg' in lvm_settings %}
{% for vg,vg_options in lvm_settings.vg.items() %}
{% if ( 'enabled' in vg_options and vg_options.enabled ) or 'enabled' not in vg_options %}    <======================

Install state works anyway.

sudo salt-call state.highstate --local
local:
----------
          ID: lvm.pkg
    Function: pkg.installed
        Name: lvm2
      Result: True
     Comment: All specified packages are already installed
     Started: 19:03:36.444193
    Duration: 609.218 ms
     Changes:   

Summary for local
------------
Succeeded: 1
Failed:    0
------------
Total states run:     1
Total run time: 609.218 ms

[noelmcloughlin]

Maybe the existing pillar.example file is not fully correct?

[noelmcloughlin]

Hi @dseira @nmadhok I raised https://github.com/saltstack-formulas/lvm-formula/pull/2 to fix legacy issue with config.sls (not caused by this PR). Could this be merged now?

[noelmcloughlin]

Friendly reminder. @dseira @nmadhok

[noelmcloughlin]

Thanks @nmadhok appreciated!!

[dseira]

Sorry @noelmcloughlin @nmadhok , I've been really busy these last month. Thanks @nmadhok for the review.

[nmadhok]

@dseira No problem! Happy to help!