anybox / anybox.buildbot.odoo Goto Github PK

This is for database upgrade tests. Seen on an OpenERP 7.0 project, these error swallowings:

 except ImportError:
    _logger.error('module %(addon)s: Unable to load %(stage)s-migration file %(file)s' % mergedict({'file': pyfile}, strfmt))
    raise
except AttributeError:
    _logger.error('module %(addon)s: Each %(stage)s-migration file must have a "migrate(cr, installed_version)" function' % strfmt)

trivia: in that case, the AttributeError was somewhere down the stack, i.e., not for the reason mentioned in the log

This is a long standing improvement wish, see the original writing on launchpad

In short, as projects evolve, their dependencies change, and nobody manages to keep the watch list up to date (it could very well be under someone else's responsibility). Consequences are the natural ones: builders keep running for addons that have long been fixed in all incarnations of the buildout, people don't think to track the bulk of odoo and get nasty surprises whereas the buildbot should have warned them.

What held this back for so long is that the polling is done from the buildmaster, hence the watch parameters are set in the master-side MANIFEST.cfg. Updating it would mean many things that the buildmaster is not really fit to perform:

1. actually grabbing the buildouts in the most frequent cases where they are on some separate VCS.
2. installing enough of the buildouts to be able parse the addons and gp.vcsdevelop directives (therefore, with bootstrap-hell)
3. according to the VCS in use, maybe also actually retrieve the sub dependencies for inspection.

This means that the buildmaster must reimplement and run a good deal what the slaves perform, and this almost defeats the idea of a distributed system.

So here's a new yet obvious idea: this just means that this work to be done on the buildslaves! And indeed, in the course of an ordinary build, the buildout lies open, ready for introspection of what is from a live revision/commit and what is not. The helper methods of the recipe are even there to serve, if needed.

At Anybox, we already have crons that does bin/buildbot reconfig every hour, so we don't even need hot-reloading of the configuration upon completion of the analysis, although that'd also be fun. The following should be enough:

• allow watch configuration to be externalized to independent enough files (one per section in MANIFEST.cfg
• have the builds dump the watch directive in a small file right after performing bin/buildout and send it back to the master with a FileUpload step.
• having one watch configuration file per build factory will allow us to put locks on the upload steps without blocking unrelated builds
• some exceptions must be made for the builds that can run on old versions of the buildouts (e.g with the hgtag scheme, or release builds that run by definition on a frozen version of the buildout). The simplest would be to introduce a new option for MANIFEST.cfg: auto-watch, defaulting to True.

That's not a very tight design, and in particular, it means that a change in dependencies would not be immediately applied master-side, but who would care about one hour lag in that context whereas our problem today is that outdated watches stay for weeks if not months ?

That being said, the parsing of those independent watch directives should not fail ever, otherwise one bogus conf would stop the whole updating for a long while, and jeopardize the next restart if needed (this is not new). That raises the question of properly reporting errors

Cc @sbidoul @jssuzanne @pverkest @ccomb for feedback.

I'm willing to come up with a prototype real soon now, being tired of having errors that should have already been detected each time I reforce a bunch of builds to test something else than the builds outcomes themselves.

Failure: twisted.internet.defer.FirstError: FirstError[#12, [Failure instance: Traceback (failure with no frames): <class 'twisted.internet.defer.FirstError'>: FirstError[#3, [Failure instance: Traceback: <type 'exceptions.ValueError'>: unsupported format character '{' (0x7b) at index 1

According to the doc, this is indeed typical of wrong format strings in Python (on which properties interpolation relies)

During some release builds, I got this on several buildslaves:

Traceback from remote host -- Traceback (most recent call last):
(...)_
  File "/srv/buildslave/local/lib/python2.7/site-packages/twisted/internet/defer.py", line 139, in maybeDeferred
    result = f(*args, **kw)
  File "/srv/buildslave/local/lib/python2.7/site-packages/buildslave/commands/transfer.py", line 285, in start
    os.makedirs(dirname)
  File "/srv/buildslave/lib/python2.7/os.py", line 157, in makedirs
    mkdir(name, mode)
exceptions.OSError: [Errno 17] File exists: '/srv/buildslave/slavedir/myproject-release-postgresql-9_3/build/../src'
exceptions.OSError: [Errno 17] File exists: '/srv/buildslave/slavedir/myproject-release-postgresql-9_3/build/../src'

Explanation: the build/ subdir does not exist, therefore os.path.exists('build/../src') returns False, and that trigger the makedirs on a directory that actually exists (something at lower level probably normalizes a bit more before creating)

Since this FileDownload has been there since the inception of the 'hgtag' token, I suppose this error can happen only after previous errors at some precise point.

Anyway, workdir=src will be cleaner than ../src

develop-eggs is known to be the most frequent source of erratic stateful behaviour in buildout, to the point that some people actually propose to wipe it afresh at each bootstrap, but that's not currently the case.

I have a build that runs with an outdated version of anybox.recipe.openerp on exactly one buildslave (out of half a dozen), just because of this.

Since we're reboostraping at each build anyway, we should also remove develop-eggs at each build before the bootstrap recreates a new one for whatever legitimate use it may have.

In a swarm with heterogeneous servers, it can happen that a few are less or more powerful than the others.

Buildbot provides the max_builds parameter to control how many parallel builds a given buildslave can take, but that's not enough. For instance, at Anybox, we have currently a slow but highly threaded server. It won't be slower for 6 builds than for one. Still, we prefer to use it only if the quicker ones are already busy.

Buildbot allows customization of the choice by means of the nextSlave callable. We could write one using a slave_priority property.

Currently, the make_pollers() method of MultiWatcher accepts an argument (default 600s), but

• the configurator just uses the default value
• that value will be used identically for all pollers

It's natural however to have different polling intervals, especially if some sources have change hooks, i.e., push notifications. Indeed in that case, a much bigger interval is in order, just to catch up if some notification got lost.

It would be much more powerful to register a callable that can return the proper value and will be called at poller instantiation time.

Somehow, changes in master seem to trigger builds on other branches (seen on Odoo, but would probably be the same for other repos):

http://buildbot.anybox.fr/builders/odoo-80-postgresql-9.3/builds/560
http://buildbot.anybox.fr/builders/odoo-80-postgresql-9.3/builds/569

(that's before first tests of #1 on this buildbot)

In some case, it's useful to deactivate some modules before launching tests or doing upgrades.

For instance, we have currently a project that depends on ldap_synchronization, and the upgrade script can't run without a properly configured LDAP server. Of course it'd be even better to setup a full integration testing harness, but we don't want to depend too hard on that, especially since in any case it'd need a modification of that module.