Merge lp:~blake-rouse/maas/commissioning-get-disk-info into lp:~maas-committers/maas/trunk

[Blake Rouse]

Populates the PhysicalBlockDevice model for a node when commissioning. Uses a combination of lsblk, udevadm, and blockdev to gather the information from the node during commissioning.

[Raphaël Badin (rvb)]

This looks generally good. I've got a bunch of comments but more importantly, I'm wondering about the failure modes for this. See below.

This uses a lot of commands and a fair share of parsing. All of which could go wrong at some point and break the commissioning. I think it's worth taking a step back and thinking about the failure modes here:
- which part of the code are likely to fail (command failure, parsing failure, etc)? Can we make this more robust against non-fatal error?
- what's the failure mode? In other words, what is going to happen when this fails, what will the consequences be, how easy will it be to the user to get information about what's wrong?

What do you think?

review: Needs Information

[Blake Rouse (blake-rouse)]

I think at the moment we should leave it how it is. I understand your concern, but the issue is that all the calls that are made are required and not optional. Currently if this script fails then all of commissioning fails, which would allow the user to view the output where the exception would be outputted. The only way I can think around this is to remove disks as commands for that disk fail, which might be even more confusing for the user.

[Raphaël Badin (rvb)]

> Currently if this script fails then all of commissioning fails, which would allow the user to
> view the output where the exception would be outputted. The only way I can think around this is
> to remove disks as commands for that disk fail, which might be even more confusing for the user.

I'm a bit concerned about releasing code that might prevent the nodes from being commissioned at all if the parsing fails (given that the commands we run and the parsing we do are non-trivial). Now, I'm fine with releasing this as is as long as we preform extensive QA on this and test it on all the machines that we have: the lab, the garage MAAS, Jason's lab, OIL, etc. Can you please do this?

fwiw, I really think this ugly code could be best done with a regular expression but I'm not going to block this branch for it :)

+ info_line = info_line.strip()
+ if info_line == "":
+ continue
+ _, info = info_line.split(" ", 1)
+ if "=" not in info:
+ continue
+ k, v = info.split("=", 1)

review: Approve

[MAAS Lander (maas-lander)]

The attempt to merge lp:~blake-rouse/maas/commissioning-get-disk-info into lp:maas failed. Below is the output from the failed tests.

Ign http://security.ubuntu.com trusty-security InRelease
Get:1 http://security.ubuntu.com trusty-security Release.gpg [933 B]
Ign http://nova.clouds.archive.ubuntu.com trusty InRelease
Get:2 http://security.ubuntu.com trusty-security Release [62.0 kB]
Ign http://nova.clouds.archive.ubuntu.com trusty-updates InRelease
Hit http://nova.clouds.archive.ubuntu.com trusty Release.gpg
Get:3 http://nova.clouds.archive.ubuntu.com trusty-updates Release.gpg [933 B]
Hit http://nova.clouds.archive.ubuntu.com trusty Release
Get:4 http://nova.clouds.archive.ubuntu.com trusty-updates Release [62.0 kB]
Get:5 http://security.ubuntu.com trusty-security/main Sources [59.7 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty/main Sources
Get:6 http://security.ubuntu.com trusty-security/universe Sources [17.4 kB]
Get:7 http://security.ubuntu.com trusty-security/main amd64 Packages [187 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Sources
Hit http://nova.clouds.archive.ubuntu.com trusty/main amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty/universe amd64 Packages
Get:8 http://security.ubuntu.com trusty-security/universe amd64 Packages [82.2 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en
Get:9 http://nova.clouds.archive.ubuntu.com trusty-updates/main Sources [152 kB]
Hit http://security.ubuntu.com trusty-security/main Translation-en
Hit http://security.ubuntu.com trusty-security/universe Translation-en
Get:10 http://nova.clouds.archive.ubuntu.com trusty-updates/universe Sources [96.5 kB]
Get:11 http://nova.clouds.archive.ubuntu.com trusty-updates/main amd64 Packages [392 kB]
Get:12 http://nova.clouds.archive.ubuntu.com trusty-updates/universe amd64 Packages [236 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/universe Translation-en
Ign http://nova.clouds.archive.ubuntu.com trusty/main Translation-en_US
Ign http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en_US
Fetched 1,348 kB in 3s (441 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
     --no-install-recommends install apache2 authbind bind9 bind9utils build-essential bzr-builddeb curl daemontools debhelper dh-apport distro-info dnsutils firefox freeipmi-tools gjs ipython isc-dhcp-common libjs-raphael libjs-yui3-full libjs-yui3-min libpq-dev make pep8 postgresql pyflakes python-amqplib python-bzrlib python-celery python-convoy python-crochet python-cssselect python-curtin python-dev python-distro-info python-django python-django-piston python-django-south python-djorm-ext-pgarray python-docutils python-extras python-fixtures python-flake8 python-formencode python-hivex python-httplib2 python-jinja2 python-jsonschema python-lockfile python-lxml python-mimeparse python-mock python-netaddr python-netifaces python-nose python-oauth python-oops python-oops-amqp python-oops-datedir-repo python-oops-twisted python-oops-wsg...

=== modified file 'src/metadataserver/models/commissioningscript.py'
--- src/metadataserver/models/commissioningscript.py	2014-12-05 21:07:48 +0000
+++ src/metadataserver/models/commissioningscript.py	2015-01-09 14:32:14 +0000
@@ -43,6 +43,7 @@
     )
 from lxml import etree
 from maasserver.fields import MAC
