Checkbox

whitelist ordering of jobs is not preserved by clients

Bug #1213893 reported by Zygmunt Krynicki on 2013-08-19

This bug affects 2 people

Affects		Status	Importance	Assigned to	Milestone
	Checkbox	Fix Released	High	Brendan Donegan	Checkbox plainbox-0.5

Bug Description

Whitelists contain patterns of job names in sequential order, eg 'job1', 'job2', 'job3.*' . When clients utilize that whitelist the order of executed jobs (forgetting about dependencies for a second) is not as it was in the whitelist. Instead the order depends on how jobs are returned by job enumeration APIs.

Clients should iterate whitelist entries and match that to jobs, not enumerate jobs and match that to a whitelist.

NOTE: it is yet unknown how this should work when clients are faced with multiple whitelists

This bug affects:

1) The dbus-mini-client
2) The checkbox-ihv-ng GUI
3) plainbox run command

Tags:

Related branches

lp:~zyga/checkbox/whitelist-ordering

Merged into lp:checkbox at revision 2633

Daniel Manrique (community): Approve on 2014-01-30

lp:~brendan-donegan/checkbox/gui_whitelist_ordering

Merged into lp:checkbox at revision 2798

Zygmunt Krynicki (community): Approve on 2014-03-18

lp:~brendan-donegan/cdts/whitelist_ordering

Merged into lp:cdts at revision 2365

Zygmunt Krynicki (community): Approve on 2014-03-20

Daniel Manrique (community): Needs Fixing on 2014-03-20

lp:~brendan-donegan/checkbox/ffe_ui_custom_transport_ubuntu

lp:ubuntu/trusty/checkbox

Revision history for this message

Ara Pulido (ara) wrote on 2013-08-19:

For more than whitelist I don't have a strong opinion, but a possible solution would be:

1) Run whitelists in alphabetical order
2) For each whitelist, use the classic ordering
3) If a test has been already added by a previous whitelist, don't add it again when generating the list of tests.

Again, I am flexible about several whitelists, as it is a new use case

Revision history for this message

Zygmunt Krynicki (zyga) wrote on 2013-08-19:

That sounds good. I'll get it fixed on our side.

Ara, quick question: in plainbox CLI we can select whitelist "in order" by passing -w many times. Do you want to preserve that order or still apply your algorithm (so ordering would be irrelevant and would be determined by rule 1, alphabetically).

Revision history for this message

Ara Pulido (ara) wrote on 2013-08-19:

Let's do it alphabetically also on Plainbox CLI, to be consistent

Zygmunt Krynicki (zyga) on 2013-08-19

Changed in checkbox-ihv-ng:
status:	New → Triaged
Changed in checkbox:
status:	New → Triaged

Ara Pulido (ara) on 2013-08-22

Changed in checkbox-ihv-ng:
milestone:	none → version1.2

Chris Gregan (cgregan) on 2013-09-05

Changed in checkbox-ihv-ng:
importance:	Undecided → Medium

Zygmunt Krynicki (zyga) on 2013-09-10

tags:	added: plainbox
Changed in checkbox:
assignee:	nobody → Zygmunt Krynicki (zkrynicki)
importance:	Undecided → High
status:	Triaged → In Progress

Sylvain Pineau (sylvain-pineau) on 2013-09-19

Changed in checkbox-ihv-ng:
milestone:	version1.2 → version1.3

Revision history for this message

Zygmunt Krynicki (zyga) wrote on 2013-10-10:

ordering.py Edit (1.3 KiB, text/x-python)

I've implemented an ordering algorithm. If nobody disapproves I'll add that to plainbox (probably applogic) and expose it over dbus for applications to use.

Revision history for this message

Sylvain Pineau (sylvain-pineau) wrote on 2013-10-11:

@Zygmunt, whitelists include regexp patterns only meant to be parsed with re, not fnmatch

Revision history for this message

Zygmunt Krynicki (zyga) wrote on 2013-10-12:

Sylvain: I'm well aware of that. This example was meant to be simple but instructive. The only difference for "production" mode is the implementation of the has_wildcards() function which speeds things up but is not affecting the semantics in any way.

Revision history for this message

Daniel Manrique (roadmr) wrote on 2013-10-18:

I don't disapprove, I think this algorithm is fine.

Chris Gregan (cgregan) on 2013-10-29

Changed in checkbox-ihv-ng:
milestone:	version1.3 → version1.4
milestone:	version1.4 → version1.3

Chris Gregan (cgregan) on 2013-10-29

Changed in checkbox-ihv-ng:
status:	Triaged → In Progress

Chris Gregan (cgregan) on 2013-10-30

Changed in checkbox-ihv-ng:
milestone:	version1.3 → version1.5

Zygmunt Krynicki (zyga) on 2013-12-02

Changed in checkbox:
milestone:	none → plainbox-0.5

Chris Gregan (cgregan) on 2013-12-18

Changed in checkbox-ihv-ng:
status:	In Progress → Triaged

Zygmunt Krynicki (zyga) on 2013-12-24

Changed in checkbox:
milestone:	plainbox-0.5 → plainbox-0.5a1

Chris Gregan (cgregan) on 2014-01-08

Changed in checkbox-ihv-ng:
milestone:	version1.5 → version1.6

Sylvain Pineau (sylvain-pineau) on 2014-01-27

Changed in checkbox-ihv-ng:
milestone:	version1.6 → version1.7

Zygmunt Krynicki (zyga) on 2014-02-03

Changed in checkbox:
milestone:	plainbox-0.5a1 → plainbox-0.5

Chris Gregan (cgregan) on 2014-02-13

Changed in checkbox-ihv-ng:
milestone:	version1.7 → version1.8

Chris Gregan (cgregan) on 2014-03-12

Changed in checkbox-ihv-ng:
importance:	Medium → High

Zygmunt Krynicki (zyga) on 2014-03-17

Changed in checkbox:
assignee:	Zygmunt Krynicki (zkrynicki) → nobody
assignee:	nobody → Brendan Donegan (brendan-donegan)

Brendan Donegan (brendan-donegan) on 2014-03-19

Changed in checkbox:
status:	In Progress → Fix Released
status:	Fix Released → Fix Committed

Chris Gregan (cgregan) on 2014-03-20

Changed in checkbox-ihv-ng:
importance:	High → Critical

Revision history for this message

Daniel Manrique (roadmr) wrote on 2014-03-21:

Brendan's code that leverages SelectJobs() is now in canonical-driver-test-suite, I marked this as Fix Committed. It's actually available in PPA builds now but will break until a recent namespacing fix also makes it into c-d-t-s. We should hold off marking it fix released until we've tested an actual cdts tarball/package set.

Still, with the right set of packages and fixes as I tested on a VM, it works very well and whitelist ordering is respected when possible, as delivered by select_jobs.