refactor(formula): align to template-formula & fix #19

noelmcloughlin commented 4 years ago

This PR aligns entire formula to the template-formula standardization and fixes the major limitation described in #19

Verification will be done on FreeBSD and GNU/Linux - results reported in comments below.

noelmcloughlin commented 4 years ago

Verified on CentOS7: Here is output: saveconfig.json.centos7.txt

noelmcloughlin commented 4 years ago

Verified on Ubuntu now: iscsi.ubuntu.txt

The tgt service did not start because I tested logical configuration only (no backing storage).

noelmcloughlin commented 4 years ago

Verified on OpenSUSE now: iscsi-opensuse-salt.output.txt

The target.service does not exist - I asked SuSE guys for help: https://github.com/yast/yast-iscsi-lio-server/issues/96 This is good enough for now.

UPDATE target was renamed to targetcli in Leap.

noelmcloughlin commented 4 years ago

Now working on OpenSUSE: iscsi-suse-lio-targetcli.out.txt

noelmcloughlin commented 4 years ago

Now working on Archlinux: archlinux-iscsi-target-fb.out.txt

Notes:

I ran users-formula first to create necessary user.
Before running iscsi formula I rebooted so kernel upgrade is appllied
I found this salt bug ...

unless: {{ var }} evaluated to unless: https://aur.archlinux.org which failed. I presumed this would check for non-null value but maybe not. The fix I used was: unless: {{ var != None }}

noelmcloughlin commented 4 years ago

Verified on FreeBSD 11.2: freebsd-istgt-output.txt

This PR is fully verified now.

myii commented 4 years ago

@noelmcloughlin Getting all failures on Travis at the moment:

https://travis-ci.org/myii/iscsi-formula/builds/586323276

Specifically:

https://travis-ci.org/myii/iscsi-formula/jobs/586323277#L1446-L1454

                 ID: iscsi-target-service-install-service-running
           Function: service.running
               Name: tgt
             Result: False
            Comment: Failed to start tgt.service: Unit tgt.service not found.
            Started: 01:17:00.449964
           Duration: 120.00000000000455 ms
            Changes:   
         Name: iscsi-target-service-install-failure-explanation - Function: test.show_notification - Result: Clean Started: - 01:17:00.572500 Duration: 9.999999999990905 ms

Can you enable Travis on your side and test it out?

baby-gnu commented 4 years ago

Hello.

