Merge pull request #10 from fguillot/retry-unordered-events

Do not ignore silently instance.delete.end events when the entity is not created

almanach/adapters/database_adapter.py

@@ -78,6 +78,10 @@ class DatabaseAdapter(object):
     def count_entity_entries(self, entity_id):
         return self.db.entity.find({"entity_id": entity_id}).count()
 
+    @database
+    def has_active_entity(self, entity_id):
+        return self.db.entity.find({"entity_id": entity_id, "end": None}).count() == 1
+
     @database
     def list_entities(self, project_id, start, end, entity_type=None):
         args = {"project_id": project_id, "start": {"$lte": end}, "$or": [{"end": None}, {"end": {"$gte": start}}]}

almanach/common/almanach_entity_not_found_exception.py

@@ -0,0 +1,2 @@
+class AlmanachEntityNotFoundException(Exception):
+    pass

almanach/core/controller.py

@@ -19,6 +19,7 @@ from datetime import timedelta
 from dateutil import parser as date_parser
 from pkg_resources import get_distribution
 
+from almanach.common.almanach_entity_not_found_exception import AlmanachEntityNotFoundException
 from almanach.common.date_format_exception import DateFormatException
 from almanach.core.model import Instance, Volume, VolumeType
 from almanach import config
@@ -66,6 +67,9 @@ class Controller(object):
         self.database_adapter.insert_entity(entity)
 
     def delete_instance(self, instance_id, delete_date):
+        if not self.database_adapter.has_active_entity(instance_id):
+            raise AlmanachEntityNotFoundException("InstanceId: {0} Not Found".format(instance_id))
+
         delete_date = self._validate_and_parse_date(delete_date)
         logging.info("instance %s deleted on %s" % (instance_id, delete_date))
         self.database_adapter.close_active_entity(instance_id, delete_date)

tests/adapters/test_bus_adapter.py

@@ -18,6 +18,7 @@ import pytz
 from datetime import datetime
 from flexmock import flexmock, flexmock_teardown
 
+from almanach.common.almanach_entity_not_found_exception import AlmanachEntityNotFoundException
 from tests import messages
 from almanach.adapters.bus_adapter import BusAdapter
 
@@ -334,12 +335,21 @@ class BusAdapterTest(unittest.TestCase):
 
     def test_failing_notification_get_retry(self):
         notification = messages.get_instance_rebuild_end_sample()
-        self.controller.should_receive('instance_rebuilded').and_raise(Exception("trololololo"))
-        self.retry.should_receive('publish_to_dead_letter').once()
-
         message = flexmock()
-        (flexmock(message)
-         .should_receive("ack"))
+
+        (flexmock(message).should_receive("ack"))
+        self.controller.should_receive('instance_rebuilded').and_raise(Exception("Foobar"))
+        self.retry.should_receive('publish_to_dead_letter').with_args(message).once()
+
+        self.bus_adapter.on_message(notification, message)
+
+    def test_that_entity_not_found_exceptions_goes_to_retry_queue(self):
+        notification = messages.get_instance_delete_end_sample(instance_id="My instance id")
+        message = flexmock()
+
+        (flexmock(message).should_receive("ack"))
+        self.controller.should_receive('delete_instance').and_raise(AlmanachEntityNotFoundException("Entity not found"))
+        self.retry.should_receive('publish_to_dead_letter').with_args(message).once()
 
         self.bus_adapter.on_message(notification, message)
 

tests/adapters/test_database_adapter.py

@@ -49,6 +49,14 @@ class DatabaseAdapterTest(unittest.TestCase):
         self.assertEqual(self.db.entity.count(), 1)
         self.assert_mongo_collection_contains("entity", fake_instance)
 
+    def test_has_active_entity_not_found(self):
+        self.assertFalse(self.adapter.has_active_entity("my_entity_id"))
+
+    def test_has_active_entity_found(self):
+        fake_instance = a(instance().with_id("my_entity_id"))
+        self.adapter.insert_entity(fake_instance)
+        self.assertTrue(self.adapter.has_active_entity("my_entity_id"))
+
     def test_get_instance_entity(self):
         fake_entity = a(instance().with_metadata({}))
 

tests/core/test_controller.py

@@ -20,6 +20,7 @@ from dateutil import parser as date_parser
 from flexmock import flexmock, flexmock_teardown
 from nose.tools import assert_raises
 from almanach import config
+from almanach.common.almanach_entity_not_found_exception import AlmanachEntityNotFoundException
 from almanach.common.date_format_exception import DateFormatException
 from almanach.core.controller import Controller
 from almanach.core.model import Instance, Volume
@@ -149,6 +150,12 @@ class ControllerTest(unittest.TestCase):
                                         fake_instance.os.version, fake_instance.name, fake_instance.metadata)
 
     def test_instance_deleted(self):
+        (flexmock(self.database_adapter)
+         .should_receive("has_active_entity")
+         .with_args("id1")
+         .and_return(True)
+         .once())
+
         (flexmock(self.database_adapter)
          .should_receive("close_active_entity")
          .with_args("id1", date_parser.parse("2015-10-21T16:25:00.000000Z"))
@@ -156,6 +163,16 @@ class ControllerTest(unittest.TestCase):
 
         self.controller.delete_instance("id1", "2015-10-21T16:25:00.000000Z")
 
+    def test_instance_deleted_when_entity_not_found(self):
+        (flexmock(self.database_adapter)
+         .should_receive("has_active_entity")
+         .with_args("id1")
+         .and_return(False)
+         .once())
+
+        with self.assertRaises(AlmanachEntityNotFoundException):
+            self.controller.delete_instance("id1", "2015-10-21T16:25:00.000000Z")
+
     def test_volume_deleted(self):
         fake_volume = a(volume())