[all][gate][stable] Pinning tox<4 in stable branch testing
Hello Everyone, As you might know, tox4 broke almost all the projects tox.ini and so do master as well as stable branches gate. On the master, we need to fix it as we should use tox4 in testing. To fix the stable branches gate, either we need to backport all the fixes which include some more issues fixes[1] or we can pin the tox<4. We discussed it in today's TC meeting and it is better to keep testing the stable branch with the tox version that they were released with and not with the tox4. Even in future, there might be the cases where latest tox might introduce more incompatible changes. By considering all these factors, it is better to pin tox<4 for stable branches (<=stable/zed) testing. I have prepared the patch to pin it in the common job/template, feel free to comment with your feedback/opinion: - https://review.opendev.org/c/openstack/openstack-zuul-jobs/+/867849/3 also, tested with nova/cinder stable branches: - https://review.opendev.org/q/I0ca55abf9975c5a3f9713ac5dd5be39083e04554 - https://review.opendev.org/q/I300e7804a27d08ecd239d1a7faaf2aaf3e07b9ee You can also test it in your project stable branches in case any different syntax in tox.ini causing the tox upgrade to the latest. [1] https://github.com/tox-dev/tox/issues/2712 -gmann
Apologies for top posting, but in addition the change gmann has proposed, I believe that you'll need to change your tox.ini file to pin tox <4. I ran into this working on [0] yesterday, where cinderclient functional tests are devstack-based, and at some point during devstack install someone [1] pip-installs tox unconstrained. The zuul ensure_tox role only ensures that tox is present. The ensure_tox_version var has a slightly misleading name in that it is only used when the role decides it needs to install tox, and then it uses the value of that var; it doesn't ensure that the available tox is that version. I've verified that the 'requires = tox<4' trick in [0] works when the tox being called is >=4 [2]; tox creates a virtualenv in .tox/.tox and installs tox<4 in there, and then runs your testenvs using the tox you required in your tox.ini. cheers, brian [0] https://review.opendev.org/c/openstack/python-cinderclient/+/869263 [1] not naming any names here; also this same situation will happen if the test node image already contains tox4 [2] it works in tox 3 too, sometime after tox 3.18.0. Hopefully it will continue to work in tox 4, though they way they're introducing bad regressions (e.g., [3]), I guess I should say that i've verified that it works through 4.2.2 (4.2.4 was released yesterday). [3] https://github.com/tox-dev/tox/issues/2811 On 1/4/23 11:08 PM, Ghanshyam Mann wrote:
Hello Everyone,
As you might know, tox4 broke almost all the projects tox.ini and so do master as well as stable branches gate. On the master, we need to fix it as we should use tox4 in testing. To fix the stable branches gate, either we need to backport all the fixes which include some more issues fixes[1] or we can pin the tox<4.
We discussed it in today's TC meeting and it is better to keep testing the stable branch with the tox version that they were released with and not with the tox4. Even in future, there might be the cases where latest tox might introduce more incompatible changes. By considering all these factors, it is better to pin tox<4 for stable branches (<=stable/zed) testing.
I have prepared the patch to pin it in the common job/template, feel free to comment with your feedback/opinion: - https://review.opendev.org/c/openstack/openstack-zuul-jobs/+/867849/3
also, tested with nova/cinder stable branches: - https://review.opendev.org/q/I0ca55abf9975c5a3f9713ac5dd5be39083e04554 - https://review.opendev.org/q/I300e7804a27d08ecd239d1a7faaf2aaf3e07b9ee
You can also test it in your project stable branches in case any different syntax in tox.ini causing the tox upgrade to the latest.
[1] https://github.com/tox-dev/tox/issues/2712
-gmann
On 2023-01-06 08:42:45 -0500 (-0500), Brian Rosmaita wrote: [...]
The zuul ensure_tox role only ensures that tox is present. The ensure_tox_version var has a slightly misleading name in that it is only used when the role decides it needs to install tox, and then it uses the value of that var; it doesn't ensure that the available tox is that version. [...]
And even if it did, it would only be able to do so at the point at which it was invoked, so if something reinstalled/upgraded tox after that point it would still be different than whatever you set in ensure_tox_version. Still, it might be possible to make ensure_tox_version enforce a specific version if set which would catch circumstances like the one you observed where something running in the job before the ensure-tox role is invoked installed a different version globally. -- Jeremy Stanley
On Fri, Jan 6, 2023, at 6:49 AM, Jeremy Stanley wrote:
On 2023-01-06 08:42:45 -0500 (-0500), Brian Rosmaita wrote: [...]
The zuul ensure_tox role only ensures that tox is present. The ensure_tox_version var has a slightly misleading name in that it is only used when the role decides it needs to install tox, and then it uses the value of that var; it doesn't ensure that the available tox is that version. [...]
And even if it did, it would only be able to do so at the point at which it was invoked, so if something reinstalled/upgraded tox after that point it would still be different than whatever you set in ensure_tox_version. Still, it might be possible to make ensure_tox_version enforce a specific version if set which would catch circumstances like the one you observed where something running in the job before the ensure-tox role is invoked installed a different version globally.
Doing so would just mask that you've improperly installed tox elsewhere (a bug itself). I think the current behavior is correct because it isn't letting us get away with that. Elsewhere we have tools (devstack) that install tempest at least 3 redundant times for every tempest job. Its a waste of effort and we shouldn't make it easier for ourselves to fall into those traps.
-- Jeremy Stanley
Attachments: * signature.asc
---- On Fri, 06 Jan 2023 05:42:45 -0800 Brian Rosmaita wrote ---
Apologies for top posting, but in addition the change gmann has proposed, I believe that you'll need to change your tox.ini file to pin tox <4. I ran into this working on [0] yesterday, where cinderclient functional tests are devstack-based, and at some point during devstack install someone [1] pip-installs tox unconstrained.
The zuul ensure_tox role only ensures that tox is present. The ensure_tox_version var has a slightly misleading name in that it is only used when the role decides it needs to install tox, and then it uses the value of that var; it doesn't ensure that the available tox is that version.
I've verified that the 'requires = tox<4' trick in [0] works when the tox being called is >=4 [2]; tox creates a virtualenv in .tox/.tox and installs tox<4 in there, and then runs your testenvs using the tox you required in your tox.ini.
I saw in the log that it is using the ensure-tox role from devstack/playbooks/tox/run-both.yaml - https://zuul.opendev.org/t/openstack/build/c957db6323dc4b42bee07f6b709fb3ad/... Which is run after pre-yaml where we pinned tox<4 via ensure_tox_version but missed doing it in run-both.yaml. Testing it by pinning in run-both.yaml also. run-both - https://review.opendev.org/c/openstack/python-cinderclient/+/869494 -gmann
cheers, brian
[0] https://review.opendev.org/c/openstack/python-cinderclient/+/869263 [1] not naming any names here; also this same situation will happen if the test node image already contains tox4 [2] it works in tox 3 too, sometime after tox 3.18.0. Hopefully it will continue to work in tox 4, though they way they're introducing bad regressions (e.g., [3]), I guess I should say that i've verified that it works through 4.2.2 (4.2.4 was released yesterday). [3] https://github.com/tox-dev/tox/issues/2811
On 1/4/23 11:08 PM, Ghanshyam Mann wrote:
Hello Everyone,
As you might know, tox4 broke almost all the projects tox.ini and so do master as well as stable branches gate. On the master, we need to fix it as we should use tox4 in testing. To fix the stable branches gate, either we need to backport all the fixes which include some more issues fixes[1] or we can pin the tox<4.
We discussed it in today's TC meeting and it is better to keep testing the stable branch with the tox version that they were released with and not with the tox4. Even in future, there might be the cases where latest tox might introduce more incompatible changes. By considering all these factors, it is better to pin tox<4 for stable branches (<=stable/zed) testing.
I have prepared the patch to pin it in the common job/template, feel free to comment with your feedback/opinion: - https://review.opendev.org/c/openstack/openstack-zuul-jobs/+/867849/3
also, tested with nova/cinder stable branches: - https://review.opendev.org/q/I0ca55abf9975c5a3f9713ac5dd5be39083e04554 - https://review.opendev.org/q/I300e7804a27d08ecd239d1a7faaf2aaf3e07b9ee
You can also test it in your project stable branches in case any different syntax in tox.ini causing the tox upgrade to the latest.
[1] https://github.com/tox-dev/tox/issues/2712
-gmann
---- On Fri, 06 Jan 2023 10:41:43 -0800 Ghanshyam Mann wrote ---
---- On Fri, 06 Jan 2023 05:42:45 -0800 Brian Rosmaita wrote ---
Apologies for top posting, but in addition the change gmann has proposed, I believe that you'll need to change your tox.ini file to pin tox <4. I ran into this working on [0] yesterday, where cinderclient functional tests are devstack-based, and at some point during devstack install someone [1] pip-installs tox unconstrained.
The zuul ensure_tox role only ensures that tox is present. The ensure_tox_version var has a slightly misleading name in that it is only used when the role decides it needs to install tox, and then it uses the value of that var; it doesn't ensure that the available tox is that version.
I've verified that the 'requires = tox<4' trick in [0] works when the tox being called is >=4 [2]; tox creates a virtualenv in .tox/.tox and installs tox<4 in there, and then runs your testenvs using the tox you required in your tox.ini.
I saw in the log that it is using the ensure-tox role from devstack/playbooks/tox/run-both.yaml - https://zuul.opendev.org/t/openstack/build/c957db6323dc4b42bee07f6b709fb3ad/...
Which is run after pre-yaml where we pinned tox<4 via ensure_tox_version but missed doing it in run-both.yaml. Testing it by pinning in run-both.yaml also.
Pinning in run-both.yaml playbook did not fix the python-cinderclient issue and pinning tox<4 in tox.ini is the way forward for this case. -gmann
- https://review.opendev.org/c/openstack/python-cinderclient/+/869494
-gmann
cheers, brian
[0] https://review.opendev.org/c/openstack/python-cinderclient/+/869263 [1] not naming any names here; also this same situation will happen if the test node image already contains tox4 [2] it works in tox 3 too, sometime after tox 3.18.0. Hopefully it will continue to work in tox 4, though they way they're introducing bad regressions (e.g., [3]), I guess I should say that i've verified that it works through 4.2.2 (4.2.4 was released yesterday). [3] https://github.com/tox-dev/tox/issues/2811
On 1/4/23 11:08 PM, Ghanshyam Mann wrote:
Hello Everyone,
As you might know, tox4 broke almost all the projects tox.ini and so do master as well as stable branches gate. On the master, we need to fix it as we should use tox4 in testing. To fix the stable branches gate, either we need to backport all the fixes which include some more issues fixes[1] or we can pin the tox<4.
We discussed it in today's TC meeting and it is better to keep testing the stable branch with the tox version that they were released with and not with the tox4. Even in future, there might be the cases where latest tox might introduce more incompatible changes. By considering all these factors, it is better to pin tox<4 for stable branches (<=stable/zed) testing.
I have prepared the patch to pin it in the common job/template, feel free to comment with your feedback/opinion: - https://review.opendev.org/c/openstack/openstack-zuul-jobs/+/867849/3
also, tested with nova/cinder stable branches: - https://review.opendev.org/q/I0ca55abf9975c5a3f9713ac5dd5be39083e04554 - https://review.opendev.org/q/I300e7804a27d08ecd239d1a7faaf2aaf3e07b9ee
You can also test it in your project stable branches in case any different syntax in tox.ini causing the tox upgrade to the latest.
[1] https://github.com/tox-dev/tox/issues/2712
-gmann
On Fri, Jan 6, 2023, at 3:56 PM, Ghanshyam Mann wrote:
---- On Fri, 06 Jan 2023 05:42:45 -0800 Brian Rosmaita wrote ---
Apologies for top posting, but in addition the change gmann has proposed, I believe that you'll need to change your tox.ini file to pin tox <4. I ran into this working on [0] yesterday, where cinderclient functional tests are devstack-based, and at some point during devstack install someone [1] pip-installs tox unconstrained.
The zuul ensure_tox role only ensures that tox is present. The ensure_tox_version var has a slightly misleading name in that it is only used when the role decides it needs to install tox, and then it uses the value of that var; it doesn't ensure that the available tox is
I've verified that the 'requires = tox<4' trick in [0] works when
---- On Fri, 06 Jan 2023 10:41:43 -0800 Ghanshyam Mann wrote --- that version. the
tox being called is >=4 [2]; tox creates a virtualenv in .tox/.tox and installs tox<4 in there, and then runs your testenvs using the tox you required in your tox.ini.
I saw in the log that it is using the ensure-tox role from devstack/playbooks/tox/run-both.yaml - https://zuul.opendev.org/t/openstack/build/c957db6323dc4b42bee07f6b709fb3ad/...
Which is run after pre-yaml where we pinned tox<4 via ensure_tox_version but missed doing it in run-both.yaml. Testing it by pinning in run-both.yaml also.
Pinning in run-both.yaml playbook did not fix the python-cinderclient issue and pinning tox<4 in tox.ini is the way forward for this case.
I don't think this is a proper fix. This goes back to the concern I already mentioned on this thread. The correct way to fix this is to ensure we aren't installing tox multiple times with the final install being the version we want. We should ensure we install it once with the correct version. The reason the python-cinderclient change failed is that devstack is blindly installing tox here: https://opendev.org/openstack/devstack/src/branch/master/lib/neutron_plugins... which is installing latest tox per this log: https://zuul.opendev.org/t/openstack/build/961c429cd9fc4d649e8714aba67f052d/.... The problem with adding requires = tox<4 in tox.ini is that this will cause tox to install a new tox in a new venv unnecessarily simply to run the target under an older tox. If we fix devstack instead then we can install tox once and everything should work.
-gmann
On 1/6/23 9:12 PM, Clark Boylan wrote:
On Fri, Jan 6, 2023, at 3:56 PM, Ghanshyam Mann wrote:
---- On Fri, 06 Jan 2023 10:41:43 -0800 Ghanshyam Mann wrote ---
---- On Fri, 06 Jan 2023 05:42:45 -0800 Brian Rosmaita wrote --- [snip] Pinning in run-both.yaml playbook did not fix the python-cinderclient issue and pinning tox<4 in tox.ini is the way forward for this case.
I don't think this is a proper fix. This goes back to the concern I already mentioned on this thread. The correct way to fix this is to ensure we aren't installing tox multiple times with the final install being the version we want. We should ensure we install it once with the correct version.
The reason the python-cinderclient change failed is that devstack is blindly installing tox here: https://opendev.org/openstack/devstack/src/branch/master/lib/neutron_plugins... which is installing latest tox per this log: https://zuul.opendev.org/t/openstack/build/961c429cd9fc4d649e8714aba67f052d/....
The problem with adding requires = tox<4 in tox.ini is that this will cause tox to install a new tox in a new venv unnecessarily simply to run the target under an older tox. If we fix devstack instead then we can install tox once and everything should work.
I think we have two separate issues here. The cinderclient functional test job just wants devstack to be up and running so that tox-based cinderclient tests can be run against devstack. I don't see that it's necessary that cinderclient have to use the same tox version to conduct its tests that devstack has installed for whatever reason devstack is installing tox. There may be good reasons for using different versions. In other words, it's not obvious to me that making devstack istelf tox-consistent implies that other projects running tox-based jobs against devstack have to use that same tox version.
-gmann
participants (4)
-
Brian Rosmaita
-
Clark Boylan
-
Ghanshyam Mann
-
Jeremy Stanley