saltstack-formulas / lvm-formula

8 stars 13 forks source link

Add support to use backing files for PV (ready) #5

Closed noelmcloughlin closed 6 years ago

noelmcloughlin commented 6 years ago

This PR adds support for using file as backing devices for LVM PVs. Its WIP until #3 and #4 are merged then it can be reviewed. The PR supports following `losetup(8)' workflow.

The following commands can be used as an example of using the loop device.
# dd if=/dev/zero of=/file bs=1k count=100
# losetup -e des /dev/loop0 /file
Password:
Init (up to 16 hex digits):
# mkfs -t ext2 /dev/loop0 100
# mount -t ext2 /dev/loop0 /mnt
 ....
# umount /dev/loop0
# losetup -d /dev/loop0

Note: some technical sources use truncate (instead of dd) so both commands are supported.

Example Pillars::

lvm:
  files:
    loopbackdir: /tmp         #Where to create backing files? Default is /tmp anyway.
    remove:
      - /tmp/testfile1.img
      - /tmp/testfile2.img
    create:
      truncate:                #Shrink or extend the size of each FILE to the specified size
        /tmp/testfile1.img
          options:
            size: 100M
      dd:                      #copy a file, converting and formatting according to the operands
        /tmp/testfile2.img
          options:
            if: /dev/urandom
            bs: 1024
            count: 204800
      losetup:                 #set up and control loop devices
        /tmp/testfile1.img:
          options:
            show: True
            find: True
         /tmp/testfile2.img:
  pv:
    create:
      /dev/loop0:               #hopefully  /tmp/testfile1.img
      /dev/loop1:                #hopefully  /tmp/testfile2.img

Existing functionality is not impacted.

noelmcloughlin commented 6 years ago

Sample output

[ERROR   ] Command '[u'pvdisplay', u'-c', u'/dev/loop1']' failed with return code: 5
[ERROR   ] stderr:   Failed to find physical volume "/dev/loop1".
[ERROR   ] retcode: 5
[ERROR   ] Command '[u'pvdisplay', u'-c', u'/dev/loop1']' failed with return code: 5
[ERROR   ] stderr:   Failed to find physical volume "/dev/loop1".
[ERROR   ] retcode: 5
local:
----------
          ID: lvm file remove /tmp/testfile1.img
    Function: file.absent
        Name: /tmp/testfile1.img
      Result: True
     Comment: Removed file /tmp/testfile1.img
     Started: 08:08:53.181565
    Duration: 7.068 ms
     Changes:   
              ----------
              removed:
                  /tmp/testfile1.img
----------
          ID: lvm file remove /tmp/testfile2.img
    Function: file.absent
        Name: /tmp/testfile2.img
      Result: True
     Comment: Removed file /tmp/testfile2.img
     Started: 08:08:53.188800
    Duration: 17.679 ms
     Changes:   
              ----------
              removed:
                  /tmp/testfile2.img
----------
          ID: lvm file truncate testfile1.img
    Function: cmd.run
        Name: truncate --size 100M testfile1.img
      Result: True
     Comment: Command "truncate --size 100M testfile1.img" run
     Started: 08:08:53.208126
    Duration: 15.375 ms
     Changes:   
              ----------
              pid:
                  10682
              retcode:
                  0
              stderr:
              stdout:
----------
          ID: lvm file dd testfile2.img
    Function: cmd.run
        Name: dd if=/dev/urandom bs=1024 count=204800 of=testfile2.img
      Result: True
     Comment: Command "dd if=/dev/urandom bs=1024 count=204800 of=testfile2.img" run
     Started: 08:08:53.223837
    Duration: 1468.856 ms
     Changes:   
              ----------
              pid:
                  10684
              retcode:
                  0
              stderr:
                  204800+0 records in
                  204800+0 records out
                  209715200 bytes (210 MB) copied, 1.45456 s, 144 MB/s
              stdout:
----------
          ID: lvm file losetup testfile1.img
    Function: cmd.run
        Name: losetup  --show --find  -f testfile1.img
      Result: True
     Comment: Command "losetup  --show --find  -f testfile1.img" run
     Started: 08:08:54.693008
    Duration: 17.948 ms
     Changes:   
              ----------
              pid:
                  10686
              retcode:
                  0
              stderr:
              stdout:
                  /dev/loop0
----------
          ID: lvm file losetup testfile2.img
    Function: cmd.run
        Name: losetup   -f testfile2.img
      Result: True
     Comment: Command "losetup   -f testfile2.img" run
     Started: 08:08:54.711169
    Duration: 15.385 ms
     Changes:   
              ----------
              pid:
                  10690
              retcode:
                  0
              stderr:
              stdout:
