openstack
/
tuning-box
Code Issues Proposed changes

Remove fake root hierarchy level values 

We don't really need them, they can be represented by parent_id=None in
their childs. This prevents race condition in flat environment (with
only "nodes" level), added comments about how to properly fix race
condition.

TODO: check that migration and code work on real environment

Closes-bug: 1569353
Change-Id: I16740b006b8276f01504eb95997319ab610dfe1c
This commit is contained in:
Yuriy Taraday 2016-04-12 21:29:27 +03:00
parent 46560c1a3f
commit 4c9e4eba61
5 changed files with 189 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
tuning_box/app.py
View File

@ -120,17 +120,13 @@ class Environment(flask_restful.Resource):
def iter_environment_level_values(environment, levels):
env_levels = db.EnvironmentHierarchyLevel.get_for_environment(environment)
level_pairs = itertools.chain(
[(None, (None, None))], # root level
zip(env_levels, levels),
)
level_pairs = zip(env_levels, levels)
parent_level_value = None
for env_level, (level_name, level_value) in level_pairs:
if env_level:
if env_level.name != level_name:
raise exceptions.BadRequest(
"Unexpected level name '%s'. Expected '%s'." % (
level_name, env_level.name))
if env_level.name != level_name:
raise exceptions.BadRequest(
"Unexpected level name '%s'. Expected '%s'." % (
level_name, env_level.name))
level_value_db = db.get_or_create(
db.EnvironmentHierarchyLevelValue,
level=env_level,
@ -142,6 +138,7 @@ def iter_environment_level_values(environment, levels):
def get_environment_level_value(environment, levels):
level_value = None
for level_value in iter_environment_level_values(environment, levels):
pass
return level_value
@ -197,7 +194,7 @@ class ResourceValues(flask_restful.Resource):
environment=environment,
).all()
result = {}
for level_value in level_values:
for level_value in itertools.chain([None], level_values):
for resource_value in resource_values:
if resource_value.level_value == level_value:
result.update(resource_value.values)
@ -205,10 +202,14 @@ class ResourceValues(flask_restful.Resource):
break
return result
else:
if not level_values:
level_value = None
else:
level_value = level_values[-1]
resource_values = db.ResourceValues.query.filter_by(
resource_definition=resdef,
environment=environment,
level_value=level_values[-1],
level_value=level_value,
).one_or_none()
if not resource_values:
return {}
@ -244,7 +245,7 @@ class ResourceOverrides(flask_restful.Resource):
def get(self, environment_id, resource_id_or_name, levels):
environment = db.Environment.query.get_or_404(environment_id)
level_values = list(iter_environment_level_values(environment, levels))
level_value = get_environment_level_value(environment, levels)
# TODO(yorik-sar): filter by environment
resdef = db.ResourceDefinition.query.get_by_id_or_name(
resource_id_or_name)
@ -259,7 +260,7 @@ class ResourceOverrides(flask_restful.Resource):
resource_values = db.ResourceValues.query.filter_by(
resource_definition=resdef,
environment=environment,
level_value=level_values[-1],
level_value=level_value,
).one_or_none()
if not resource_values:
return {}

3
tuning_box/db.py
View File

@ -184,6 +184,7 @@ class EnvironmentHierarchyLevelValue(ModelMixin, db.Model):
def parent(cls):
return db.relationship(cls, remote_side=cls.id)
# TODO(yorik-sar): add UniqueConstraint for all fields
__repr_attrs__ = ('id', 'level', 'parent', 'value')
@ -211,6 +212,8 @@ def get_or_create(cls, **attrs):
if not item:
item = cls(**attrs)
db.session.add(item)
# TODO(yorik-sar): handle constraints failure in case of
# race condition
return item

90
tuning_box/migrations/versions/9ae15c85fa92_remove_fake_root_hierarchy_level_values.py Normal file
View File

