openstack team mailing list archive

[Mark McLoughlin <markmc@xxxxxxxxxx>]

Hey,

I've posted a proposal for how the stable branch could work here:

  http://wiki.openstack.org/StableBranch

and a proposed diablo branch for nova:

  http://github.com/markmc/nova/tree/diablo

The wiki page seems to be 404 for folks for some bizarre reason, but you
can get to it from:

  wiki.openstack.org/RecentChanges

I've also appended it below so folks can reply to specific points
inline.

Cheers,
Mark.

= Stable Branch =

At the Essex Design Summit it was agreed that a working group would be established to maintain stable/backport/fixes branches for the Diablo
release.

This effort is essentially an experiment to establish:

 * That the idea makes sense
 * That there is enough people willing to help maintain it for it to be viable
 * The policies for what belongs in such a stable branch
 * How changes should be proposed to the branch
 * The workflow for maintaining the branch
 * Whether updates should be released from the branch

This page tries to cover most of these topics and should be considered an RFC.

== Overview ==

Each project will have a branch named after the previous release. For example, the stable branch for the diablo release will be simply called 'diablo'.

The stable branch will only be maintained until the next release is out.

The maintainers, in conjunction with the release team, may choose to cut a new release from the branch if there are enough worthwhile fixes and the branch has been tested enough.

The branch maintainer(s) will monitor master for changes and periodically cherry-pick patches into the branch. They may choose to stop actively monitoring master after a period (e.g. 2 months) if they feel there are so few backport candidates that it is no longer worth the effort. If so, they will notify developers via the mailing list.

Anyone can propose a cherry-pick to the maintainers. This helps ensure that the maintainers don't miss anything. To catch the maintainers attention, simply add

 Cc: stable@xxxxxxxxxxxxx

to the commit message. If the patch you're proposing will not cherry-pick cleanly, you can help by resolving the conflicts yourself and proposing the resulting patch.

All changes to the stable branch will go through gerrit. The branch maintainers and core teams of each project can +1 the changes.

If a bug in launchpad looks like a good candidate for backporting - e.g. if it's a significant bug with the previous release - then just tagging it with 'diablo-backport' will bring it to the attention of the maintainers e.g. [[https://bugs.launchpad.net/nova/+bugs?field.tag=diablo-backport|for nova]] and [[https://bugs.launchpad.net/glance/+bugs?field.tag=diablo-backport|for glance]].

Every change on the stable branch must pass all the tests.

Security patches will follow a different process, whereby the patch can be embargoed and pushed at the same time as the patch on master, once the issue has been made public.

== Policy ==

The most obvious question we need to answer is "What belongs in a stable branch?"

However, we don't need to ignore prior art here. Plenty of projects maintain such a branch and there's a very rough consensus across projects on what the policy should be.

