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

mdadm error in raid.present #43517

Open
prutschman opened this Issue Sep 15, 2017 · 6 comments

Comments

Projects
None yet
6 participants
@prutschman

prutschman commented Sep 15, 2017

Description of Issue/Question

raid.present invokes mdadm with inappropriate options during assembly

Setup

ssd_bulk_raid:
    raid.present:
        - level: 0
        - devices:
            - /dev/sdb
            - /dev/sdc
        - chunk: 256
        - run: True

Steps to Reproduce Issue

salt-ssh <myhost> state.apply <mystate>
Excerpt of output:

----------
          ID: ssd_bulk_raid
    Function: raid.present
      Result: False
     Comment: Raid ssd_bulk_raid failed to be assembled.
     Started: 16:48:23.038326
    Duration: 70.032 ms
     Changes:   

from salt-call.log on target machine:

2017-09-14 16:48:23,094 [salt.loaded.int.module.cmdmod][ERROR   ][11953] Command '['mdadm', '-A', 'ssd_bulk_raid', '-v', '--chunk', 256, '--run', '/dev/sdb', '/dev/sdc']' failed with return code: 2
2017-09-14 16:48:23,094 [salt.loaded.int.module.cmdmod][ERROR   ][11953] output: mdadm: :option --chunk not valid in assemble mode
2017-09-14 16:48:23,107 [salt.state       ][ERROR   ][11953] Raid ssd_bulk_raid failed to be assembled.

I believe the problem may be in

salt/salt/states/mdadm.py

Lines 134 to 137 in 792cc0e

if do_assemble:
__salt__['raid.assemble'](name,
devices,
**kwargs)

The chunk size is passed through to raid.assemble as a kwarg, but chunk is not a valid assembly option.

Versions Report

This is using salt-ssh, no masters.

salt-ssh 2017.7.1 (Nitrogen)
@garethgreenaway

This comment has been minimized.

Member

garethgreenaway commented Sep 15, 2017

@prutschman Thanks for the report. Looks like it is potentially a quick fix if you're able to submit a PR, it would be greatly appreciated.

@prutschman

This comment has been minimized.

prutschman commented Sep 18, 2017

@garethgreenaway I'm not familiar with Salt's architecture, so I don't know if this is how you'd typically handle a case like this. I'm dropping the chunk argument if present but only in the state module, and only when passing to the assemble command.

@BadgerOps

This comment has been minimized.

Contributor

BadgerOps commented Jan 9, 2018

Howdy, I just ran across this same issue and manually applied @prutschman 's patch (it works!). I'd like to see this merged and would be happy to test if there's any edge cases that need to be checked first @garethgreenaway

Thanks!

@benceszikora

This comment has been minimized.

benceszikora commented Jun 11, 2018

@garethgreenaway I just ran into the same thing as well and the patch does seem to fix the issue, does this PR need any more testing?

@SEJeff

This comment has been minimized.

Member

SEJeff commented Jun 11, 2018

@prutschman can you open the PR against the saltstack/salt repo, and not against your fork?

One of my coworkers ran into this exact issue and I came in here to find the bug and write a fix. Since you've already written one, it makes sense to get credit as a contributor!

@junster1

This comment has been minimized.

Contributor

junster1 commented Jun 13, 2018

@prutschman we ran into this issue also, can you submit a PR against upstream?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment