Skip to content
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

Add unitary molecule tests for dhcp_server role #413

Merged
merged 11 commits into from
Nov 4, 2020

Conversation

osmocl
Copy link
Contributor

@osmocl osmocl commented Oct 14, 2020

  • Assing static ip in docker containers
  • Using a fake host declared in molecule.yml
  • Run unitary tests

Copy link
Member

@btravouillon btravouillon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @osmocl ,
Thank you for this new test! A few comments inline.

.github/TEST_MATRIX.md Outdated Show resolved Hide resolved
roles/core/dhcp_server/molecule/default/molecule.yml Outdated Show resolved Hide resolved
roles/core/dhcp_server/readme.rst Outdated Show resolved Hide resolved
roles/core/dhcp_server/molecule/default/molecule.yml Outdated Show resolved Hide resolved
@osmocl
Copy link
Contributor Author

osmocl commented Oct 16, 2020

In my platform the check E301 for ansible-lint is working perfectly in skip_list and warn_list.
I dot not know why it is not working here.

@btravouillon
Copy link
Member

In my platform the check E301 for ansible-lint is working perfectly in skip_list and warn_list.
I dot not know why it is not working here.

ansible-lint runs at the root of the project, it won't look at roles/core/dhcp_server/.ansible-lint.

This is the same failure than in #412, with the same command. You can safely add changed_when: False.

@hmeScaler
Copy link
Contributor

Hello,
I just remembered that the python module python3-netaddr.noarch will be required to be able to calculate the DHCP server configuration files.
Can we add this verification here?
Normally it's the bluebanquise role molecule test that does this verification.

- name: Test ipaddr filter in molecule controller

It is smart to re-check here?
Regards

@btravouillon
Copy link
Member

I just remembered that the python module python3-netaddr.noarch will be required to be able to calculate the DHCP server configuration files.
Can we add this verification here?

This module is required on the ansible controller, not the dhcp server. It is explicitly installed here for this scenario, there is no need to verify it on the ansible controller.

@hmeScaler
Copy link
Contributor

I just remembered that the python module python3-netaddr.noarch will be required to be able to calculate the DHCP server configuration files.
Can we add this verification here?

This module is required on the ansible controller, not the dhcp server. It is explicitly installed here for this scenario, there is no need to verify it on the ansible controller.

Great,
That's what I wanted to know,
Thanks @btravouillon

@hmeScaler hmeScaler closed this Oct 22, 2020
@hmeScaler hmeScaler reopened this Oct 22, 2020
@btravouillon btravouillon self-requested a review November 3, 2020 14:44
Copy link
Member

@btravouillon btravouillon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. 👍
Thank you @osmocl.

Copy link
Member

@oxedions oxedions left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dear @osmocl
Could you also increment role version, like you did here for nfs_server:

https://github.com/bluebanquise/bluebanquise/pull/412/files

@btravouillon
Copy link
Member

Could you also increment role version, like you did here for nfs_server:

Well, I explicitly asked him to remove it. There is no functional change in the role, only a new molecule scenario.

@oxedions
Copy link
Member

oxedions commented Nov 4, 2020

Could you also increment role version, like you did here for nfs_server:

Well, I explicitly asked him to remove it. There is no functional change in the role, only a new molecule scenario.

OK. Since it was incremented for the nfs_server, I was wondering. Let's keep it like this.

@oxedions oxedions merged commit eefbb36 into master Nov 4, 2020
@btravouillon btravouillon deleted the osmocl/molecule_tests_dhcp_server_role branch November 4, 2020 12:20
@btravouillon
Copy link
Member

Could you also increment role version, like you did here for nfs_server:

Well, I explicitly asked him to remove it. There is no functional change in the role, only a new molecule scenario.

OK. Since it was incremented for the nfs_server, I was wondering. Let's keep it like this.

That's a mistake. nfs_client introduced a new variable, not nfs_server.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants