ansible-lint broken when called from pre-commit

Bug #1848512 reported by Sorin Sbarnea
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
tripleo
Fix Released
Critical
Sorin Sbarnea

Bug Description

All jobs using ansible-lint via pre-commit (aka tox-molecule-linters) got broken when a newer version of setuptools was released.

The bug itself is not caused by setuptools but by the ugly-hack present inside setup.py from the ansible-lint project.

I already proposed the PR to fix it but projects maintenance status is not one of the best right now, it may take more time to get it merged and a new release made.

Meanwhile I was able to find a workaround that looks like:
https://review.rdoproject.org/r/#/c/23215/4/tox.ini

Fixes need to be done in all repositories where we encounter them.

Upstream references:

* https://github.com/ansible/ansible-lint/issues/590 <-- please comment here, and also cross post to #ansible-galaxy channel (they own the project)
There are already two PRs that are fixing the issue but they were not reviewed/merged/released.

* https://github.com/pypa/setuptools/issues/1869

I will mark this as a promotion blocker because is high-likely that most CR would be blocked by the broken linters jobs until we implement workarounds everywhere.

Temporary workarounds:

deps =
    # workaround for https://github.com/ansible/ansible-lint/issues/590
    virtualenv==16.3.0 # 16.7.6 not working
    pre-commit

By pinning the virtualenv on tox.ini we can avoid this breakage, that is because newer virtualenv bring a newer setuptools, one that is not compatible with the dirty hacks made in setup.py of ansible-lint.

wes hayutin (weshayutin)
tags: added: alert
Sorin Sbarnea (ssbarnea)
description: updated
description: updated
Sorin Sbarnea (ssbarnea)
Changed in tripleo:
status: Triaged → In Progress
Revision history for this message
Sorin Sbarnea (ssbarnea) wrote :

Apparently the workaround above does work only for some cases and not for all of them.

Pinged #ansible-devel team daily since last week to review my two possible bugfixes but still without tangible results.

Meanwhile I ended up creating fork of ansible-lint with the patch on it and got the confirmation that this works for us.

This could be used as a temporary workaround:
https://review.opendev.org/#/c/689719/3/.pre-commit-config.yaml

Revision history for this message
Marios Andreou (marios-b) wrote :

bunch of changes merged with the workaround but we did a terrible job of tracking those and did not include this bug (bug was filed after some of the changes merged)

capturing related changes here at least as many as i could quickly find

https://review.opendev.org/689719
https://review.opendev.org/689527
https://review.opendev.org/689144
https://review.opendev.org/689170
https://review.opendev.org/689148
https://review.opendev.org/689129

Revision history for this message
Marios Andreou (marios-b) wrote :

i think this should be mostly done but we keep the bug around for tracking the 'proper' fix once it merges upstream. Removing alert from the bug for now the patches in comment #2 are merged now except the first one

tags: added: ci
removed: alert
Revision history for this message
Sorin Sbarnea (ssbarnea) wrote :

My fixed merge to ansible-lint master branch. We can change revision to that changeset in hook config and avoid the workaround. This is a much better workaround because it will work fine with `pre-commit autoupdate`, mainly not requiring any undoing later.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to tripleo-quickstart (master)

Reviewed: https://review.opendev.org/689719
Committed: https://git.openstack.org/cgit/openstack/tripleo-quickstart/commit/?id=9afdcca1bc2cb7d87a433fb4f02e5569067d7a4b
Submitter: Zuul
Branch: master

commit 9afdcca1bc2cb7d87a433fb4f02e5569067d7a4b
Author: Sorin Sbarnea <email address hidden>
Date: Mon Oct 21 12:31:54 2019 +0100

    Workaround ansible-lint installation failure

    * Bumps ansible-lint to master vesion (which got fix)
    * Resolves bug with vars file placed inside tasks folder which caused
      linter to crash parsing them.
      https://github.com/ansible/ansible-lint/issues/595
    * Includes tox.ini fix for HTTP proxies

    Partial-Bug: #1848512
    Partial-Bug: https://github.com/ansible/ansible-lint/issues/590
    Partial-Bug: https://github.com/ansible/ansible-lint/issues/595
    Change-Id: Ib505b05ef909d0ded9dd87a67166b8f821d62d14

Revision history for this message
Marios Andreou (marios-b) wrote :
Revision history for this message
Carlos Camacho (ccamacho) wrote :

Fix for tripleo-upgrade proposed here: https://review.opendev.org/#/c/691686

Revision history for this message
Marios Andreou (marios-b) wrote :

@Sorin do we want to keep this around - not clear to me from comments #3/4 do we have anything else to merge ? The job should no longer be failing in any TripleO repos now, but is there still a workaround/undoing of the workaround that we need to track?

Revision history for this message
Sorin Sbarnea (ssbarnea) wrote :

We can safely remove alert on it but the ticket should be kept open for another week or so.

Now we have a long-term solution: removal of workaround (unpinning virtualenv) and doing "precommit autoupdate" (raise CR after) which will pick a newer version of ansible-lint that is not affected by this bug.

Anyone interested about ansible-list is welcomed to contact me directly if they have questions, bugs or other requests. I started to work with Ansible team more on it and we will be addressing most of its issues.

Sorin Sbarnea (ssbarnea)
Changed in tripleo:
status: In Progress → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.