----------
          ID: lvm_pv_create_/dev/loop0
    Function: lvm.pv_present
        Name: /dev/loop0
      Result: True
     Comment: unless condition is true
     Started: 08:08:54.728220
    Duration: 2597.658 ms
     Changes:   
----------
          ID: lvm_pv_create_/dev/loop1
    Function: lvm.pv_present
        Name: /dev/loop1
      Result: True
     Comment: Created Physical Volume /dev/loop1
     Started: 08:08:57.326167
    Duration: 172.974 ms
     Changes:   
              ----------
              created:
                  True

Summary for local
------------
Succeeded: 8 (changed=7)
Failed:    0
noelmcloughlin commented 6 years ago

Hey @dseira I appreciate your testing this. Glad you spotted that problem. Thanks!!

I decided to use -a. Its most portable across all versions of losetup command and is unlikely to disappear soon (even if depreciated). This commit is verified on both Centos7 and Ubuntu.

          ID: lvm file losetup delete /dev/loop0 if attached to /tmp/testfile1.img
    Function: cmd.run
        Name: losetup --detach /dev/loop0
      Result: True
     Comment: Command "losetup --detach /dev/loop0" run
     Started: 14:06:00.478475
    Duration: 72.061 ms
     Changes:   
              ----------
              pid:
                  6127
              retcode:
                  0
              stderr:
              stdout:
----------
          ID: lvm file losetup delete /dev/loop1 if attached to /tmp/testfile1.img
    Function: cmd.run
        Name: losetup --detach /dev/loop1
      Result: True
     Comment: onlyif condition is false
     Started: 14:06:00.551050
    Duration: 15.106 ms
     Changes:   
----------
          ID: lvm file remove /tmp/testfile1.img
    Function: file.absent
        Name: /tmp/testfile1.img
      Result: True
     Comment: Removed file /tmp/testfile1.img
     Started: 14:06:00.569924
    Duration: 7.781 ms
     Changes:   
              ----------
              removed:
                  /tmp/testfile1.img
----------
          ID: lvm file losetup delete /dev/loop0 if attached to /tmp/testfile2.img
    Function: cmd.run
        Name: losetup --detach /dev/loop0
      Result: True
     Comment: onlyif condition is false
     Started: 14:06:00.577878
    Duration: 16.513 ms
     Changes:   
----------
          ID: lvm file losetup delete /dev/loop1 if attached to /tmp/testfile2.img
    Function: cmd.run
        Name: losetup --detach /dev/loop1
      Result: True
     Comment: Command "losetup --detach /dev/loop1" run
     Started: 14:06:00.594639
    Duration: 43.652 ms
     Changes:   
              ----------
              pid:
                  6142
              retcode:
                  0
              stderr:
              stdout:
----------
          ID: lvm file remove /tmp/testfile2.img
    Function: file.absent
        Name: /tmp/testfile2.img
      Result: True
     Comment: Removed file /tmp/testfile2.img
     Started: 14:06:00.638552
    Duration: 19.767 ms
     Changes:   
              ----------
              removed:
                  /tmp/testfile2.img
----------
          ID: lvm file truncate testfile1.img
    Function: cmd.run
        Name: truncate --size 100M testfile1.img
      Result: True
     Comment: Command "truncate --size 100M testfile1.img" run
     Started: 14:06:00.658600
    Duration: 11.966 ms
     Changes:   
              ----------
              pid:
                  6146
              retcode:
                  0
              stderr:
              stdout:
----------
          ID: lvm file dd testfile2.img
    Function: cmd.run
        Name: dd if=/dev/urandom bs=1024 count=204800 of=testfile2.img
      Result: True
     Comment: Command "dd if=/dev/urandom bs=1024 count=204800 of=testfile2.img" run
     Started: 14:06:00.670789
    Duration: 15363.74 ms
     Changes:   
              ----------
              pid:
                  6148
              retcode:
                  0
              stderr:
                  204800+0 records in
                  204800+0 records out
                  209715200 bytes (210 MB, 200 MiB) copied, 15.3543 s, 13.7 MB/s
              stdout:
----------
          ID: lvm file losetup testfile1.img as loopback device
    Function: cmd.run
        Name: losetup  --show --find testfile1.img
      Result: True
     Comment: Command "losetup  --show --find testfile1.img" run
     Started: 14:06:16.034840
    Duration: 23.313 ms
     Changes:   
              ----------
              pid:
                  6150
              retcode:
                  0
              stderr:
              stdout:
                  /dev/loop0
