Merge lp:~andreserl/maas/fix_lp1469846 into lp:~maas-committers/maas/trunk

[Andres Rodriguez]

When probe-and-enlist a UCSM chassis, only probe for servers/blades that have a service profile / MAC address created for them.

[Raphaël Badin (rvb)]

Looks okay but:
- David suggested a more thorough check, can you explain why you implemented only part of it?
- Missing test(s).

review: Needs Fixing

[Andres Rodriguez (andreserl)]

1. David's suggestion came after this patch.
2. If I have to write tests for this change, that means there's definitely something wrong as I would spend exponentially more time writing tests than the actual code, and that's unproductive :).

[Raphaël Badin (rvb)]

Looks better but some of the new code is still untested.

review: Needs Fixing

[Andres Rodriguez (andreserl)]

Hi Raphael,

I've addressed your comments other than adding the test for probe_lan_boot_options. I've spend over an hour trying to get it right, but I couldn't. If you can help with that would be great!

Thanks

[Raphaël Badin (rvb)]

> Hi Raphael,
>
> I've addressed your comments other than adding the test for
> probe_lan_boot_options. I've spend over an hour trying to get it right, but I
> couldn't. If you can help with that would be great!
>
> Thanks

Yeah, this one is painful because we don't have a proper test double so we're down for some serious patching ;).

http://paste.ubuntu.com/11806452/

[Raphaël Badin (rvb)]

Approved with my change ;)

review: Approve

[Andres Rodriguez (andreserl)]

thank you sir!

[MAAS Lander (maas-lander)]

The attempt to merge lp:~andreserl/maas/fix_lp1469846 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]
Get:2 http://security.ubuntu.com trusty-security Release [63.5 kB]
Ign http://nova.clouds.archive.ubuntu.com trusty InRelease
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 [63.5 kB]
Get:5 http://security.ubuntu.com trusty-security/main Sources [87.4 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty/main Sources
Get:6 http://security.ubuntu.com trusty-security/universe Sources [28.1 kB]
Get:7 http://security.ubuntu.com trusty-security/main amd64 Packages [304 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
Hit http://nova.clouds.archive.ubuntu.com trusty/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en
Get:8 http://nova.clouds.archive.ubuntu.com trusty-updates/main Sources [212 kB]
Get:9 http://security.ubuntu.com trusty-security/universe amd64 Packages [111 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 [123 kB]
Get:11 http://nova.clouds.archive.ubuntu.com trusty-updates/main amd64 Packages [562 kB]
Get:12 http://nova.clouds.archive.ubuntu.com trusty-updates/universe amd64 Packages [292 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,848 kB in 3s (563 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
     --no-install-recommends install apache2 authbind bind9 bind9utils build-essential bzr-builddeb chromium-browser chromium-chromedriver curl daemontools debhelper dh-apport dh-systemd distro-info dnsutils firefox freeipmi-tools git gjs ipython isc-dhcp-common libjs-angularjs libjs-jquery libjs-jquery-hotkeys libjs-yui3-full libjs-yui3-min libpq-dev make nodejs-legacy npm pep8 phantomjs postgresql pyflakes python-apt python-bson python-bzrlib python-convoy python-coverage 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-iscpy python-jinja2 python-jsonschema python-lockfile python-lxml python-mock python-netaddr python-netifaces python-n...

[MAAS Lander (maas-lander)]

The attempt to merge lp:~andreserl/maas/fix_lp1469846 into lp:maas failed. Below is the output from the failed tests.

