graphics_card_resource puts spaces in product names which results in broken job ids

Bug #1425722 reported by Daniel Manrique
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Checkbox Provider - Resource
Fix Released
High
Daniel Manrique

Bug Description

Picture a job like:

unit: template
template-unit: job
template-resource: graphics_card_resource
plugin: shell
id: graphics/{index}_screen-capture-internal_{product}
_summary: Obtains a simple screen capture of {product}
estimated_duration: 1.0
requires: package.name == 'gnome-screenshot'
command: gnome-screenshot --file $PLAINBOX_SESSION_SHARE/screen-capture-{index}.png
_description: Obtains a simple screen capture of {product} using gnome-screenshot if present

Now picture output of graphics_card_resource like:

bus: pci
category: VIDEO
driver: i915
index: 1
path: /devices/pci0000:00/0000:00:02.0
product: Haswell-ULT Integrated Graphics Controller
product_id: 2582
subproduct_id: 1546
subvendor_id: 4136
vendor: Intel Corporation
vendor_id: 32902

The product has spaces, so it results in a job id like:

graphics/1_screen-capture-internal_Haswell-ULT Integrated Graphics Controller

This obviously breaks things resulting in errors similar to (in this case, the bogus job was specified as a dependency):

There were some problems with the selected jobs
 * missing dependency: '2013.com.canonical.certification::Integrated' (direct)
Problematic jobs will not be considered

Previously this was handled in the job's ID which in non-template jobs is done like so:

id: graphics/`echo ${index}`_resolution-change_`echo "${product}" | sed 's/ /_/g;s/[^_a-zA-Z0-9-]//g'`

Since we no longer have that shell processing, maybe the resource should do these substitutions. Though I wonder if munging the product name that much would cause other repercussions. Graphics_card_resource is only ever used in conjunction with that sed substitution, so perhaps it would be OK for the resource itself to do it.

Related branches

Daniel Manrique (roadmr)
Changed in checkbox:
status: New → Triaged
importance: Undecided → High
affects: checkbox → plainbox-provider-resource
Revision history for this message
Daniel Manrique (roadmr) wrote :

--- providers/plainbox-provider-resource-generic/bin/graphics_card_resource 2015-01-27 14:34:02 +0000
+++ providers/plainbox-provider-resource-generic/bin/graphics_card_resource 2015-02-25 22:29:43 +0000
@@ -18,6 +18,7 @@
 # along with Checkbox. If not, see <http://www.gnu.org/licenses/>.

 import argparse
+import re
 import subprocess
 import shlex

@@ -115,6 +116,12 @@
                 except ValueError:
                     fake_product = "PCI ID unknown"
                 record['product'] = fake_product
+ # replace characters that would make for an invalid job id
+ record['product'] = re.sub(r"[^_a-zA-Z0-9-]",
+ "",
+ record['product'].replace(
+ " ",
+ "_"))
             # Finally, print the record
             items = ["{key}: {value}".format(key=k, value=record[k])
                      for k in sorted(record.keys())]

Daniel Manrique (roadmr)
Changed in plainbox-provider-resource:
status: Triaged → In Progress
assignee: nobody → Daniel Manrique (roadmr)
Revision history for this message
Zygmunt Krynicki (zyga) wrote :

The solution is to quote the desired pattern in the test plan.

Revision history for this message
Zygmunt Krynicki (zyga) wrote :

In other words, we still support arbitrary identifiers, including spaces, we just need to quote them.

Revision history for this message
Zygmunt Krynicki (zyga) wrote :

Quick question, how did you get this error:
(to quote your description)

This obviously breaks things resulting in errors similar to (in this case, the bogus job was specified as a dependency):

There were some problems with the selected jobs
 * missing dependency: '2013.com.canonical.certification::Integrated' (direct)
Problematic jobs will not be considered

Zygmunt Krynicki (zyga)
Changed in plainbox-provider-resource:
status: In Progress → Fix Committed
milestone: none → 0.18
Changed in plainbox-provider-resource:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.