ansible / molecule

Molecule aids in the development and testing of Ansible content: collections, playbooks and roles
https://ansible.readthedocs.io/projects/molecule/
MIT License
3.87k stars 659 forks source link

Removal of testinfra verifier #3920

Closed ssbarnea closed 1 year ago

ssbarnea commented 1 year ago

As part of v6, we intend to make Ansible to be the default verifier and the only verifier to be supported in this project. This will mean removing testinfra as a dependency of Molecule. testinfra can still be used as a verifier with Molecule. It just will become a separate optional component for developers that wish to use it and not maintained as part of the main Molecule code base and distribution. The APIs for other verifiers such as testinfra will remain.

This is not in any way a reflection on the quality and efforts of testinfra. It is a great and powerful library. It is a general purpose infrastructure library for python programmers with support for not only Ansible, but Chef, Puppet, Saltstack and a lot more though. In refocusing Molecule as test runner specifically for Ansible playbooks and roles, testinfra is no longer a good fit for Molecule's purpose. Besides requiring python coding skills that most Ansible content creators do not possess, it either meant supporting testinfra itself or trying to carve out in documentation what is or is not supported and maintained as part of this project’s work. (This is guaranteed to be complicated and confusing.) So in order to clarify what every release tests and maintains, testinfra will need to be removed from the dependencies of this project's packages.

tima commented 1 year ago

The reasoning for this move as I understand it, is that testinfra is for writing general purpose testing for infrastructure with python. While a very powerful library, its features and dependencies go well beyond the scope of Ansible content testing and require Ansible content creators to have python programming skills to verify their content. This is not consistent with Ansible’s “no programming skills required” ethos for authoring playbooks and roles that Molecule is intended to test.

Removing testinfra from the Molecule distribution will also clarify what every release tests and maintains. This streamlining will also improve its overall supportability and streamline the tool and its installation process.

apatard commented 1 year ago

there may be bugs but the testinfra verifier is working and as I said in the past, I (and maybe others?) prefer using it rather than the ansible verifier.

While I understand the concerns like "no programming skills required" that doesn't mean that people with the knowledge should be forced to use the ansible verifier.

When looking at this issue and the other "for v6" changes, it more or less feels like the idea is to kill molecule and replace it with a ansible-playbook command line.

tima commented 1 year ago

There is nothing stopping anyone from using testinfra to verify the state of the infrastructure, it's just that it won't be in molecule distribution. Besides requiring python coding skills most Ansible content creators do not possess, testinfra does a lot more than can be maintained and supported within the context of Ansible content testing. This will also better position Molecule for integration with VS Code and all of the containerized Ansible effort.

So, we are not trying to kill Molecule. Quite the opposite. Red Hat wouldn't be putting it on the path the supportability if it was getting rid of it. These changes are being made so it has a path forward as an effective tool for testing Ansible content that is focused, maintainable and better supported.

pimvh commented 1 year ago

Thanks for this suggestion. First off, what does not supporting it mean? Just not packaging it or also stop allowing to provide a custom input to the verifier? From what I gather from @tima it does not mean the latter.

I understand that in general maintaining external things can be troublesome, but could you elaborate a little bit more on the cost of maintenance? Where is it currently costly? I don't think I fully understand. :)

Additionally I also understand the "no programming skills required", I also value accessibility very highly! In order to lessen the burden for people to understand testinfra I would be happy to contribute to the documentation. Just point me to the right place!

For me, a counter argument to this idea, is that using a tool to verify itself is generally an anti-pattern. It's like a person that's marking his/her own exam. Moving away from other verifiers means that bugs in Ansible core modules cannot be caught by Molecule anymore, if you get what I mean. I get that testing (core) Ansible functionality is (partially) out of scope for this tool, but this could still be helpful for system administrators and for finding bugs in general.

EDIT: I just though of this, does the Ansible-based verifier currently support the same flow of execution as testinfra, that is, that all test are executed regardless of whether they are successful or failed? I think that could also be a reason to keep support testinfra, unless you implement a similar feature in the Ansible-based verifier.

davedittrich commented 1 year ago

