The idea behind the AdvancedServices (Metadata, *aaS) services
to be singletons was to (1) offer a way to get an instance of such
a service in a convinient manner, and to (2) ensure that only one
service registered per service type.
(1) Since the AdvancedService.instance method required an instance
of a L3 agent, this point was kind of missed. If you have a L3 agent
instance in your hand, just use it to access the service you're looking
for.
(2) This is now fulfilled by asserting that only one driver is registered
(per type) in the .add method.
The motivation to make them non-singletons is that:
a) They don't need to be
b) The code is simplified this way
c) I've been facing crazy issues during functional testing where we're
constantly instantiating L3 agents, and the (singleton) metadata
service was referencing the wrong configuration object. The service
was re-initialized during a test, screwing up the configuration of
other mid-run tests.
Closes-Bug: #
1425759
Depends-On: I155aece9b7028f1bdd3d9709facef392b38aec27
Depends-On: I199f6a967c64e921624b397fa5b7dfc56572d63a
Change-Id: I192e4d12380346c53c8ae0246371ccc41a836c4c
self.use_ipv6 = ipv6_utils.is_enabled()
if self.conf.enable_metadata_proxy:
- driver = metadata_driver.MetadataDriver.instance(self)
- self.event_observers.add(driver)
+ self.metadata_driver = metadata_driver.MetadataDriver(self)
+ self.event_observers.add(self.metadata_driver)
def _check_config_params(self):
"""Check items in configuration files.
def __init__(self):
self.observers = set()
- def add(self, observer):
+ def add(self, new_observer):
"""Add a listener for L3 agent notifications."""
- self.observers.add(observer)
+ for observer in self.observers:
+ if type(observer) == type(new_observer):
+ raise ValueError('Only a single instance of AdvancedService '
+ 'may be registered, per type of service.')
+
+ self.observers.add(new_observer)
def notify(self, l3_event_action, *args, **kwargs):
"""Give interested parties a chance to act on event.
# License for the specific language governing permissions and limitations
# under the License.
-from oslo_concurrency import lockutils
-
from neutron.openstack.common import log as logging
LOG = logging.getLogger(__name__)
Instead, a child class is defined for each service type and instantiated
by the corresponding service agent. The instances will have a back
reference to the L3 agent, and will register as an observer of events.
- A singleton is used to create only one service object per service type.
This base class provides a definition for all of the L3 event handlers
that a service could "observe". A child class for a service type will
implement handlers, for events of interest.
"""
- _instance = None
-
def __init__(self, l3_agent):
"""Base class for an advanced service.
# TODO(pcm): Address this in future refactorings.
self.conf = l3_agent.conf
- @classmethod
- def instance(cls, l3_agent):
- """Creates instance (singleton) of service.
-
- Do not directly call this for the base class. Instead, it should be
- called by a child class, that represents a specific service type.
-
- This ensures that only one instance is created for all agents of a
- specific service type.
- """
- if not cls._instance:
- with lockutils.lock('instance'):
- if not cls._instance:
- cls._instance = cls(l3_agent)
-
- return cls._instance
-
# NOTE: Handler definitions for events generated by the L3 agent.
# Subclasses of AdvancedService can override these to perform service
# specific actions. Unique methods are defined for add/update, as
class TestL3NATAgent(agent.L3NATAgentWithStateReport):
NESTED_NAMESPACE_SEPARATOR = '@'
- def __init__(self, host, conf=None):
- super(TestL3NATAgent, self).__init__(host, conf)
- self.event_observers.observers = set(
- observer.__class__(self) for observer in
- self.event_observers.observers)
-
def get_ns_name(self, router_id):
ns_name = super(TestL3NATAgent, self).get_ns_name(router_id)
return "%s%s%s" % (ns_name, self.NESTED_NAMESPACE_SEPARATOR, self.host)
# under the License.
import mock
+import testtools
from neutron.agent.l3 import event_observers
from neutron.services import advanced_service as adv_svc
self.event_observers.add(observer)
self.assertIn(observer, self.event_observers.observers)
- def test_add_duplicate_observer_is_ignored(self):
- observer = object()
+ def test_add_duplicate_observer_type_raises(self):
+ agent = mock.Mock()
+ observer = DummyService1(agent)
self.event_observers.add(observer)
- try:
- self.event_observers.add(observer)
- except Exception:
- self.fail('Duplicate additions of observers should be ignored')
+
+ observer2 = DummyService1(agent)
+ with testtools.ExpectedException(ValueError):
+ self.event_observers.add(observer2)
+
self.assertEqual(1, len(self.event_observers.observers))
def test_observers_in_service_notified(self):
"""Test that correct handlers for multiple services are called."""
l3_agent = mock.Mock()
router_info = mock.Mock()
- observer1 = DummyService1.instance(l3_agent)
- observer2 = DummyService2.instance(l3_agent)
+ observer1 = DummyService1(l3_agent)
+ observer2 = DummyService2(l3_agent)
observer1_before_add = mock.patch.object(
DummyService1, 'before_router_added').start()
observer2_before_add = mock.patch.object(
super(TestAdvancedService, self).setUp()
self.agent = mock.Mock()
self.test_observers = event_observers.L3EventObservers()
- # Ensure no instances for each test
- FakeServiceA._instance = None
- FakeServiceB._instance = None
def test_create_service(self):
"""Test agent saved and service added to observer list."""
- my_service = FakeServiceA.instance(self.agent)
+ my_service = FakeServiceA(self.agent)
self.test_observers.add(my_service)
self.assertIn(my_service, self.test_observers.observers)
self.assertEqual(self.agent, my_service.l3_agent)
- def test_service_is_singleton(self):
- """Test that two services of same time use same instance."""
- a1 = FakeServiceA.instance(self.agent)
- a2 = FakeServiceA.instance(self.agent)
- self.assertIs(a1, a2)
-
def test_shared_observers_for_different_services(self):
"""Test different service type instances created.
The services are unique instances, with different agents, but
sharing the same observer list.
"""
- a = FakeServiceA.instance(self.agent)
+ a = FakeServiceA(self.agent)
self.test_observers.add(a)
self.assertEqual(self.agent, a.l3_agent)
self.assertIn(a, self.test_observers.observers)
another_agent = mock.Mock()
- b = FakeServiceB.instance(another_agent)
+ b = FakeServiceB(another_agent)
self.test_observers.add(b)
self.assertNotEqual(a, b)
self.assertEqual(another_agent, b.l3_agent)
The services are unique instances, shared the same agent, but
are using different observer lists.
"""
- a = FakeServiceA.instance(self.agent)
+ a = FakeServiceA(self.agent)
self.test_observers.add(a)
other_observers = event_observers.L3EventObservers()
- b = FakeServiceB.instance(self.agent)
+ b = FakeServiceB(self.agent)
other_observers.add(b)
self.assertNotEqual(a, b)
Code Review / openstack-build / neutron-build.git / commitdiff