Co-ordinating glance/nova/keystone changes with devstack

[Mark McLoughlin]

We've talked about the problem of needing to merge a change into the core projects which require a change in devstack. Because the devstack gating tests fail when jenkins tests the gerrit review, we get stuck.

A good example is this change:

  https://review.openstack.org/3036
    Split out paste deployment config from the core glance *.conf files into corresponding *-paste.ini files.

Which can't be merged before this devstack change:

  https://review.openstack.org/3092
    Corresponds to the Glance patch that splits paste

And the devstack change can't be merged until the glance change has been merged. Chicken and egg. Nasty.

There's been some offline discussion about how the devstack patch might support glance's old and new behaviour and then the support for the old behaviour could be merged later. It seems like wasted effort, though, because do we really need latest devstack to work at the exact moment a core project change gets merged?

How about this for an alternative:

  1) The core project change is approved and ready to go, but people notice that devstack needs updating
  2) Someone cooks up a devstack change, tests it with the proposed core project change, proposes it and gets approval in principle
  3) Someone adds a tag to the core project's commit message which means "this change temporarily breaks devstack"
  4) The patch gets approved, only after ${project}-core is confident that the devstack change is ready to go
  5) The devstack gating job in Jenkins sees the tag and skips doing any testing
  6) No more changes to any project can now be merged until devstack is updated
  7) The devstack change is approved, Jenkins devstack gating job tests it and merges it
  8) Everything is back to normal again

AFAICT, this would only need something like this in devstack-vm-gate.sh:

     if [[ $GERRIT_PROJECT == $PROJECT ]]; then
         echo " Merging proposed change"
         git fetch https://review.openstack.org/p/$PROJECT $GERRIT_REFSPEC
         git merge FETCH_HEAD
         if git show | grep 'THIS COMMIT TEMPORARILY BREAKS DEVSTACK' > /dev/null; then
             echo 'THIS COMMIT TEMPORARILY BREAKS DEVSTACK'
             exit 0
         fi

Once we have that going, we could proceed to a more cunning plan:

  ...
  3) Someone adds a tag to the commit message pointing at the devstack fix in gerrit
  ...
  5) devstack-vm-gate sees the tag and merges the devstack fix before proceeding

We would still have the period at (6) where everything blocks until the devstack fix is merged

[Jay Pipes (jaypipes)]

Setting to Critical because this is now holding up multiple projects...

[Monty Taylor (mordred)]

Jim made a suggestion on the list about how we solved this for nova - seems like the same approach could be take with glance, no?

I'd like to see if we can solve the specific issue before we start instituting policies for how we can bypass testing. (the procedure indicated above seems technically sound - but there's a reason that we don't let people skip having tests run, so this does wind up being a pretty powerful hammer to hand out)

[James E. Blair (corvus)]

Nova recently faced a similar problem. It was solved by actually making
the devstack configuration of nova simpler. Before, nova had a sample
paste config, and devstack had a nova paste config that was nearly a
duplicate of that. By changing devstack to just use the sample paste
config from nova (adding a few items to it when it copied it into
place), some of the configuration of nova is kept in the nova repository
so that it can be handled by a single change. It also makes devstack
slightly simpler and reduces duplication of configuration files. Here
is the change that implemented that in devstack:

https://review.openstack.org/#change,2978

From a quick look at the changes you mentioned, I think a similar
approach may work here. Otherwise, the changes need to merge in
sequence, so making devstack handle both forms (perhaps temporarily) is
the second best way to go.

[Eoghan Glynn (eglynn)]

Applying the devstack nova config logic to glance config also would definitely be a step forward, in the sense of reducing duplication and avoiding devstack brittleness on future glance config changes.

Now in this particular case, the config has not only been changed but also split out over two files (e.g. glance-api.conf & glance-api-paste.ini) whereas previously there would have only been a single config file (e.g. glance-api.conf) for each glance service.

So as long as the devstack changes were (initially at least) made tolerant of the glance-*-paste.ini files being missing, then I don't see any reason why the approach suggested by Jim above wouldn't work.

i.e. stack.sh would just need to check if [ -e $GLANCE_DIR/etc/glance-api-paste.ini ] before attempting to copy.

This approach would also have the advantage of not requiring the glance commits to be squashed.

[Eoghan Glynn (eglynn)]

A-ha, I see this proposed patch:

https://review.openstack.org/#patch,sidebyside,3095,3,stack.sh

already makes already makes stack.sh tolerant of missing glance-*-paste.ini files.

[Mark McLoughlin (markmc)]

In terms of the general issue of co-ordinating changes with devstack, I suggested the above way for folks to allow devstack-breaking changes to be pushed because I don't see the benefit to the hoops we're making people jump through

e.g. when I develop a nice series of patches, each logical change in its own patch, each patch passing the unit tests, etc. I often have to jump through hoops like we're doing here - making some change in patch A to allow it to pass the unit tests even though you know that patch B will remove the change

For a patch series, this has definite benefits - the commit history is bisectable because every commit passes the tests, it makes it easier for folks to review and/or cherry-pick patches and the history is more useful when you're looking at it later

But I don't see the benefit to the hoops we're jumping through here. If we allow devstack-breaking changes through only when the corresponding devstack fix is queued and ready to immediately push, I think we've got all bases covered

[James E. Blair (corvus)]

Your approach is technically sound. There's nothing stopping us from testing changes together as well, so we can even go one step further and say "This change requires pending devstack change FOO" and that would even allow the change to be merged after being tested with the change in devstack. But neither of these satisfies one of the goals of this project which is to ensure that trunk is always tested automatically and never broken. To have a system where a developer can opt-out of testing puts the rest of the development community at risk of developing on top of a broken trunk or having their changes erroneously rejected.

Until cross-project dependencies are codified in gerrit, we don't have a way of ensuring that two or more related changes are actually merged together. Even if we test them together in devstack, one of them could still fail other tests (eg, unit tests) and not be merged. When we merge a change without a corresponding change, the trunk is broken until that change is merged. It's possible there could be a flaw in the second change preventing it from being merged in a timely manner. There may not be anyone with approval authority for the second change (necessarily in a different project) to merge it in a reasonable time. An unrelated change in a third project may be merged before even the first change we're considering is merged that could cause the pending second change to fail to merge. In all these cases, and surely many more, one developer has broken the tree for all other developers for an undefined length of time.

I would rather this not be complicated. I'm looking forward to CPD support in gerrit. And I understand that what we have may not actually be tenable and that we have to implement something like your suggestion (or my enhancement of it). But so far, we've seen two problems, and they are very similar. In both cases we've (hopefully) been able to solve them by moving the closely related bits into a single project. By having a sample config file in nova that's actually _used_ by devstack in testing is an improvement to the documentation of the nova project itself. Moving duplicate information out of devstack in turn makes it simpler. We may yet see a class of change that would cause us to do what you suggest, but so far, the obstacles we've encountered have actually led to improvements, so I'd like to continue to see how the system works as-is.

[Monty Taylor (mordred)]

I'm marking this as wont fix for the moment, because we were able to work around the problem.