x/poppy
Code Issues Proposed changes

Fixes race condition in DELETE endpoint

Browse Source 
Change-Id: Ic48c6b31e2beae0de57ed173d0fbf4e7ab651a22
Closes-Bug: 1421349
This commit is contained in:
Obulpathi
2015-02-12 15:51:18 -05:00
parent 535578d432
commit 54ac272981
7 changed files with 74 additions and 178 deletions
Download Patch File Download Diff File Expand all files Collapse all files

33
poppy/manager/default/service_async_workers/delete_service_worker.py
View File

@@ -14,36 +14,40 @@
# limitations under the License.
import argparse
import json
import logging
import os
import sys
from oslo.config import cfg
from poppy import bootstrap
from poppy.openstack.common import log
from poppy.transport.pecan.models.request import (
provider_details as req_provider_details
)
LOG = log.getLogger(__file__)
conf = cfg.CONF
conf(project='poppy', prog='poppy', args=[])
def service_delete_worker(provider_details,
project_id, service_id):
def service_delete_worker(project_id, service_id):
LOG.logger.setLevel(logging.INFO)
bootstrap_obj = bootstrap.Bootstrap(conf)
service_controller = bootstrap_obj.manager.services_controller
provider_details = json.loads(provider_details)
storage_controller = service_controller.storage_controller
storage_controller._driver.connect()
try:
service_obj = storage_controller.get(project_id, service_id)
except ValueError:
LOG.info('Deleting service {0} from Poppy failed. '
'No such service exists'.format(service_id))
sys.exit(0)
provider_details = service_obj.provider_details
responders = []
# try to delete all service from each provider presented
# in provider_details
for provider in provider_details:
provider_details[provider] = (
req_provider_details.load_from_json(provider_details[provider]))
LOG.info('Starting to delete service from %s' % provider)
responder = service_controller.provider_wrapper.delete(
service_controller._driver.providers[provider.lower()],
@@ -75,18 +79,13 @@ def service_delete_worker(provider_details,
# delete service successful, remove this provider detail record
del provider_details[provider_name]
service_controller.storage_controller._driver.connect()
if provider_details != {}:
# Store failed provider details with error infomation for further
# action, maybe for debug and/or support.
LOG.info('Delete failed for one or more providers'
'Updating poppy service provider details for %s' %
service_id)
service_controller.storage_controller.update_provider_details(
project_id,
service_id,
provider_details)
storage_controller.update(project_id, service_id, service_obj)
# always delete from Poppy. Provider Details will contain
# any provider issues that may have occurred.
@@ -105,12 +104,10 @@ if __name__ == '__main__':
parser = argparse.ArgumentParser(description='Delete service async worker'
' script arg parser')
parser.add_argument('provider_details', action="store")
parser.add_argument('project_id', action="store")
parser.add_argument('service_id', action="store")
result = parser.parse_args()
provider_details = result.provider_details
project_id = result.project_id
service_id = result.service_id
service_delete_worker(provider_details, project_id, service_id)
service_delete_worker(project_id, service_id)

16
poppy/manager/default/services.py
View File

@@ -202,18 +202,14 @@ class DefaultServicesController(base.ServicesController):
:param service_id
:raises LookupError
"""
provider_details = self._get_provider_details(project_id, service_id)
service_obj = self.storage_controller.get(project_id, service_id)
# change each provider detail's status to delete_in_progress
# TODO(tonytan4ever): what if this provider is in 'failed' status?
# Maybe raising a 400 error here ?
for provider in provider_details:
provider_details[provider].status = "delete_in_progress"
for provider in service_obj.provider_details:
service_obj.provider_details[provider].status = (
u'delete_in_progress')
self.storage_controller.update_provider_details(
project_id,
service_id,
provider_details)
self.storage_controller.update(project_id, service_id, service_obj)
proxy_path = os.path.join(os.path.dirname(os.path.abspath(__file__)),
'service_async_workers',
@@ -228,8 +224,6 @@ class DefaultServicesController(base.ServicesController):
cmd_list = [executable,
proxy_path,
script_path,
json.dumps(dict([(k, v.to_dict())
for k, v in provider_details.items()])),
project_id, service_id]
LOG.info('Starting delete service subprocess: %s' % cmd_list)
p = subprocess.Popen(cmd_list, env=os.environ.copy())

16
poppy/model/helpers/provider_details.py
View File

@@ -13,6 +13,11 @@
# See the License for the specific language governing permissions and
# limitations under the License.
try:
import ordereddict as collections
except ImportError: # pragma: no cover
import collections # pragma: no cover
from poppy.model import common
VALID_STATUSES = [
@@ -94,6 +99,17 @@ class ProviderDetail(common.DictSerializableModel):
def error_message(self, value):
self._error_message = value
def to_dict(self):
result = collections.OrderedDict()
result["id"] = self.provider_service_id
result["access_urls"] = self.access_urls
result["status"] = self.status
result["name"] = self.name
result["error_info"] = self.error_info
result["error_message"] = self.error_message
return result
@classmethod
def init_from_dict(cls, dict_obj):
"""Construct a model instance from a dictionary.

6
poppy/storage/cassandra/services.py
View File

@@ -380,6 +380,10 @@ class ServicesController(base.ServicesController):
restrictions = [json.dumps(r.to_dict())
for r in service_obj.restrictions]
pds = {provider:
json.dumps(service_obj.provider_details[provider].to_dict())
for provider in service_obj.provider_details}
# updates an existing service
args = {
'project_id': project_id,
@@ -390,7 +394,7 @@ class ServicesController(base.ServicesController):
'origins': origins,
'caching_rules': caching_rules,
'restrictions': restrictions,
'provider_details': {}
'provider_details': pds
}
stmt = query.SimpleStatement(

3
poppy/transport/pecan/controllers/v1/services.py
View File

@@ -185,10 +185,13 @@ class ServicesController(base.Controller, hooks.HookController):
)
def delete(self, service_id):
services_controller = self._driver.manager.services_controller
try:
services_controller.delete(self.project_id, service_id)
except LookupError as e:
pecan.abort(404, detail=str(e))
except ValueError as e:
pecan.abort(404, detail=str(e))
return pecan.Response(None, 202)

15
tests/api/services/test_services.py
View File

@@ -425,6 +425,21 @@ class TestServiceActions(base.TestBase):
self.skipTest(
'Until we figure out how to create provider side failure.')
def test_delete_service_race_condition(self):
resp1 = self.client.delete_service(location=self.service_url)
resp2 = self.client.delete_service(location=self.service_url)
self.assertEqual(resp1.status_code, 202)
self.assertEqual(resp2.status_code, 202)
time.sleep(10)
exception = False
try:
self.client.get_service(location=self.service_url)
except Exception:
exception = True
self.assertFalse(exception)
def test_get_service(self):
resp = self.client.get_service(location=self.service_url)
self.assertEqual(resp.status_code, 200)

163
tests/unit/manager/default/test_services.py
View File

@@ -22,7 +22,6 @@ from oslo.config import cfg
from poppy.manager.default import driver
from poppy.manager.default.service_async_workers import create_service_worker
from poppy.manager.default.service_async_workers import delete_service_worker
from poppy.manager.default.service_async_workers import purge_service_worker
from poppy.manager.default import services
from poppy.model import flavor
@@ -34,9 +33,9 @@ from tests.unit import base
@ddt.ddt
class DefaultManagerServiceTests(base.TestCase):
@mock.patch('poppy.storage.base.driver.StorageDriverBase')
@mock.patch('poppy.dns.base.driver.DNSDriverBase')
def setUp(self, mock_driver, mock_dns):
@mock.patch('poppy.storage.base.driver.StorageDriverBase')
def setUp(self, mock_storage, mock_dns):
super(DefaultManagerServiceTests, self).setUp()
# create mocked config and driver
@@ -55,7 +54,7 @@ class DefaultManagerServiceTests(base.TestCase):
mock_providers = mock.MagicMock()
mock_providers.__getitem__.side_effect = get_provider_by_name
manager_driver = driver.DefaultManagerDriver(conf,
mock_driver,
mock_storage,
mock_providers,
mock_dns)
@@ -98,6 +97,8 @@ class DefaultManagerServiceTests(base.TestCase):
"flavor_id": "standard"
}
self.service_obj = service.load_from_json(self.service_json)
def test_create(self):
service_obj = service.load_from_json(self.service_json)
# fake one return value
@@ -299,160 +300,26 @@ class DefaultManagerServiceTests(base.TestCase):
provider_detail_dict = json.loads(
provider_details_json[provider_name]
)
provider_service_id = provider_detail_dict.get("id", None)
access_urls = provider_detail_dict.get("access_urls", None)
status = provider_detail_dict.get("status", u'deployed')
provider_service_id = provider_detail_dict.get('id', None)
access_urls = provider_detail_dict.get('access_urls', [])
status = provider_detail_dict.get('status', u'unknown')
provider_detail_obj = provider_details.ProviderDetail(
provider_service_id=provider_service_id,
access_urls=access_urls,
status=status)
self.provider_details[provider_name] = provider_detail_obj
self.sc.storage_controller._get_provider_details.return_value = (
self.provider_details
)
self.service_obj.provider_details = self.provider_details
sc = self.sc.storage_controller
sc.get.return_value = self.service_obj
self.sc.delete(self.project_id, self.service_id)
# ensure the manager calls the storage driver with the appropriate data
# break into 2 lines.
sc = self.sc.storage_controller
sc.get_provider_details.assert_called_once_with(
self.project_id,
self.service_id)
@ddt.file_data('data_provider_details.json')
def test_detele_service_worker_success(self, provider_details_json):
self.provider_details = {}
for provider_name in provider_details_json:
provider_detail_dict = json.loads(
provider_details_json[provider_name]
)
provider_service_id = provider_detail_dict.get("id", None)
access_urls = provider_detail_dict.get("access_urls", None)
status = provider_detail_dict.get("status", u'deployed')
provider_detail_obj = provider_details.ProviderDetail(
provider_service_id=provider_service_id,
access_urls=access_urls,
status=status)
self.provider_details[provider_name] = provider_detail_obj
providers = self.sc._driver.providers
def get_provider_extension_by_name(name):
if name == 'cloudfront':
return_mock = {
'CloudFront': {
'id':
'08d2e326-377e-11e4-b531-3c15c2b8d2d6',
}
}
service_controller = mock.Mock(
delete=mock.Mock(return_value=return_mock)
)
return mock.Mock(obj=mock.Mock(
provider_name='CloudFront',
service_controller=service_controller)
)
elif name == 'maxcdn':
return_mock = {
'MaxCDN': {'id': "pullzone345"}
}
service_controller = mock.Mock(
delete=mock.Mock(return_value=return_mock)
)
return mock.Mock(obj=mock.Mock(
provider_name='MaxCDN',
service_controller=service_controller)
)
else:
return_mock = {
name.title(): {
'id':
'08d2e326-377e-11e4-b531-3c15c2b8d2d6',
}
}
service_controller = mock.Mock(
delete=mock.Mock(return_value=return_mock)
)
return mock.Mock(obj=mock.Mock(
provider_name=name.title(),
service_controller=service_controller)
)
providers.__getitem__.side_effect = get_provider_extension_by_name
delete_service_worker.service_delete_worker(
json.dumps(dict([(k, v.to_dict())
for k, v in
self.provider_details.items()])),
self.project_id,
self.service_id)
@ddt.file_data('data_provider_details.json')
def test_delete_service_worker_with_error(self, provider_details_json):
self.provider_details = {}
for provider_name in provider_details_json:
provider_detail_dict = json.loads(
provider_details_json[provider_name]
)
provider_service_id = provider_detail_dict.get("id", None)
access_urls = provider_detail_dict.get("access_urls", None)
status = provider_detail_dict.get("status", u'deployed')
provider_detail_obj = provider_details.ProviderDetail(
provider_service_id=provider_service_id,
access_urls=access_urls,
status=status)
self.provider_details[provider_name] = provider_detail_obj
providers = self.sc._driver.providers
def get_provider_extension_by_name(name):
if name == 'cloudfront':
return mock.Mock(
obj=mock.Mock(
provider_name='CloudFront',
service_controller=mock.Mock(
delete=mock.Mock(
return_value={
'CloudFront': {
'id':
'08d2e326-377e-11e4-b531-3c15c2b8d2d6',
}}),
)))
elif name == 'maxcdn':
return mock.Mock(obj=mock.Mock(
provider_name='MaxCDN',
service_controller=mock.Mock(
delete=mock.Mock(return_value={
'MaxCDN': {'error': "fail to create servcice",
'error_detail':
'MaxCDN delete service'
' failed because of XYZ'}})
)
))
else:
return mock.Mock(
obj=mock.Mock(
provider_name=name.title(),
service_controller=mock.Mock(
delete=mock.Mock(
return_value={
name.title(): {
'id':
'08d2e326-377e-11e4-b531-3c15c2b8d2d6',
}}),
)))
providers.__getitem__.side_effect = get_provider_extension_by_name
delete_service_worker.service_delete_worker(
json.dumps(dict([(k, v.to_dict()
)
for k, v in
self.provider_details.items()])),
self.project_id,
self.service_id)
sc.get.assert_called_once_with(self.project_id, self.service_id)
sc.update.assert_called_once_with(self.project_id,
self.service_id,
self.service_obj)
@ddt.file_data('data_provider_details.json')
def test_purge(self, provider_details_json):