[PATCH] image: add mechanism to run QA checks on the image once it's built

[PATCH] image: add mechanism to run QA checks on the image once it's built

[Joshua Lock]

Add a mechanism to run QA checks on a constructed image once it's
complete. All checks will be run with any one failure resulting in
a failed build.

QA checks should be bitbake functions which throw a
NotImplementedError when the check QA fails, with any error
messages passed to the exception.

Specify which checks to run by adding them to IMAGE_QA_COMMANDS.

i.e.

IMAGE_QA_COMMANDS += " \
    image_check_everything_ok \
"

python image_check_everything_ok () {
    raise NotImplementedError('This check always fails')
}

This code is based heavily on the configuration upgrade code in
sanity.bbclass.

[YOCTO #9448]

Signed-off-by: Joshua Lock <joshua.g.lock@intel.com>
---
 meta/classes/image.bbclass | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
index 9f4c83f..40bcd16 100644
--- a/meta/classes/image.bbclass
+++ b/meta/classes/image.bbclass
@@ -260,6 +260,35 @@ do_image_complete[dirs] = "${TOPDIR}"
 do_image_complete[umask] = "022"
 addtask do_image_complete after do_image before do_build
 
+# Add image-level QA/sanity checks to IMAGE_QA_COMMANDS
+#
+# IMAGE_QA_COMMANDS += " \
+#     image_check_everything_ok \
+# "
+# This task runs all functions in IMAGE_QA_COMMANDS after the image
+# construction has completed in order to validate the resulting image.
+# Image sanity checks should raise a NotImplementedError when they fail,
+# passing any failure messages to the Exception. For example:
+#
+#    python image_check_everything_ok () {
+#        raise NotImplementedError('This check always fails')
+#    }
+fakeroot python do_image_qa () {
+    qa_cmds = (d.getVar('IMAGE_QA_COMMANDS', True) or '').split()
+    imageqasane = True
+
+    for cmd in qa_cmds:
+        try:
+            bb.build.exec_func(cmd, d, pythonexception=True)
+        except NotImplementedError as e:
+            imageqasane = False
+            bb.error(str(e))
+
+    if not imageqasane:
+        bb.fatal("QA errors found whilst validating image: %s" % d.getVar('IMAGE_NAME', True))
+}
+addtask do_image_qa after do_image_complete before do_build
+
 #
 # Write environment variables used by wic
 # to tmp/sysroots/<machine>/imgdata/<image>.env
-- 
2.7.4

Re: [PATCH] image: add mechanism to run QA checks on the image once it's built

[Christopher Larson]

[-- Attachment #1: Type: text/plain, Size: 1370 bytes --]

On Tue, Jun 7, 2016 at 7:50 AM, Joshua Lock <joshua.g.lock@intel.com> wrote:

> Add a mechanism to run QA checks on a constructed image once it's
> complete. All checks will be run with any one failure resulting in
> a failed build.
>
> QA checks should be bitbake functions which throw a
> NotImplementedError when the check QA fails, with any error
> messages passed to the exception.
>
> Specify which checks to run by adding them to IMAGE_QA_COMMANDS.
>
> i.e.
>
> IMAGE_QA_COMMANDS += " \
>     image_check_everything_ok \
> "
>
> python image_check_everything_ok () {
>     raise NotImplementedError('This check always fails')
> }
>
> This code is based heavily on the configuration upgrade code in
> sanity.bbclass.
>
> [YOCTO #9448]
>
> Signed-off-by: Joshua Lock <joshua.g.lock@intel.com>
>


What's the behavior if your qa function fails with a different exception?
What if a user writes a shell qa check function, what's the behavior? Also,
this seems like overloading the purpose of NotImplementedError. IMO It'd be
cleaner to either use a custom exception or re-use python unit testing bits
/ use assert rather than subverting this one to a different purpose.
-- 
Christopher Larson
clarson at kergoth dot com
Founder - BitBake, OpenEmbedded, OpenZaurus
Maintainer - Tslib
Senior Software Engineer, Mentor Graphics

[-- Attachment #2: Type: text/html, Size: 1992 bytes --]

Re: [PATCH] image: add mechanism to run QA checks on the image once it's built

[Joshua G Lock]

[-- Attachment #1: Type: text/plain, Size: 3234 bytes --]

On Tue, 2016-06-07 at 08:48 -0700, Christopher Larson wrote:
On Tue, Jun 7, 2016 at 7:50 AM, Joshua Lock <joshua.g.lock@intel.com>
wrote:
> Add a mechanism to run QA checks on a constructed image once it's> 

> complete. All checks will be run with any one failure resulting in> 

> a failed build.> 
> 
> 

> QA checks should be > bitbake>  functions which throw a> 

> NotImplementedError when the check QA fails, with any error> 

> messages passed to the exception.> 
> 
> 

> Specify which checks to run by adding them to IMAGE_QA_COMMANDS.> 
> 
> 

> i.e.> 
> 
> 

> IMAGE_QA_COMMANDS += " \> 

>     image_check_everything_ok \> 

> "> 
> 
> 

> python image_check_everything_ok () {> 

>     raise NotImplementedError('This check always fails')> 

> }> 
> 
> 

> This code is based heavily on the configuration upgrade code in> 

> sanity.> bbclass> .> 
> 
> 

> [YOCTO #9448]> 
> 
> 

> Signed-off-by: Joshua Lock <joshua.g.lock@intel.com>

> > Add a mechanism to run QA checks on a constructed image once it's
> > complete. All checks will be run with any one failure resulting in
> > a failed build.
> > 
> > QA checks should be bitbake functions which throw a
> > NotImplementedError when the check QA fails, with any error
> > messages passed to the exception.
> > 
> > Specify which checks to run by adding them to IMAGE_QA_COMMANDS.
> > 
> > i.e.
> > 
> > IMAGE_QA_COMMANDS += " \
> >     image_check_everything_ok \
> > "
> > 
> > python image_check_everything_ok () {
> >     raise NotImplementedError('This check always fails')
> > }
> > 
> > This code is based heavily on the configuration upgrade code in
> > sanity.bbclass.
> > 
> > [YOCTO #9448]
> > 
> > Signed-off-by: Joshua Lock <joshua.g.lock@intel.com>
> > 
> 
> 
> What's the behavior if your qa function fails with a different
> exception? What if a user writes a shell qa check function, what's
> the behavior? Also, this seems like overloading the purpose of
> NotImplementedError. IMO It'd be cleaner to either use a custom
> exception or re-use python unit testing bits / use assert rather than
> subverting this one to a different purpose.
> 
> 
> 
> 
> 
> What's the behavior if your qa function fails with a different
> exception? What if a user writes a shell qa check function, what's
> the behavior? Also, this seems like overloading the purpose of
> NotImplementedError. IMO It'd be cleaner to either use a custom
> exception or re-use python unit testing bits / use assert rather than
> subverting this one to a different purpose.
> 
> 
> 
Different exceptions to NotImplementedError result in a backtrace… 
The python exception pattern using NotImplementedError I copied from
the configuration upgrade code in sanity.bbclass, because I want to
replicate that same behaviour where the checks are able to pass a
reason back to the task which calls the check functions.
Shell functions are a good point, not least of all because I suspect
many checks could be much more concise with sh.
I'll go back to the drawing board for a more generic solution that
supports both sh and python tasks.
Thanks for the review,
Joshua

[-- Attachment #2: Type: text/html, Size: 4049 bytes --]

Re: [PATCH] image: add mechanism to run QA checks on the image once it's built

[Richard Purdie]

On Tue, 2016-06-07 at 15:50 +0100, Joshua Lock wrote:
> Add a mechanism to run QA checks on a constructed image once it's
> complete. All checks will be run with any one failure resulting in
> a failed build.

How is that different to the testimage class (which tests images after
they're built)?

Cheers,

Richard

Re: [PATCH] image: add mechanism to run QA checks on the image once it's built

[Joshua G Lock]

On Tue, 2016-06-07 at 17:59 +0100, Richard Purdie wrote:
> On Tue, 2016-06-07 at 15:50 +0100, Joshua Lock wrote:
> > Add a mechanism to run QA checks on a constructed image once it's
> > complete. All checks will be run with any one failure resulting in
> > a failed build.
> 
> How is that different to the testimage class (which tests images
> after
> they're built)?

The intended purpose of this task is to run checks on the contents of
the image before it's booted.

I implemented a similar task in meta-swupd to run various checks such
as ensuring a generated image doesn't contain dangling symlinks (a sign
of unexpected consequences from alternatives configuration).

Another check one might run is ensuring /etc and /var are empty in a
stateless distro, or checking /bin, /lib and /sbin are links in a
distro with a merged /usr.

When I have a v2 of this which addresses some of Chris' concerns I'll
try and improve the commit message, variable naming, etc to avoid
confusion.

Regards,

Joshua