Merge lp:~allenap/maas/there-can-be-only-one--bug-1462320 into lp:~maas-committers/maas/trunk

Proposed by Gavin Panella
Status: Merged
Approved by: Gavin Panella
Approved revision: no longer in the source branch.
Merged at revision: 3992
Proposed branch: lp:~allenap/maas/there-can-be-only-one--bug-1462320
Merge into: lp:~maas-committers/maas/trunk
Diff against target: 197 lines (+114/-13)
2 files modified
src/maasserver/rpc/regionservice.py (+23/-4)
src/maasserver/rpc/tests/test_regionservice.py (+91/-9)
To merge this branch: bzr merge lp:~allenap/maas/there-can-be-only-one--bug-1462320
Reviewer Review Type Date Requested Status
Raphaël Badin (community) Approve
Review via email: mp+261223@code.launchpad.net

Commit message

Remove overlapping event-loop records where it's obvious they're outdated.

In addition, don't allow RegionAdvertisingService to crash when there's an error, log instead.

To post a comment you must log in.
Revision history for this message
Raphaël Badin (rvb) wrote :

Looks good. Tested in my lab and it works as expected.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/maasserver/rpc/regionservice.py'
--- src/maasserver/rpc/regionservice.py 2015-06-02 15:50:27 +0000
+++ src/maasserver/rpc/regionservice.py 2015-06-05 13:23:18 +0000
@@ -698,7 +698,12 @@
698698
699 def __init__(self, interval=60):699 def __init__(self, interval=60):
700 super(RegionAdvertisingService, self).__init__(700 super(RegionAdvertisingService, self).__init__(
701 interval, deferToThread, self.update)701 interval, self.try_update)
702
703 def try_update(self):
704 return deferToThread(self.update).addErrback(
705 log.err, "Failed to update this event-loop's advertisement; "
706 "%s's record may be out of date" % (eventloop.loop.name,))
702707
703 @asynchronous708 @asynchronous
704 def startService(self):709 def startService(self):
@@ -784,9 +789,11 @@
784 event-loop running in the same process, and deletes - garbage789 event-loop running in the same process, and deletes - garbage
785 collects - old records related to any event-loop.790 collects - old records related to any event-loop.
786 """791 """
792 addresses = frozenset(self._get_addresses())
787 with closing(connection.cursor()) as cursor:793 with closing(connection.cursor()) as cursor:
788 self._do_delete(cursor)794 self._do_delete(cursor)
789 self._do_insert(cursor)795 self._do_delete_overlap(cursor, addresses)
796 self._do_insert(cursor, addresses)
790 self._do_collect(cursor)797 self._do_collect(cursor)
791798
792 @synchronous799 @synchronous
@@ -881,13 +888,25 @@
881 def _do_delete(self, cursor):888 def _do_delete(self, cursor):
882 cursor.execute(self._delete_statement, [eventloop.loop.name])889 cursor.execute(self._delete_statement, [eventloop.loop.name])
883890
891 _delete_overlap_statement = "DELETE FROM eventloops WHERE "
892 _delete_overlap_values_statement = "(address = %s AND port = %s)"
893
894 def _do_delete_overlap(self, cursor, addresses):
895 statement, values = [], []
896 for addr, port in addresses:
897 statement.append(self._delete_overlap_values_statement)
898 values.extend([addr, port])
899 if len(statement) != 0:
900 statement = self._delete_overlap_statement + " OR ".join(statement)
901 cursor.execute(statement, values)
902
884 _insert_statement = "INSERT INTO eventloops (name, address, port) VALUES "903 _insert_statement = "INSERT INTO eventloops (name, address, port) VALUES "
885 _insert_values_statement = "(%s, %s, %s)"904 _insert_values_statement = "(%s, %s, %s)"
886905
887 def _do_insert(self, cursor):906 def _do_insert(self, cursor, addresses):
888 name = eventloop.loop.name907 name = eventloop.loop.name
889 statement, values = [], []908 statement, values = [], []
890 for addr, port in self._get_addresses():909 for addr, port in addresses:
891 statement.append(self._insert_values_statement)910 statement.append(self._insert_values_statement)
892 values.extend([name, addr, port])911 values.extend([name, addr, port])
893 if len(statement) != 0:912 if len(statement) != 0:
894913
=== modified file 'src/maasserver/rpc/tests/test_regionservice.py'
--- src/maasserver/rpc/tests/test_regionservice.py 2015-06-02 15:50:27 +0000
+++ src/maasserver/rpc/tests/test_regionservice.py 2015-06-05 13:23:18 +0000
@@ -1718,7 +1718,25 @@
1718 def test_init(self):1718 def test_init(self):
1719 ras = RegionAdvertisingService()1719 ras = RegionAdvertisingService()
1720 self.assertEqual(60, ras.step)1720 self.assertEqual(60, ras.step)
1721 self.assertEqual((deferToThread, (ras.update,), {}), ras.call)1721 self.assertEqual((ras.try_update, (), {}), ras.call)
1722
1723 @wait_for_reactor
1724 @inlineCallbacks
1725 def test_try_update_logs_all_errors(self):
1726 ras = RegionAdvertisingService()
1727 error = factory.make_exception_type()
1728 self.patch(ras, "update").side_effect = error
1729 with TwistedLoggerFixture() as logger:
1730 yield ras.try_update()
1731 self.assertDocTestMatches(
1732 """
1733 Failed to update this event-loop's advertisement;
1734 %s's record may be out of date
1735 Traceback (most recent call last):
1736 ...
1737 maastesting.factory.TestException#...
1738 """ % eventloop.loop.name,
1739 logger.output)
17221740
1723 @wait_for_reactor1741 @wait_for_reactor
1724 def test_starting_and_stopping_the_service(self):1742 def test_starting_and_stopping_the_service(self):
@@ -1926,6 +1944,70 @@
1926 [("vic", "192.168.1.1", 1111)],1944 [("vic", "192.168.1.1", 1111)],
1927 service.dump())1945 service.dump())
19281946
1947 def patch_port(self, port):
1948 getServiceNamed = self.patch(eventloop.services, "getServiceNamed")
1949 getPort = getServiceNamed.return_value.getPort
1950 getPort.side_effect = asynchronous(lambda: port)
1951
1952 def patch_addresses(self, addresses):
1953 get_all_interface_addresses = self.patch(
1954 regionservice, "get_all_interface_addresses")
1955 get_all_interface_addresses.return_value = addresses
1956
1957 def test_update_deletes_overlapping_records(self):
1958 service = RegionAdvertisingService()
1959 service.prepare()
1960
1961 # The name of a fictional old event-loop.
1962 example_name = factory.make_name("loop")
1963 # Port and addresses on which the old and current loops are listening.
1964 example_port = factory.pick_port()
1965 example_addrs = {
1966 factory.make_ipv4_address(),
1967 factory.make_ipv4_address(),
1968 }
1969 self.patch_port(example_port)
1970 self.patch_addresses(example_addrs)
1971
1972 # A non-overlapping host and port for the old event-loop.
1973 for example_addr in iter(factory.make_ipv4_address, None):
1974 if example_addr not in example_addrs:
1975 example_non_overlapping_record = (
1976 example_name, example_addr, example_port)
1977 break
1978
1979 # Populate the eventloops table with records for the old event-loop.
1980 with closing(connection):
1981 with closing(connection.cursor()) as cursor:
1982 # Create the non-overlapping record.
1983 cursor.execute("""\
1984 INSERT INTO eventloops (name, address, port, updated)
1985 VALUES (%s, %s, %s, DEFAULT)
1986 """, example_non_overlapping_record)
1987 # Create the overlapping records.
1988 for example_addr in example_addrs:
1989 cursor.execute("""\
1990 INSERT INTO eventloops (name, address, port, updated)
1991 VALUES (%s, %s, %s, DEFAULT)
1992 """, [example_name, example_addr, example_port])
1993
1994 # All records for the old event-loop are present.
1995 self.assertItemsEqual(
1996 [example_non_overlapping_record] + [
1997 (example_name, example_addr, example_port)
1998 for example_addr in example_addrs
1999 ],
2000 service.dump())
2001 # Updating replaces overlapping records with records for the current
2002 # event-loop. The non-overlapping record remains.
2003 service.update()
2004 self.assertItemsEqual(
2005 [example_non_overlapping_record] + [
2006 (eventloop.loop.name, example_addr, example_port)
2007 for example_addr in example_addrs
2008 ],
2009 service.dump())
2010
1929 def test_dump(self):2011 def test_dump(self):
1930 example_addresses = [2012 example_addresses = [
1931 (factory.make_ipv4_address(), factory.pick_port()),2013 (factory.make_ipv4_address(), factory.pick_port()),
@@ -1975,9 +2057,7 @@
1975 service = RegionAdvertisingService()2057 service = RegionAdvertisingService()
19762058
1977 example_port = factory.pick_port()2059 example_port = factory.pick_port()
1978 getServiceNamed = self.patch(eventloop.services, "getServiceNamed")2060 self.patch_port(example_port)
1979 getPort = getServiceNamed.return_value.getPort
1980 getPort.side_effect = asynchronous(lambda: example_port)
19812061
1982 example_ipv4_addrs = {2062 example_ipv4_addrs = {
1983 factory.make_ipv4_address(),2063 factory.make_ipv4_address(),
@@ -1991,9 +2071,7 @@
1991 factory.pick_ip_in_network(netaddr.ip.IPV4_LINK_LOCAL),2071 factory.pick_ip_in_network(netaddr.ip.IPV4_LINK_LOCAL),
1992 factory.pick_ip_in_network(netaddr.ip.IPV6_LINK_LOCAL),2072 factory.pick_ip_in_network(netaddr.ip.IPV6_LINK_LOCAL),
1993 }2073 }
1994 get_all_interface_addresses = self.patch(2074 self.patch_addresses(
1995 regionservice, "get_all_interface_addresses")
1996 get_all_interface_addresses.return_value = (
1997 example_ipv4_addrs | example_ipv6_addrs |2075 example_ipv4_addrs | example_ipv6_addrs |
1998 example_link_local_addrs)2076 example_link_local_addrs)
19992077
@@ -2003,8 +2081,12 @@
2003 [(addr, example_port) for addr in example_ipv4_addrs],2081 [(addr, example_port) for addr in example_ipv4_addrs],
2004 service._get_addresses())2082 service._get_addresses())
20052083
2006 getServiceNamed.assert_called_once_with("rpc")2084 self.assertThat(
2007 get_all_interface_addresses.assert_called_once_with()2085 eventloop.services.getServiceNamed,
2086 MockCalledOnceWith("rpc"))
2087 self.assertThat(
2088 regionservice.get_all_interface_addresses,
2089 MockCalledOnceWith())
20082090
2009 def test__get_addresses_when_rpc_down(self):2091 def test__get_addresses_when_rpc_down(self):
2010 service = RegionAdvertisingService()2092 service = RegionAdvertisingService()