+from maasserver.models.physicalblockdevice import PhysicalBlockDevice
 from maasserver.models.tag import Tag
 from metadataserver import DefaultMeta
 from metadataserver.enum import RESULT_TYPE
@@ -354,6 +355,122 @@
     'find /sys -name modalias -print0 | xargs -0 cat | sort -u'
 
 
+def gather_physical_block_devices(print_output=True):
+    """Gathers information about a nodes physical block devices.
+
+    The following commands are ran in order to gather the required information.
+
+    lsblk       Gathers the initial block devices not including slaves or
+                holders. Gets the name, read-only, removable, model, and
+                if rotary.
+
+    udevadm     Grabs the device path, serial number, and if connected over
+                SATA.
+
+    blockdev    Grabs the block size and size of the disk in bytes.
+
+    :param print_output: False will return the output instead of
+        printing. (Used only for testing.)
+    """
+    import shlex
+    from subprocess import check_output
+
+    # Grab the block devices from lsblk.
+    blockdevs = []
+    block_list = check_output(
+        ("lsblk", "-d", "-P", "-o", "NAME,RO,RM,MODEL,ROTA"))
+    for blockdev in block_list.splitlines():
+        tokens = shlex.split(blockdev)
+        current_block = {}
+        for token in tokens:
+            k, v = token.split("=", 1)
+            current_block[k] = v.strip()
+        blockdevs.append(current_block)
+
+    # Grab the device path, serial number, and sata connection.
+    UDEV_MAPPINGS = {
+        "DEVNAME": "PATH",
+        "ID_SERIAL_SHORT": "SERIAL",
+        "ID_ATA_SATA": "SATA",
+        }
+    del_blocks = []
+    for block_info in blockdevs:
+        # Some RAID devices return the name of the device seperated with "!",
+        # but udevadm expects it to be a "/".
+        block_name = block_info["NAME"].replace("!", "/")
+        udev_info = check_output(
+            ("udevadm", "info", "-q", "all", "-n", block_name))
+        for info_line in udev_info.splitlines():
+            info_line = info_line.strip()
+            if info_line == "":
+                continue
+            _, info = info_line.split(" ", 1)
+            if "=" not in info:
+                continue
+            k, v = info.split("=", 1)
+            if k in UDEV_MAPPINGS:
+                block_info[UDEV_MAPPINGS[k]] = v.strip()
+            if k == "ID_CDROM" and v == "1":
+                # Remove any type of CDROM from the blockdevs, as we
+                # cannot use this device for installation.
+                del_blocks.append(block_name)
+
+    # Remove any devices that need to be removed.
+    blockdevs = [
+        block_info
+        for block_info in blockdevs
+        if block_info["NAME"] not in del_blocks
+        ]
+
+    # Grab the size of the device and block size.
+    for block_info in blockdevs:
+        block_path = block_info["PATH"]
+        device_size = check_output(
+            ("blockdev", "--getsize64", block_path))
+        device_block_size = check_output(
+            ("blockdev", "--getbsz", block_path))
+        block_info["SIZE"] = device_size.strip()
+        block_info["BLOCK_SIZE"] = device_block_size.strip()
+
+    # Output block device information in json
+    json_output = json.dumps(blockdevs, indent=True)
+    if print_output:
+        print(json_output)
+    else:
+        return json_output
+
+
+def update_node_physical_block_devices(node, output, exit_status):
+    """Process the results of `gather_physical_block_devices`.
+
+    This updates the physical block devices that are attached to a node.
+
+    If `exit_status` is non-zero, this function returns without doing
+    anything.
+    """
+    assert isinstance(output, bytes)
+    if exit_status != 0:
+        return
+    blockdevs = json.loads(output)
+    PhysicalBlockDevice.objects.filter(node=node).delete()
+    for block_info in blockdevs:
+        # Skip the read-only devices. We keep them in the output for
+        # the user to view but they do not get an entry in the database.
+        if block_info["RO"] == "1":
+            continue
+        model = block_info.get("MODEL", "")
+        serial = block_info.get("SERIAL", "")
+        PhysicalBlockDevice.objects.create(
+            node=node,
+            name=block_info["NAME"],
+            path=block_info["PATH"],
+            size=long(block_info["SIZE"]),
+            block_size=int(block_info["BLOCK_SIZE"]),
+            model=model,
+            serial=serial,
+            )
+
+
 def null_hook(node, output, exit_status):
     """Intentionally do nothing.
 
@@ -401,6 +518,10 @@
         'content': make_function_call_script(dhcp_explore),
         'hook': null_hook,
     },
+    '00-maas-06-block-devices.out': {
+        'content': make_function_call_script(gather_physical_block_devices),
+        'hook': update_node_physical_block_devices,
+    },
     '99-maas-01-wait-for-lldpd.out': {
         'content': make_function_call_script(
             lldpd_wait, "/var/run/lldpd.socket", time_delay=60),

=== modified file 'src/metadataserver/models/tests/test_noderesults.py'
--- src/metadataserver/models/tests/test_noderesults.py	2014-12-05 21:07:48 +0000
+++ src/metadataserver/models/tests/test_noderesults.py	2015-01-09 14:32:14 +0000
@@ -17,11 +17,13 @@
 import doctest
 from inspect import getsource
 from io import BytesIO
+import json
 from math import (
     ceil,
     floor,
     )
 import os.path
+import random
 from random import randint
 import subprocess
 from subprocess import (
@@ -35,6 +37,7 @@
 
 from fixtures import FakeLogger
 from maasserver.fields import MAC
+from maasserver.models.physicalblockdevice import PhysicalBlockDevice
 from maasserver.models.tag import Tag
 from maasserver.testing.factory import factory
 from maasserver.testing.orm import reload_object
@@ -56,6 +59,7 @@
 from metadataserver.models.commissioningscript import (
     ARCHIVE_PREFIX,
     extract_router_mac_addresses,
+    gather_physical_block_devices,
     inject_lldp_result,
     inject_lshw_result,
     inject_result,
@@ -65,6 +69,7 @@
     set_node_routers,
     set_virtual_tag,
     update_hardware_details,
+    update_node_physical_block_devices,
     )
 from metadataserver.models.noderesult import NodeResult
 from mock import (
@@ -676,3 +681,344 @@
         logger = self.useFixture(FakeLogger(name='commissioningscript'))
         update_hardware_details(factory.make_Node(), b"garbage", exit_status=1)
         self.assertEqual("", logger.output)
+
+
+class TestGatherPhysicalBlockDevices(MAASServerTestCase):
+
+    def make_lsblk_output(
+            self, name=None, read_only=False, removable=False,
+            model=None, rotary=True):
+        if name is None:
+            name = factory.make_name('name')
+        if model is None:
+            model = factory.make_name('model')
+        read_only = "1" if read_only else "0"
+        removable = "1" if removable else "0"
+        rotary = "1" if rotary else "0"
+        return 'NAME="%s" RO="%s" RM="%s" MODEL="%s" ROTA="%s"' % (
+            name, read_only, removable, model, rotary)
+
+    def make_udevadm_output(self, name, serial=None, sata=True, cdrom=False):
+        if serial is None:
+            serial = factory.make_name('serial')
+        sata = "1" if sata else "0"
+        output = dedent("""\
+            P: /devices/pci0000:00/ata3/host2/target2:0:0/2:0:0:0/block/{name}
+            N: {name}
+            E: DEVNAME=/dev/{name}
+            E: DEVTYPE=disk
+            E: ID_ATA_SATA={sata}
+            E: ID_SERIAL_SHORT={serial}
+            """).format(name=name, serial=serial, sata=sata)
+        if cdrom:
+            output += "E: ID_CDROM=1"
+        return output
+
+    def call_gather_physical_block_devices(self):
+        return json.loads(gather_physical_block_devices(print_output=False))
+
+    def test__calls_lsblk(self):
+        check_output = self.patch(subprocess, "check_output")
+        check_output.return_value = ""
+        self.call_gather_physical_block_devices()
+        self.assertThat(check_output, MockCalledOnceWith(
+            ("lsblk", "-d", "-P", "-o", "NAME,RO,RM,MODEL,ROTA")))
+
+    def test__returns_empty_list_when_no_disks(self):
+        check_output = self.patch(subprocess, "check_output")
+        check_output.return_value = ""
+        self.assertEquals([], self.call_gather_physical_block_devices())
+
+    def test__calls_lsblk_then_udevadm(self):
+        name = factory.make_name('name')
+        check_output = self.patch(subprocess, "check_output")
+        check_output.side_effect = [
+            self.make_lsblk_output(
+                name=name),
+            self.make_udevadm_output(
+                name, cdrom=True),
+            ]
+        self.call_gather_physical_block_devices()
+        self.assertThat(check_output, MockCallsMatch(
+            call(("lsblk", "-d", "-P", "-o", "NAME,RO,RM,MODEL,ROTA")),
+            call(("udevadm", "info", "-q", "all", "-n", name))))
+
+    def test__returns_empty_list_when_cdrom_only(self):
+        name = factory.make_name('name')
+        check_output = self.patch(subprocess, "check_output")
+        check_output.side_effect = [
+            self.make_lsblk_output(
+                name=name),
+            self.make_udevadm_output(
+                name, cdrom=True),
+            ]
+        self.assertEquals([], self.call_gather_physical_block_devices())
+
+    def test__calls_lsblk_udevadm_then_blockdev(self):
+        name = factory.make_name('name')
+        model = factory.make_name('model')
+        serial = factory.make_name('serial')
+        size = random.randint(1000 * 1000, 1000 * 1000 * 1000)
+        block_size = random.choice([512, 1024, 4096])
+        check_output = self.patch(subprocess, "check_output")
+        check_output.side_effect = [
+            self.make_lsblk_output(name=name, model=model),
+            self.make_udevadm_output(name, serial=serial),
+            '%s' % size,
+            '%s' % block_size,
+            ]
+        self.call_gather_physical_block_devices()
+        self.assertThat(check_output, MockCallsMatch(
+            call(("lsblk", "-d", "-P", "-o", "NAME,RO,RM,MODEL,ROTA")),
+            call(("udevadm", "info", "-q", "all", "-n", name)),
+            call(("blockdev", "--getsize64", "/dev/%s" % name)),
+            call(("blockdev", "--getbsz", "/dev/%s" % name))))
+
+    def test__returns_block_device(self):
+        name = factory.make_name('name')
+        model = factory.make_name('model')
+        serial = factory.make_name('serial')
+        size = random.randint(1000 * 1000, 1000 * 1000 * 1000)
+        block_size = random.choice([512, 1024, 4096])
+        check_output = self.patch(subprocess, "check_output")
+        check_output.side_effect = [
+            self.make_lsblk_output(name=name, model=model),
+            self.make_udevadm_output(name, serial=serial),
+            '%s' % size,
+            '%s' % block_size,
+            ]
+        self.assertEquals([{
+            "NAME": name,
+            "PATH": "/dev/%s" % name,
+            "RO": "0",
+            "RM": "0",
+            "MODEL": model,
+            "ROTA": "1",
+            "SATA": "1",
+            "SERIAL": serial,
+            "SIZE": "%s" % size,
+            "BLOCK_SIZE": "%s" % block_size,
+            }], self.call_gather_physical_block_devices())
+
+    def test__returns_block_device_readonly(self):
+        name = factory.make_name('name')
+        model = factory.make_name('model')
+        serial = factory.make_name('serial')
+        size = random.randint(1000 * 1000, 1000 * 1000 * 1000)
+        block_size = random.choice([512, 1024, 4096])
+        check_output = self.patch(subprocess, "check_output")
+        check_output.side_effect = [
+            self.make_lsblk_output(name=name, model=model, read_only=True),
+            self.make_udevadm_output(name, serial=serial),
+            '%s' % size,
+            '%s' % block_size,
+            ]
+        self.assertEquals([{
+            "NAME": name,
+            "PATH": "/dev/%s" % name,
+            "RO": "1",
+            "RM": "0",
+            "MODEL": model,
+            "ROTA": "1",
+            "SATA": "1",
+            "SERIAL": serial,
+            "SIZE": "%s" % size,
+            "BLOCK_SIZE": "%s" % block_size,
+            }], self.call_gather_physical_block_devices())
+
+    def test__returns_block_device_ssd(self):
+        name = factory.make_name('name')
+        model = factory.make_name('model')
+        serial = factory.make_name('serial')
+        size = random.randint(1000 * 1000, 1000 * 1000 * 1000)
+        block_size = random.choice([512, 1024, 4096])
+        check_output = self.patch(subprocess, "check_output")
+        check_output.side_effect = [
+            self.make_lsblk_output(name=name, model=model, rotary=False),
+            self.make_udevadm_output(name, serial=serial),
+            '%s' % size,
+            '%s' % block_size,
+            ]
+        self.assertEquals([{
+            "NAME": name,
+            "PATH": "/dev/%s" % name,
+            "RO": "0",
+            "RM": "0",
+            "MODEL": model,
+            "ROTA": "0",
+            "SATA": "1",
+            "SERIAL": serial,
+            "SIZE": "%s" % size,
+            "BLOCK_SIZE": "%s" % block_size,
+            }], self.call_gather_physical_block_devices())
+
+    def test__returns_block_device_not_sata(self):
+        name = factory.make_name('name')
+        model = factory.make_name('model')
+        serial = factory.make_name('serial')
+        size = random.randint(1000 * 1000, 1000 * 1000 * 1000)
+        block_size = random.choice([512, 1024, 4096])
+        check_output = self.patch(subprocess, "check_output")
+        check_output.side_effect = [
+            self.make_lsblk_output(name=name, model=model),
+            self.make_udevadm_output(name, serial=serial, sata=False),
+            '%s' % size,
+            '%s' % block_size,
+            ]
+        self.assertEquals([{
+            "NAME": name,
+            "PATH": "/dev/%s" % name,
+            "RO": "0",
+            "RM": "0",
+            "MODEL": model,
+            "ROTA": "1",
+            "SATA": "0",
+            "SERIAL": serial,
+            "SIZE": "%s" % size,
+            "BLOCK_SIZE": "%s" % block_size,
+            }], self.call_gather_physical_block_devices())
+
+    def test__returns_block_device_removable(self):
+        name = factory.make_name('name')
+        model = factory.make_name('model')
+        serial = factory.make_name('serial')
+        size = random.randint(1000 * 1000, 1000 * 1000 * 1000)
+        block_size = random.choice([512, 1024, 4096])
+        check_output = self.patch(subprocess, "check_output")
+        check_output.side_effect = [
+            self.make_lsblk_output(name=name, model=model, removable=True),
+            self.make_udevadm_output(name, serial=serial),
+            '%s' % size,
+            '%s' % block_size,
+            ]
+        self.assertEquals([{
+            "NAME": name,
+            "PATH": "/dev/%s" % name,
+            "RO": "0",
+            "RM": "1",
+            "MODEL": model,
+            "ROTA": "1",
+            "SATA": "1",
+            "SERIAL": serial,
+            "SIZE": "%s" % size,
+            "BLOCK_SIZE": "%s" % block_size,
+            }], self.call_gather_physical_block_devices())
+
+    def test__returns_multiple_block_devices_in_order(self):
+        names = [factory.make_name('name') for _ in range(3)]
+        lsblk = [
+            self.make_lsblk_output(name=name)
+            for name in names
+            ]
+        call_outputs = []
+        call_outputs.append("\n".join(lsblk))
+        for name in names:
+            call_outputs.append(self.make_udevadm_output(name))
+        for name in names:
+            call_outputs.append(
+                "%s" % random.randint(1000 * 1000, 1000 * 1000 * 1000))
+            call_outputs.append(
+                "%s" % random.choice([512, 1024, 4096]))
+        check_output = self.patch(subprocess, "check_output")
+        check_output.side_effect = call_outputs
+        device_names = [
+            block_info['NAME']
+            for block_info in self.call_gather_physical_block_devices()
+            ]
+        self.assertEquals(names, device_names)
+
+
+class TestUpdateNodePhysicalBlockDevices(MAASServerTestCase):
+
+    def make_block_device(
+            self, name=None, path=None, size=None, block_size=None,
+            model=None, serial=None):
+        if name is None:
+            name = factory.make_name('name')
+        if path is None:
+            path = '/dev/%s' % name
+        if size is None:
+            size = random.randint(1000 * 1000, 1000 * 1000 * 1000)
+        if block_size is None:
+            block_size = random.choice([512, 1024, 4096])
+        if model is None:
+            model = factory.make_name('model')
+        if serial is None:
+            serial = factory.make_name('serial')
+        return {
+            "NAME": name,
+            "PATH": path,
+            "SIZE": '%s' % size,
+            "BLOCK_SIZE": '%s' % block_size,
+            "MODEL": model,
+            "SERIAL": serial,
+            "RO": "0",
+            "RM": "0",
+            "ROTA": "1",
+            }
+
+    def test__does_nothing_when_exit_status_is_not_zero(self):
+        node = factory.make_Node()
+        block_device = factory.make_PhysicalBlockDevice(node=node)
+        update_node_physical_block_devices(node, b"garbage", exit_status=1)
+        self.assertIsNotNone(reload_object(block_device))
+
+    def test__clears_previous_physical_block_devices(self):
+        node = factory.make_Node()
+        block_device = factory.make_PhysicalBlockDevice(node=node)
+        update_node_physical_block_devices(node, b"[]", 0)
+        self.assertIsNone(reload_object(block_device))
+
+    def test__creates_physical_block_devices(self):
+        devices = [self.make_block_device() for _ in range(3)]
+        device_names = [device['NAME'] for device in devices]
+        node = factory.make_Node()
+        json_output = json.dumps(devices).encode('utf-8')
+        update_node_physical_block_devices(node, json_output, 0)
+        created_names = [
+            device.name
+            for device in PhysicalBlockDevice.objects.filter(node=node)
+            ]
+        self.assertItemsEqual(device_names, created_names)
+
+    def test__creates_physical_block_devices_in_order(self):
+        devices = [self.make_block_device() for _ in range(3)]
+        device_names = [device['NAME'] for device in devices]
+        node = factory.make_Node()
+        json_output = json.dumps(devices).encode('utf-8')
+        update_node_physical_block_devices(node, json_output, 0)
+        created_names = [
+            device.name
+            for device in (
+                PhysicalBlockDevice.objects.filter(node=node).order_by('id'))
+            ]
+        self.assertEquals(device_names, created_names)
+
+    def test__creates_physical_block_device(self):
+        name = factory.make_name('name')
+        path = '/dev/%s' % name
+        size = random.randint(1000 * 1000, 1000 * 1000 * 1000)
+        block_size = random.choice([512, 1024, 4096])
+        model = factory.make_name('model')
+        serial = factory.make_name('serial')
+        device = self.make_block_device(
+            name=name, path=path, size=size, block_size=block_size,
+            model=model, serial=serial)
+        node = factory.make_Node()
+        json_output = json.dumps([device]).encode('utf-8')
+        update_node_physical_block_devices(node, json_output, 0)
+        self.assertThat(
+            PhysicalBlockDevice.objects.filter(node=node).first(),
+            MatchesStructure.byEquality(
+                name=name, path=path, size=size, block_size=block_size,
+                model=model, serial=serial))
+
+    def test__creates_physical_block_device_only_for_node(self):
+        device = self.make_block_device()
+        node = factory.make_Node()
+        other_node = factory.make_Node()
+        json_output = json.dumps([device]).encode('utf-8')
+        update_node_physical_block_devices(node, json_output, 0)
+        self.assertEquals(
+            0, PhysicalBlockDevice.objects.filter(node=other_node).count(),
+            "Created physical block device for the incorrect node.")