[openstack-dev] [nova] minimum review period for functional changes that break backwards compatibility
clint at fewbar.com
Sat Dec 28 15:57:58 UTC 2013
Hi Phil. Thanks for the well reasoned and poignant message urging
caution and forethought in change management. I agree with all of the
sentiments and think that we can do better in reasoning about the impact
of changes. I think this just puts further exposure on the fact that
Nova needs reviewers desperately so that reviewers can slow down.
However, I think this is primarily an exposure in our gate testing. If
there are older OS's we want to be able to support, we should be booting
them in the gate and testing that the ephemeral disk works on them. What
is a cloud that can't boot workloads?
While our ability to reason is a quite effective way to stop emergent
problems, we know these are precious and scarce resources, and thus
we should use mechanical methods before falling back to reviewers and
So, I'd suggest that we add a test that the ephemeral disk mounts in
any desired OS's to tempest. If that is infeasible (due to nested KVM
in the gate being slllooooowwww) then I'm afraid I don't have a solution.
Excerpts from Day, Phil's message of 2013-12-28 07:21:16 -0800:
> Hi Folks,
> I know it may seem odd to be arguing for slowing down a part of the review process, but I'd like to float the idea that there should be a minimum review period for patches that change existing functionality in a way that isn't backwards compatible.
> The specific change that got me thinking about this is https://review.openstack.org/#/c/63209/ which changes the default fs type from ext3 to ext4. I agree with the comments in the commit message that ext4 is a much better filesystem, and it probably does make sense to move to that as the new default at some point, however there are some old OS's that may still be in use that don't support ext4. By making this change to the default without any significant notification period this change has the potential to brake existing images and snapshots. It was already possible to use ext4 via existing configuration values, so there was no urgency to this change (and no urgency implied in the commit messages, which is neither a bug or blueprint).
> I'm not trying to pick out the folks involved in this change in particular, it just happened to serve as a good and convenient example of something that I think we need to be more aware of and think about having some specific policy around. On the plus side the reviewers did say they would wait 24 hours to see if anyone objected, and the actual review went over 4 days - but I'd suggest that is still far too quick even in a non-holiday period for something which is low priority (the functionality could already be achieved via existing configuration options) and which is a change in default behaviour. (In the period around a major holiday there probable needs to be an even longer wait). I know there are those that don't want to see blueprints for every minor functional change to the system, but maybe this is a case where a blueprint being proposed and reviewed may have caught the impact of the change. With a number of people now using a continual deployment approach any chan
ge in default behaviour needs to be considered not just for the benefits it brings but what it might break. The advantage we have as a community is that there are lot of different perspectives that can be brought to bear on the impact of functional changes, but we equally have to make sure there is sufficient time for those perspectives to emerge.
> Somehow it feels that we're getting the priorities on reviews wrong when a low priority changes like this which can go through in a matter of days, when there are bug fixes such as https://review.openstack.org/#/c/57708/ which have been sitting for over a month with a number of +1's which don't seem to be making any progress.
More information about the OpenStack-dev