Problem: Duplicate user_allocation_source Events (#351)

* Problem : Duplicate user_allocation_source Events * Problem: Can't freeze time more than once Solution: If `context.freezer` already exists, set `context.freezer.time_to_freeze` to `context.frozen_current_time`. * Problem: No test for #350 - Duplicate `user_allocation_source` Events Solution: Added behave scenario to ensure that we don't create duplicate events when checking the TAS API multiple times. * Problem: No migration to remove existing duplicate events re #350 Solution: Added migration to remove duplicate events with name `user_allocation_source_created`. * Added an entry to the CHANGELOG.md file for #350 * Renamed the projects.
cyverse · May 1, 2017 · e971c94 · e971c94
1 parent 653ec46
commit e971c94
Show file tree

Hide file tree

Showing 6 changed files with 162 additions and 46 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,8 @@ Bugfixes:
  - Fixed application tags (https://github.com/cyverse/atmosphere/commit/fed9aae578025d8024f4a255ee109e12f1ff0483)
  - Behave fail for allocation settings (https://github.com/cyverse/atmosphere/commit/62686fd387203e2b5abe057d807503eabbddade4)
  - Unknown sizes appear when sizes are disabled (https://github.com/cyverse/atmosphere/pull/321)
+ - Fixed duplicate `user_allocation_source` events (https://github.com/cyverse/atmosphere/issues/350)
+    - Also has a migration to delete old duplicate events
 
 Internal:
  - Bail out conditions for Celery task when MockDriver is used

diff --git a/core/migrations/0080_delete_duplicate_user_allocation_source_events.py b/core/migrations/0080_delete_duplicate_user_allocation_source_events.py
@@ -0,0 +1,41 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.9.8 on 2017-03-29 18:34
+from __future__ import unicode_literals
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ('core', '0079_add_events_for_old_allocationsource'),
+    ]
+
+    rename_duplicate_events = '''UPDATE event_table
+SET name = 'user_allocation_source_created_DELETE'
+FROM (
+       SELECT
+         entity_id,
+         name,
+         payload,
+         count(*),
+         min(timestamp) AS min_timestamp,
+         max(timestamp) AS max_timestamp
+       FROM event_table
+       WHERE name = 'user_allocation_source_created'
+       GROUP BY entity_id, name, payload
+       HAVING count(*) > 1
+     ) AS duplicates
+WHERE
+  event_table.entity_id = duplicates.entity_id
+  AND event_table.name = duplicates.name
+  AND event_table.payload = duplicates.payload
+  AND event_table.timestamp > duplicates.min_timestamp;'''
+
+    delete_renamed_duplicate_events = '''DELETE FROM event_table
+WHERE name = 'user_allocation_source_created_DELETE';
+    '''
+
+    operations = [
+        migrations.RunSQL(rename_duplicate_events),
+        migrations.RunSQL(delete_renamed_duplicate_events),
+    ]
diff --git a/jetstream/allocation.py b/jetstream/allocation.py
@@ -392,15 +392,22 @@ def get_or_create_user_allocation_source(user, allocation_source):
     from core.models import AtmosphereUser
     assert isinstance(user, AtmosphereUser)
     assert isinstance(allocation_source, AllocationSource)
-    payload = {
-        'allocation_source_name': allocation_source.name
-    }
 
-    created_event = EventTable.objects.create(name='user_allocation_source_created',
-                                              entity_id=user.username,
-                                              payload=payload)
-    assert isinstance(created_event, EventTable)
-    user_allocation_source = UserAllocationSource.objects.get(user=user, allocation_source=allocation_source)
+    user_allocation_source = UserAllocationSource.objects.filter(user=user, allocation_source=allocation_source)
+
+    if not user_allocation_source:
+        payload = {
+            'allocation_source_name': allocation_source.name
+        }
+
+        created_event = EventTable.objects.create(name='user_allocation_source_created',
+                                                  entity_id=user.username,
+                                                  payload=payload)
+        assert isinstance(created_event, EventTable)
+        user_allocation_source = UserAllocationSource.objects.get(user=user, allocation_source=allocation_source)
+    else:
+        user_allocation_source = user_allocation_source.last()
+
     return user_allocation_source
 
 

diff --git a/jetstream/features/environment.py b/jetstream/features/environment.py
@@ -1,3 +1,4 @@
+import dateutil.parser
 import freezegun
 
 
@@ -17,6 +18,8 @@ def before_step(context, step):
     if hasattr(context, 'frozen_current_time'):
         if not hasattr(context, 'freezer'):
             context.freezer = freezegun.freeze_time(context.frozen_current_time, tick=True)
+        else:
+            context.freezer.time_to_freeze = dateutil.parser.parse(context.frozen_current_time)
         context.freezer.start()
 
 

diff --git a/jetstream/features/monitor_jetstream_allocation_sources.feature b/jetstream/features/monitor_jetstream_allocation_sources.feature
@@ -21,32 +21,32 @@ Feature: Monitor Jetstream Allocation Sources
       | user111        | tacc_user111  |
     And the following TAS projects
       | id    | chargeCode   |
-      | 29444 | TG-BIO150062 |
-      | 29456 | TG-TRA160003 |
+      | 29444 | TG-ENG150061 |
+      | 29456 | TG-TRA160006 |
     And the following TAS allocations
       | id    | projectId | project      | computeAllocated | computeUsed | start                | end                  | status   | resource  |
-      | 38229 | 29444     | TG-BIO150062 | 1000000          | 781768.01   | 2016-01-01T06:00:00Z | 2017-06-30T05:00:00Z | Active   | Jetstream |
-      | 38271 | 29456     | TG-TRA160003 | 500000           | 583803.28   | 2016-01-22T06:00:00Z | 2017-01-21T06:00:00Z | Inactive | Jetstream |
-      | 45184 | 29456     | TG-TRA160003 | 600000           | 87914.06    | 2017-01-22T06:00:00Z | 2018-01-21T06:00:00Z | Active   | Jetstream |
-      | 55184 | 29456     | TG-TRA160003 | 700000           | 0           | 2018-01-22T06:00:00Z | 2019-01-21T06:00:00Z | Approved | Jetstream |
+      | 38229 | 29444     | TG-ENG150061 | 1000000          | 781768.01   | 2016-01-01T06:00:00Z | 2017-06-30T05:00:00Z | Active   | Jetstream |
+      | 38271 | 29456     | TG-TRA160006 | 500000           | 583803.28   | 2016-01-22T06:00:00Z | 2017-01-21T06:00:00Z | Inactive | Jetstream |
+      | 45184 | 29456     | TG-TRA160006 | 600000           | 87914.06    | 2017-01-22T06:00:00Z | 2018-01-21T06:00:00Z | Active   | Jetstream |
+      | 55184 | 29456     | TG-TRA160006 | 700000           | 0           | 2018-01-22T06:00:00Z | 2019-01-21T06:00:00Z | Approved | Jetstream |
     And the following TACC usernames for TAS projects
       | project      | tacc_usernames            |
-      | TG-BIO150062 | tacc_user108,tacc_user109 |
-      | TG-TRA160003 | tacc_user109,tacc_user111 |
+      | TG-ENG150061 | tacc_user108,tacc_user109 |
+      | TG-TRA160006 | tacc_user109,tacc_user111 |
 
 
   Scenario: Get all projects
     When we get all projects
     Then we should have the following local projects
       | id    | chargeCode   |
-      | 29444 | TG-BIO150062 |
-      | 29456 | TG-TRA160003 |
+      | 29444 | TG-ENG150061 |
+      | 29456 | TG-TRA160006 |
     And we should have the following local allocations
       | id    | projectId | project      | computeAllocated | computeUsed | start                | end                  | status   | resource  |
-      | 38229 | 29444     | TG-BIO150062 | 1000000          | 781768.01   | 2016-01-01T06:00:00Z | 2017-06-30T05:00:00Z | Active   | Jetstream |
-      | 38271 | 29456     | TG-TRA160003 | 500000           | 583803.28   | 2016-01-22T06:00:00Z | 2017-01-21T06:00:00Z | Inactive | Jetstream |
-      | 45184 | 29456     | TG-TRA160003 | 600000           | 87914.06    | 2017-01-22T06:00:00Z | 2018-01-21T06:00:00Z | Active   | Jetstream |
-      | 55184 | 29456     | TG-TRA160003 | 700000           | 0           | 2018-01-22T06:00:00Z | 2019-01-21T06:00:00Z | Approved | Jetstream |
+      | 38229 | 29444     | TG-ENG150061 | 1000000          | 781768.01   | 2016-01-01T06:00:00Z | 2017-06-30T05:00:00Z | Active   | Jetstream |
+      | 38271 | 29456     | TG-TRA160006 | 500000           | 583803.28   | 2016-01-22T06:00:00Z | 2017-01-21T06:00:00Z | Inactive | Jetstream |
+      | 45184 | 29456     | TG-TRA160006 | 600000           | 87914.06    | 2017-01-22T06:00:00Z | 2018-01-21T06:00:00Z | Active   | Jetstream |
+      | 55184 | 29456     | TG-TRA160006 | 700000           | 0           | 2018-01-22T06:00:00Z | 2019-01-21T06:00:00Z | Approved | Jetstream |
 
 
   Scenario: Fill user allocation sources
@@ -60,50 +60,50 @@ Feature: Monitor Jetstream Allocation Sources
       | user111 | tacc_user111 |
     And we should have the following local projects
       | id    | chargeCode   |
-      | 29444 | TG-BIO150062 |
-      | 29456 | TG-TRA160003 |
+      | 29444 | TG-ENG150061 |
+      | 29456 | TG-TRA160006 |
     And we should have the following allocation sources
       | name         | compute_allowed |
-      | TG-BIO150062 | 1000000         |
-      | TG-TRA160003 | 600000          |
+      | TG-ENG150061 | 1000000         |
+      | TG-TRA160006 | 600000          |
     And we should have the following user allocation sources
       | atmosphere_username | allocation_source |
-      | user108             | TG-BIO150062      |
-      | user109             | TG-BIO150062      |
-      | user109             | TG-TRA160003      |
-      | user111             | TG-TRA160003      |
+      | user108             | TG-ENG150061      |
+      | user109             | TG-ENG150061      |
+      | user109             | TG-TRA160006      |
+      | user111             | TG-TRA160006      |
 
 
   Scenario: Remove user from allocation source
     When we get all projects
     And we fill user allocation sources from TAS
     Then we should have the following user allocation sources
       | atmosphere_username | allocation_source |
-      | user108             | TG-BIO150062      |
-      | user109             | TG-BIO150062      |
-      | user109             | TG-TRA160003      |
-      | user111             | TG-TRA160003      |
+      | user108             | TG-ENG150061      |
+      | user109             | TG-ENG150061      |
+      | user109             | TG-TRA160006      |
+      | user111             | TG-TRA160006      |
     Given a current time of '2017-02-17T05:00:00Z'
     And the following TACC usernames for TAS projects
       | project      | tacc_usernames            |
-      | TG-BIO150062 | tacc_user108              |
-      | TG-TRA160003 | tacc_user109,tacc_user111 |
+      | TG-ENG150061 | tacc_user108              |
+      | TG-TRA160006 | tacc_user109,tacc_user111 |
     When we fill user allocation sources from TAS
     Then we should have the following user allocation sources
       | atmosphere_username | allocation_source |
-      | user108             | TG-BIO150062      |
-      | user109             | TG-TRA160003      |
-      | user111             | TG-TRA160003      |
+      | user108             | TG-ENG150061      |
+      | user109             | TG-TRA160006      |
+      | user111             | TG-TRA160006      |
     Given a current time of '2017-02-19T05:00:00Z'
     And the following TACC usernames for TAS projects
       | project      | tacc_usernames |
-      | TG-BIO150062 | tacc_user108   |
-      | TG-TRA160003 | tacc_user111   |
+      | TG-ENG150061 | tacc_user108   |
+      | TG-TRA160006 | tacc_user111   |
     When we fill user allocation sources from TAS
     Then we should have the following user allocation sources
       | atmosphere_username | allocation_source |
-      | user108             | TG-BIO150062      |
-      | user111             | TG-TRA160003      |
+      | user108             | TG-ENG150061      |
+      | user111             | TG-TRA160006      |
 
 
   Scenario: Calculate allocation source snapshots
@@ -112,9 +112,51 @@ Feature: Monitor Jetstream Allocation Sources
     And we update snapshots
     Then we should have the following allocation source snapshots
       | name         | compute_used |
-      | TG-BIO150062 | 781768.010   |
-      | TG-TRA160003 | 87914.060    |
+      | TG-ENG150061 | 781768.010   |
+      | TG-TRA160006 | 87914.060    |
 
 
+  Scenario: Correct events, with no duplicates when checking twice
+    When we get all projects
+    Then we should have the following events
+      | entity_id | name | payload | timestamp |
+    When we fill user allocation sources from TAS
+    Then we should have the following user allocation sources
+      | atmosphere_username | allocation_source |
+      | user108             | TG-ENG150061      |
+      | user109             | TG-ENG150061      |
+      | user109             | TG-TRA160006      |
+      | user111             | TG-TRA160006      |
+    # TODO: `allocation_source_created_or_renewed` & `allocation_source_compute_allowed_changed` should have `entity_id`
+    And we should have the following events
+      | entity_id | name                                      | payload                                                                                                                                          | timestamp                |
+      |           | allocation_source_created_or_renewed      | {"allocation_source_name": "TG-ENG150061", "start_date": "2016-01-01T06:00:00Z", "end_date": "2017-06-30T05:00:00Z", "compute_allowed": 1000000} | 2017-02-15 05:00:00+0000 |
+      |           | allocation_source_compute_allowed_changed | {"allocation_source_name": "TG-ENG150061", "start_date": "2016-01-01T06:00:00Z", "end_date": "2017-06-30T05:00:00Z", "compute_allowed": 1000000} | 2017-02-15 05:00:00+0000 |
+      | user108   | user_allocation_source_created            | {"allocation_source_name": "TG-ENG150061"}                                                                                                       | 2017-02-15 05:00:00+0000 |
+      | user109   | user_allocation_source_created            | {"allocation_source_name": "TG-ENG150061"}                                                                                                       | 2017-02-15 05:00:00+0000 |
+      |           | allocation_source_created_or_renewed      | {"allocation_source_name": "TG-TRA160006", "start_date": "2017-01-22T06:00:00Z", "end_date": "2018-01-21T06:00:00Z", "compute_allowed": 600000}  | 2017-02-15 05:00:00+0000 |
+      |           | allocation_source_compute_allowed_changed | {"allocation_source_name": "TG-TRA160006", "start_date": "2017-01-22T06:00:00Z", "end_date": "2018-01-21T06:00:00Z", "compute_allowed": 600000}  | 2017-02-15 05:00:00+0000 |
+      | user109   | user_allocation_source_created            | {"allocation_source_name": "TG-TRA160006"}                                                                                                       | 2017-02-15 05:00:00+0000 |
+      | user111   | user_allocation_source_created            | {"allocation_source_name": "TG-TRA160006"}                                                                                                       | 2017-02-15 05:00:00+0000 |
+    Given a current time of '2017-02-16T06:00:00Z'
+    When we get all projects
+    And we fill user allocation sources from TAS
+    Then we should have the following user allocation sources
+      | atmosphere_username | allocation_source |
+      | user108             | TG-ENG150061      |
+      | user109             | TG-ENG150061      |
+      | user109             | TG-TRA160006      |
+      | user111             | TG-TRA160006      |
+    And we should have the following events
+      | entity_id | name                                      | payload                                                                                                                                          | timestamp                |
+      |           | allocation_source_created_or_renewed      | {"allocation_source_name": "TG-ENG150061", "start_date": "2016-01-01T06:00:00Z", "end_date": "2017-06-30T05:00:00Z", "compute_allowed": 1000000} | 2017-02-15 05:00:00+0000 |
+      |           | allocation_source_compute_allowed_changed | {"allocation_source_name": "TG-ENG150061", "start_date": "2016-01-01T06:00:00Z", "end_date": "2017-06-30T05:00:00Z", "compute_allowed": 1000000} | 2017-02-15 05:00:00+0000 |
+      | user108   | user_allocation_source_created            | {"allocation_source_name": "TG-ENG150061"}                                                                                                       | 2017-02-15 05:00:00+0000 |
+      | user109   | user_allocation_source_created            | {"allocation_source_name": "TG-ENG150061"}                                                                                                       | 2017-02-15 05:00:00+0000 |
+      |           | allocation_source_created_or_renewed      | {"allocation_source_name": "TG-TRA160006", "start_date": "2017-01-22T06:00:00Z", "end_date": "2018-01-21T06:00:00Z", "compute_allowed": 600000}  | 2017-02-15 05:00:00+0000 |
+      |           | allocation_source_compute_allowed_changed | {"allocation_source_name": "TG-TRA160006", "start_date": "2017-01-22T06:00:00Z", "end_date": "2018-01-21T06:00:00Z", "compute_allowed": 600000}  | 2017-02-15 05:00:00+0000 |
+      | user109   | user_allocation_source_created            | {"allocation_source_name": "TG-TRA160006"}                                                                                                       | 2017-02-15 05:00:00+0000 |
+      | user111   | user_allocation_source_created            | {"allocation_source_name": "TG-TRA160006"}                                                                                                       | 2017-02-15 05:00:00+0000 |
+
   @skip
   Scenario: Generate reports
diff --git a/jetstream/features/steps/tas_api_steps.py b/jetstream/features/steps/tas_api_steps.py
@@ -1,4 +1,6 @@
 import copy
+import datetime
+import json
 from decimal import Decimal
 
 import mock
@@ -39,6 +41,7 @@ def _get_user_projects(context, url):
     data = {'status': 'success', 'message': None, 'result': user_projects}
     return data
 
+
 def _make_mock_tacc_api_get(context):
     def _mock_tacc_api_get(*args, **kwargs):
         url = args[0]
@@ -205,7 +208,7 @@ def should_have_allocation_source_snapshots(context):
     context.test.assertDictEqual(expected_allocation_source_snapshots, allocation_source_snapshots)
 
 
-@then(u'we should have the following user allocation sources')
+@step(u'we should have the following user allocation sources')
 def should_have_user_allocation_sources(context):
     expected_user_allocation_sources = [(row.cells[0], row.cells[1] or None,) for row in context.table]
     from core.models import UserAllocationSource
@@ -218,3 +221,21 @@ def should_have_user_allocation_sources(context):
                                    'allocation_source__name'
                                )]
     context.test.assertListEqual(expected_user_allocation_sources, user_allocation_sources)
+
+
+@step(u'we should have the following events')
+@step(u'we should have the following "{event_name}" events')
+def should_have_following_events(context, event_name=None):
+    expected_events = [dict(zip(row.headings, row.cells)) for row in context.table]
+    for expected_event in expected_events:
+        expected_event['payload'] = json.loads(expected_event['payload'])
+    from core.models import EventTable
+    query = EventTable.objects.all().order_by('id')
+    if event_name is not None:
+        query = query.filter(name=event_name)
+    events = [event for event in query.values('entity_id', 'name', 'payload', 'timestamp')]
+    for event in events:
+        event['timestamp'] = event['timestamp'].replace(microsecond=0)
+        event['timestamp'] = datetime.datetime.strftime(event['timestamp'], '%Y-%m-%d %H:%M:%S%z')
+    context.test.maxDiff = None
+    context.test.assertListEqual(expected_events, events)