diff --git a/watcher/db/sqlalchemy/alembic/versions/15f7375ca737_change_efficiacy_indicator_decimals.py b/watcher/db/sqlalchemy/alembic/versions/15f7375ca737_change_efficiacy_indicator_decimals.py new file mode 100644 index 000000000..00166b9bd --- /dev/null +++ b/watcher/db/sqlalchemy/alembic/versions/15f7375ca737_change_efficiacy_indicator_decimals.py @@ -0,0 +1,20 @@ +"""change_efficiacy_indicator_decimals + +Revision ID: 15f7375ca737 +Revises: 609bec748f2a +Create Date: 2025-03-24 10:15:19.269061 + +""""" + +# revision identifiers, used by Alembic. +revision = '15f7375ca737' +down_revision = '609bec748f2a' + +from alembic import op +import sqlalchemy as sa + +def upgrade(): + op.add_column('efficacy_indicators', + sa.Column('data', sa.Float()) + ) + diff --git a/watcher/db/sqlalchemy/api.py b/watcher/db/sqlalchemy/api.py index 33ed128bf..a9bebe326 100644 --- a/watcher/db/sqlalchemy/api.py +++ b/watcher/db/sqlalchemy/api.py @@ -905,15 +905,34 @@ class Connection(api.BaseConnection): # ### EFFICACY INDICATORS ### # def get_efficacy_indicator_list(self, *args, **kwargs): - return self._get_model_list(models.EfficacyIndicator, - self._add_efficacy_indicators_filters, - *args, **kwargs) + eff_ind_models = self._get_model_list( + models.EfficacyIndicator, + self._add_efficacy_indicators_filters, + *args, **kwargs + ) + for indicator in eff_ind_models: + if indicator.data is not None: + # jgilaber: use the data value since it stores the value + # properly, see https://bugs.launchpad.net/watcher/+bug/2103458 + # for more details + indicator.value = indicator.data + else: + # if data is None, it means that we're reading data from a + # database created before the 15f7375ca737 revision, use the + # value column + indicator.data = indicator.value + + return eff_ind_models def create_efficacy_indicator(self, values): # ensure defaults are present for new efficacy indicators if not values.get('uuid'): values['uuid'] = utils.generate_uuid() + # jgilaber: use the data column since it stores the value + # properly, see https://bugs.launchpad.net/watcher/+bug/2103458 + # for more details + values['data'] = values.get('value') try: efficacy_indicator = self._create(models.EfficacyIndicator, values) except db_exc.DBDuplicateEntry: @@ -922,8 +941,22 @@ class Connection(api.BaseConnection): def _get_efficacy_indicator(self, context, fieldname, value, eager): try: - return self._get(context, model=models.EfficacyIndicator, - fieldname=fieldname, value=value, eager=eager) + efficacy_indicator = self._get(context, + model=models.EfficacyIndicator, + fieldname=fieldname, + value=value, eager=eager) + + if efficacy_indicator.data is not None: + # jgilaber: use the data value since it stores the value + # properly, see https://bugs.launchpad.net/watcher/+bug/2103458 + # for more details + efficacy_indicator.value = efficacy_indicator.data + else: + # if data is None, it means that we're reading data from a + # database created before the 15f7375ca737 revision, use the + # value column + efficacy_indicator.data = efficacy_indicator.value + return efficacy_indicator except exception.ResourceNotFound: raise exception.EfficacyIndicatorNotFound(efficacy_indicator=value) diff --git a/watcher/db/sqlalchemy/models.py b/watcher/db/sqlalchemy/models.py index 6ae06378f..41778c15a 100644 --- a/watcher/db/sqlalchemy/models.py +++ b/watcher/db/sqlalchemy/models.py @@ -238,7 +238,10 @@ class EfficacyIndicator(Base): name = Column(String(63)) description = Column(String(255), nullable=True) unit = Column(String(63), nullable=True) + # this column is deprecated due to bug + # https://bugs.launchpad.net/watcher/+bug/2103458 value = Column(Numeric()) + data = Column(Float()) action_plan_id = Column(Integer, ForeignKey('action_plans.id'), nullable=False) diff --git a/watcher/tests/db/test_efficacy_indicator.py b/watcher/tests/db/test_efficacy_indicator.py index a0b55b53a..aa39fd437 100644 --- a/watcher/tests/db/test_efficacy_indicator.py +++ b/watcher/tests/db/test_efficacy_indicator.py @@ -452,12 +452,8 @@ class MySQLDbEfficacyIndicatorTestCase(base.MySQLDbTestCase): action_plan_id=self.action_plan.id, id=3, uuid=None, name="efficacy_indicator3", description="Test Indicator 3") - def test_efficacy_indicator_value_decimals(self): + def test_efficacy_indicator_value(self): db_efficacy_indicator = self.dbapi.get_efficacy_indicator_by_id( self.context, 1) - self.assertAlmostEqual(float(db_efficacy_indicator.value), - 0.00, places=2) - # FIXME: once the database bug is fixed check that the value is stored - # correctly - # self.assertAlmostEqual(float(db_efficacy_indicator.value), - # 0.01, places=2) + self.assertAlmostEqual(db_efficacy_indicator.value, 0.012, places=3) + self.assertAlmostEqual(db_efficacy_indicator.data, 0.012, places=3) diff --git a/watcher/tests/db/test_migrations.py b/watcher/tests/db/test_migrations.py new file mode 100644 index 000000000..efee6d5e1 --- /dev/null +++ b/watcher/tests/db/test_migrations.py @@ -0,0 +1,205 @@ +# Copyright 2025 Red Hat, Inc. +# +# 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. +# + +""""Tests for database migration for Watcher. + +These are "opportunistic" tests which allow testing against all three databases +(sqlite in memory, mysql, pg) in a properly configured unit test environment. + +For the opportunistic testing you need to set up DBs named 'openstack_citest' +with user 'openstack_citest' and password 'openstack_citest' on localhost. The +test will then use that DB and username/password combo to run the tests. Refer +to the 'tools/test-setup.sh' for an example of how to configure this. +""" + +from alembic import command as alembic_api +from alembic.script import ScriptDirectory +from oslo_config import cfg +from oslo_db.sqlalchemy import enginefacade +from oslo_db.sqlalchemy import test_fixtures +from oslo_log import log as logging +import sqlalchemy + +from watcher.common import utils as w_utils +from watcher.db import api as dbapi +from watcher.db.sqlalchemy import migration +from watcher.tests import base +from watcher.tests.db import utils + +LOG = logging.getLogger(__name__) + + +class MySQLDbMigrationsTestCase(test_fixtures.OpportunisticDBTestMixin, + base.TestCase): + + FIXTURE = test_fixtures.MySQLOpportunisticFixture + + def setUp(self): + conn_str = "mysql+pymysql://root:insecure_slave@127.0.0.1" + # to use mysql db + cfg.CONF.set_override("connection", conn_str, + group="database") + super().setUp() + self.engine = enginefacade.writer.get_engine() + self.dbapi = dbapi.get_instance() + self.alembic_config = migration._alembic_config() + self.revisions_tested = set(["15f7375ca737"]) + + def _apply_migration(self, connection, revision): + if revision not in self.revisions_tested: + # if we don't have tests for this version, just upgrade to it + alembic_api.upgrade(self.alembic_config, revision) + return + + pre_upgrade = getattr(self, f"_pre_upgrade_{revision}", None) + if pre_upgrade: + pre_upgrade(connection) + + alembic_api.upgrade(self.alembic_config, revision) + + post_upgrade = getattr(self, f"_check_{revision}", None) + if post_upgrade: + post_upgrade(connection) + + def _pre_upgrade_15f7375ca737(self, connection): + inspector = sqlalchemy.inspect(connection) + columns = inspector.get_columns("efficacy_indicators") + for column in columns: + if column['name'] != "value": + continue + value_type = column['type'] + self.assertIsInstance(value_type, sqlalchemy.Numeric) + self.assertEqual(value_type.scale, 0) + self.assertEqual(value_type.precision, 10) + + def _check_15f7375ca737(self, connection): + inspector = sqlalchemy.inspect(connection) + columns = inspector.get_columns("efficacy_indicators") + for column in columns: + if column['name'] == "value": + value_type = column['type'] + self.assertIsInstance(value_type, sqlalchemy.Numeric) + self.assertEqual(value_type.scale, 0) + self.assertEqual(value_type.precision, 10) + elif column['name'] == "data": + value_type = column['type'] + self.assertIsInstance(value_type, sqlalchemy.Float) + + def test_migration_revisions(self): + with self.engine.begin() as connection: + self.alembic_config.attributes["connection"] = connection + script = ScriptDirectory.from_config(self.alembic_config) + revisions = [x.revision for x in script.walk_revisions()] + + # for some reason, 'walk_revisions' gives us the revisions in + # reverse chronological order so we have to invert this + revisions.reverse() + + for revision in revisions: + LOG.info('Testing revision %s', revision) + self._apply_migration(connection, revision) + + +class MySQLDbDataMigrationsTestCase(MySQLDbMigrationsTestCase): + + def _create_manual_efficacy_indicator(self, connection, **kwargs): + eff_ind_values = utils.get_test_efficacy_indicator(**kwargs) + metadata = sqlalchemy.MetaData() + metadata.reflect(bind=self.engine) + eff_ind_table = sqlalchemy.Table('efficacy_indicators', metadata) + connection.execute(eff_ind_table.insert(), eff_ind_values) + connection.commit() + + def _pre_upgrade_15f7375ca737(self, connection): + """Add data to the database before applying the 15f7375ca737 revision. + + This data will then be checked after applying the revision to ensure it + was not affected by the db upgrade. + + """ + self.goal = utils.create_test_goal( + id=1, uuid=w_utils.generate_uuid(), + name="GOAL_1", display_name='Goal 1') + self.strategy = utils.create_test_strategy( + id=1, uuid=w_utils.generate_uuid(), + name="STRATEGY_ID_1", display_name='My Strategy 1') + self.audit_template = utils.create_test_audit_template( + name="Audit Template", id=1, uuid=None) + self.audit = utils.create_test_audit( + audit_template_id=self.audit_template.id, id=1, uuid=None, + name="AUDIT_1") + self.action_plan = utils.create_test_action_plan( + audit_id=self.audit.id, id=1, uuid=None) + + self._create_manual_efficacy_indicator( + connection, + action_plan_id=self.action_plan.id, id=1, uuid=None, + name="efficacy_indicator1", description="Test Indicator 1", + value=1.01234567912345678) + + def _check_15f7375ca737(self, connection): + """Check data integrity after the database migration.""" + # check that creating a new efficacy_indicator after the migration + # works + utils.create_test_efficacy_indicator( + action_plan_id=self.action_plan.id, id=2, uuid=None, + name="efficacy_indicator1", description="Test Indicator 2", + value=0.01234567912345678) + db_efficacy_indicator = self.dbapi.get_efficacy_indicator_by_id( + self.context, 2) + self.assertAlmostEqual(db_efficacy_indicator.value, 0.012, places=3) + # check that getting the efficacy_indicator using the + # get_efficacy_indicator_list method reports the correct values + efficacy_indicators = self.dbapi.get_efficacy_indicator_list( + self.context) + self.assertEqual(len(efficacy_indicators), 2) + self.assertEqual(efficacy_indicators[0].id, 1) + self.assertAlmostEqual(efficacy_indicators[0].value, + 1.00, places=3) + + self.assertEqual(efficacy_indicators[1].id, 2) + self.assertAlmostEqual(efficacy_indicators[1].value, + 0.012, places=3) + # check that the existing data is there after the migration + db_goal = self.dbapi.get_goal_by_id(self.context, 1) + self.assertEqual(db_goal.name, "GOAL_1") + db_strategy = self.dbapi.get_strategy_by_id(self.context, 1) + self.assertEqual(db_strategy.name, "STRATEGY_ID_1") + db_audit_template = self.dbapi.get_audit_template_by_id( + self.context, 1) + self.assertEqual(db_audit_template.name, "Audit Template") + db_audit = self.dbapi.get_audit_by_id(self.context, 1) + self.assertEqual(db_audit.name, "AUDIT_1") + db_efficacy_indicator_1 = self.dbapi.get_efficacy_indicator_by_id( + self.context, 1) + self.assertAlmostEqual(db_efficacy_indicator_1.data, + 1.00, places=2) + self.assertAlmostEqual(db_efficacy_indicator_1.value, + 1.00, places=2) + self.assertEqual(db_efficacy_indicator_1.name, "efficacy_indicator1") + + def test_migration_revisions(self): + with self.engine.begin() as connection: + self.alembic_config.attributes["connection"] = connection + script = ScriptDirectory.from_config(self.alembic_config) + revisions = [x.revision for x in script.walk_revisions()] + + # for some reason, 'walk_revisions' gives us the revisions in + # reverse chronological order so we have to invert this + revisions.reverse() + + for revision in revisions: + LOG.info('Testing revision %s', revision) + self._apply_migration(connection, revision)