ovirt-ansible-collection icon indicating copy to clipboard operation
ovirt-ansible-collection copied to clipboard

Unable to run current versions of ansible-lint on this collection

Open ssbarnea opened this issue 3 years ago • 5 comments

I am quite curious about what is happening with this collection because I observed that cannot even run ansible-lint on it due its use of templating for its name @NAMESPACE@.@NAME@. Ansible-lint v5 does identify these as invalid (for good reasons). Also galaxy was updated to use ansible-lint v5 which is the first version that really supports collections.

# ansible-lint
We were unable to read either as JSON nor YAML, these are the errors we got from each:
JSON: Expecting value: line 1 column 1 (char 0)

Syntax Error while loading YAML.
  found character that cannot start any token

The error appears to be in '/Users/ssbarnea/c/a/ovirt-ansible-collection/roles/infra/roles/datacenters/tasks/main.yml': line 25, column 11, but may
be elsewhere in the file depending on the exact syntax problem.

The offending line appears to be:

  import_role:
    name: @NAMESPACE@.@[email protected]_cleanup
          ^ here

I guess that this templating is the reason why you forced used of old linter in build.sh. Any plans to fix it?

I am afraid that in this particular case, the issue is not a bug in the linter, the file causing the error is is invalid YAML.

PS. In case you need help with the linter, please let me know, I am its maintainer.

ssbarnea avatar Apr 25 '21 10:04 ssbarnea

Hi @ssbarnea, thanks for the issue.

I guess that this templating is the reason why you forced used of old linter in build.sh. Any plans to fix it?

It is not because of the templating but because the v5 added few checks which broke our CI and as a hotfix, we kept the v4. I have in plan to move to v5 but currently, the v5 ends with Finished with 349 failure(s), 0 warning(s) on 165 files. so we need to do quite a lot changes.

I am afraid that in this particular case, the issue is not a bug in the linter, the file causing the error is is invalid YAML.

We do build of the project before the lint and it replaces the @NAMESPACE@.@NAME@ with corresponding ovirt.ovirt/redhat.rhv

If you want to try it by yourself you can by ./build.sh build ovirt ~/.ansible/collections and cd to the created collection.

mnecas avatar Apr 26 '21 11:04 mnecas

Adding a new level of indirection (templating) seems to be the panacea for all coding problems ;)

Do not get scared about the multiple problems reports by v5, it is not supposed to generate so many. In fact number of effective rule changes between the two is very small.

I think that templating ansible code will not work in long run as it will break everywhere in the ecosystem. For example, this will prevent you from directly using ansible-test, tox-ansible and vscode-ansible at least.

When you "compile" code and run tests that fail, you will be send to the generated file, making any fix a PITA as you will have to identify the template that was used to generate that file. When you run the tests directly from the cloned repo, you do not have this problem, you click on the filename and is opened directly in your editor. You make the fix and run again. If it passes you make a PR.

I do remember one thing that really annoyed me was the hardcoded version inside galaxy.yml which required manual version update.

ssbarnea avatar Apr 26 '21 12:04 ssbarnea

We need source code templating to be able to have a signle repository for upstream ovirt.ovirt collection and downstream redhat.rhv collection. As Martin mentioned "untemplating" is a part of build process, which can contain several phases:

  1. Untemplapting
  2. Linting
  3. Building a package

mwperina avatar Apr 26 '21 14:04 mwperina

I know that ansible has some internal support for aliasing collections but I am not sure if it ended up being usable by users to add their own aliases. If that is true it may prove as a viable solution to only use the upstream name and only include the alias as part of installation.

I think that this issue in particular needs some feedback from Ansible core team. That is not the first and the last collection that would need to be distributed in both ways and I am sure that templating the codebase does NOT assure an easy development or maintenance.

@bcoca Do you know who can give some feedback on this subject?

ssbarnea avatar May 15 '21 13:05 ssbarnea

I know that ansible has some internal support for aliasing collections but I am not sure if it ended up being usable by users to add their own aliases. If that is true it may prove as a viable solution to only use the upstream name and only include the alias as part of installation.

The indirection functionality will not help here because it was designed to provide backward compatibility for content that moved from Ansible to collection (and possibly from one collection to another). It would be possible to have a redirect-only collection that just redirects all ovirt.ovirt.* names to redhat.rhv.* names, but since the redhat.rhv is not freely available, this will not work. ANd I would assume that redirects in the other way are also not possible because it would require paying customers to install non-certified collections.

tadeboro avatar May 15 '21 15:05 tadeboro