Merge pull request #497 from coreos-inc/delrepofix

Fix interleaved repo delete with RAC via a transaction
This commit is contained in:
josephschorr 2015-09-16 16:53:15 -04:00
commit b91660f87b
3 changed files with 36 additions and 13 deletions

View file

@ -119,6 +119,7 @@ db = Proxy()
read_slave = Proxy() read_slave = Proxy()
db_random_func = CallableProxy() db_random_func = CallableProxy()
db_for_update = CallableProxy() db_for_update = CallableProxy()
db_transaction = CallableProxy()
def validate_database_url(url, db_kwargs, connect_timeout=5): def validate_database_url(url, db_kwargs, connect_timeout=5):
@ -164,6 +165,10 @@ def configure(config_object):
if read_slave_uri is not None: if read_slave_uri is not None:
read_slave.initialize(_db_from_url(read_slave_uri, db_kwargs)) read_slave.initialize(_db_from_url(read_slave_uri, db_kwargs))
def _db_transaction():
return config_object['DB_TRANSACTION_FACTORY'](db)
db_transaction.initialize(_db_transaction)
def random_string_generator(length=16): def random_string_generator(length=16):
def random_string(): def random_string():
@ -373,6 +378,7 @@ class Repository(BaseModel):
return sorted_models.index(cmp_fk.model_class.__name__) return sorted_models.index(cmp_fk.model_class.__name__)
filtered_ops.sort(key=sorted_model_key) filtered_ops.sort(key=sorted_model_key)
with db_transaction():
for query, fk in filtered_ops: for query, fk in filtered_ops:
model = fk.model_class model = fk.model_class
if fk.null and not delete_nullable: if fk.null and not delete_nullable:

View file

@ -1,4 +1,4 @@
from data.database import db from data.database import db, db_transaction
class DataModelException(Exception): class DataModelException(Exception):
@ -80,10 +80,6 @@ class Config(object):
config = Config() config = Config()
def db_transaction():
return config.app_config['DB_TRANSACTION_FACTORY'](db)
# There MUST NOT be any circular dependencies between these subsections. If there are fix it by # There MUST NOT be any circular dependencies between these subsections. If there are fix it by
# moving the minimal number of things to _basequery # moving the minimal number of things to _basequery
# TODO document the methods and modules for each one of the submodules below. # TODO document the methods and modules for each one of the submodules below.

View file

@ -1,6 +1,7 @@
# coding=utf-8 # coding=utf-8
import unittest import unittest
import datetime
import json as py_json import json as py_json
from urllib import urlencode from urllib import urlencode
@ -15,6 +16,7 @@ from endpoints.trigger import BuildTriggerHandler
from app import app from app import app
from initdb import setup_database_for_testing, finished_database_for_testing from initdb import setup_database_for_testing, finished_database_for_testing
from data import database, model from data import database, model
from data.database import RepositoryActionCount
from endpoints.api.team import TeamMember, TeamMemberList, TeamMemberInvite, OrganizationTeam from endpoints.api.team import TeamMember, TeamMemberList, TeamMemberInvite, OrganizationTeam
from endpoints.api.tag import RepositoryTagImages, RepositoryTag, RevertTag, ListRepositoryTags from endpoints.api.tag import RepositoryTagImages, RepositoryTag, RevertTag, ListRepositoryTags
@ -1468,6 +1470,10 @@ class TestDeleteRepository(ApiTestCase):
def test_deleterepo(self): def test_deleterepo(self):
self.login(ADMIN_ACCESS_USER) self.login(ADMIN_ACCESS_USER)
# Verify the repo exists.
self.getResponse(Repository,
params=dict(repository=self.SIMPLE_REPO))
self.deleteResponse(Repository, params=dict(repository=self.SIMPLE_REPO)) self.deleteResponse(Repository, params=dict(repository=self.SIMPLE_REPO))
# Verify the repo was deleted. # Verify the repo was deleted.
@ -1478,6 +1484,10 @@ class TestDeleteRepository(ApiTestCase):
def test_deleterepo2(self): def test_deleterepo2(self):
self.login(ADMIN_ACCESS_USER) self.login(ADMIN_ACCESS_USER)
# Verify the repo exists.
self.getResponse(Repository,
params=dict(repository=self.COMPLEX_REPO))
self.deleteResponse(Repository, params=dict(repository=self.COMPLEX_REPO)) self.deleteResponse(Repository, params=dict(repository=self.COMPLEX_REPO))
# Verify the repo was deleted. # Verify the repo was deleted.
@ -1488,7 +1498,11 @@ class TestDeleteRepository(ApiTestCase):
def test_populate_and_delete_repo(self): def test_populate_and_delete_repo(self):
self.login(ADMIN_ACCESS_USER) self.login(ADMIN_ACCESS_USER)
# Make sure the repository has come images and tags. # Verify the repo exists.
self.getResponse(Repository,
params=dict(repository=self.COMPLEX_REPO))
# Make sure the repository has some images and tags.
self.assertTrue(len(list(model.image.get_repository_images(ADMIN_ACCESS_USER, 'complex'))) > 0) self.assertTrue(len(list(model.image.get_repository_images(ADMIN_ACCESS_USER, 'complex'))) > 0)
self.assertTrue(len(list(model.tag.list_repository_tags(ADMIN_ACCESS_USER, 'complex'))) > 0) self.assertTrue(len(list(model.tag.list_repository_tags(ADMIN_ACCESS_USER, 'complex'))) > 0)
@ -1524,6 +1538,13 @@ class TestDeleteRepository(ApiTestCase):
model.repository.create_email_authorization_for_repo(ADMIN_ACCESS_USER, 'complex', 'a@b.com') model.repository.create_email_authorization_for_repo(ADMIN_ACCESS_USER, 'complex', 'a@b.com')
model.repository.create_email_authorization_for_repo(ADMIN_ACCESS_USER, 'complex', 'b@c.com') model.repository.create_email_authorization_for_repo(ADMIN_ACCESS_USER, 'complex', 'b@c.com')
# Create some repository action count entries.
RepositoryActionCount.create(repository=repository, date=datetime.datetime.now(), count=1)
RepositoryActionCount.create(repository=repository,
date=datetime.datetime.now() - datetime.timedelta(days=1), count=2)
RepositoryActionCount.create(repository=repository,
date=datetime.datetime.now() - datetime.timedelta(days=3), count=6)
# Delete the repository. # Delete the repository.
self.deleteResponse(Repository, params=dict(repository=self.COMPLEX_REPO)) self.deleteResponse(Repository, params=dict(repository=self.COMPLEX_REPO))