The docs/_static/css/custom.css, docs/conf.py and docs/index.rst are deprecated and should not be included (https://github.com/saltstack-formulas/libvirt-formula/pull/43#pullrequestreview-253285365).

I think they should be removed from template-formula.

Regards.

noelmcloughlin commented 4 years ago

Can you enable Travis on your side and test it out?

Could we disable Debian/Ubuntu on Travis for the moment? The tgt service will not start because the logical configuration has no backing storage (created by lvm formula for example). It's not something I can easily fix so a separate PR would be better.

noelmcloughlin commented 4 years ago

The docs/_static/css/custom.css, docs/conf.py and docs/index.rst are deprecated and should not be inc

Thanks @baby-gnu for reviewing. I will remove those files from here.

myii commented 4 years ago

Could we disable Debian/Ubuntu on Travis for the moment? The tgt service will not start because the logical configuration has no backing storage (created by lvm formula for example). It's not something I can easily fix so a separate PR would be better.

That's not a problem but we should get it passing on some of the other platforms instead. It's possible to set dependencies on other formulas in kitchen.yml like this example. Please try to do the testing in your own account in Travis, so that it doesn't block other formulas that are using Travis at the same time; at the current time, we've only got 3 platforms that can run concurrently across all of the formulas in the whole org.

noelmcloughlin commented 4 years ago

Travis is 50% working now: https://travis-ci.org/noelmcloughlin/iscsi-formula/builds/586794185

Once https://github.com/saltstack-formulas/lvm-formula/pull/13 is merged I'll try again as this may fix RedHat family (centos, fedora, amazon) otherwise I'll fix travis without using lvm.

noelmcloughlin commented 4 years ago

Travis status

INSTANCE=default-centos-7-2019-2-py3 image lacks target_mod_core kernel module so target service cannot start.
INSTANCE=default-centos-6-2017-7-py2 isnsd service fails with "none of the trigger conditions were met".
INSTANCE=default-amazonlinux-2-2018-3-py2 iscsi service fails with "none of the trigger conditions were met".

I will drop CentOS6 because technologically it has an obsolete iscsi implementation. I hope to get CentOS7 & Amazonlinux working

noelmcloughlin commented 4 years ago

Hi @myii @n-rodriguez I can get 5 of 6 Travis jobs passing. That is the best I can achieve in this PR. https://travis-ci.org/noelmcloughlin/iscsi-formula/builds/587598962

What is the next stage? thanks!!

myii commented 4 years ago

@noelmcloughlin Noticed that you're making progress and only one instance failing -- good work. I ran the lint stage as well and found that there are going to be numerous yamllint errors:

https://travis-ci.com/saltstack-formulas/iscsi-formula/jobs/237749889#L222-L279

$ yamllint -s .
./kitchen.yml
  154:5     error    wrong indentation: expected 6 but found 4  (indentation)
  182:5     error    wrong indentation: expected 6 but found 4  (indentation)
  217:5     error    wrong indentation: expected 6 but found 4  (indentation)

./pillar.example
  639:1     error    too many blank lines (1 > 0)  (empty-lines)
  4:6       error    syntax error: found character '%' that cannot start any token

./iscsi/defaults.yaml
  6:21      warning  missing starting space in comment  (comments)
  13:21     warning  truthy value should be one of [false, true]  (truthy)
  19:12     error    trailing spaces  (trailing-spaces)
  30:16     warning  truthy value should be one of [false, true]  (truthy)
  31:15     warning  truthy value should be one of [false, true]  (truthy)
  35:32     warning  missing starting space in comment  (comments)
  39:32     warning  missing starting space in comment  (comments)
  40:32     warning  missing starting space in comment  (comments)
  41:32     warning  missing starting space in comment  (comments)
  42:32     warning  missing starting space in comment  (comments)
  43:32     warning  missing starting space in comment  (comments)
  44:32     warning  missing starting space in comment  (comments)
  62:14     error    empty value in block mapping  (empty-values)
  66:14     error    empty value in block mapping  (empty-values)
  69:14     error    empty value in block mapping  (empty-values)
  72:14     error    empty value in block mapping  (empty-values)
  94:14     warning  truthy value should be one of [false, true]  (truthy)
  96:14     warning  truthy value should be one of [false, true]  (truthy)
  105:14    warning  truthy value should be one of [false, true]  (truthy)
  106:17    warning  truthy value should be one of [false, true]  (truthy)
  108:14    warning  truthy value should be one of [false, true]  (truthy)
  118:17    warning  truthy value should be one of [false, true]  (truthy)
  119:14    warning  truthy value should be one of [false, true]  (truthy)
  121:14    warning  truthy value should be one of [false, true]  (truthy)

./iscsi/osfamilymap.yaml
  18:14     warning  truthy value should be one of [false, true]  (truthy)
  18:22     warning  missing starting space in comment  (comments)
  35:25     warning  too few spaces before comment  (comments)
  35:26     warning  missing starting space in comment  (comments)
  36:26     warning  missing starting space in comment  (comments)
  59:17     error    trailing spaces  (trailing-spaces)
  106:8     error    wrong indentation: expected 8 but found 7  (indentation)
  114:17    warning  truthy value should be one of [false, true]  (truthy)
  137:17    warning  truthy value should be one of [false, true]  (truthy)

./iscsi/osfingermap.yaml
  36:17     warning  truthy value should be one of [false, true]  (truthy)
  56:17     warning  truthy value should be one of [false, true]  (truthy)

./iscsi/oscodename.yaml
  4:2       error    syntax error: found character '%' that cannot start any token
  20:89     error    line too long (94 > 88 characters)  (line-length)
  26:89     error    line too long (105 > 88 characters)  (line-length)

./test/salt/pillar/centos6.sls
  4:6       error    syntax error: mapping values are not allowed here

When you finally get the tests working, we can discuss how to fix (or ignore) the errors above.

noelmcloughlin commented 4 years ago

Thanks @myii I have fixed the lint issues.

Travis has regressed after https://github.com/saltstack-formulas/lvm-formula/pull/14 got merged (I think) so I raised a new PR https://github.com/saltstack-formulas/lvm-formula/pull/15

The dependency (lvm) is causing the Travis failures now afaik. Once the Lvm PR is merged the Travis tests should work (fingers crossed).

noelmcloughlin commented 4 years ago

Travis CI is fixed now.

saltstack-formulas-travis commented 4 years ago

:tada: This PR is included in version 1.0.0 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket:

saltstack-formulas / iscsi-formula

refactor(formula): align to template-formula & fix #19 #23