As part of v6, we are considering making Ansible the only verifier to be supported.

If I read the initial proposal correctly, making Ansible the only verifier to be supported (as in, molecule will only allow Ansible to be run in the verifier stage), I oppose this strongly. As mentioned in other comments, testinfra is a very powerful, yet fairly simple mechanism for testing on hosts. It is a pytest plugin, so the real proposal here is to also remove pytest from use as a verifier, not testinfra alone. If that is not correct, please clarify this.

In the past, when a proposal is made to remove a feature for "supportability" reasons, that means that the primary maintainer of the code is not willing or able to fix it. That is what open source PRs are for, but they, too, are often rejected in situations where it doesn't seem to make sense in terms of UX. I have code that fixes the bugs, but it will require me making simultaneous PRs to two different projects and getting them both accepted, which is adding friction to the process.

The only bugs I can recall at the moment related to testinfra (months ago, and I haven't touched my code in a long time, which I will come back to in a moment) had to do with changes to output formatting in an attempt to produce colorful output and conflicts with command line options with another pytest plugin. Programmers who believe in and practice test-driven development might use pytest and this proposal restricts that ability. While adding colorful output is "nice to have", it isn't "necessary to have," and I think the user experience of having flexibility and the option to use the same Python testing tools for the Python source of your Ansible plugins, filters, etc. that go into your Ansible Collection you are testing with molecule should take higher priority to buggy attempts to add colorful output.

Getting back to the "not touching my code in a while" comment, I have found that "supportability" and "nice-to-have" feature additions often result in bugs, some breaking bugs, that are added without sufficient functional and regression testing frequently break my projects. Deprecations of features followed shortly after with complete removal contribute to instability, further negatively impact UX. These breaking changes require that I spend a non-trivial amount of time to get things working again before I can get back to progressing on features of my own code. I'm almost at the point of abandoning all Ansible products due to things like difficulty in finding current documentation, lack of good examples, regression bugs, rapid deprecation and removal of features I am using (based on examples in high-ranking search results), and the friction and instability these tools are adding to my own projects.

I think that at least allowing an optional hook to use any tool a programmer wants to linting, verification, etc., maintaining stability in a tool like molecule and over all UX should be given higher priority, whether that means adding more maintainers or being more flexible about accepting PRs. Maybe encourage users who are capable of helping maintain features though a "bug-bounty," cash rewards, contracts, ??? Or maybe just slow down the pace of change enough to improve test-driven development of Ansible tooling and documentation?

davedittrich commented 1 year ago

The reasoning for this move as I understand it, is that testinfra is for writing general purpose testing for infrastructure with python. While a very powerful library, its features and dependencies go well beyond the scope of Ansible content testing and require Ansible content creators to have python programming skills to verify their content.

Ansible Collections include plugins, filters, etc., all written in Python. A powerful library (actually pytest - testinfra is just a plugin for pytest) actually improves Ansible content testing. Making it harder for people to produce Ansible Collections seems to the opposite of what you would want.

This is not consistent with Ansible’s “no programming skills required” ethos for authoring playbooks and roles that Molecule is intended to test.

If molecule cannot be used for easy and reliable test-driven of new Ansible roles, plugins, filters, etc., you may end up with buggy roles, plugins, filters, etc. that will all look like Ansible is the fault (as it calls these things behind the scenes). If someone isn't a good Python programmer, they can't easily debug the problems and thus will simply add more Issues and increase maintenance cost, not decrease it.

Removing testinfra from the Molecule distribution will also clarify what every release tests and maintains. This streamlining will also improve its overall supportability and streamline the tool and its installation process.

This comment seems to talk about installation, not execution of the verifier. Are these concepts being conflated here, or is the description of the proposal inadequately expressed and being misunderstood by all of us who are commenting? I think more specifics should be added to the description of the proposal (even if that means killing this thread and starting a new, more clearly defined, proposal.)

tima commented 1 year ago

@pimvh and @davedittrich: I appreciate all of the feedback so far and want to hopefully put your mind at ease and clarify some things. I'm reading a lot of assumptions that are not accurate.

One thing I want to emphasize here is that Molecule is being refocused as a test runner for functional testing of Ansible playbooks and roles using Ansible itself. This is to say it is not a tool for doing unit and integration testing of modules and plugins that are written in python. There has been ansible-test in the past and there is a lot of effort going into pytest-ansible and tox-ansible right now to provide standard python tools for testing the content written in python.

I need to update this milestone and underlying issues to better reflect what is being communicated through other channels.

First off, what does not supporting it mean? Just not packaging it or also stop allowing to provide a custom input to the verifier? From what I gather from @tima it does not mean the latter.

Correct. I meant the former. Supporting means that it is a dependency of the molecule distribution and is being tested and maintained as part of each release. Red Hat's efforts won't go into the "not supported" parts and will not be part of the downstream distribution made available to AAP subscribers. I have not seen or heard any plans to rip out the APIs that these unsupported parts use. I'd personally be opposed to that.

I understand that in general maintaining external things can be troublesome, but could you elaborate a little bit more on the cost of maintenance? Where is it currently costly? I don't think I fully understand. :)

Molecule contains drivers performing operations that can be found in Ansible collections such as amazon.aws etc. So that is two code bases to maintain that do the same thing. Relying upon the delegated driver means we can leave testing and maintenance to those projects rather than executing them in this project. testinfra poses similar burdens and then some that I'll cover in a bit.

In order to lessen the burden for people to understand testinfra I would be happy to contribute to the documentation. Just point me to the right place!

Regardless how trivial it seems to a seasoned coder, the problem is that testinfra requires python programming skills. No amount of documentation can address that.

... tool to verify itself is generally an anti-pattern.

It's not verifying itself. It is verifying content (playbooks and roles) that run through it. I don't see this as being any different than using python to unit test your python libraries and applications.

I just though of this, does the Ansible-based verifier currently support the same flow of execution as testinfra...

That I will need to leave to the engineers.

If I read the initial proposal correctly, making Ansible the only verifier to be supported (as in, molecule will only allow Ansible to be run in the verifier stage), I oppose this strongly.

I hear you. That is not the case. You can use other verifiers. They won't be part of this project's pipeline and distribution that will get supported and maintained in this project though.

In the past, when a proposal is made to remove a feature for "supportability" reasons, that means that the primary maintainer of the code is not willing or able to fix it.

This is not in anyway a reflection on the quality and efforts of testinfra. It is a great and powerful library. It is a general purpose infrastructure library for python programmers with support for not only Ansible, but Chef, Puppet, Saltstack and a lot more though. In refocusing Molecule as test runner specifically for Ansible playbooks and roles, testinfra is not longer a good fit for Molecule's purpose. Besides requiring python coding skills that most Ansible content creators do not possess, it either meant supporting testinfra itself or trying to carve out in documentation what is or is not supported and maintained. (This is guaranteed to be complicated and confusing.) So in order to clarify what every release tests and maintains, testinfra will need to be removed from the dependencies of this project's packages.

Ansible Collections include plugins, filters, etc., all written in Python. A powerful library (actually pytest - testinfra is just a plugin for pytest) actually improves Ansible content testing. Making it harder for people to produce Ansible Collections seems to the opposite of what you would want.

We wholeheartedly agree! That is why Red Hat is going to be investing its effort in getting this project to a state that is more accessible and useful to all Ansible content creators to test their playbooks and roles in addition to effort into the pytest-ansible and tox-ansible plugins for python developers creating module and plugin content.

This comment seems to talk about installation, not execution of the verifier. Are these concepts being conflated here, or is the description of the proposal inadequately expressed and being misunderstood by all of us who are commenting?

I believe they are. I didn't file these issues and need to get permission to edit them to expand and clarify on things I've covered in my response. You can still use testinfra with Molecule if you want. We are not ripping out the APIs that would be used to do that. It's just that it will be an independent optional component.

pimvh commented 1 year ago

Thank you for the clarification.

tima commented 1 year ago

I've updated the description of this issue to expand and clarify on things I covered in my earlier comments here.

trishnaguha commented 1 year ago

@ssbarnea I believe we added this as stretch for v6. Can we remove v6 then?