← Back to team overview

ubuntu-phone team mailing list archive

Re: LP: #1260712 post-mortem and improving our processes

 

On 13 December 2013 22:25, Barry Warsaw <barry@xxxxxxxxxx> wrote:
>
> So, what steps can be taken to avoid such problems in the future, and what
> lessons were learned that might be of use across the project?
>

dbus apis are hard, hence a few dbus apis are provided as
shared-libraries (essentially generate client code and compile it into
shared library).
When looking at dbus api, as if it's a compiled generated shared
library the ABI/API break is obvious and one typically renames the
library package as per Debian policy.

Looking at other well-behaved dbus exported daemons, they encode
version name in the well-known name (e.g. PolicyKit1, RealtimeKit1,
UDisks2, login1) or the interface names (e.g. com.ubuntu.Upstart0_6 ,
althought that might be simply a cool looking emoticon 0_6 )


> Steve brought up a very good point in my private discussions with him:
> versioning the API.  In this case, it might have been better to leave
> ApplyUpdate() alone and introduce a new D-Bus API method with the new
> behavior.  Decoupling s-i and s-s in this way would allow an unchanged
> system-settings to have continued to work, and s-s could change to the new,
> improved API at its leisure.  Of course, this would mean that an unchanged
> system-settings would still be affected by the re-downloading bug, since that
> bug could not be fixed without the change in the API.
>

Keeping ApplyUpdate() as it was, and introducing e.g.
ApplyAsyncUpdate() would have been better.

Or e.g. using nih-dbus-tool from the start which explicitly supports
both sync and async dbus method calls.

> (There's a possible middle ground in *this* instance, where the old API could
> have been faked closely enough to fool s-s, but that isn't a general solution.
> It's essentially what Didier's hot fix implements.)
>
> API versioning is an important principle in general, and especially important
> in our touch images since we already have such a difficult time coordinating
> all the ongoing changes into a clean image that can be promoted.  An example
> here would be future changes to the D-Bus API for ubuntu-download-manager, for
> which there are multiple clients.  Is it even feasible to expect that all
> those components are released or promoted together so that nothing at all
> breaks?
>

I think breaking API is bad. And thus should generally be avoided. If
possible keep API/ABI compatible, if not leave it as it is (support
it) and introduce new api with fixed behaviors.
It does come at a cost. But for example from the perspective of the
ubuntu-framework-13.10 - Everything which is not forbidden is allowed.
So all DBus chatter that can be opened up by stock apparmor profiles
must stay compatible, and typical API non-breakage rules apply - only
add, don't change/remove api.

> Can we use packaging metadata to help enforce coordinated promotion?  E.g. if
> we knew that s-i 2.0 "broke" system-settings, a Breaks could have been added,
> but that would still require better coordination between the two projects so
> that we knew what version of s-s would satisfy the API requirements of s-i
> 2.0.
>

coordinated promotion is easy, simply make sure the package will get
stuck in the -proposed pocket and not promoted into release pocket.
e.g. system-image-update declaring breaks: system-settings (<< X.Y)
would have been sufficient. But i'm not sure it could have been used
effectively in this case, thanks to daily-release version numbers are
not predictable and one cannot know ahead of time which version number
of fixed system-settings will be.

So as the other tool, you could have opened a "block-proposed" bug
against system-image-update (and/or other affected packages). It's a
manual hammer, which needs a human to both set and remove. So it needs
to be coordinated, to avoid clogging up proposed migration. But at
least packages well be available from proposed pocket for ease of
manual testing & validation, and if very broken be even remove from
proposed pocket without affecting the release pocket.

> How could we have written automated tests to catch this?  Even with
> autopkgtests in s-i, without autopilot tests of the u/i, this would have gone
> unnoticed.  What could I have done to better Q/A the change, or to have better
> helped the Q/A team to test or notice such a change?
>

.... autogenerate tests against all dbus api calls used and validate
that they do what one expect, make it runable on stock i386/amd64 and
make sure the reverse depends call them out ....
very heavy weight, but it could have worked. also quite pointless, as
changing dbus api will be hard (even if coordinated). Imho it's best
handled by policy, e.g. unit-test and validate your dbus api calls to
not change (keep api/abi stability) I'm not sure if there is a tool
like that already, e.g. i do know that libnih-dbus has test-suite
generators (althought it still does have a lot of boiler plate).

Did the ApplyUpdates() dbus signature change? or did it really changed
behaviour i.e. instead of returning string and empty string got
returned?

> This email is already long enough, and I have probably raised more questions
> than I've answered.  I hope this is taken in the spirit in which it is written
> -- to openly investigate the problem, discuss solutions, and drive together as
> a team toward better processes which improve overall quality and confidence in
> our images.  It's not to cast any blame, nor to shirk my responsibility for
> this bug.  I'm just glad that the fire was quickly put out, and that we have
> the opportunity to discuss this in the light of a small flame.
>

i think such post-mortem was very useful. thanks for raising this. I
did read the previous API change announce email, but didn't flag it up
as potential cause for bugs.


> In any case, I don't anticipate any more changes to system-image before next
> year, so enjoy the coming holidays with many happy and successful updates to
> your mobile devices. :)
>

May the delta updates small, be with you ? =))))

-- 
Regards,

Dimitri.


Follow ups

References