[dev][requirements] Upcoming changes to constraints handling in tox.ini
Hi folks, This is a heads-up to describe 3 sets of changes you'll start seeing starting next week.
1) lower-constraints.txt handling TL;DR: Make sure projects do not specify a constraint file in install_command 2) Switch to the new canonical constraints URL on master TR;DR: Make sure you use https://releases.openstack.org/constraints/upper/master 3) Switch to the new canonical constraints URL on stable branches TR;DR: Make sure you use https://releases.openstack.org/constraints/upper/$series
These will be generated from a member of the requirements team[1], and will be on the gerrit topic constraints-updates. We'll start next week to give y'all a few days to digest this email
Now for slightly more details:
1) lower-constraints.txt handling
If a projects has an install_command that includes a constraint file *and* the also have a lower-constraints test env what may happen is something like:
given a tox.ini that contains: --- [testenv] install_command = pip install -U -c upper-constraints.txt {opts} {deps} deps = -r requirements.txt -r test-requirements.txt [testenv:lower-constraints] deps = -c lower-constrtaints.txt -r requirements.txt -r test-requirements.txt ---
pyXX: virtualenv -p $python_version .tox/$testenv_name .tox/$testenv_name/bin/pip install -U -c upper-constratints.txt -r requirements.txt -rtest-requirements.txt .tox/$testenv_name/bin/pip install .
lower-constraints: virtualenv -p $python_version .tox/$testenv_name .tox/$testenv_name/bin/pip install -U -c upper-constratints.txt -c lower-constrtaints.txt -r requirements.txt -rtest-requirements.txt .tox/$testenv_name/bin/pip install .
pip will constrain a library to the first item it encounters, which pretty much means that the lower-constraints testenv is still testing against upper-constraints.txt[3]
The fix is to move all constraints into deps rather than install_command. This impacts ~40 projects and will be fixed by hand, rather than a tool so we can preserve each projects style for tox.ini.
There's a pretty good chance that this change will break the lower-constraints testenv, project teams are encouraged to help identify the incorrect values in lower-constraints, the requirements team will pick the current value in upper-constraints which almost certainly raises a value well past the minimum. https://review.opendev.org/#/c/601188/ is the change we did for keystone way back when
Some projects have upper-constraints.txt in install_command but explicitly override it in lower-constratints.txt this works and wont be changed at this point but ideally all projects will use the same format to reduce the impact in switching between projects.
2) Switch to the new canonical constraints URL on master
Way back at the first Denver PTG thr requirements, release and infrastructure teams discussed avoiding using gitweb urls for constraints urls. This was discussed[4] in more detail after the liberty EOL broke consumers still wanting to use tox as: https://git.openstack.org/cgit/openstack/requirements/plain/upper-constraint... 'vanished' The idea was re-raised[5] in Feb. this year where we devised a plan to create URLs that abstract the git hosting and allow us to switch between branches and tags at the right times in the OpenStack life-cycle.
Other, more dynamic, ideas were discussed but over complicate things and make OpenStack slightly harder to understand or create more work.
for example we could: a. Add an install_script that looks up $metadata and grab the right constraints We're trying avoid scripts like this as it very rapidly gets us to the point where we're doing "managed copy-and-paste" between projects *or* projects install scripts diverge b. Add to tox itself the ability to derive the URL from $metadata This is doable but frankly requires more time than we have right now.
So what we have is:
redirect 301 /constraints/upper/master http://git.openstack.org/cgit/openstack/requirements/plain/upper-constraints... redirect 301 /constraints/upper/train http://git.openstack.org/cgit/openstack/requirements/plain/upper-constraints... redirect 301 /constraints/upper/stein http://git.openstack.org/cgit/openstack/requirements/plain/upper-constraints... redirect 301 /constraints/upper/rocky http://git.openstack.org/cgit/openstack/requirements/plain/upper-constraints... redirect 301 /constraints/upper/queens http://git.openstack.org/cgit/openstack/requirements/plain/upper-constraints... redirect 301 /constraints/upper/pike http://git.openstack.org/cgit/openstack/requirements/plain/upper-constraints... redirect 301 /constraints/upper/ocata http://git.openstack.org/cgit/openstack/requirements/plain/upper-constraints... redirect 301 /constraints/upper/newton http://git.openstack.org/cgit/openstack/requirements/plain/upper-constraints... redirect 301 /constraints/upper/mitaka http://git.openstack.org/cgit/openstack/requirements/plain/upper-constraints... redirect 301 /constraints/upper/liberty http://git.openstack.org/cgit/openstack/requirements/plain/upper-constraints... redirect 301 /constraints/upper/kilo http://git.openstack.org/cgit/openstack/requirements/plain/upper-constraints... redirect 301 /constraints/upper/juno http://git.openstack.org/cgit/openstack/requirements/plain/upper-constraints...
[Yes I know we need to use opendev that's tomorrows problem]
so that means that: http{s,}://releases.openstack.org/constraints/upper/master http{s,}://releases.openstack.org/constraints/upper/$series
will always do the right thing based on the state of each OpenStack release.
So we're going to generate changes that point to the right place.
For master we have 2 options: a. Use .../train
If we use train in the constraints URLs now things will work and when we branch for 'U' we'd need to update master to point to the right series name. This is very doable. The release team already generates a change on master at this point in time to update reno. We also need to update the new stable branch so that .gitreview points to the new branch.
b. Use .../master
If we use 'master' in the constratints URLs master will always work and when we branch for 'U' we'd need to update the stable/train branch to point to train. The release team already does this today along with the .gitreview change.
So the bottom line is that using master (over train) is *slighty* less work for the release team and *slightly* less work for each project team. as such that's what we're going to do.
There are several changes in the works that set the constraints url to either https://opendev.org/openstack/requirements/raw/branch/master/upper-constrain... or https://releases.openstack.org/constraints/upper/train
These will work and will not break anything when the project branches, however for the sake of OpenStack wide consistency, and avoiding merge conflicts project teams are discouraged from merging them, or if the change also fixes other references to git.openstack.org just have the tox.ini updates removed.
Once tox.ini is done the requirements team will look for additional places where the 'old style' urls are used and follow-up but right now there's a lot of noise to sift through
3) Switch to the new canonical constraints URL on stable branches
Hopefully with all the background from 2 above this should be pretty self-explanatory. This is however much lower priority and probably wont start until the bulk of 1 and 2 is complete
Yours Tony.
[1] https://review.opendev.org/#/admin/groups/131,members [2] [2] Probably me actually ;P [3] There is some nuance here but this is the general rule [4] In the thread that starts with: http://lists.openstack.org/pipermail/openstack-dev/2017-September/122333.htm... [5] In the thread that starts with: http://lists.openstack.org/pipermail/openstack-discuss/2019-February/002682....
Hi Tony,
Thanks for the write-up.
- Switch to the new canonical constraints URL on master
At the last Denver PTG we also discussed the switch from UPPER_CONSTRAINTS_FILE environment variable to TOX_CONSTRAINTS_FILE. As this change and the switch from UPPER_CONSTRAINTS_FILE to TOX_CONSTRAINTS_FILE would touch the very same line of text in the tox.ini, I would suggest that we combine that into one review as that is ~ 300 reviews less to conflict-merge and resolve when both would happen independently at the same time.
I started the patch series to add TOX_CONSTRAINTS_FILE in addition to UPPER_CONSTRAINTS_FILE so that lower-constraints setting looks less odd here:
https://review.opendev.org/657886 https://review.opendev.org/660187
Would be good to get this in in-time so that requirements team can do both changes in one review set.
Thanks, Dirk
On Wed, May 22, 2019 at 08:45:29AM +0200, Dirk Müller wrote:
Hi Tony,
Thanks for the write-up.
- Switch to the new canonical constraints URL on master
At the last Denver PTG we also discussed the switch from UPPER_CONSTRAINTS_FILE environment variable to TOX_CONSTRAINTS_FILE. As this change and the switch from UPPER_CONSTRAINTS_FILE to TOX_CONSTRAINTS_FILE would touch the very same line of text in the tox.ini, I would suggest that we combine that into one review as that is ~ 300 reviews less to conflict-merge and resolve when both would happen independently at the same time.
I started the patch series to add TOX_CONSTRAINTS_FILE in addition to UPPER_CONSTRAINTS_FILE so that lower-constraints setting looks less odd here:
https://review.opendev.org/657886 https://review.opendev.org/660187
Would be good to get this in in-time so that requirements team can do both changes in one review set.
Yup if they're approved when we start we can do this at the same time. It's just another line in the shell script :)
Yours Tony.
On Wed, 2019-05-22 at 13:02 +1000, Tony Breeds wrote:
Hi folks, This is a heads-up to describe 3 sets of changes you'll start seeing starting next week.
- lower-constraints.txt handling TL;DR: Make sure projects do not specify a constraint file in install_command
- Switch to the new canonical constraints URL on master TR;DR: Make sure you use https://releases.openstack.org/constraints/upper/master
- Switch to the new canonical constraints URL on stable branches TR;DR: Make sure you use https://releases.openstack.org/constraints/upper/$series
These will be generated from a member of the requirements team[1], and will be on the gerrit topic constraints-updates. We'll start next week to give y'all a few days to digest this email
All looks good to me. I'd been fixing this in a piecemeal fashion with oslo but who knows what other projects do iffy stuff here.
I realize this is bound to be controversial, but would it be possible to just auto-merge these patches assuming they pass CI? We've had a lot of these initiatives before and, invariably, there are some projects that won't get around to merging these for a long time (if ever). We had to do this recently with the opendev updates to the '.gitreview' files (I think?) so there is precedent here.
Stephen
[1] https://review.opendev.org/#/admin/groups/131,members [2] [2] Probably me actually ;P
On 2019-05-22 09:26:19 +0100 (+0100), Stephen Finucane wrote: [...]
I realize this is bound to be controversial, but would it be possible to just auto-merge these patches assuming they pass CI? We've had a lot of these initiatives before and, invariably, there are some projects that won't get around to merging these for a long time (if ever). We had to do this recently with the opendev updates to the '.gitreview' files (I think?) so there is precedent here.
Well, there were two approaches we used in the OpenDev migration:
1. Backward-compatible mass changes which fixed things we knew would otherwise break were proposed, given a brief opportunity for projects to review and approve or -2, and then at an pre-announced deadline any which were still open but passing their jobs and had no blocking votes were bulk-approved by a Gerrit administrator who temporarily elevated their access to act as a core reviewer for all projects. More specifically, this was the changes to replace git:// URLs with https:// because we were dropping support for the protocol.
2. Non-backward-compatible mass changes which fixed things we knew would otherwise be broken by the transition were committed directly into the on-disk copies of repositories in Gerrit while the service was offline for maintenance, entirely bypassing CI and code review. These were changes for things like .gitreview files and zuul pipelines/jobs/playbooks/roles.
I think something similar to #1 might be appropriate here. I could see, for example, requiring Gerrit ACLs for official OpenStack deliverable repositories to inherit from a parent ACL (Gerrit supports this) which includes core reviewer permissions for a group that the Release team can temporarily add themselves to, for the purposes of bulk approving relevant changes at or shortly following the coordinated release. The release process they follow already involves some automated group updates for reassigning control of branches, so this probably wouldn't be too hard to incorporate.
On Wed, May 22, 2019 at 03:18:36PM +0000, Jeremy Stanley wrote:
On 2019-05-22 09:26:19 +0100 (+0100), Stephen Finucane wrote: [...]
I realize this is bound to be controversial, but would it be possible to just auto-merge these patches assuming they pass CI? We've had a lot of these initiatives before and, invariably, there are some projects that won't get around to merging these for a long time (if ever). We had to do this recently with the opendev updates to the '.gitreview' files (I think?) so there is precedent here.
Well, there were two approaches we used in the OpenDev migration:
- Backward-compatible mass changes which fixed things we knew would
otherwise break were proposed, given a brief opportunity for projects to review and approve or -2, and then at an pre-announced deadline any which were still open but passing their jobs and had no blocking votes were bulk-approved by a Gerrit administrator who temporarily elevated their access to act as a core reviewer for all projects. More specifically, this was the changes to replace git:// URLs with https:// because we were dropping support for the protocol.
- Non-backward-compatible mass changes which fixed things we knew
would otherwise be broken by the transition were committed directly into the on-disk copies of repositories in Gerrit while the service was offline for maintenance, entirely bypassing CI and code review. These were changes for things like .gitreview files and zuul pipelines/jobs/playbooks/roles.
Yeah clearly not this one ;P
I think something similar to #1 might be appropriate here. I could see, for example, requiring Gerrit ACLs for official OpenStack deliverable repositories to inherit from a parent ACL (Gerrit supports this) which includes core reviewer permissions for a group that the Release team can temporarily add themselves to,
We could use "Project Bootstrappers" for this, which I think has all the right permissions. It would be a matter of adding the right people to that group for a short period of time. It'd be the requirements team rather than release team.
for the purposes of bulk approving relevant changes at or shortly following the coordinated release. The release process they follow already involves some automated group updates for reassigning control of branches, so this probably wouldn't be too hard to incorporate.
We could add a group similar to "Project Bootstrappers" but with slightly fewer permissions if we think using this approach from time-to-time is an acceptable idea.
Yours Tony.
On Tue, May 21, 2019 at 8:02 PM Tony Breeds tony@bakeyournoodle.com wrote:
Hi folks, This is a heads-up to describe 3 sets of changes you'll start seeing starting next week.
- lower-constraints.txt handling TL;DR: Make sure projects do not specify a constraint file in install_command
- Switch to the new canonical constraints URL on master TR;DR: Make sure you use https://releases.openstack.org/constraints/upper/master
- Switch to the new canonical constraints URL on stable branches TR;DR: Make sure you use https://releases.openstack.org/constraints/upper/$series
These will be generated from a member of the requirements team[1], and will be on the gerrit topic constraints-updates. We'll start next week to give y'all a few days to digest this email
I'm seeing a lot of changes for #2 being proposed by people who are not members of the requirements team (e.g. [1] and [2]).
Is it ok to approve those changes or should we wait for the official changes from a member of the requirements team?
[1] https://review.opendev.org/#/c/666947/ [2] https://review.opendev.org/#/c/666950/
On Wed, Jun 26, 2019 at 05:11:23PM -0700, Duc Truong wrote:
On Tue, May 21, 2019 at 8:02 PM Tony Breeds tony@bakeyournoodle.com wrote:
Hi folks, This is a heads-up to describe 3 sets of changes you'll start seeing starting next week.
- lower-constraints.txt handling TL;DR: Make sure projects do not specify a constraint file in install_command
- Switch to the new canonical constraints URL on master TR;DR: Make sure you use https://releases.openstack.org/constraints/upper/master
- Switch to the new canonical constraints URL on stable branches TR;DR: Make sure you use https://releases.openstack.org/constraints/upper/$series
These will be generated from a member of the requirements team[1], and will be on the gerrit topic constraints-updates. We'll start next week to give y'all a few days to digest this email
I'm seeing a lot of changes for #2 being proposed by people who are not members of the requirements team (e.g. [1] and [2]).
Is it ok to approve those changes or should we wait for the official changes from a member of the requirements team?
[1] https://review.opendev.org/#/c/666947/ [2] https://review.opendev.org/#/c/666950/
Thanks for checking Duc. I think as long as they are using the correct URL, it should be fine to approve those patches. We may end up with some duplicates if we run our script to create patches for repos that have not been updated yet, but those can just be abandoned.
Thanks! Sean
participants (6)
-
Dirk Müller
-
Duc Truong
-
Jeremy Stanley
-
Sean McGinnis
-
Stephen Finucane
-
Tony Breeds