----------
          ID: lvm file losetup testfile2.img as loopback device
    Function: cmd.run
        Name: losetup --show --find testfile2.img
      Result: True
     Comment: Command "losetup --show --find testfile2.img" run
     Started: 14:06:16.058478
    Duration: 28.166 ms
     Changes:   
              ----------
              pid:
                  6155
              retcode:
                  0
              stderr:
              stdout:
                  /dev/loop1
----------
    <<CUT >>
----------
          ID: lvm_pv_create_/dev/loop0
    Function: lvm.pv_present
        Name: /dev/loop0
      Result: True
     Comment: unless condition is true
     Started: 14:06:16.089426
    Duration: 983.373 ms
     Changes:   
----------
          ID: lvm_pv_create_/dev/loop1
    Function: lvm.pv_present
        Name: /dev/loop1
      Result: True
     Comment: unless condition is true
     Started: 14:06:17.073070
    Duration: 160.918 ms
     Changes:   

Summary for local
-------------
Succeeded: 16 (changed=8)
Failed:     0
-------------
Total states run:     16
Total run time:   16.747 s
vagrant@ubuntu-xenial:~$ losetup
NAME       SIZELIMIT OFFSET AUTOCLEAR RO BACK-FILE
/dev/loop0         0      0         0  0 /tmp/testfile1.img
/dev/loop1         0      0         0  0 /tmp/testfile2.img
dseira commented 6 years ago

Now I'm getting the following error in all versions:

2018-09-17 12:15:45,053 [salt.utils.templates:181 ][ERROR   ][50296] Rendering exception occurred
Traceback (most recent call last):
  File "/usr/lib/python2.7/site-packages/salt/utils/templates.py", line 170, in render_tmpl
    output = render_str(tmplstr, context, tmplpath)
  File "/usr/lib/python2.7/site-packages/salt/utils/templates.py", line 399, in render_jinja_tmpl
    buf=tmplstr)
SaltRenderError: Jinja variable 'list object' has no attribute 'items'
2018-09-17 12:15:45,054 [salt.state       :3454][CRITICAL][50296] Rendering SLS 'base:lvm.files.remove' failed: Jinja variable 'list object' has no attribute 'items'

I'm using the following pillar:

lvm:
  files:
    loopbackdir: /tmp         #Where to create backing files? Default is /tmp anyway.
    remove:
      - /tmp/testfile1.img
      - /tmp/testfile2.img
    create:
      truncate:                #Shrink or extend the size of each FILE to the specified size
        /tmp/testfile1.img:
          options:
            size: 100M
      dd:                      #copy a file, converting and formatting according to the operands
        /tmp/testfile2.img:
          options:
            if: /dev/urandom
            bs: 1024
            count: 204800
      losetup:                 #set up and control loop devices
        /tmp/testfile1.img:
          options:
            show: True
            find: True
        /tmp/testfile2.img:
  pv:
    create:
      /dev/loop0:               #hopefully  /tmp/testfile1.img
      /dev/loop1:                #hopefully  /tmp/testfile2.img
noelmcloughlin commented 6 years ago

Hmm. I'm bit puzzled. Works fine for me on Ubuntu (two pv.create states fail but that is expected).

Can you run sudo salt-call pillar.items --local which should not fail? Hmm.

# git pull origin master
From https://github.com/noelmcloughlin/lvm-formula
 * branch            master     -> FETCH_HEAD
Auto-merging lvm/defaults.yaml
Merge made by the 'recursive' strategy.
 lvm/defaults.yaml    |  4 +++-
 lvm/install.sls      | 10 +++++++---
 lvm/osfamilymap.yaml |  3 ---
 3 files changed, 10 insertions(+), 7 deletions(-)

$ sudo salt-call state.highstate --local
<< CUT >>
-------------
Succeeded: 24 (changed=8)
Failed:     2

$ losetup -a
/dev/loop1: []: (/var/lib/snapd/snaps/core_5328.snap)
/dev/loop4: []: (/var/lib/snapd/snaps/core_4917.snap)
/dev/loop2: []: (/var/lib/snapd/snaps/core_5145.snap)
/dev/loop0: []: (/var/lib/snapd/snaps/hello-world_27.snap)
/dev/loop5: []: (/tmp/testfile2.img)
/dev/loop3: []: (/tmp/testfile1.img)