@ -0,0 +1,90 @@
# Licensed under the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License. You may obtain
# a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
"""Remove fake root hierarchy level values
Revision ID: 9ae15c85fa92
Revises: d054eefc4c5b
Create Date: 2016-04-12 20:22:06.323291
"""
# revision identifiers, used by Alembic.
revision = '9ae15c85fa92'
down_revision = 'd054eefc4c5b'
branch_labels = None
depends_on = None
from alembic import context
from alembic import op
import sqlalchemy as sa
from sqlalchemy.ext import automap
def _get_autobase(table_prefix, bind):
metadata = sa.MetaData(bind=bind)
table_name = table_prefix + 'environment_hierarchy_level_value'
metadata.reflect(only=[table_name])
AutoBase = automap.automap_base(metadata=metadata)
def classname_for_table(base, refl_table_name, table):
assert refl_table_name.startswith(table_prefix)
noprefix_name = refl_table_name[len(table_prefix):]
uname = u"".join(s.capitalize() for s in noprefix_name.split('_'))
if not isinstance(uname, str):
return uname.encode('utf-8')
else:
return uname
AutoBase.prepare(classname_for_table=classname_for_table)
return AutoBase
def _get_ehlv_class():
table_prefix = context.config.get_main_option('table_prefix')
bind = op.get_bind()
AutoBase = _get_autobase(table_prefix, bind)
return AutoBase.classes.EnvironmentHierarchyLevelValue
def _get_session():
return sa.orm.Session(bind=op.get_bind(), autocommit=True)
def upgrade():
EHLV = _get_ehlv_class()
session = _get_session()
with session.begin():
fake_roots = session.query(EHLV) \
.filter_by(level_id=None, parent_id=None, value=None) \
.all()
if fake_roots:
fake_root_ids = [r.id for r in fake_roots]
session.query(EHLV) \
.filter(EHLV.parent_id.in_(fake_root_ids)) \
.update({EHLV.parent_id: None}, synchronize_session=False)
for r in fake_roots:
session.delete(r)
def downgrade():
EHLV = _get_ehlv_class()
session = _get_session()
with session.begin():
fake_root = EHLV(level_id=None, parent_id=None, value=None)
session.add(fake_root)
session.flush()
session.query(EHLV) \
.filter(EHLV.parent_id == None, # noqa
EHLV.id != fake_root.id) \
.update({EHLV.parent_id: fake_root.id},
synchronize_session=False)

21
tuning_box/tests/test_app.py
View File

@ -237,9 +237,7 @@ class TestApp(base.TestCase):
db.Environment(id=9),
[],
)
self.assertIsNone(level_value.level)
self.assertIsNone(level_value.parent)
self.assertIsNone(level_value.value)
self.assertIsNone(level_value)
def test_get_environment_level_value_deep(self):
self._fixture()
@ -254,10 +252,7 @@ class TestApp(base.TestCase):
level_value = level_value.parent
self.assertEqual(level_value.level.name, 'lvl1')
self.assertEqual(level_value.value, 'val1')
level_value = level_value.parent
self.assertIsNone(level_value.level)
self.assertIsNone(level_value.parent)
self.assertIsNone(level_value.value)
def test_get_environment_level_value_bad_level(self):
self._fixture()
@ -284,9 +279,7 @@ class TestApp(base.TestCase):
environment_id=9, resource_definition_id=5).one_or_none()
self.assertIsNotNone(resource_values)
self.assertEqual(resource_values.values, {'k': 'v'})
self.assertIsNone(resource_values.level_value.level)
self.assertIsNone(resource_values.level_value.parent)
self.assertIsNone(resource_values.level_value.value)
self.assertIsNone(resource_values.level_value)
def test_put_resource_values_deep(self):
self._fixture()
@ -307,10 +300,7 @@ class TestApp(base.TestCase):
level_value = level_value.parent
self.assertEqual(level_value.level.name, 'lvl1')
self.assertEqual(level_value.value, 'val1')
level_value = level_value.parent
self.assertIsNone(level_value.level)
self.assertIsNone(level_value.parent)
self.assertIsNone(level_value.value)
def test_put_resource_values_overrides_root(self):
self._fixture()
@ -323,9 +313,7 @@ class TestApp(base.TestCase):
environment_id=9, resource_definition_id=5).one_or_none()
self.assertIsNotNone(resource_values)
self.assertEqual(resource_values.overrides, {'k': 'v'})
self.assertIsNone(resource_values.level_value.level)
self.assertIsNone(resource_values.level_value.parent)
self.assertIsNone(resource_values.level_value.value)
self.assertIsNone(resource_values.level_value)
def test_put_resource_values_overrides_deep(self):
self._fixture()
@ -346,10 +334,7 @@ class TestApp(base.TestCase):
level_value = level_value.parent
self.assertEqual(level_value.level.name, 'lvl1')
self.assertEqual(level_value.value, 'val1')
level_value = level_value.parent
self.assertIsNone(level_value.level)
self.assertIsNone(level_value.parent)
self.assertIsNone(level_value.value)
def test_put_resource_values_bad_level(self):
self._fixture()

