← Back to team overview

ubuntu-phone team mailing list archive

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

 

On Sat, Dec 14, 2013 at 12:05 AM, Dimitri John Ledkov <xnox@xxxxxxxxxx> wrote:
> 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.
>

Seconded. Treating DBus as an implementation detail and not leaking it
to customers of your service helps a lot in versioning access to your
API/service. In addition, our tooling and packaging supports
versioning on a library/symbol level quite nicely.

> 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?
>>

Only if the packaging machinery is able to notice the breakage. For
that: Try to avoid providing "raw" access to your DBus API.

>
> 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.
>
>

Seconded!

Cheers,

  Thomas

>> 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.
>
> --
> Mailing list: https://launchpad.net/~ubuntu-phone
> Post to     : ubuntu-phone@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~ubuntu-phone
> More help   : https://help.launchpad.net/ListHelp


Follow ups

References