$ sudo salt-call pillar.items --local
local:
    ----------
    lvm:
        ----------
        files:
            ----------
            loopbackdir:
                /tmp
            remove:
                - /tmp/testfile1.img
                - /tmp/testfile2.img
            create:
                ----------
                truncate:
                    ----------
                    /tmp/testfile1.img:
                        ----------
                        options:
                            ----------
                            size:
                                100M
                dd:
                    ----------
                    /tmp/testfile2.img:
                        ----------
                        options:
                            ----------
                            if:
                                /dev/urandom
                            bs:
                                1024
                            count:
                                204800
                losetup:
                    ----------
                    /tmp/testfile1.img:
                        ----------
                        options:
                            ----------
                            show:
                                True
                            find:
                                True
                    /tmp/testfile2.img:
                        None
        pv:
            ----------
            create:
                ----------
                /dev/loop0:
                    None
                /dev/loop1:
                    None
dseira commented 6 years ago

Sorry, my fault. I didn't pull properly. It works in my tested OSes (ubuntu14/16, centos 6/7).

I'm still getting the following error in all the OS:

          ID: lvm.pv.create_create
    Function: lvm.pv_present
        Name: create
      Result: False
     Comment: An exception occurred in this state: Traceback (most recent call last):
                File "/usr/lib/python2.7/dist-packages/salt/state.py", line 1837, in call
                  **cdata['kwargs'])
                File "/usr/lib/python2.7/dist-packages/salt/loader.py", line 1794, in wrapper
                  return f(*args, **kwargs)
                File "/usr/lib/python2.7/dist-packages/salt/states/lvm.py", line 66, in pv_present
                  changes = __salt__['lvm.pvcreate'](name, **kwargs)
                File "/usr/lib/python2.7/dist-packages/salt/modules/linux_lvm.py", line 225, in pvcreate
                  raise CommandExecutionError('{0} does not exist'.format(device))
              CommandExecutionError: create does not exist
     Started: 13:10:07.668416
    Duration: 40.695 ms
     Changes:   

But the PV are created successfully.

noelmcloughlin commented 6 years ago

Hi @dseira thank you for reviewing further.

I admit to being a bit stumped because either lvm.pv.create.items() evaluates to ('create', '/dev/loop0', '/dev/loop1') in your environment (I cannot replicate in my testing), or some other environment issue is happening. The key question is why "create" object is passed to jinja loop?

In the latest commit I changed ....

{%- if lvm.pv and "create" in lvm.pv and lvm.pv.create is mapping %}
  {% for pv, pvdata in lvm.pv.create.items() %}

to

{%- if lvm.pv and "create" in lvm.pv and lvm.pv.create is mapping %}
  {% for pv, pvdata in lvm.pv.create.items() if pv not in ('create', 'delete',) %}

Could you please pull and try to replicate this create does not exist error again?

dseira commented 6 years ago

Hi @noelmcloughlin , I'm working on it (I've not forgotten XD).

I think it is something related to /usr/lib/python2.7/site-packages/salt/modules/linux_lvm.py.

Which version of master/minion are you using?

Mine are:

master: 2018.3.2+ds-1
minion centos 7: salt-2018.3.2-1.el7.noarch
minion centos 6: salt-2018.3.2-1.el6.noarch
minion ubuntu 16: 2017.7.0+ds-1
minion ubuntu 14: 2017.7.0+ds-1
noelmcloughlin commented 6 years ago

No bother @dseira I'm sure you have more memory than free time ;-) Here is my Centos7/Vagrant setup.

Salt Version:
           Salt: 2018.3.2

Dependency Versions:
           cffi: 1.11.5
       cherrypy: 5.6.0
       dateutil: 2.7.3
      docker-py: 2.4.2
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.10
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: 1.0.7
   msgpack-pure: Not Installed
 msgpack-python: 0.5.6
   mysql-python: Not Installed
      pycparser: 2.18
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 2.7.5 (default, Jul 13 2018, 13:06:57)
   python-gnupg: Not Installed
         PyYAML: 3.13
          PyZMQ: 15.3.0
           RAET: Not Installed
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.2.1
            ZMQ: 4.1.4
noelmcloughlin commented 6 years ago

I wonder if salt bug is to blame. If your list evaluates to ['create', '/dev/loop0', '/dev/loop1'] then the inclusion of extra unwanted create keyword is vaguely similar to https://github.com/saltstack/salt/issues/49712 problem?

- names:
      - /tmp/opensds
      - /var/log/opensds
      - /opt/opensds-linux-amd64
      - /etc/opensds/driver
      - 0700                   <======= weird https://github.com/saltstack/salt/issues/49712
      - /opt/opensds-linux-amd64-devstack
      - /etc/opensds
      - root                <===== weird https://github.com/saltstack/salt/issues/49712
noelmcloughlin commented 6 years ago

@dseira could you approve/merge this please. issues/improvements can be addressed in followup PR.