79
tuning_box/tests/test_db.py
View File

@ -18,9 +18,11 @@ os.environ.setdefault("OS_TEST_DBAPI_ADMIN_CONNECTION", "sqlite:///testdb")
from alembic import command as alembic_command
from alembic import config as alembic_config
from alembic import script as alembic_script
import flask
from oslo_db.sqlalchemy import test_base
from oslo_db.sqlalchemy import test_migrations
import sqlalchemy as sa
import testscenarios
from werkzeug import exceptions
@ -191,3 +193,80 @@ class TestMigrationsSyncPrefixed(_RealDBPrefixedTest,
return super(TestMigrationsSyncPrefixed, self).include_object(
object_, name, type_, reflected, compare_to)
class TestRemoveFakeRootMigration(_RealDBTest):
revision = '9ae15c85fa92'
prefix = ''
def setUp(self):
super(TestRemoveFakeRootMigration, self).setUp()
self.alembic_config = self.get_alembic_config(self.engine)
script_dir = alembic_script.ScriptDirectory.from_config(
self.alembic_config)
self.migration_module = script_dir.get_revision(self.revision).module
self.down_revision = self.migration_module.down_revision
self.session = sa.orm.Session(bind=self.engine)
self.addCleanup(self.session.close)
def test_upgrade(self):
alembic_command.upgrade(self.alembic_config, self.down_revision)
abase = self.migration_module._get_autobase(self.prefix, self.engine)
clss = abase.classes
env = clss.Environment()
env_level = clss.EnvironmentHierarchyLevel(environment=env, name='lvl')
self.session.add(env_level)
fake_root_1 = clss.EnvironmentHierarchyLevelValue(
level_id=None, value=None, parent_id=None)
self.session.add(fake_root_1)
self.session.flush()
child_1 = clss.EnvironmentHierarchyLevelValue(
level_id=env_level.id, value="1", parent_id=fake_root_1.id)
self.session.add(child_1)
fake_root_2 = clss.EnvironmentHierarchyLevelValue(
level_id=None, value=None, parent_id=None)
self.session.add(fake_root_2)
self.session.flush()
child_2 = clss.EnvironmentHierarchyLevelValue(
level_id=env_level.id, value="2", parent_id=fake_root_2.id)
self.session.add(child_2)
self.session.commit()
alembic_command.upgrade(self.alembic_config, self.revision)
ehlvs = self.session.query(clss.EnvironmentHierarchyLevelValue).all()
for ehlv in ehlvs:
self.assertIsNotNone(ehlv.level_id)
self.assertIsNone(ehlv.parent_id)
def test_downgrade(self):
alembic_command.upgrade(self.alembic_config, self.revision)
abase = self.migration_module._get_autobase(self.prefix, self.engine)
clss = abase.classes
env = clss.Environment()
env_level = clss.EnvironmentHierarchyLevel(environment=env, name='lvl')
self.session.add(env_level)
self.session.flush()
child_1 = clss.EnvironmentHierarchyLevelValue(
level_id=env_level.id, value="1", parent_id=None)
self.session.add(child_1)
child_2 = clss.EnvironmentHierarchyLevelValue(
level_id=env_level.id, value="2", parent_id=None)
self.session.add(child_2)
self.session.commit()
alembic_command.downgrade(self.alembic_config, self.down_revision)
fake_root = self.session.query(clss.EnvironmentHierarchyLevelValue) \
.filter_by(parent_id=None) \
.one()
self.assertIsNone(fake_root.level_id)
self.assertIsNone(fake_root.value)
children = self.session.query(clss.EnvironmentHierarchyLevelValue) \
.filter_by(parent_id=fake_root.id) \
.all()
self.assertItemsEqual(["1", "2"], [c.value for c in children])
for child in children:
self.assertEqual(fake_root.id, child.parent_id)
self.assertEqual(env_level.id, child.level_id)
class TestRemoveFakeRootMigrationPrefixed(_RealDBPrefixedTest,
TestRemoveFakeRootMigration):
prefix = 'test_prefix_'