Merge branch 'master' into letter-branding

app/celery/nightly_tasks.py

@@ -42,14 +42,14 @@ from app.cronitor import cronitor
 @notify_celery.task(name="remove_sms_email_jobs")
 @cronitor("remove_sms_email_jobs")
 @statsd(namespace="tasks")
-def remove_sms_email_csv_files(job_types):
+def remove_sms_email_csv_files():
     _remove_csv_files([EMAIL_TYPE, SMS_TYPE])
 
 
 @notify_celery.task(name="remove_letter_jobs")
 @cronitor("remove_letter_jobs")
 @statsd(namespace="tasks")
-def remove_letter_csv_files(job_types):
+def remove_letter_csv_files():
     _remove_csv_files([LETTER_TYPE])
 
 

manifest-delivery-base.yml

@@ -68,7 +68,7 @@ applications:
 
   - name: notify-delivery-worker-sender
     command: scripts/run_multi_worker_app_paas.sh celery multi start 3 -c 10 -A run_celery.notify_celery --loglevel=INFO -Q send-sms-tasks,send-email-tasks
-    memory: 2G
+    memory: 3G
     env:
       NOTIFY_APP_NAME: delivery-worker-sender
 

scripts/run_multi_worker_app_paas.sh

@@ -69,7 +69,10 @@ function get_celery_pids {
   # get the PIDs of the process whose parent is the root process
   # print only pid and their command, get the ones with "celery" in their name
   # and keep only these PIDs
+
+  set +o pipefail # so grep returning no matches does not premature fail pipe
   APP_PIDS=$(pgrep -P 1 | xargs ps -o pid=,command= -p | grep celery | cut -f1 -d/)
+  set -o pipefail # pipefail should be set everywhere else
 }
 
 function send_signal_to_celery_processes {
@@ -98,9 +101,32 @@ function start_logs_tail {
   echo "tail pid: ${LOGS_TAIL_PID}"
 }
 
+function ensure_celery_is_running {
+  if [ "${APP_PIDS}" = "" ]; then
+    echo "There are no celery processes running, this container is bad"
+
+    echo "Exporting CF information for diagnosis"
+
+    env | grep CF
+
+    echo "Sleeping 15 seconds for logs to get shipped"
+
+    sleep 15
+
+    echo "Killing awslogs_agent and tail"
+    kill -9 ${AWSLOGS_AGENT_PID}
+    kill -9 ${LOGS_TAIL_PID}
+
+    exit 1
+  fi
+}
+
 function run {
   while true; do
     get_celery_pids
+
+    ensure_celery_is_running
+
     for APP_PID in ${APP_PIDS}; do
         kill -0 ${APP_PID} 2&>/dev/null || return 1
     done

tests/app/celery/test_nightly_tasks.py

@@ -16,7 +16,8 @@ from app.celery.nightly_tasks import (
     delete_letter_notifications_older_than_seven_days,
     delete_sms_notifications_older_than_seven_days,
     raise_alert_if_letter_notifications_still_sending,
-    _remove_csv_files,
+    remove_letter_csv_files,
+    remove_sms_email_csv_files,
     remove_transformed_dvla_files,
     s3,
     send_daily_performance_platform_stats,
@@ -44,11 +45,7 @@ from tests.app.db import (
     create_service_data_retention
 )
 
-from tests.app.conftest import (
+from tests.app.conftest import datetime_in_past
-    sample_job as create_sample_job,
-    sample_notification_history as create_notification_history,
-    datetime_in_past
-)
 
 
 def mock_s3_get_list_match(bucket_name, subfolder='', suffix='', last_modified=None):
@@ -82,13 +79,13 @@ def test_will_remove_csv_files_for_jobs_older_than_seven_days(
     just_under_nine_days = nine_days_ago + timedelta(seconds=1)
     nine_days_one_second_ago = nine_days_ago - timedelta(seconds=1)
 
-    create_sample_job(notify_db, notify_db_session, created_at=nine_days_one_second_ago, archived=True)
+    create_job(sample_template, created_at=nine_days_one_second_ago, archived=True)
-    job1_to_delete = create_sample_job(notify_db, notify_db_session, created_at=eight_days_ago)
+    job1_to_delete = create_job(sample_template, created_at=eight_days_ago)
-    job2_to_delete = create_sample_job(notify_db, notify_db_session, created_at=just_under_nine_days)
+    job2_to_delete = create_job(sample_template, created_at=just_under_nine_days)
-    dont_delete_me_1 = create_sample_job(notify_db, notify_db_session, created_at=seven_days_ago)
+    dont_delete_me_1 = create_job(sample_template, created_at=seven_days_ago)
-    create_sample_job(notify_db, notify_db_session, created_at=just_under_seven_days)
+    create_job(sample_template, created_at=just_under_seven_days)
 
-    _remove_csv_files(job_types=[sample_template.template_type])
+    remove_sms_email_csv_files()
 
     assert s3.remove_job_from_s3.call_args_list == [
         call(job1_to_delete.service_id, job1_to_delete.id),
@@ -120,21 +117,15 @@ def test_will_remove_csv_files_for_jobs_older_than_retention_period(
     eight_days_ago = datetime.utcnow() - timedelta(days=8)
     thirty_one_days_ago = datetime.utcnow() - timedelta(days=31)
 
-    _create_job = partial(
+    job1_to_delete = create_job(sms_template_service_1, created_at=four_days_ago)
-        create_sample_job,
+    job2_to_delete = create_job(email_template_service_1, created_at=eight_days_ago)
-        notify_db,
+    create_job(email_template_service_1, created_at=four_days_ago)
-        notify_db_session,
-    )
 
-    job1_to_delete = _create_job(service=service_1, template=sms_template_service_1, created_at=four_days_ago)
+    create_job(email_template_service_2, created_at=eight_days_ago)
-    job2_to_delete = _create_job(service=service_1, template=email_template_service_1, created_at=eight_days_ago)
+    job3_to_delete = create_job(email_template_service_2, created_at=thirty_one_days_ago)
-    _create_job(service=service_1, template=email_template_service_1, created_at=four_days_ago)
+    job4_to_delete = create_job(sms_template_service_2, created_at=eight_days_ago)
 
-    _create_job(service=service_2, template=email_template_service_2, created_at=eight_days_ago)
+    remove_sms_email_csv_files()
-    job3_to_delete = _create_job(service=service_2, template=email_template_service_2, created_at=thirty_one_days_ago)
-    job4_to_delete = _create_job(service=service_2, template=sms_template_service_2, created_at=eight_days_ago)
-
-    _remove_csv_files(job_types=[SMS_TYPE, EMAIL_TYPE])
 
     s3.remove_job_from_s3.assert_has_calls([
         call(job1_to_delete.service_id, job1_to_delete.id),
@@ -158,7 +149,7 @@ def test_remove_csv_files_filters_by_type(mocker, sample_service):
     job_to_delete = create_job(template=letter_template, created_at=eight_days_ago)
     create_job(template=sms_template, created_at=eight_days_ago)
 
-    _remove_csv_files(job_types=[LETTER_TYPE])
+    remove_letter_csv_files()
 
     assert s3.remove_job_from_s3.call_args_list == [
         call(job_to_delete.service_id, job_to_delete.id),
@@ -265,30 +256,26 @@ def test_send_total_sent_notifications_to_performance_platform_calls_with_correc
         notify_db,
         notify_db_session,
         sample_template,
+        sample_email_template,
         mocker
 ):
+    sms = sample_template
+    email = sample_email_template
+
     perf_mock = mocker.patch(
         'app.celery.nightly_tasks.total_sent_notifications.send_total_notifications_sent_for_day_stats')  # noqa
 
-    notification_history = partial(
+    create_notification(email, status='delivered')
-        create_notification_history,
+    create_notification(sms, status='delivered')
-        notify_db,
-        notify_db_session,
-        sample_template,
-        status='delivered'
-    )
-
-    notification_history(notification_type='email')
-    notification_history(notification_type='sms')
 
     # Create some notifications for the day before
     yesterday = datetime(2016, 1, 10, 15, 30, 0, 0)
     with freeze_time(yesterday):
-        notification_history(notification_type='sms')
+        create_notification(sms, status='delivered')
-        notification_history(notification_type='sms')
+        create_notification(sms, status='delivered')
-        notification_history(notification_type='email')
+        create_notification(email, status='delivered')
-        notification_history(notification_type='email')
+        create_notification(email, status='delivered')
-        notification_history(notification_type='email')
+        create_notification(email, status='delivered')
 
     with patch.object(
             PerformancePlatformClient,