Should we keep the fortinet collections (fortinet.fortimanager, fortinet.fortios) in the community Ansible package?

Right now Ansible contains two collections by fortinet:

Both collections together happen to be the largest part of an Ansible installation: the total ansible_collections directory is ~500 MB, while its fortinet subdirectory is ~220 MB. Out of these ~155 MB are for fortinet.fortimanager, and ~60 MB for fortinet.fortios. (The next largest subdirectory of ansible_collections is community with 73 MB for 29 different collections.)

Both collections have a lot of modules, many which seem to be auto-generated, or contain large auto-generated parts. The largest modules in both collections are 1.7 MB (!), the largest in each being ansible-galaxy-fortimanager-collection/plugins/modules/fmgr_fsp_vlan.py at main · fortinet-ansible-dev/ansible-galaxy-fortimanager-collection · GitHub and ansible-galaxy-fortios-collection/plugins/modules/fortios_wireless_controller_wtp_profile.py at main · fortinet-ansible-dev/ansible-galaxy-fortios-collection · GitHub. (Look at the bottom of these files and scroll around a bit - you’ll see one big reason why these modules are so large, and I’m pretty sure I’m not the only one who will think that it shouldn’t be that hard to massively shrink these files.)

If you look at their documentation, it looks mostly auto-generated from more or less helpful sources (less helpful: fortinet.fortimanager.fmgr_fsp_vlan module – no description — Ansible Community Documentation - more helpful: fortinet.fortios.fortios_wireless_controller_wtp_profile module – Configure WTP profiles or FortiAP profiles that define radio settings for manageable FortiAP platforms in Fortinet’s FortiOS and FortiGate. — Ansible Community Documentation, though it doesn’t say which unit is used for this option).

When we tried to run all sanity tests against all collections included in Ansible, a full sanity test run for these collections took way too long (I’m not even sure whether it ever completed, maybe @rfc2549 remembers).

All in all it seems that these two collections are really huge, their quality is not that great, and given that they only cater to a small subset of all Ansible users, I think we should discuss whether we want to remove them from Ansible. Users who need them can still install them manually with ansible-galaxy collection install <name>. What do you think? Also, it would be nice to hear from actual users of these collections!

(This would also be the largest reduction of the size of the ansible package since removal of the tests :slight_smile: )

5 Likes

I fully support this. The Ansible package is already very large, and these collections take up a disproportionate amount of space without providing value to the average ansible user. They also violate our Collection Requirements about CI testing.

2 Likes

+1 to the removal, I haven’ t seen any examples, blogs, or documentation from Fortinet users installing the Ansible Community Package and using the included collection. Even if they do install ansible instead of ansible-core most of the content related to Fortinet instructs them to install the collection with ansible-galaxy collection install afterward.

Have the Fortinet collection developers ever interacted with the Community WG or Steering Committee?

2 Likes

Indeed.

They have never reached out to us, but we have filed various issues with them when we found Collection Requirements violations during automated testing.

fortinet.fortimanager had a few interactions:

The sanity tests issue had no reaction though:

Interactions with fortinet.fortios:

The boolean values issue had no reaction:

(Also note that there is a community.fortios collection, which was part of Ansible until Ansible 9: GitHub - ansible-collections/community.fortios It was basically a dumping ground for some modules from Ansible 2.9 that they didn’t want to take over.)

2 Likes

I agree that those two collections are absurdly large. But I think we’ve never said that this is a reason to remove a collection, did we?

Don’t get me wrong, I think it would benefit the Ansible package to remove those collections if they don’t slim down considerably. But trying to find any other inclusion requirement violations in order to remove them somehow feels like… well, jailing Al Capone for tax violations, if you know what I mean.

3 Likes

@felixfontein thanks for bringing up such an important topic! I support the removal as, yes, presence of this content in the package is obviously disproportionate with its impact on the overall user/package maintainer experience.
@mariolenz i agree that we should fix this issue somehow differently:)

i think we should introduce a statement in the requirements and the SC policies that will allow the Committee to act based on particular circumstances. I opened the PR that adds it.
If there’s support, we could vote. I deliberately didn’t go in details in the PR as I’m sure the Committee members are selected as sane people and there won’t be outrage:)
Please take a look. Let’s make the package slim.

IMO we can already do this now by explicitly voting on something like that, but having this explicitly mentioned definitely doesn’t hurt. (I’m also somewhat sure we already mentioned this explicitly somewhere, but I right now can only find overview/removal_from_ansible.rst at main · ansible-collections/overview · GitHub which mainly mentions emergencies.)

1 Like

@felixfontein ah, great, so we can start working on exclusion right now! And, yeah, putting it explicitly in the SC policy and mention it in inclusion process would be a good thing for future similar cases. So PTAL at the PR:)
But it’s not an obstacle for proceeding with this IMO

Before starting a vote, I think we should inform the maintainers that we are planning to remove those collections because of their size and give them a chance to slim them down.

If they don’t, I’m +1 on voting to remove them.

Personally, I would vote -1 on removing a collection for size alone, and I don’t think we should approach them on that basis.

If there are other violations, and there’s been insufficient response or resolution, let’s move to remove like we would on any other collection, but I don’t think the size should be a factor.

It’s not just size, but also the inability to run sanity tests in an acceptable amount of time.

run sanity tests in an acceptable amount of time

problems like these are one of the reasons I was not really in favor of us centrally running sanity tests for collections :slight_smile: I would have preferred a model where test results are gathered centrally (lack of results also being detectable).

I don’t think we should really care if a collection’s tests (sanity or otherwise) take a long time to run, that’s for them to maintain.


In any case, sanity failures are a violation, and we should use the normal process to raise those issues and eventually remove the collections if they don’t comply, but I think we should leave size out of it.

but I think we should leave size out of it.

Shouldn’t we be concerned about this? I know it’s hard to define a limit but the fortinet plugins are pretty large, especially when compared to every other collection that is bundled. Not only does that make the ansible package larger for distribution and storage but it also really slows down the execution time when users go to use these modules are they need to be shipped across the wire for each invocation. I’ve got no skin in the game but I personally think the size should be something we are concerned about when it reaches what the fortinet collections contains.

2 Likes

I’ve created issues in both fortinet.fortimanager and fortinet.fortios to give them the opportunity to join the discussion.

2 Likes

Shouldn’t we be concerned about this?

Yes, but I think we should formally define the limits and criteria.

These issues don’t mention the sanity test failures, which seem the more critical to fix as that is already an inclusion requirement. Should we open separate issues for the CI failures?

1 Like

Since I’ve mentioned “some” problems, maybe just add a comment mentioning the CI failures instead of opening separate issues. Not really sure, just decide for yourself :slight_smile:

I’m against having explicit size limits. There’s no way to pick explicit numbers that are really useful. Ballpark figures can work, but even picking these is more like picking random numbers.

We should care more about quality (which can be related to size, as in the case of the two modules I pointed out in the initial post of this thread). And/or about whether the size of a collection roughly corresponds to how useful it is for all users.

There are already issues for them:

One problem (coming from the size) is that it’s very hard (or let’s say expensive) to check whether sanity errors are still present, since running all sanity tests takes a very long time. But at least for fortinet.fortimanager there are a lot of failing sanity tests: https://raw.githubusercontent.com/ansible-community/package-test-results/main/rendered/9.1.0/fortinet.fortimanager.md - this lists 13015 errors! For fortinet.fortios the sanity tests that were run by @gotmax23 passed (but these are just a subset of all of them).

1 Like

will do. I believe I can just point them to Ansible community package collections requirements — Ansible Documentation but want to doublecheck…