Ign http://security.ubuntu.com trusty-security InRelease
Hit http://security.ubuntu.com trusty-security Release.gpg
Ign http://nova.clouds.archive.ubuntu.com trusty InRelease
Hit http://security.ubuntu.com trusty-security Release
Ign http://nova.clouds.archive.ubuntu.com trusty-updates InRelease
Hit http://nova.clouds.archive.ubuntu.com trusty Release.gpg
Hit http://nova.clouds.archive.ubuntu.com trusty-updates Release.gpg
Hit http://nova.clouds.archive.ubuntu.com trusty Release
Hit http://nova.clouds.archive.ubuntu.com trusty-updates Release
Hit http://security.ubuntu.com trusty-security/main Sources
Hit http://security.ubuntu.com trusty-security/universe Sources
Hit http://security.ubuntu.com trusty-security/main amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty/main Sources
Hit http://security.ubuntu.com trusty-security/universe amd64 Packages
Hit http://security.ubuntu.com trusty-security/main Translation-en
Hit http://security.ubuntu.com trusty-security/universe Translation-en
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
Hit http://nova.clouds.archive.ubuntu.com trusty/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/main Sources
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/universe Sources
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/main amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/universe amd64 Packages
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
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
     --no-install-recommends install apache2 authbind bind9 bind9utils build-essential bzr-builddeb chromium-browser chromium-chromedriver curl daemontools debhelper dh-apport dh-systemd distro-info dnsutils firefox freeipmi-tools git gjs ipython isc-dhcp-common libjs-angularjs libjs-jquery libjs-jquery-hotkeys libjs-yui3-full libjs-yui3-min libpq-dev make nodejs-legacy npm pep8 phantomjs postgresql pyflakes python-apt python-bson python-bzrlib python-convoy python-coverage 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-iscpy python-jinja2 python-jsonschema python-lockfile python-lxml python-mock python-netaddr python-netifaces python-nose python-oauth python-openssl python-paramiko python-pexpect python-pip python-pocket-lint python-psycopg2 python-pyinotify python-pyparsing python-seamicroclient python...

[MAAS Lander (maas-lander)]

The attempt to merge lp:~andreserl/maas/fix_lp1469846 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]
Get:2 http://security.ubuntu.com trusty-security Release [63.5 kB]
Ign http://nova.clouds.archive.ubuntu.com trusty InRelease
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 [63.5 kB]
Get:5 http://security.ubuntu.com trusty-security/main Sources [87.4 kB]
Get:6 http://security.ubuntu.com trusty-security/universe Sources [28.1 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty/main Sources
Get:7 http://security.ubuntu.com trusty-security/main amd64 Packages [304 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 [111 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 [212 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 [123 kB]
Get:11 http://nova.clouds.archive.ubuntu.com trusty-updates/main amd64 Packages [562 kB]
Get:12 http://nova.clouds.archive.ubuntu.com trusty-updates/universe amd64 Packages [292 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,848 kB in 3s (592 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
     --no-install-recommends install apache2 authbind bind9 bind9utils build-essential bzr-builddeb chromium-browser chromium-chromedriver curl daemontools debhelper dh-apport dh-systemd distro-info dnsutils firefox freeipmi-tools git gjs ipython isc-dhcp-common libjs-angularjs libjs-jquery libjs-jquery-hotkeys libjs-yui3-full libjs-yui3-min libpq-dev make nodejs-legacy npm pep8 phantomjs postgresql pyflakes python-apt python-bson python-bzrlib python-convoy python-coverage 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-iscpy python-jinja2 python-jsonschema python-lockfile python-lxml python-mock python-netaddr python-netifaces python-n...

=== modified file 'src/provisioningserver/drivers/hardware/tests/test_ucsm.py'
--- src/provisioningserver/drivers/hardware/tests/test_ucsm.py	2015-05-07 18:14:38 +0000
+++ src/provisioningserver/drivers/hardware/tests/test_ucsm.py	2015-07-02 04:20:13 +0000
@@ -38,6 +38,7 @@
     ANY,
     call,
     Mock,
