Merge lp:~gz/juju-ci-tools/endpoints_bindings into lp:juju-ci-tools

[Martin Packman]

Work in progress on making endpoint bindings test that can be run on shared maas

Basic outline is that the networking parts are set up and torn down using the underlying maas commands each run, and that machines that need interfaces reconfigured are constrained from being used on other maas jobs. There are still some details here that need discussing in more depth, but the basic outline here should provide a start.

I have chopped out most of the test complexity that I was adding locally for ease of review. Also, had to make some changes to how the network configuration was done in light of how finfolk is actually set up, so for now have deleted the tests based on real data for the maas configuration, will update with the new behaviour and re-add.

There's a bunch more to talk about here as well, happy to answer any questions.

[Curtis Hovey (sinzui)]

Thank you. I have some advice, questions, and comments inline. Nothing blocks this branch from merging.

review: Approve (code)

[Martin Packman (gz)]

Thanks for all the comments, have replied in line and with push some changes.

[lp:~gz/juju-ci-tools/endpoints_bindings]

1659. By Martin Packman on 2016-10-21

Address review comments from sinzui and use persistent spaces

1660. By Martin Packman on 2016-10-21

Give gateway_ip when creating subnets

=== added file 'assess_endpoint_bindings.py'
--- assess_endpoint_bindings.py	1970-01-01 00:00:00 +0000
+++ assess_endpoint_bindings.py	2016-10-21 22:18:30 +0000
@@ -0,0 +1,344 @@
+#!/usr/bin/env python
+"""Validate endpoint bindings functionality on MAAS."""
+
+from __future__ import print_function
+
+import argparse
+import contextlib
+import logging
+import os
+import sys
+import yaml
+
+from deploy_stack import (
+    BootstrapManager,
+)
+from jujucharm import (
+    Charm,
+)
+from substrate import (
+    maas_account_from_config,
+)
+from utility import (
+    add_basic_testing_arguments,
+    configure_logging,
+    temp_dir,
+)
+
+
+log = logging.getLogger("assess_endpoint_bindings")
+
+
+script_identifier = "endpoint-bindings"
+
+# To avoid clashes with other tests these space names must be seperately
+# registered in jujupy to populate constraints.
+space_control = script_identifier + "-control"
+space_data = script_identifier + "-data"
+space_public = script_identifier + "-public"
+
+
+def _generate_vids(start=10):
+    """
+    Generate a series of vid values beginning with start.
+
+    Ideally these values would be carefully chosen to not clash with existing
+    vlans, but for now just hardcode.
+    """
+    for vid in range(start, 4096):
+        yield vid
+
+
+def _generate_cidrs(start=40, inc=10, block_pattern="10.0.{}.0/24"):
+    """
+    Generate a series of cidrs based on block_pattern beginning with start.
+
+    Would be good not to hardcode but inspecting network for free ranges is
+    also non-trivial.
+    """
+    for n in range(start, 255, inc):
+        yield block_pattern.format(n)
+
+
+def ensure_spaces(manager, required_spaces):
+    """Return details for each given required_spaces creating spaces as needed.
+
+    :param manager: MAAS account manager.
+    :param required_spaces: List of space names that may need to be created.
+    """
+    existing_spaces = manager.spaces()
+    log.info("Have spaces: %s", ", ".join(s["name"] for s in existing_spaces))
+    spaces_map = dict((s["name"], s) for s in existing_spaces)
+    spaces = []
+    for space_name in required_spaces:
+        space = spaces_map.get(space_name)
+        if space is None:
+            space = manager.create_space(space_name)
+            log.info("Created space: %r", space)
+        spaces.append(space)
+    return spaces
+
+
[e-mail]
+def reconfigure_networking(manager, required_spaces):
+    """Create new MAAS networking primitives to prepare for testing.
+
+    :param manager: MAAS account manager.
+    :param required_spaces: List of spaces to make with vlans and subnets.
+    """
+    new_subnets = []
+    new_vlans = []
+    fabrics = manager.fabrics()
+    log.info("Have fabrics: %s", ", ".join(f["name"] for f in fabrics))
+    new_fabric = manager.create_fabric(script_identifier)
+    try:
+        log.info("Created fabric: %r", new_fabric)
+
+        spaces = ensure_spaces(manager, required_spaces)
+
+        for vid, space_name in zip(_generate_vids(), required_spaces):
+            name = space_name + "-vlan"
+            new_vlans.append(manager.create_vlan(new_fabric["id"], vid, name))
+            log.info("Created vlan: %r", new_vlans[-1])
+
+        for cidr, vlan, space in zip(_generate_cidrs(), new_vlans, spaces):
+            new_subnets.append(manager.create_subnet(
+                cidr, fabric_id=new_fabric["id"], vlan_id=vlan["id"],
+                space=space["id"], gateway_ip=cidr.replace(".0/24", ".1")))
+            log.info("Created subnet: %r", new_subnets[-1])
+
+        yield new_fabric, spaces, list(new_vlans), list(new_subnets)
+
+    finally:
+        for subnet in new_subnets:
+            manager.delete_subnet(subnet["id"])
+            log.info("Deleted subnet: %s", subnet["name"])
+
+        for vlan in new_vlans:
+            manager.delete_vlan(new_fabric["id"], vlan["vid"])
+            log.info("Deleted vlan: %s", vlan["name"])
+
+        try:
+            manager.delete_fabric(new_fabric["id"])
+        except Exception:
+            log.exception("Failed to delete fabric: %s", new_fabric["id"])
+            raise
+        else:
+            log.info("Deleted fabric: %s", new_fabric["id"])
+
+
[e-mail]
+def reconfigure_machines(manager, fabric, required_machine_subnets):
+    """
+    Reconfigure MAAS machines with new interfaces to prepare for testing.
+
+    There are some unavoidable races if multiple jobs attempt to reconfigure
+    machines at the same time. Also, in heterogenous environments an
+    inadequate machine may be reserved at this time.
+
+    Ideally this function would just allocate some machines before operating
+    on them. Alas, MAAS doesn't allow interface changes on allocated machines
+    and Juju will not select them for deployment.
+
+    :param manager: MAAS account manager.
+    :param fabric: Data from MAAS about the fabric to be used.
+    :param required_machine_subnets: List of list of vlan and subnet ids.
+    """
+
+    # Find all machines not currently being used
+    all_machines = manager.machines()
+    candidate_machines = [
+        m for m in all_machines if m["status"] == manager.STATUS_READY]
+    # Take the id of the default vlan on the new fabric
+    default_vlan = fabric["vlans"][0]["id"]
+
+    configured_machines = []
+    machine_interfaces = {}
+    try:
+        for machine_subnets in required_machine_subnets:
+            if not candidate_machines:
+                raise Exception("No ready maas machines to configure")
+
+            machine = candidate_machines.pop()
+            system_id = machine["system_id"]
+            # TODO(gz): Add logic to pick sane parent?
+            existing_interface = [
+                interface for interface in machine["interface_set"]
+                if not any("subnet" in link for link in interface["links"])
+                ][0]
+            previous_vlan_id = existing_interface["vlan"]["id"]
+            new_interfaces = []
+            machine_interfaces[system_id] = (
+                existing_interface, previous_vlan_id, new_interfaces)
+            manager.interface_update(
+                system_id, existing_interface["id"], vlan_id=default_vlan)
+            log.info("Changed existing interface: %s %s", system_id,
+                     existing_interface["name"])
+            parent = existing_interface["id"]
+
+            for vlan_id, subnet_id in machine_subnets:
+                links = []
+                interface = manager.interface_create_vlan(
+                    system_id, parent, vlan_id)
+                new_interfaces.append(interface)
+                log.info("Created interface: %r", interface)
+
+                updated_subnet = manager.interface_link_subnet(
+                    system_id, interface["id"], "AUTO", subnet_id)
+                # TODO(gz): Need to pick out right link if multiple are added.
+                links.append(updated_subnet["links"][0])
+                log.info("Created link: %r", links[-1])
+
+            configured_machines.append(machine)
+        yield configured_machines
+    finally:
+        log.info("About to reset machine interfaces to original states.")
+        for system_id in machine_interfaces:
+            parent, vlan, children = machine_interfaces[system_id]
+            for child in children:
+                manager.delete_interface(system_id, child["id"])
+                log.info("Deleted interface: %s %s", system_id, child["id"])
+            manager.interface_update(system_id, parent["id"], vlan_id=vlan)
+            log.info("Reset original interface: %s", parent["name"])
+
+
+def create_test_charms():
+    """Create charms for testing and bundle using them."""
+    charm_datastore = Charm("datastore", "Testing datastore charm.")
+    charm_datastore.metadata["provides"] = {
+        "datastore": {"interface": "data"},
+    }
+
+    charm_frontend = Charm("frontend", "Testing frontend charm.")
+    charm_frontend.metadata["provides"] = {
+        "website": {"interface": "http"},
+    }
+    charm_frontend.metadata["requires"] = {
+        "datastore": {"interface": "data"},
+    }
+
+    bundle = {
+        "services": {
+            "datastore": {
+                "charm": "./xenial/datastore",
+                "series": "xenial",
+                "num-units": 1,
+                "bindings": {
+                    "datastore": space_data,
+                },
+            },
+            "frontend": {
+                "charm": "./xenial/frontend",
+                "series": "xenial",
+                "num-units": 1,
+                "bindings": {
+                    "website": space_public,
+                    "datastore": space_data,
+                },
+            },
+        },
+        "relations": [
+            ["datastore:datastore", "frontend:datastore"],
+        ],
+    }
+    return bundle, [charm_datastore, charm_frontend]
+
+
[e-mail]
+def using_bundle_and_charms(bundle, charms, bundle_name="bundle.yaml"):
+    """Commit bundle and charms to disk and gives path to bundle."""
+    with temp_dir() as working_dir:
+        for charm in charms:
+            charm.to_repo_dir(working_dir)
+
+        # TODO(gz): Create a bundle abstration in jujucharm module
+        bundle_path = os.path.join(working_dir, bundle_name)
+        with open(bundle_path, "w") as f:
+            yaml.safe_dump(bundle, f)
+
+        yield bundle_path
+
+
+def machine_spaces_for_bundle(bundle):
+    """Return a list of sets of spaces required for machines in bundle."""
+    machines = []
+    for service in bundle["services"].values():
+        spaces = frozenset(service.get("bindings", {}).values())
+        num_units = service.get("num_units", 1)
+        machines.extend([spaces] * num_units)
+    return machines
+
+
+def bootstrap_and_test(bootstrap_manager, bundle_path, machine):
+    with bootstrap_manager.booted_context(False, to=machine):
+        client = bootstrap_manager.client
+        log.info("Deploying bundle.")
+        client.deploy(bundle_path)
+        log.info("Waiting for all units to start.")
+        client.wait_for_started()
+        client.wait_for_workloads()
+        log.info("Validating bindings.")
+        validate(client)
+
+
+def validate(client):
+    """Ensure relations are bound to the correct spaces."""
+
+
+def assess_endpoint_bindings(maas_manager, bootstrap_manager):
+    required_spaces = [space_data, space_public]
+
+    bundle, charms = create_test_charms()
+
+    machine_spaces = machine_spaces_for_bundle(bundle)
+    # Add a bootstrap machine in all spaces
+    machine_spaces.insert(0, frozenset().union(*machine_spaces))
+
+    log.info("About to write charms to disk.")
+    with using_bundle_and_charms(bundle, charms) as bundle_path:
+        log.info("About to reconfigure maas networking.")
+        with reconfigure_networking(maas_manager, required_spaces) as nets:
+
+            fabric, spaces, vlans, subnets = nets
+            # Derive the vlans and subnets that need to be added to machines
+            vlan_subnet_per_machine = []
+            for spaces in machine_spaces:
+                idxs = sorted(required_spaces.index(space) for space in spaces)
+                vlans_subnets = [
+                    (vlans[i]["id"], subnets[i]["id"]) for i in idxs]
+                vlan_subnet_per_machine.append(vlans_subnets)
+
+            log.info("About to add new interfaces to machines.")
+            with reconfigure_machines(
+                    maas_manager, fabric, vlan_subnet_per_machine) as machines:
+
+                bootstrap_manager.client.use_reserved_spaces(required_spaces)
+
+                base_machine = machines[0]["hostname"]
+
+                log.info("About to bootstrap.")
+                bootstrap_and_test(
+                    bootstrap_manager, bundle_path, base_machine)
+
+
+def parse_args(argv):
+    """Parse all arguments."""
+    parser = argparse.ArgumentParser(description="assess endpoint bindings")
+    add_basic_testing_arguments(parser)
+    args = parser.parse_args(argv)
+    if args.upload_tools:
+        parser.error("giving --upload-tools meaningless on 2.0 only test")
+    return args
+
+
+def main(argv=None):
+    args = parse_args(argv)
+    configure_logging(args.verbose)
+    bs_manager = BootstrapManager.from_args(args)
+    with maas_account_from_config(bs_manager.client.env.config) as account:
+        assess_endpoint_bindings(account, bs_manager)
+    return 0
+
+
+if __name__ == '__main__':
+    sys.exit(main())

