Add nbde_client role

[sergio-correia]

No description provided.

[richm]

in general, looks good - would like to get another reviewer from the system roles team (@pcahyna , @nhosoi), and someone familiar with the clevis cli tools

[richm]

Also, please review the travis CI test failures

[sergio-correia]

Also, please review the travis CI test failures

Yeah, this seems more complex to get running correctly. I would appreciate help here.

[sergio-correia]

Thanks for the first review, @richm; I will update this shortly.

[richm]

Also, please review the travis CI test failures

Yeah, this seems more complex to get running correctly. I would appreciate help here.

If you just want to skip black and flake8 and pylint for now - see https://github.com/linux-system-roles/nbde_client/blob/master/.travis/config.sh - e.g. edit the file to set

export RUN_PYLINT_DISABLED=true
export RUN_BLACK_DISABLED=true
export RUN_FLAKE8_DISABLED=true

For black - if you want black to format your code - edit tox.ini like this:

commands =
#    bash {toxinidir}/.travis/runblack.sh --check --diff .
    bash {toxinidir}/.travis/runblack.sh .

then run tox -e black - black will reformat your python code to be conformant. Then revert tox.ini.

You can also review the linter issues - you can edit the various config files to completely disable that particular issue (e.g. disabling line length checking is probably the biggest one). The config.sh variables such as RUN_BLACK_EXTRA_ARGS and RUN_FLAKE8_EXTRA_ARGS can be used to pass suppressions on the command line to those programs.

You can also use code comments to suppress specific occurrences.

[sergio-correia]

@richm: I have removed the pytest test for now and added playbook-based tests. I will work on pytest next week. I believe this should be ready for review now.

[pcahyna]

"of the backing" - backing what? shouldn't it read "of the backing device"?

[pcahyna]

@dwlehman what do you think?

[sergio-correia]

I updated this to use "of the backing device"

[richm]

prefer only 1 empty line between tasks instead of 2

[sergio-correia]

is there a way to enforce this in yamllint?

[richm]

is there a way to enforce this in yamllint?

not sure - will investigate

[richm]

hmm - I don't think this is a yaml thing, I think it is an ansible thing - https://docs.ansible.com/ansible-lint/ doesn't seem to have a check for that @pcahyna @tyll do you know if there is a linter for this? OTOH, I don't recall if 'one blank line between tasks' is in any of the best practices, but it is a common convention.

[tyll]

I hear about this for the first time. Would be great to have a linter or actually formatter for this.

[richm]

Is this role idempotent? If you run the role again with the same nbde_client_bindings, will it do all of these steps again, and report that something changed? If so, I'm not sure how to prevent that from happening - how would you know ahead of time that you have already brought the managed hosts to the desired state? I realize it may be a little late for this particular PR, and I apologize for not noticing this sooner, so perhaps we can address this in a subsequent PR if necessary.

[sergio-correia]

Not fully. We discovered an issue with at least passphrase_temporary, which will be addressed after the initial merge.

[sergio-correia]

@pcahyna opened #4 to track this issue.