+    sentinel,
 )
 from provisioningserver.drivers.hardware import ucsm
 from provisioningserver.drivers.hardware.ucsm import (
@@ -55,6 +56,7 @@
     power_control_ucsm,
     power_state_ucsm,
     probe_and_enlist_ucsm,
+    probe_lan_boot_options,
     probe_servers,
     RO_KEYS,
     set_lan_boot_default,
@@ -334,6 +336,30 @@
         self.assertThat(mock, MockCalledOnceWith('computeItem', ANY))
 
 
+class TestProbeLanBootOptions(MAASTestCase):
+    """Tests for ``probe_lan_boot_options``."""
+
+    def test_returns_result(self):
+        api = make_api()
+        server = sentinel.server
+        mock_service_profile = Mock()
+        mock_get_service_profile = self.patch(ucsm, 'get_service_profile')
+        mock_get_service_profile.return_value = mock_service_profile
+        mock_service_profile.get.return_value = sentinel.profile_get
+        fake_result = make_fake_result('tag', 'lsbootLan')
+        mock_config_resolve_children = self.patch(
+            api, 'config_resolve_children')
+        mock_config_resolve_children.return_value = fake_result
+        self.assertEqual(1, len(probe_lan_boot_options(api, server)))
+        self.assertThat(
+            mock_config_resolve_children,
+            MockCalledOnceWith(sentinel.profile_get))
+        self.assertThat(
+            mock_service_profile.get, MockCalledOnceWith('operBootPolicyName'))
+        self.assertThat(
+            mock_get_service_profile, MockCalledOnceWith(api, server))
+
+
 class TestGetChildren(MAASTestCase):
     """Tests for ``get_children``."""
 
@@ -397,9 +423,29 @@
         api = make_api()
         self.patch(ucsm, 'get_servers').return_value = servers
         self.patch(ucsm, 'get_macs').return_value = [mac]
+        self.patch(ucsm, 'probe_lan_boot_options').return_value = ['option']
         server_list = probe_servers(api)
         self.assertEqual([(servers[0], [mac])], server_list)
 
+    def test_no_results_with_no_server_macs(self):
+        servers = [{'uuid': factory.make_UUID()}]
+        api = make_api()
+        self.patch(ucsm, 'get_servers').return_value = servers
+        self.patch(ucsm, 'get_macs').return_value = []
+        self.patch(ucsm, 'probe_lan_boot_options').return_value = ['option']
+        server_list = probe_servers(api)
+        self.assertEqual([], server_list)
+
+    def test_no_results_with_no_boot_options(self):
+        servers = [{'uuid': factory.make_UUID()}]
+        mac = 'mac'
+        api = make_api()
+        self.patch(ucsm, 'get_servers').return_value = servers
+        self.patch(ucsm, 'get_macs').return_value = mac
+        self.patch(ucsm, 'probe_lan_boot_options').return_value = []
+        server_list = probe_servers(api)
+        self.assertEqual([], server_list)
+
 
 class TestGetServerPowerControl(MAASTestCase):
     """Tests for ``get_server_power_control``."""

=== modified file 'src/provisioningserver/drivers/hardware/ucsm.py'
--- src/provisioningserver/drivers/hardware/ucsm.py	2015-05-07 18:14:38 +0000
+++ src/provisioningserver/drivers/hardware/ucsm.py	2015-07-02 04:20:13 +0000
@@ -260,10 +260,29 @@
     return macs
 
 
+def probe_lan_boot_options(api, server):
+    """Probe for LAN boot options available on a server."""
+    service_profile = get_service_profile(api, server)
+    boot_profile_dn = service_profile.get('operBootPolicyName')
+    response = api.config_resolve_children(boot_profile_dn)
+    return response.xpath('//outConfigs/lsbootLan')
+
+
 def probe_servers(api):
     """Retrieve the UUID and MAC addresses for servers from the UCS Manager."""
     servers = get_servers(api)
-    server_list = [(s, get_macs(api, s)) for s in servers]
+
+    server_list = []
+    for s in servers:
+        # If the server does not have any MAC, then we don't add it.
+        if not get_macs(api, s):
+            continue
+        # If the server does not have LAN boot option (can't boot from LAN),
+        # then we don't add it.
+        if not probe_lan_boot_options(api, s):
+            continue
+        server_list.append((s, get_macs(api, s)))
+
     return server_list