As an example, [[http://github.com/torvalds/linux/blob/master/Documentation/stable_kernel_rules.txt|stable_kernel_rules.txt]] says:

 * It must be obviously correct and tested.
 * It cannot be bigger than 100 lines, with context.
 * It must fix only one thing.
 * It must fix a real bug that bothers people (not a, "This could be a problem..." type thing).
 * It must fix a problem that causes a build error [..], an oops, a hang, data corruption, a real security issue, or some "oh, that's not good" issue.  In short, something critical.
 * No "theoretical race condition" issues, unless an explanation of how the race can be exploited is also provided.
 * It cannot contain any "trivial" fixes in it (spelling changes, whitespace cleanups, etc).
 * It or an equivalent fix must [...] (upstream).

We can also add some rules specific to OpenStack:

 * No changes to the external HTTP APIs
 * No changes to the internal AMQP API
 * No DB schema changes

The branch is intended to be a safe source of fixes for significant issues which have been fixed on master since the release. New features, regressions, compatibility issues, subtle behavioural changes, etc. have no place here.

The maintainers need to balance the risk of any given patch with the value that it will provide to users of the stable branch. A large,
risky patch for a major data corruption issue might make sense. As might a trivial fix for a fairly obscure error handling case.

== Notes On The Nova Diablo Branch ==

A proposed diablo branch for Nova is [[https://github.com/markmc/nova/tree/fedora-patches|here]].

The first few changes are about getting tools/rfc.sh added and making the tests pass on git. This is only needed because the switch to git
happened after the Diablo release but we want to use it for the stable branch.

The set of fixes backported from master are:

 * [[https://github.com/openstack/nova/commit/33e5892|33e5892]] Include original exception in ClassNotFound exception
 * [[https://github.com/openstack/nova/commit/2915e6b|2915e6b]] Make sure unknown extensions return 404
 * [[https://github.com/openstack/nova/commit/a25f106|a25f106]] Handle pidfile exception for dnsmasq
 * [[https://github.com/openstack/nova/commit/beee11e|beee11e]] Stop returning correct password on api calls
 * [[https://github.com/openstack/nova/commit/f225ea4|f225ea4]] Deallocate ip if build fails.
 * [[https://github.com/openstack/nova/commit/7a10619|7a10619]] Fixes bug 862633 -- OS api consoles create() broken
 * [[https://github.com/openstack/nova/commit/f3fb16a|f3fb16a]] Adds the tenant id to the create images response Location header Fixes bug 862672
 * [[https://github.com/openstack/nova/commit/eb4bd86|eb4bd86]] Added ==1.0.4 version specifier to kombu in pip-requires to ensure tests pass in a clean venv.
 * [[https://github.com/openstack/nova/commit/2dd0445|2dd0445]] bug lp845714
 * [[https://github.com/openstack/nova/commit/eb64323|eb64323]] Enforce snapshot cleanup.
 * [[https://github.com/openstack/nova/commit/0546616|0546616]] bug 861310
 * [[https://github.com/openstack/nova/commit/b9aac11|b9aac11]] Fixed bug lp850602. Adding backing file copy operation on kvm block migration.
 * [[https://github.com/openstack/nova/commit/b5abd8e|b5abd8e]] Snapshots/backups can no longer happen simultaneously. Tests included.
 * [[https://github.com/openstack/nova/commit/6574d1b|6574d1b]] Make snapshots private by default
 * [[https://github.com/openstack/nova/commit/7dba1d9|7dba1d9]] Raise InsufficientFreeMemory
 * [[https://github.com/openstack/nova/commit/f3dd56e|f3dd56e]] Removed db_pool complexities from nova.db.sqlalchemy.session. Fixes bug 838581.
 * [[https://github.com/openstack/nova/commit/14eb9c6|14eb9c6]] Call endheaders when auth_token is None. Fixes bug 856721.
 * [[https://github.com/openstack/nova/commit/114ca0b|114ca0b]] Fix the grantee group loading for source groups
 * [[https://github.com/openstack/nova/commit/6415999|6415999]] put fully qualified domain name in local-hostname
 * [[https://github.com/openstack/nova/commit/569b310|569b310]] Makes sure to recreate gateway for moved ip

Four of these were [[https://bugs.launchpad.net/nova/+bugs?field.tag=diablo-backport|tagged for backporting]] in launchpad.

Some of the fixes not explicitly not backported were:

 * [[https://github.com/openstack/nova/commit/9b12a6c|9b12a6c]]  Fixes an issue where 'invalid literal for int' would occur when listing images after making a v1.1 server snapshot (with a UUID).

Looks like a correctness issue for future where images may be UUIDs.

 * [[https://github.com/openstack/nova/commit/e15258d|e15258d]] Fix bug 856664 overLimit errors now return 413

Correctness issue; problem existed in cactus too.

 * [[https://github.com/openstack/nova/commit/bb410b3|bb410b3]] Change 'recurse_zones' to 'local_zone_only'.

API change?

 * [[https://github.com/openstack/nova/commit/4584e55|4584e55]] Fixes euca-describe-instances failing or not showing IPs

Fixes an essex regression?

 * [[https://github.com/openstack/nova/commit/eff0a63|eff0a63]] Fixes bug 862658 -- ec2 metadata issue getting IPs

Another essex regression fix?

 * [[https://github.com/openstack/nova/commit/981f527|981f527]] Set error state on spawn error + integration test.

Tests require _wait_for_state_change(). Patch looks risky.

Everything on master since the diablo RBP has been reviewed up until [[https://github.com/openstack/nova/commit/f5f429b|f5f429b]].

The backport of b9aac118 is a good example of why logically separate changes should be committed separately. There were a bunch of changes
to test_libvirt.py which weren't related to the fix and, therefore, not appropriate for the branch. Manually eliding them is error prone. If those changes had been applied first as a standalone commit, cherry-picking the patch would have been easier.

== Random Notes and Tips ==

The maintainers need to be able to collaborate on the process of monitoring master for suitable fixes - e.g. you need to be able to record which commits you have reviewed, which commits you have rejected etc. Using git notes is one way we could do this. We need to experiment with them to see what works best.

In Essex, the RBP will be when the RCs are considered "good enough". It is at that point the stable maintainers need to start monitoring master. For Diablo, the PTLs and release manager did this work. That implies that the PTLs and RM will initially take an active interest in the stable branch, but that interest will want after a time. Which makes perfect sense.

Switching between master and diablo is a pain because you end up with .pyc files for migrations that exist only on master and diablo blows up when trying to run them.

When cherry-picking a commit, in order to ensure a new Change-Id, you need to do:

{{{
 $> git cherry-pick -e -x $commit
 <remove the Change-Id tag>
 $> git commit --amend
 <this will add a new Change-Id>
}}}

Alternatively, you can skip the latter step and use 'git rebase -i' and the 'reword' action to add Change-Ids later.

We could probably fix this in the commit-msg hook ... perhaps by recognizing when its a cherry-pick. Or maybe there's a hook specific to cherry-picking.

Try 'git log --graph' ... I found it helpful when reviewing the changes on master since diablo.

If you're looking at a sequence of commits on a branch which got merged into master and you want to see exactly what changes were merged in, then look at the merge commit:

{{{
 commit f5f429bac6446db084ca3f5d86c64127a1e539f2
 Merge: e95e923 f3fb16a
}}}

and do:

{{{
 $> git diff e95e923..f5f429b
}}}