State module.run not passing all arguments

[timwsuqld]

Description of Issue/Question

When using state module.run to call the extfs.mkfs it's not passing the label argument through.

Setup

format_borg_repository_disk:
  module.run:
    - name: extfs.mkfs
    - device: /dev/disk/by-id/scsi-0Google_PersistentDisk_borg-repository
    - fs_type: ext4
    - label: "Borg-Repository-label"
    - unless: blkid /dev/disk/by-id/scsi-0Google_PersistentDisk_borg-repository | grep ext4

Steps to Reproduce Issue

Run state.sls expecting it to format /dev/disk/by-id/scsi-0Google_PersistentDisk_borg-repository with the label "Borg-Repository-label", the format works but the label is not set.

Below are snipped debug logs. In that run, I also had m_label set to see if that worked, and there is a second mount state that follows the format state. The OrderedDict clearly has label as a parameter, but it's not passed to mke2fs.

[DEBUG   ] Results of YAML rendering: 
OrderedDict([(u'format_borg_repository_disk', OrderedDict([(u'module.run', [OrderedDict([(u'name', u'extfs.mkfs')]), OrderedDict([(u'device', u'/dev/disk/by-id/scsi-0Google_PersistentDisk_borg-repository')]), OrderedDict([(u'fs_type', u'ext4')]), OrderedDict([(u'm_label', u'Borg-Repository')]), OrderedDict([(u'label', u'Borg-Repository-label')]), OrderedDict([(u'unless', u'blkid /dev/disk/by-id/scsi-0Google_PersistentDisk_borg-repository | grep ext4')])])])), (u'mount_borg_repository_disk', OrderedDict([(u'mount.mounted', [OrderedDict([(u'name', u'/backups/')]), OrderedDict([(u'device', u'LABEL=Borg-Repository')]), OrderedDict([(u'persist', True)]), OrderedDict([(u'fstype', u'ext4')]), OrderedDict([(u'mkmnt', True)]), OrderedDict([(u'opts', u'defaults,discard')])])]))])
<snip>
[INFO    ] Executing command 'blkid /dev/disk/by-id/scsi-0Google_PersistentDisk_borg-repository | grep ext4' in directory '/root'
[DEBUG   ] retcode: 1
[DEBUG   ] Last command return code: 1
[WARNING ] The function "module.run" is using its deprecated version and will expire in version "Sodium".
[INFO    ] Executing command 'mke2fs -F -t ext4 /dev/disk/by-id/scsi-0Google_PersistentDisk_borg-repository' in directory '/root'
[DEBUG   ] stdout: mke2fs 1.44.1 (24-Mar-2018)
Discarding device blocks: done                            
Creating filesystem with 65536000 4k blocks and 16384000 inodes
<snip>
Writing superblocks and filesystem accounting information: done     
[INFO    ] {u'ret': [u'Creating filesystem with 65536000 4k blocks and 16384000 inodes', u'Filesystem UUID: 0a9580af-9196-429d-881d-1be913a0f0d5', u'Superblock backups stored on blocks: ', u'\t32768, 98304, 163840, 229376, 294912, 819200, 884736, 1605632, 2654208, ', u'\t4096000, 7962624, 11239424, 20480000, 23887872']}
[INFO    ] Completed state [extfs.mkfs] at time 14:32:05.114086 (duration_in_ms=16353.437)

Running the module directly instead of the state shows that label is correctly used, so it's something to do with module.run not passing things correctly, but I can't see in the code where the problem is.

salt-call --state-verbose=False -l debug state.module.run name=extfs.mkfs device=/dev/disk/by-id/scsi-0Google_PersistentDisk_borg-repository label=Borg-Repository fs_type=ext4

Versions Report

Client is Ubuntu 18.04, master is 16.04 still. But both on 2019.2.0 salt.

$ salt-call --versions-report
Salt Version:
           Salt: 2019.2.0

Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: 2.6.1
      docker-py: Not Installed
          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: Not Installed
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 2.7.15rc1 (default, Nov 12 2018, 14:31:15)
   python-gnupg: 0.4.1
         PyYAML: 3.12
          PyZMQ: 16.0.2
           RAET: Not Installed
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.5.3
            ZMQ: 4.2.5

System Versions:
           dist: Ubuntu 18.04 bionic
         locale: UTF-8
        machine: x86_64
        release: 4.15.0-1027-gcp
         system: Linux
        version: Ubuntu 18.04 bionic

[garethgreenaway]

@timwsuqld Thanks for the report. Was this working correctly in 2018.3.x?

[timwsuqld]

@garethgreenaway I'm not sure if it was working on 2018.3.x. I was writing new states and couldn't get it working, we upgraded to 2019.2 a few weeks back, so never tried it on 2018.3.

Thanks

[garethgreenaway]

@timwsuqld No worries. Just wanted to determine if it was something that broke with the 2019.2.0 release or if it was broken in 2018.3 as well. Thanks! :smile:

[dustyketchum]

number_of_inodes has the same issue, and I would assume that many other mkfs options are being silently ignored as well. Not quite sure why this issue status is blocked because this is in one piece of source code, I assume the fix is reasonably straightforward (but no I have not tried)