=== modified file 'jujupy.py'
--- jujupy.py	2016-10-21 19:50:45 +0000
+++ jujupy.py	2016-10-21 22:18:30 +0000
@@ -940,6 +940,9 @@
 
     controller_permissions = frozenset(['login', 'addmodel', 'superuser'])
 
+    reserved_spaces = frozenset([
+        'endpoint-bindings-data', 'endpoint-bindings-public'])
+
     @classmethod
     def preferred_container(cls):
         for container_type in [LXD_MACHINE, LXC_MACHINE]:
@@ -1032,6 +1035,7 @@
         feature_flags = self.feature_flags.intersection(cls.used_feature_flags)
         backend = self._backend.clone(full_path, version, debug, feature_flags)
         other = cls.from_backend(backend, env)
+        other.excluded_spaces = set(self.excluded_spaces)
         return other
 
     @classmethod
@@ -1108,6 +1112,7 @@
                     env.juju_home = get_juju_home()
             else:
                 env.juju_home = juju_home
+        self.excluded_spaces = set(self.reserved_spaces)
 
     @property
     def version(self):
@@ -1141,6 +1146,12 @@
         return self._backend.shell_environ(self.used_feature_flags,
                                            self.env.juju_home)
 
+    def use_reserved_spaces(self, spaces):
+        """Allow machines in given spaces to be allocated and used."""
+        if not self.reserved_spaces.issuperset(spaces):
+            raise ValueError('Space not reserved: {}'.format(spaces))
+        self.excluded_spaces.difference_update(spaces)
+
     def add_ssh_machines(self, machines):
         for count, machine in enumerate(machines):
             try:
@@ -1163,11 +1174,7 @@
             credential=None, auto_upgrade=False, metadata_source=None,
             to=None, no_gui=False, agent_version=None):
         """Return the bootstrap arguments for the substrate."""
-        if self.env.joyent:
-            # Only accept kvm packages by requiring >1 cpu core, see lp:1446264
-            constraints = 'mem=2G cpu-cores=1'
-        else:
-            constraints = 'mem=2G'
+        constraints = self._get_substrate_constraints()
         cloud_region = self.get_cloud_region(self.env.get_cloud(),
                                              self.env.get_region())
         # Note cloud_region before controller name
@@ -1523,6 +1530,10 @@
         if self.env.joyent:
             # Only accept kvm packages by requiring >1 cpu core, see lp:1446264
             return 'mem=2G cpu-cores=1'
+        elif self.env.maas:
+            # For now only maas support spaces in a meaningful way.
+            return 'mem=2G spaces={}'.format( ','.join(
+                '^' + space for space in sorted(self.excluded_spaces)))
         else:
             return 'mem=2G'
 
@@ -2559,6 +2570,13 @@
         else:
             return unqualified_model_name(self.model_name)
 
+    def _get_substrate_constraints(self):
+        if self.env.joyent:
+            # Only accept kvm packages by requiring >1 cpu core, see lp:1446264
+            return 'mem=2G cpu-cores=1'
+        else:
+            return 'mem=2G'
+
     def get_bootstrap_args(self, upload_tools, bootstrap_series=None,
                            credential=None):
         """Return the bootstrap arguments for the substrate."""

