Yesterday I made an initial attempt at adding support for the No Frozen
Rawhide[0] and Critical Path Packages[1] policies in bodhi.
From a bodhi/releng perspective, here is what the process will look like so far:
Releng adds F13 to bodhi as a `locked` release:
>>> Release(name='F13', long_name='Fedora 13', dist_tag='dist-f13', locked=True)
Developer pushes update to F13:
If the package is in the critical path, force it to go to testing until approved by releng/qa
QA/Releng test F13 critical path testing updates, and promote them to stable.
Releng kicks off a push of all updates:
If an update is for a pending release, and headed to stable, only move it's tags:
dist-f13-updates-candidate => dist-f13
If an update is for a pending release, and headed to testing, do things as normal:
dist-f13-updates-candidate => dist-f13-updates-testing
Release unlocks F13 release in bodhi upon RC, and everything returns to normal:
>>> Release.byName('F13').locked = False
Assuming I've understood everything correctly, here are the actions that I took
to implement this, and the test cases that I've written to validate the policy:
[X] 100% Bodhi No Frozen Rawhide & Critical Path support
[X] Ability to flag a release as 'pending'
[X] Disable the ability to push critpath updates directly to stable for pending releases
[X] If a package is critical path, force it to go to testing
[X] Push testing updates to pending releases normally
[X] Only allow it to go to stable if approved by releng/qa
[X] Strongly discourage developers from pushing critpath packages directly to stable, for all releases
[X] Strongly discourage pushing non-critpath updates directly to stable for pending releases
[X] Tag stable updates to pending releases with Release.dist_tag
[X] Don't mash stable updates for pending releases, only move their tags
[_] 85% Test cases
[X] Create a pending release
[X] Submit an update for this pending release, as normal
[X] Ensure we can't push directly to stable
[X] Ensure it has a testing request
[X] Try pushing it to stable
[X] Ensure we can't as a normal developer
[X] Ensure we can as a member of the proper groups (releng/qa)
[X] Ensure devs cannot push critpath updates in testing to stable
[X] Ensure releng/qa can push critpath updates in testing to stable
[X] Ensure noncritpath updates in testing can be pushed to stable
[_] Try pushing updates (currently not possible via unit tests)
[_] Ensure the stable updates only get tagged
[_] Ensure the testing update gets pushed normally
Attached is the initial bodhi patch. Thanks to some clever re-use of an
existing DB column, I was able to implement this without making any schema
modifications, so deployment should be trivial. I've also hardcoded the
critical path package list in bodhi's config file until we can query the pkgdb
for it.
Please let me know if I'm misunderstanding any parts of this proposal,
if I am missing something, or if you find any bugs in my patch. I'll
try and get this into staging ASAP so we can test the masher portion of
this.
Thanks,
luke
[0]: https://fedoraproject.org/wiki/No_Frozen_Rawhide_Proposal
[1]: https://fedoraproject.org/wiki/Critical_Path_Packages_Proposal
diff --git a/bodhi/admin.py b/bodhi/admin.py
index db096dd..7d80d25 100644
--- a/bodhi/admin.py
+++ b/bodhi/admin.py
@@ -110,9 +110,14 @@ class AdminController(Controller, SecureResource):
else:
# Get a list of all updates with a request that aren't
# unapproved security updates, or for a locked release
- requests = filter(lambda update: not update.release.locked,
- PackageUpdate.select(
- PackageUpdate.q.request != None))
+ requests = PackageUpdate.select(PackageUpdate.q.request != None)
+
+ # Come F13+, bodhi will not have locked releases. It will
+ # implement the 'No Frozen Rawhide' proposal, and treat 'locked'
+ # releases as pending.
+ #requests = filter(lambda update: not update.release.locked,
+ # PackageUpdate.select(
+ # PackageUpdate.q.request != None))
for update in requests:
if update.type == 'security' and not update.approved:
continue
diff --git a/bodhi/controllers.py b/bodhi/controllers.py
index fa2e7e1..7b134e5 100644
--- a/bodhi/controllers.py
+++ b/bodhi/controllers.py
@@ -831,6 +831,20 @@ class Root(controllers.RootController):
flash_log(str(e))
raise InvalidUpdateException(params)
+ # Politely discourage devs from pushing critpath straight to stable
+ if (update.request == 'stable' and update.critpath and
+ 'qa' not in identity.current.groups and
+ 'releng' not in identity.current.groups):
+ note.append("You're pushing a critical path package directly to "
+ "stable, which is strongly discouraged. Please "
+ "consider pushing to testing first!")
+ # Discourage devs from pushing directly to stable for pending releases
+ elif update.request == 'stable' and update.release.locked:
+ note.append("This update is bypassing updates-testing for a "
+ "pending release, which is strongly discouraged. "
+ "Please ensure that it is properly tested, or "
+ "consider pushing it to testing first.")
+
flash_log('. '.join(note))
if request_format() == 'json':
diff --git a/bodhi/masher.py b/bodhi/masher.py
index c890d92..6ae1ee6 100644
--- a/bodhi/masher.py
+++ b/bodhi/masher.py
@@ -351,6 +351,11 @@ class MashTask(Thread):
"""
for update in self.updates:
release = update.release
+
+ # [No Frozen Rawhide] Don't mash stable repos for pending releases
+ if update.request == 'stable' and release.locked:
+ continue
+
if self.resume:
self.repos.add(release.stable_repo)
self.repos.add(release.testing_repo)
@@ -380,6 +385,10 @@ class MashTask(Thread):
for update in self.updates:
if update.request == 'stable':
self.tag = update.release.stable_tag
+ # [No Frozen Rawhide] Move stable builds going to a pending
+ # release to the Release.dist-tag
+ if update.release.locked:
+ self.tag = update.release.dist_tag
elif update.request == 'testing':
self.tag = update.release.testing_tag
elif update.request == 'obsolete':
diff --git a/bodhi/model.py b/bodhi/model.py
index 819b571..a05ca2d 100644
--- a/bodhi/model.py
+++ b/bodhi/model.py
@@ -50,9 +50,12 @@ class Release(SQLObject):
updates = MultipleJoin('PackageUpdate', joinColumn='release_id')
id_prefix = UnicodeCol(notNone=True)
dist_tag = UnicodeCol(notNone=True) # ie dist-fc7
- locked = BoolCol(default=False)
metrics = PickleCol(default=None) # {metric: {data}}
+ # [No Frozen Rawhide] We're going to re-use this column to flag 'pending'
+ # releases, since we'll no longer need to lock releases in this case.
+ locked = BoolCol(default=False)
+
def get_version(self):
regex = re.compile('\D+(\d+)$')
return int(regex.match(self.name).groups()[0])
@@ -404,6 +407,17 @@ class PackageUpdate(SQLObject):
flash_log('%s does not have a request to revoke' % self.title)
return
+ # [No Frozen Rawhide] Disable pushing critical path updates for
+ # pending releases directly to stable.
+ if action == 'stable' and self.release.locked and self.critpath:
+ if ('releng' in identity.current.groups or
+ 'qa' in identity.current.groups):
+ self.comment('Critical path update approved by %s' %
+ identity.current.user_name, author='bodhi')
+ else:
+ log.info('Forcing critical path update into testing')
+ action = 'testing'
+
self.request = action
self.pushed = False
#self.date_pushed = None
@@ -811,6 +825,17 @@ class PackageUpdate(SQLObject):
return None
return int(self.updateid.split('-')[-1])
+ @property
+ def critpath(self):
+ """ Return whether or not this update is in the critical path """
+ critical = False
+ critpath_pkgs = config.get('critpath').split()
+ for build in self.builds:
+ if build.package.name in critpath_pkgs:
+ critical = True
+ break
+ return critical
+
class Comment(SQLObject):
timestamp = DateTimeCol(default=datetime.utcnow)
diff --git a/bodhi/templates/show.kid b/bodhi/templates/show.kid
index ea01bef..8e7c8af 100644
--- a/bodhi/templates/show.kid
+++ b/bodhi/templates/show.kid
@@ -67,12 +67,24 @@ karma = "<img src=\"%s\" align=\"top\" /> <b>%d</b>" % (tg.url('/static/images/k
Push to Testing
</a>
</td>
+ <span py:if="update.release.locked and update.critpath">
+ <span py:if="'qa' in tg.identity.groups or 'releng' in tg.identity.groups">
+ <td>
+ <a href="${util.url('/request/stable/%s' % update.title)}" class="list">
+ <img src="${tg.url('/static/images/submit.png')}" border="0"/>
+ Push Critical Path update to Stable
+ </a>
+ </td>
+ </span>
+ <span py:if="not update.release.locked or not update.critpath">
<td>
<a href="${util.url('/request/stable/%s' % update.title)}" class="list">
<img src="${tg.url('/static/images/submit.png')}" border="0"/>
Push to Stable
</a>
</td>
+ </span>
+ </span>
<td>
<a href="${util.url('/confirm_delete?nvr=%s' % update.title)}" class="list">
<img src="${tg.url('/static/images/trash.png')}" border="0"/>
@@ -95,14 +107,26 @@ karma = "<img src=\"%s\" align=\"top\" /> <b>%d</b>" % (tg.url('/static/images/k
</a>
</td>
<span py:if="update.status == 'testing'">
- <span py:if="update.request == None">
- <td>
- <a href="${util.url('/request/stable/%s' % update.title)}" class="list">
- <img src="${tg.url('/static/images/submit.png')}" border="0"/>
- Mark as Stable
- </a>
- </td>
- </span>
+ <span py:if="update.request == None">
+ <span py:if="update.release.locked and update.critpath">
+ <span py:if="'qa' in tg.identity.groups or 'releng' in tg.identity.groups">
+ <td>
+ <a href="${util.url('/request/stable/%s' % update.title)}" class="list">
+ <img src="${tg.url('/static/images/submit.png')}" border="0"/>
+ Mark Critical Path update as Stable
+ </a>
+ </td>
+ </span>
+ </span>
+ <span py:if="not update.release.locked or not update.critpath">
+ <td>
+ <a href="${util.url('/request/stable/%s' % update.title)}" class="list">
+ <img src="${tg.url('/static/images/submit.png')}" border="0"/>
+ Mark as Stable
+ </a>
+ </td>
+ </span>
+ </span>
<td>
<a href="${util.url('/edit/%s' % update.title)}" class="list">
<img src="${tg.url('/static/images/edit.png')}" border="0"/>
diff --git a/bodhi/tests/test_controllers.py b/bodhi/tests/test_controllers.py
index 2228c0a..916d297 100644
--- a/bodhi/tests/test_controllers.py
+++ b/bodhi/tests/test_controllers.py
@@ -21,9 +21,9 @@ from bodhi.exceptions import DuplicateEntryError
cherrypy.root = Root()
-def create_release(num='7', dist='dist-fc'):
+def create_release(num='7', dist='dist-fc', **kw):
rel = Release(name='F'+num, long_name='Fedora '+num, id_prefix='FEDORA',
- dist_tag=dist+num)
+ dist_tag=dist+num, **kw)
assert rel
assert Release.byName('F'+num)
return rel
@@ -1243,3 +1243,223 @@ class TestControllers(testutil.DBTest):
assert False, "Old obsolete build still exists!!"
except SQLObjectNotFound:
pass
+
+ def test_push_critpath_to_release(self):
+ session = login()
+ create_release()
+ params = {
+ 'builds' : 'kernel-2.6.31-1.fc7',
+ 'release' : 'Fedora 7',
+ 'type_' : 'bugfix',
+ 'bugs' : '',
+ 'notes' : 'foobar',
+ 'stable_karma' : 1,
+ 'request': 'stable',
+ 'unstable_karma' : -1,
+ }
+ testutil.capture_log(["bodhi.util", "bodhi.controllers", "bodhi.model"])
+ self.save_update(params, session)
+ log = testutil.get_log()
+ assert "Update successfully created. You're pushing a critical path package directly to stable. Please consider pushing to testing first!" in log, log
+ update = PackageUpdate.byTitle(params['builds'])
+ assert update.request == 'stable'
+
+ def test_push_critpath_to_frozen_release(self):
+ session = login()
+ create_release(locked=True)
+ params = {
+ 'builds' : 'kernel-2.6.31-1.fc7',
+ 'release' : 'Fedora 7',
+ 'type_' : 'bugfix',
+ 'bugs' : '',
+ 'notes' : 'foobar',
+ 'stable_karma' : 1,
+ 'request': 'stable',
+ 'unstable_karma' : -1,
+ }
+ self.save_update(params, session)
+ update = PackageUpdate.byTitle(params['builds'])
+ assert update.request == 'testing'
+
+ def test_push_critpath_to_frozen_release_and_request_stable(self):
+ session = login()
+ create_release(locked=True)
+ params = {
+ 'builds' : 'kernel-2.6.31-1.fc7',
+ 'release' : 'Fedora 7',
+ 'type_' : 'bugfix',
+ 'bugs' : '',
+ 'notes' : 'foobar',
+ 'stable_karma' : 1,
+ 'request': 'stable',
+ 'unstable_karma' : -1,
+ }
+ self.save_update(params, session)
+ update = PackageUpdate.byTitle(params['builds'])
+ assert update.request == 'testing'
+
+ # Ensure we can't create a stable request
+ testutil.capture_log(["bodhi.util", "bodhi.controllers", "bodhi.model"])
+ testutil.create_request('/updates/request/stable/%s' % params['builds'],
+ method='POST', headers=session)
+ log = testutil.get_log()
+ assert "Forcing critical path update into testing" in log
+ update = PackageUpdate.byTitle(params['builds'])
+ assert update.request == 'testing'
+
+ def test_push_critpath_to_frozen_release_and_request_stable_as_releng(self):
+ session = login(group='releng')
+ create_release(locked=True)
+ params = {
+ 'builds' : 'kernel-2.6.31-1.fc7',
+ 'release' : 'Fedora 7',
+ 'type_' : 'bugfix',
+ 'bugs' : '',
+ 'notes' : 'foobar',
+ 'stable_karma' : 1,
+ 'request': 'stable',
+ 'unstable_karma' : -1,
+ }
+ self.save_update(params, session)
+ update = PackageUpdate.byTitle(params['builds'])
+ assert update.request == 'stable'
+
+ def test_critpath_to_frozen_release_available_actions(self):
+ """
+ Ensure devs can attempt to push critpath updates for pending releases
+ to stable, but make sure that it can only go to testing.
+ """
+ session = login()
+ create_release(locked=True)
+ params = {
+ 'builds' : 'kernel-2.6.31-1.fc7',
+ 'release' : 'Fedora 7',
+ 'type_' : 'bugfix',
+ 'bugs' : '',
+ 'notes' : 'foobar',
+ 'stable_karma' : 1,
+ 'request': None,
+ 'unstable_karma' : -1,
+ }
+ self.save_update(params, session)
+ update = PackageUpdate.byTitle(params['builds'])
+ testutil.create_request('/updates/%s' % params['builds'],
+ method='GET', headers=session)
+
+ assert "Push to Testing" in cherrypy.response.body[0]
+ assert "Push to Stable" not in cherrypy.response.body[0]
+
+ testutil.create_request('/updates/request/stable/%s' % params['builds'],
+ method='POST', headers=session)
+ update = PackageUpdate.byTitle(params['builds'])
+ assert update.request == 'testing'
+
+ def test_critpath_to_frozen_release_available_actions_for_releng(self):
+ """
+ Ensure releng/qa can push critpath updates to stable for pending releases
+ """
+ session = login(group='releng')
+ create_release(locked=True)
+ params = {
+ 'builds' : 'kernel-2.6.31-1.fc7',
+ 'release' : 'Fedora 7',
+ 'type_' : 'bugfix',
+ 'bugs' : '',
+ 'notes' : 'foobar',
+ 'stable_karma' : 1,
+ 'request': None,
+ 'unstable_karma' : -1,
+ }
+ self.save_update(params, session)
+ update = PackageUpdate.byTitle(params['builds'])
+ testutil.create_request('/updates/%s' % params['builds'],
+ method='GET', headers=session)
+
+ assert "Push to Testing" in cherrypy.response.body[0]
+ assert "Push Critical Path update to Stable" in cherrypy.response.body[0]
+
+ def test_critpath_to_frozen_release_testing(self):
+ """
+ Ensure devs can *not* push critpath updates directly to stable
+ for pending releases
+ """
+ session = login()
+ create_release(locked=True)
+ params = {
+ 'builds' : 'kernel-2.6.31-1.fc7',
+ 'release' : 'Fedora 7',
+ 'type_' : 'bugfix',
+ 'bugs' : '',
+ 'notes' : 'foobar',
+ 'stable_karma' : 1,
+ 'request': None,
+ 'unstable_karma' : -1,
+ }
+ self.save_update(params, session)
+ update = PackageUpdate.byTitle(params['builds'])
+
+ # Pretend it's pushed to testing
+ update.pushed = True
+ update.status = 'testing'
+
+ testutil.create_request('/updates/%s' % params['builds'],
+ method='GET', headers=session)
+
+ # Ensure the dev cannot push it to stable
+ assert "/updates/request/stable" not in cherrypy.response.body[0]
+
+ def test_non_critpath_to_frozen_release_testing(self):
+ """
+ Ensure non-critpath packages can still be pushed to stable as usual
+ """
+ session = login()
+ create_release(locked=True)
+ params = {
+ 'builds' : 'nethack-2.6.31-1.fc7',
+ 'release' : 'Fedora 7',
+ 'type_' : 'bugfix',
+ 'bugs' : '',
+ 'notes' : 'foobar',
+ 'stable_karma' : 1,
+ 'request': None,
+ 'unstable_karma' : -1,
+ }
+ self.save_update(params, session)
+ update = PackageUpdate.byTitle(params['builds'])
+
+ # Pretend it's pushed to testing
+ update.pushed = True
+ update.status = 'testing'
+
+ testutil.create_request('/updates/%s' % params['builds'],
+ method='GET', headers=session)
+
+ assert "/updates/request/stable" in cherrypy.response.body[0]
+
+ def test_critpath_to_frozen_release_testing_admin_actions(self):
+ """
+ Ensure admins can submit critpath updates for pending releases to stable.
+ """
+ session = login(group='qa')
+ create_release(locked=True)
+ params = {
+ 'builds' : 'kernel-2.6.31-1.fc7',
+ 'release' : 'Fedora 7',
+ 'type_' : 'bugfix',
+ 'bugs' : '',
+ 'notes' : 'foobar',
+ 'stable_karma' : 1,
+ 'request': None,
+ 'unstable_karma' : -1,
+ }
+ self.save_update(params, session)
+ update = PackageUpdate.byTitle(params['builds'])
+
+ # Pretend it's pushed to testing
+ update.pushed = True
+ update.status = 'testing'
+
+ testutil.create_request('/updates/%s' % params['builds'],
+ method='GET', headers=session)
+
+ assert "Mark Critical Path update as Stable" in cherrypy.response.body[0]
Attachment:
pgpcukh6GgzOi.pgp
Description: PGP signature
-- fedora-devel-list mailing list fedora-devel-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/fedora-devel-list
