From 3a23c547238af9410042e09c34d6f822093649e9 Mon Sep 17 00:00:00 2001 From: dandds Date: Wed, 18 Sep 2019 11:12:48 -0400 Subject: [PATCH 1/4] Add a beat processing schedule for environment provisioning jobs. The beat schedule is set to once per minute for each of the three environment provisioning tasks. Adding a beat schedule surfaced two problems that are addressed here with the following changes: - Commit the SQLALchemy session in order to release the environment lock. Otherwise the change to the `claimed_until` field is not persisted. - Set `none_as_null` on the JSOB fields on the `Environment`. This avoids problems with querying on Postgres JSON fields that are empty. This also adds a small change to the development command for the Celery worker. Multiple child processes were executing the beat jobs, which lead to exceptions for environment locks and confusing log output. This contrains the dev command to a single Celery worker. --- atst/domain/environments.py | 8 ++++---- atst/models/environment.py | 4 ++-- atst/models/utils.py | 1 + atst/queue.py | 15 ++++++++++++++- script/dev_queue | 2 +- 5 files changed, 22 insertions(+), 8 deletions(-) diff --git a/atst/domain/environments.py b/atst/domain/environments.py index 829fe3fa..ba723cc5 100644 --- a/atst/domain/environments.py +++ b/atst/domain/environments.py @@ -1,4 +1,4 @@ -from sqlalchemy import text, func, or_ +from sqlalchemy import func, or_ from sqlalchemy.orm.exc import NoResultFound from typing import List from uuid import UUID @@ -130,7 +130,7 @@ class Environments(object): results = ( cls.base_provision_query(now) .filter(Environment.cloud_id != None) - .filter(Environment.root_user_info == text("'null'")) + .filter(Environment.root_user_info == None) ).all() return [id_ for id_, in results] @@ -143,7 +143,7 @@ class Environments(object): results = ( cls.base_provision_query(now) .filter(Environment.cloud_id != None) - .filter(Environment.root_user_info != text("'null'")) - .filter(Environment.baseline_info == text("'null'")) + .filter(Environment.root_user_info != None) + .filter(Environment.baseline_info == None) ).all() return [id_ for id_, in results] diff --git a/atst/models/environment.py b/atst/models/environment.py index fb30e6d4..4fa11bfc 100644 --- a/atst/models/environment.py +++ b/atst/models/environment.py @@ -26,8 +26,8 @@ class Environment( creator = relationship("User") cloud_id = Column(String) - root_user_info = Column(JSONB) - baseline_info = Column(JSONB) + root_user_info = Column(JSONB(none_as_null=True)) + baseline_info = Column(JSONB(none_as_null=True)) claimed_until = Column(TIMESTAMP(timezone=True)) diff --git a/atst/models/utils.py b/atst/models/utils.py index a3df73e3..6059d33e 100644 --- a/atst/models/utils.py +++ b/atst/models/utils.py @@ -47,3 +47,4 @@ def claim_for_update(resource, minutes=30): db.session.query(Model).filter(Model.id == resource.id).filter( Model.claimed_until != None ).update({"claimed_until": None}, synchronize_session="fetch") + db.session.commit() diff --git a/atst/queue.py b/atst/queue.py index d71532b1..5fc12a3f 100644 --- a/atst/queue.py +++ b/atst/queue.py @@ -5,7 +5,20 @@ celery = Celery(__name__) def update_celery(celery, app): celery.conf.update(app.config) - celery.conf.CELERYBEAT_SCHEDULE = {} + celery.conf.CELERYBEAT_SCHEDULE = { + "beat-dispatch_create_environment": { + "task": "atst.jobs.dispatch_create_environment", + "schedule": 60, + }, + "beat-dispatch_create_atat_admin_user": { + "task": "atst.jobs.dispatch_create_atat_admin_user", + "schedule": 60, + }, + "beat-dispatch_create_environment_baseline": { + "task": "atst.jobs.dispatch_create_environment_baseline", + "schedule": 60, + }, + } class ContextTask(celery.Task): def __call__(self, *args, **kwargs): diff --git a/script/dev_queue b/script/dev_queue index 4e8720ee..ef0d5ccc 100755 --- a/script/dev_queue +++ b/script/dev_queue @@ -4,7 +4,7 @@ set -e -WORKER="pipenv run celery -A celery_worker.celery worker --loglevel=info -B" +WORKER="pipenv run celery -A celery_worker.celery worker --loglevel=info -B -c 1" if [[ `command -v entr` ]]; then find atst | entr -r $WORKER From 2fc7a0d460bc250bb5434e77e942aed5d4150a0a Mon Sep 17 00:00:00 2001 From: richard-dds Date: Wed, 18 Sep 2019 14:39:00 -0400 Subject: [PATCH 2/4] Add small sleep to claims test to avoid random failures --- tests/test_jobs.py | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/tests/test_jobs.py b/tests/test_jobs.py index 09c3e645..9aaa0ee9 100644 --- a/tests/test_jobs.py +++ b/tests/test_jobs.py @@ -3,8 +3,8 @@ import pytest from uuid import uuid4 from unittest.mock import Mock from threading import Thread +from time import sleep -from atst.models import Environment from atst.domain.csp.cloud import MockCloudProvider from atst.jobs import ( RecordEnvironmentFailure, @@ -19,12 +19,7 @@ from atst.jobs import ( ) from atst.models.utils import claim_for_update from atst.domain.exceptions import ClaimFailedException -from tests.factories import ( - EnvironmentFactory, - EnvironmentRoleFactory, - UserFactory, - PortfolioFactory, -) +from tests.factories import EnvironmentFactory, EnvironmentRoleFactory, PortfolioFactory def test_environment_job_failure(celery_app, celery_worker): @@ -262,6 +257,7 @@ def test_claim_for_update(session): class FirstThread(Thread): def run(self): with claim_for_update(environment): + sleep(0.1) # doing some work satisfied_claims.append("FirstThread") class SecondThread(Thread): From 82c34ee9b1552e539f02e6e085bc433262b5e982 Mon Sep 17 00:00:00 2001 From: dandds Date: Wed, 18 Sep 2019 15:50:26 -0400 Subject: [PATCH 3/4] Display real environment processing status. --- atst/models/environment.py | 4 ++++ atst/routes/applications/settings.py | 1 + templates/portfolios/applications/settings.html | 9 ++++----- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/atst/models/environment.py b/atst/models/environment.py index 4fa11bfc..f3d85502 100644 --- a/atst/models/environment.py +++ b/atst/models/environment.py @@ -68,6 +68,10 @@ class Environment( else: return self.ProvisioningStatus.COMPLETED + @property + def is_pending(self): + return self.provisioning_status == self.ProvisioningStatus.PENDING + def __repr__(self): return "".format( self.name, diff --git a/atst/routes/applications/settings.py b/atst/routes/applications/settings.py index 8ea3b612..f6849fc1 100644 --- a/atst/routes/applications/settings.py +++ b/atst/routes/applications/settings.py @@ -25,6 +25,7 @@ def get_environments_obj_for_app(application): env_data = { "id": env.id, "name": env.name, + "pending": env.is_pending, "edit_form": EditEnvironmentForm(obj=env), "member_count": len(env.roles), "members": [env_role.application_role.user_name for env_role in env.roles], diff --git a/templates/portfolios/applications/settings.html b/templates/portfolios/applications/settings.html index a4b1c38d..b297765d 100644 --- a/templates/portfolios/applications/settings.html +++ b/templates/portfolios/applications/settings.html @@ -218,7 +218,6 @@