ci: introduce unit testing of roles with Molecule

[strus38]

• Start with roles time and dns_server
• Added molecule lint/converge
• Remove unnecessary empty lines to pass the linter phase

[btravouillon]

Hi @strus38,

Thank you for the PR. That's a huge work.

Almost all molecule test will failed if run automatically since roles needs some rework especially on default values, but this should be part of separated PR.

Indeed this is something I noticed a couple of days ago, most if not all of our default parameters are not defined in defaults/main.yml. PRs should come soon.

Force LF instead of CRLF

Do you mean force CRLF instead of LF? It looks like you changed line-endings from LF to CRLF. Anyway, you can't change the line-endings like that, it would need to be discussed first. :-)

Remove unecessary empty lines to pass the linter phase

Nice catch! Our static code analysis runs ansible-lint, but not yamllint. It may need a separate PR however. (check fc2f313 to see how I made ansible-lint happy before enabling static code analysis)

Overall this is very interesting and I will test this as soon as possible! I'm wondering if it could be easier to introduce support of molecule with a single role first, to see how it behaves and how we can integrate it with GitHub actions.
Also, I don't understand why the test_hosts_file(host) is defined in all the modules?

Finally, you need to configure your git (Author is root <root@LAPTOP-063FLOTR.localdomain>):

$ git commit --amend --author="Author Name <email@address.com>" --no-edit

[btravouillon]

By the way, the cause of the conflicts is because your commit is not on top of master:

$ git log --oneline --no-merges --name-only strus38/feature/molecule..master 
629db3f (HEAD -> master, origin/master, origin/HEAD) [advanced_dhcp_sever] remove references to ntp_ip variable (#153)
resources/documentation/configure_bluebanquise.rst
resources/examples/multi_icebergs_cluster/inventory/group_vars/all/networks/ice1-1.yml
resources/examples/multi_icebergs_cluster/inventory/group_vars/all/networks/ice2-1.yml
resources/examples/multi_icebergs_cluster/inventory/group_vars/all/networks/ice3-1.yml
resources/examples/multi_icebergs_cluster/inventory/group_vars/all/networks/ice3-2.yml
roles/advanced-core/advanced_dhcp_server/templates/dhcpd.networks.conf.j2
3a34479 fix: install jinja2 macros in rpm
bluebanquise.spec
fed7da3 fix: require clustershell in specfile
bluebanquise.spec
f7ffaf0 Fix: Adding missing role Ansible
roles/core/ansible/readme.rst
roles/core/ansible/tasks/main.yml

[strus38]

I had to do a dos2unix on all files to have the linter working. Clear for the master branch, normally I never work on the master branch, since for me it should always be reflecting the last stable version. I will change my name. Thanks Le sam. 11 avr. 2020 à 00:46, Bruno <notifications@github.com> a écrit :

…

By the way, the cause of the conflicts is because your commit is not on top of master: $ git log --oneline --no-merges --name-only strus38/feature/molecule..master 629db3f (HEAD -> master, origin/master, origin/HEAD) [advanced_dhcp_sever] remove references to ntp_ip variable (#153) resources/documentation/configure_bluebanquise.rst resources/examples/multi_icebergs_cluster/inventory/group_vars/all/networks/ice1-1.yml resources/examples/multi_icebergs_cluster/inventory/group_vars/all/networks/ice2-1.yml resources/examples/multi_icebergs_cluster/inventory/group_vars/all/networks/ice3-1.yml resources/examples/multi_icebergs_cluster/inventory/group_vars/all/networks/ice3-2.yml roles/advanced-core/advanced_dhcp_server/templates/dhcpd.networks.conf.j2 3a34479 fix: install jinja2 macros in rpm bluebanquise.spec fed7da3 fix: require clustershell in specfile bluebanquise.spec f7ffaf0 Fix: Adding missing role Ansible roles/core/ansible/readme.rst roles/core/ansible/tasks/main.yml — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#162 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABM6DUYU6SONGUKXXK5JLALRL6OWBANCNFSM4MFTDZDA> .

[strus38]

Hi

I fixed some of your requests:

• change root to my id
• re-sync with master to avoid conflicts
• the dos2unix had to be passed on all for yamlint/ansible-lint to pass on the yaml files (so I did not cganged that)
• the test_hosts_file(host) is there by default when you init molecule within a role: molecule init scenario -d docker --verifier-name testinfra, but as you can see, this part is not called yet since the 'verifier' phase is currently disabled in molecule since none of the role are currently running correctly in the converge phase because of lack of default variables. So, I want to keep that to help next people an exemple of what they have to do in such file.

If that is all, I guess you can attempt another review. Thanks.

[strus38]

Note: on windows10 WSL running Ubuntu 18.04, you must define in your Ubuntu bash:
git config --global core.autocrlf input to prevent LF/CRLF issues.

So I am reworking the PR to just validate the approach on 2 roles.

[btravouillon]

Nice change, thanks!
Until now, we were defining default values in the examples inventories. This PR demonstrates we should use Ansible Roles mechanism and define the default values in the role (defaults/main.yml). Will add some reviewers to get feedback.

[strus38]

Fixed all the remarks. Should be ok now. Thanks

[strus38]

Travis result:

The failing ones are because of a package is not available. Please, suggest how to fix it.

Fedora31

Ubuntu18.04

Also, I cannot submit code to integrate the travis build badges because it is enabled only on my fork/github account:

[btravouillon]

We're almost done!
I noticed an issue with the chrony.conf.j2 template (in current implementation, not introduced by this PR). Please revert your changes to this file, I will open a ticket to fix this.
Also:

• no need to check if time_zone is defined since it now has a default value.
• fedora is not supported

Thank you for the huge work on this.

[btravouillon]

✌️ 👍 Nice! Needs a second review before squash & merge.

[strus38]

Note: I have just completed a new github projet: docker-ubi7-ansible so we will be able to update travis config with ubi7 images.
The image is available on docker hub docker pull strusfr/docker-ubi7-ansible:latest

[oxedions]

Hello,

Thank you for this PR.
Please do not merge it, I wish to take time to investigate all of this.

Have a nice weekend

Ox

[oxedions]

Thank you for this PR. Many nice quality improvement. :-)
Please have a look on my comments regarding defaults.

[strus38]

Fixed the PR as suggested by comments. Please review again.

[btravouillon]

Good for me.

[strus38]

all fixed I guess.

[btravouillon]

Seems good.

[oxedions]

Ok for me, many thanks for this huge work.