=== modified file 'substrate.py'
--- substrate.py	2016-09-28 12:51:00 +0000
+++ substrate.py	2016-10-21 22:18:30 +0000
@@ -440,6 +440,8 @@
 
     _API_PATH = 'api/2.0/'
 
+    STATUS_READY = 4
+
     SUBNET_CONNECTION_MODES = frozenset(('AUTO', 'DHCP', 'STATIC', 'LINK_UP'))
 
     def __init__(self, profile, url, oauth):
@@ -493,6 +495,10 @@
                if v['ip_addresses']}
         return ips
 
+    def machines(self):
+        """Return list of all machines."""
+        return self._maas(self.profile, 'machines', 'read')
+
     def fabrics(self):
         """Return list of all fabrics."""
         return self._maas(self.profile, 'fabrics', 'read')
@@ -538,6 +544,22 @@
         """Return list of interfaces belonging to node with given system_id."""
         return self._maas(self.profile, 'interfaces', 'read', system_id)
 
+    def interface_update(self, system_id, interface_id, name=None,
+                         mac_address=None, tags=None, vlan_id=None):
+        """Update fields of existing interface on node with given system_id."""
+        args = [
+            self.profile, 'interface', 'update', system_id, str(interface_id),
+        ]
+        if name is not None:
+            args.append('name=' + name)
+        if mac_address is not None:
+            args.append('mac_address=' + mac_address)
+        if tags is not None:
+            args.append('tags=' + tags)
+        if vlan_id is not None:
+            args.append('vlan=' + str(vlan_id))
+        return self._maas(*args)
+
     def interface_create_vlan(self, system_id, parent, vlan_id):
         """Create a vlan interface on machine with given system_id."""
         args = [

=== added file 'tests/test_assess_endpoint_bindings.py'
--- tests/test_assess_endpoint_bindings.py	1970-01-01 00:00:00 +0000
+++ tests/test_assess_endpoint_bindings.py	2016-10-21 22:18:30 +0000
@@ -0,0 +1,177 @@
+"""Tests for assess_endpoint_bindings module."""
+
+import logging
+from mock import Mock, patch
+import StringIO
+
+from assess_endpoint_bindings import (
+    assess_endpoint_bindings,
+    ensure_spaces,
+    parse_args,
+    machine_spaces_for_bundle,
+    main,
+)
+from tests import (
+    parse_error,
+    TestCase,
+)
+from tests.test_jujupy import fake_juju_client
+
+
+class TestParseArgs(TestCase):
+
+    def test_common_args(self):
+        args = parse_args(["an-env", "/bin/juju", "/tmp/logs", "an-env-mod"])
+        self.assertEqual("an-env", args.env)
+        self.assertEqual("/bin/juju", args.juju_bin)
+        self.assertEqual("/tmp/logs", args.logs)
+        self.assertEqual("an-env-mod", args.temp_env_name)
+        self.assertEqual(False, args.debug)
+
+    def test_no_upload_tools(self):
+        with parse_error(self) as fake_stderr:
+            parse_args(["an-env", "/bin/juju", "--upload-tools"])
+        self.assertIn(
+            "error: giving --upload-tools meaningless on 2.0 only test",
+            fake_stderr.getvalue())
+
+    def test_help(self):
+        fake_stdout = StringIO.StringIO()
+        with parse_error(self) as fake_stderr:
+            with patch("sys.stdout", fake_stdout):
+                parse_args(["--help"])
+        self.assertEqual("", fake_stderr.getvalue())
+        self.assertIn("endpoint bindings", fake_stdout.getvalue())
+
+
+class TestEnsureSpaces(TestCase):
+
+    default_space = {
+        "name": "Default space",
+        "id": 0,
+        "resource_uri": "/MAAS/api/2.0/spaces/0/",
+        "subnets": [
+            {
+                "space": "Default space",
+                "id": 2,
+            },
+        ],
+    }
+    alpha_space = {
+        "name": "alpha-space",
+        "id": 60,
+        "resource_uri": "/MAAS/api/2.0/spaces/60/",
+        "subnets": [],
+    }
+    beta_space = {
+        "name": "beta-space",
+        "id": 61,
+        "resource_uri": "/MAAS/api/2.0/spaces/61/",
+        "subnets": [],
+    }
+
+    def test_all_existing(self):
+        manager = Mock(spec=["spaces"])
+        manager.spaces.return_value = [self.default_space, self.alpha_space]
+        spaces = ensure_spaces(manager, ["alpha-space"])
+        self.assertEqual(spaces, [self.alpha_space])
+        manager.spaces.assert_called_once_with()
+        self.assertEqual(
+            "INFO Have spaces: Default space, alpha-space\n",
+            self.log_stream.getvalue())
+
+    def test_some_existing(self):
+        manager = Mock(spec=["create_space", "spaces"])
+        manager.create_space.return_value = self.alpha_space
+        manager.spaces.return_value = [self.default_space, self.beta_space]
+        spaces = ensure_spaces(manager, ["alpha-space", "beta-space"])
+        self.assertEqual(spaces, [self.alpha_space, self.beta_space])
+        manager.spaces.assert_called_once_with()
+        manager.create_space.assert_called_once_with("alpha-space")
+        self.assertRegexpMatches(
+            self.log_stream.getvalue(),
+            r"^INFO Have spaces: Default space, beta-space\n"
+            r"INFO Created space: \{.*\}\n$")
+
+
+class TestMachineSpacesForBundle(TestCase):
+
+    def test_no_bindings(self):
+        bundle_without_bindings = {
+            "services": {
+                "ubuntu": {
+                    "charm": "cs:ubuntu",
+                    "num_units": 3,
+                },
+            },
+        }
+        machines = machine_spaces_for_bundle(bundle_without_bindings)
+        self.assertEqual(machines, [frozenset(), frozenset(), frozenset()])
+
+    def test_single_binding(self):
+        bundle_without_bindings = {
+            "services": {
+                "anapp": {
+                    "charm": "./anapp",
+                    "series": "xenial",
+                    "num_units": 1,
+                    "bindings": {
+                        "website": "space-public",
+                    },
+                },
+            },
+        }
+        machines = machine_spaces_for_bundle(bundle_without_bindings)
+        self.assertEqual(machines, [frozenset(["space-public"])])
+
+    def test_multiple_bindings(self):
+        bundle_without_bindings = {
+            "services": {
+                "anapp": {
+                    "charm": "./anapp",
+                    "series": "xenial",
+                    "num_units": 1,
+                    "bindings": {
+                        "website": "space-public",
+                        "data": "space-data",
+                        "monitoring": "space-ctl",
+                    },
+                },
+                "adb": {
+                    "charm": "./adb",
+                    "series": "xenial",
+                    "num_units": 2,
+                    "bindings": {
+                        "data": "space-data",
+                        "monitoring": "space-ctl",
+                    },
+                },
+            },
+        }
+        machines = machine_spaces_for_bundle(bundle_without_bindings)
+        app_spaces = frozenset(["space-data", "space-ctl", "space-public"])
+        db_spaces = frozenset(["space-data", "space-ctl"])
+        self.assertEqual(machines, [app_spaces, db_spaces, db_spaces])
+
+
+class TestMain(TestCase):
+
+    def test_main(self):
+        argv = ["an-env", "/bin/juju", "/tmp/logs", "an-env-mod", "--verbose"]
+        env = Mock(spec=["config"])
+        client = Mock(spec=["env", "is_jes_enabled"])
+        client.env = env
+        with patch("assess_endpoint_bindings.configure_logging",
+                   autospec=True) as mock_cl:
+            with patch("assess_endpoint_bindings.maas_account_from_config",
+                       autospec=True) as mock_ma:
+                with patch("deploy_stack.client_from_config",
+                           return_value=client) as mock_c:
+                    with patch("assess_endpoint_bindings.assess_endpoint_bindings",
+                               autospec=True) as mock_assess:
+                        main(argv)
+        mock_cl.assert_called_once_with(logging.DEBUG)
+        mock_c.assert_called_once_with(
+            "an-env", "/bin/juju", debug=False, soft_deadline=None)
+        self.assertEqual(mock_ma.call_count, 1)
+        self.assertEqual(mock_assess.call_count, 1)

=== modified file 'tests/test_jujupy.py'
--- tests/test_jujupy.py	2016-10-21 20:02:20 +0000
+++ tests/test_jujupy.py	2016-10-21 22:18:30 +0000
@@ -796,7 +796,8 @@
                     client.bootstrap()
             mock.assert_called_with(
                 'bootstrap', (
-                    '--constraints', 'mem=2G',
+                    '--constraints', 'mem=2G spaces=^endpoint_bindings_data,'
+                        '^endpoint_bindings_public',
                     'foo/asdf', 'maas',
                     '--config', config_file.name, '--default-model', 'maas',
                     '--agent-version', '2.0'),

=== modified file 'tests/test_substrate.py'
--- tests/test_substrate.py	2016-10-14 19:07:39 +0000
+++ tests/test_substrate.py	2016-10-21 22:18:30 +0000
@@ -877,6 +877,14 @@
             ('maas', 'mas', 'machines', 'list-allocated'))
         self.assertEqual({}, ips)
 
+    def test_machines(self):
+        account = self.get_account()
+        with patch('subprocess.check_output', autospec=True,
+                   return_value='[]') as co_mock:
+            machines = account.machines()
+        co_mock.assert_called_once_with(('maas', 'mas', 'machines', 'read'))
+        self.assertEqual([], machines)
+
     def test_fabrics(self):
         account = self.get_account()
         with patch('subprocess.check_output', autospec=True,
@@ -967,6 +975,16 @@
             'maas', 'mas', 'interfaces', 'read', 'node-xyz'))
         self.assertEqual([], interfaces)
 
+    def test_interface_update(self):
+        account = self.get_account()
+        with patch('subprocess.check_output', autospec=True,
+                   return_value='{"id": 10}') as co_mock:
+            interface = account.interface_update('node-xyz', 10, vlan=5000)
+        co_mock.assert_called_once_with((
+            'maas', 'mas', 'interface', 'update', 'node-xyz', '10',
+            'vlan=5000'))
+        self.assertEqual({'id': 10}, interface)
+
     def test_interface_create_vlan(self):
         account = self.get_account()
         with patch('subprocess.check_output', autospec=True,