crash while processing results bundle after dashboard bundle format update

IRC chat with discussion:

<terceiro> nicks: actually not - I see the same issue as you
<terceiro> will report a bug
<terceiro> nicks: https://bugs.launchpad.net/lava-dispatcher/+bug/1166935
<terceiro> stylesen: ^
<stylesen> terceiro: description is a mandatory field in all the test definitions
<stylesen> terceiro: description, name, format and version (only if the test def comes from a URL, version is mandatory)
<terceiro> stylesen: that testdef used to work before
<terceiro> I didn't change it for ages
<stylesen> terceiro: yes it used to, but now due to format upgrade and also since none of the fields in testdef_metadata are not optional, it will not work
<stylesen> terceiro: if we want to make it work, then we need have empty strings initialized in code
<terceiro> stylesen: or make the attribute optional?
<stylesen> terceiro: anyways, add your thoughts on the bug, I shall look into it tomorrow
<terceiro> stylesen: ok, np - thanks
<tyler-baker> I like the attribute as optional, :)
<stylesen> terceiro: it used to be optional, but zyga did not want everything to be optional
<terceiro> stylesen: I think we can allow a free form text field to be optional :)
<zyga> stylesen: hmm
<zyga> stylesen: my comments on optionality were well founded
<stylesen> zyga: I just made all fields mandatory after your comment
<zyga> stylesen: I don't recall the details anymore
<zyga> stylesen: but If you want I can explain why
<terceiro> stylesen: so before this we could submit a testdef with no metadata whatsoever?
<stylesen> zyga: I remember that it was because, there is chances that we accumulate more incomplete testdef_metadata
<zyga> stylesen: IIRC the situation was that it was possible to break existing submissions when you added testdef meta-data to 1.5 schema
<stylesen> terceiro: before this there was no testdef_metadata accumulation on the database, now there is a model and we strive to make it complete
<zyga> stylesen: if you do 1.6 schema it's still optional because otherwise you cannot upgrade
<terceiro> stylesen: ok, I got it. But will it break existing testdefs people are submitting?
<terceiro> stylesen: I known the ones in qa/test-definitions are complete
<stylesen> terceiro: yes it will break
<terceiro> stylesen: we have to analyze the disruption before deploying that.
<terceiro> we cannot break everything :)
<stylesen> zyga: testdef_metadata dict is optional still, but if exists then the metadata should be complete with all attributes
<zyga> stylesen: yes that makes sense
<terceiro> stylesen: do we know whether there are users submitting testdefs with incomplete metadata?
<terceiro> (other than ourselves)
<stylesen> zyga: that is what is currently available in code, but it will break test definitions that are incomplete with metadata, which is correct, terceiro? what do you feel?
<terceiro> stylesen: correct. I think we should investigate users that would be impacted by this and notify then a few days before upgrading
<stylesen> terceiro: apart from us, everyone has incomplete metadata, hence it will break, this will be an opportunity to fix their test definitions with correct metadata and have complete data in our database models
<zyga> stylesen:...

So, the situation is that

- no metadata was mandatory before
- now if you include a metadata entry, it has to have *all* entries: name, version, format, and description. any one of those will make lava-test-shell break
- existing test definitions will break when we upgrade production

I suggest that before rolling this out we:

- identify users who submit lava-test-shell tests to v.l.o and have incomplete testdefs
- notify those users that starting at 2013-04-XX, v.l.o will require the metadata to be complete.
- we wait at least a couple days after before we upgrade so people have time to fix their testdefs.

This has to be done quickly, because we should not wait too long with undeployed code in trunk.

sample failure:
http://staging.validation.linaro.org/scheduler/job/44582/log_file

Obtained a list of active users who are using lava-test-shell from production database.

I did a quick grep through:
https://git.linaro.org/gitweb?p=qa/test-definitions.git;a=summary

The case is that everyone of the those test definitions _has_ a name,
description, and format defined. However, no test definition includes a
version definition. My guess is that most of the other test definitions
will be lacking the version definition as well.

I agree with Antonio on the roll out plan. I would add that:

- Document the changes to the test definitions, and the rationale. i.e.
that name, description, format, version are required, and why.
- Update the test definitions from
https://git.linaro.org/gitweb?p=qa/test-definitions.git;a=summary
*- *Identify V.L.O users who use lava-test-shell, point them to the
documentation, and ask them to update their tests.

 We need to preemptive, and transparent if we plan to break compatibility.

On 9 April 2013 11:40, Antonio Terceiro <email address hidden> wrote:

> sample failure:
> http://staging.validation.linaro.org/scheduler/job/44582/log_file
>
> --
> You received this bug notification because you are a member of Linaro
> Validation Team, which is subscribed to LAVA Dispatcher.
> https://bugs.launchpad.net/bugs/1166935
>
> Title:
> crash while processing results bundle after dashboard bundle forma
> update
>
> Status in LAVA Dispatcher:
> New
>
> Bug description:
> Running everything from trunk on a freshly created instance, I see the
> following error:
>
> <LAVA_DISPATCHER>2013-04-09 05:57:07 PM WARNING: [ACTION-E]
> lava_test_shell is finished with error (ValidationError: Object has
> incorrect type (expected string)
> object_expr='object.test_runs[0].testdef_metadata.description',
> schema_expr='schema.properties.test_runs.items.properties.testdef_metadata.properties.description.type')).
> <LAVA_DISPATCHER>2013-04-09 05:57:07 PM INFO: Submitting the test result
> with parameters = {u'token': '<HIDDEN>', u'stream': u'/anonymous/qemu/',
> u'server': u'http://terceiro@localhost/RPC2/'}
> <LAVA_DISPATCHER>2013-04-09 05:57:07 PM ERROR: Error adding host result
> bundle /tmp/tmp8TL_hY/lava-test-shellW5j2JL.bundle
> Traceback (most recent call last):
> File
> "/srv/lava/.<email address hidden>/lava_dispatcher/actions/launch_control.py",
> line 159, in _get_results_from_host
> doc = DocumentIO.load(f)[1]
> File
> "/srv/lava/.cache/branch<email address hidden>/linaro_dashboard_bundle/io.py",
> line 123, in load
> doc = json.load(stream, parse_float=decimal.Decimal,
> object_pairs_hook=object_pairs_hook)
> File
> "/srv/lava/.cache/eggs/simplejson-2.4.0-py2.7-linux-x86_64.egg/simplejson/__init__.py",
> line 372, in load
> use_decimal=use_decimal, **kw)
> File
> "/srv/lava/.cache/eggs/simplejson-2.4.0-py2.7-linux-x86_64.egg/simplejson/__init__.py",
> line 445, in loads
> return cls(encoding=encoding, **kw).decode(s)
> File
> "/srv/lava/.cache/eggs/simplejson-2.4.0-py2.7-linux-x86_64.egg/simplejson/decoder.py",
> line 402, in decode
> obj, end = self.raw_decode(s, ...

Hi Tyler,

On Thu, Apr 11, 2013 at 9:41 AM, Tyler Baker <email address hidden> wrote:
> The case is that everyone of the those test definitions _has_ a name,
> description, and format defined. However, no test definition includes a
> version definition. My guess is that most of the other test definitions
> will be lacking the version definition as well.

This is because version is not required for those test definitions
which reside in git or bzr repository. lava-test-shell will get the
version from git/bzr repo automatically. Only if the test definition
is referred from a URL the version parameter should be explicit.

> - Document the changes to the test definitions, and the rationale. i.e.
> that name, description, format, version are required, and why.

I shall document it in lava-test-shell sometime tomorrow.

> - Update the test definitions from
> https://git.linaro.org/gitweb?p=qa/test-definitions.git;a=summary

This is up-to-date now.

> *- *Identify V.L.O users who use lava-test-shell, point them to the
> documentation, and ask them to update their tests.

These users are identified. As soon the document is available we will
point them to it.

> We need to preemptive, and transparent if we plan to break
> compatibility.

I agree :)

Thank You.
--
Senthil Kumaran S
http://www.stylesen.org/
http://www.sasenthilkumaran.com/

Hi Senthil,

On 11 April 2013 10:14, Senthil Kumaran S <email address hidden>wrote:

> Hi Tyler,
>
> On Thu, Apr 11, 2013 at 9:41 AM, Tyler Baker <email address hidden>
> wrote:
> > The case is that everyone of the those test definitions _has_ a name,
> > description, and format defined. However, no test definition includes a
> > version definition. My guess is that most of the other test definitions
> > will be lacking the version definition as well.
>
> This is because version is not required for those test definitions
> which reside in git or bzr repository. lava-test-shell will get the
> version from git/bzr repo automatically. Only if the test definition
> is referred from a URL the version parameter should be explicit.
>

I understand now, thanks for the clarification. I would suggest "Only if
the test definition
is referred from a URL the version parameter should be explicit." be
included in the documentation if it is not already.

> > - Document the changes to the test definitions, and the rationale. i.e.
> > that name, description, format, version are required, and why.
>
> I shall document it in lava-test-shell sometime tomorrow.
>

Much appreciated.

> > - Update the test definitions from
> > https://git.linaro.org/gitweb?p=qa/test-definitions.git;a=summary
>
> This is up-to-date now.
>
> > *- *Identify V.L.O users who use lava-test-shell, point them to the
> > documentation, and ask them to update their tests.
>
> These users are identified. As soon the document is available we will
> point them to it.
>

Excellent!

> > We need to preemptive, and transparent if we plan to break
> > compatibility.
>
> I agree :)
>
> Thank You.
> --
> Senthil Kumaran S
> http://www.stylesen.org/
> http://www.sasenthilkumaran.com/
>
> --
> You received this bug notification because you are a member of Linaro
> Validation Team, which is subscribed to LAVA Dispatcher.
> https://bugs.launchpad.net/bugs/1166935
>
> Title:
> crash while processing results bundle after dashboard bundle format
> update
>
> Status in LAVA Dispatcher:
> In Progress
>
> Bug description:
> Running everything from trunk on a freshly created instance, I see the
> following error:
>
> <LAVA_DISPATCHER>2013-04-09 05:57:07 PM WARNING: [ACTION-E]
> lava_test_shell is finished with error (ValidationError: Object has
> incorrect type (expected string)
> object_expr='object.test_runs[0].testdef_metadata.description',
> schema_expr='schema.properties.test_runs.items.properties.testdef_metadata.properties.description.type')).
> <LAVA_DISPATCHER>2013-04-09 05:57:07 PM INFO: Submitting the test result
> with parameters = {u'token': '<HIDDEN>', u'stream': u'/anonymous/qemu/',
> u'server': u'http://terceiro@localhost/RPC2/'}
> <LAVA_DISPATCHER>2013-04-09 05:57:07 PM ERROR: Error adding host result
> bundle /tmp/tmp8TL_hY/lava-test-shellW5j2JL.bundle
> Traceback (most recent call last):
> File
> "/srv/lava/.<email address hidden>/lava_dispatcher/actions/launch_control.py",
> line 159, in _get_results_from_host
> doc = DocumentIO.load(f)[1]
> File
> "/srv/lava/.cach...

Added documentation to lava-test-shell in r579 @ lava-dispatcher trunk branch. Attached is the patch for reference.

We sent out a notification about this and now the related code is in production.