From deffe8a1679b2fd0af803de9fa3aee46d41d5d84 Mon Sep 17 00:00:00 2001 From: Steven Hardy Date: Wed, 14 Nov 2012 15:41:17 +0000 Subject: [PATCH] WatchRule refer to stack by id not name Rework WatchRule to refer to stacks by uuid not name, this will help us move to allowing non-unique stack names containing WatchRule resources Ref bug 1078779 Change-Id: Idbbbd65a05d7036860cc2feb044d568210071d21 Signed-off-by: Steven Hardy --- heat/api/cloudwatch/watch.py | 8 +-- .../versions/014_watch_stackid.py | 25 +++++++ heat/db/sqlalchemy/models.py | 5 +- heat/engine/api.py | 6 +- heat/engine/resources/cloud_watch.py | 2 +- heat/engine/watchrule.py | 23 +++---- heat/tests/test_api_cloudwatch.py | 14 ++-- heat/tests/test_engine_service.py | 46 +++++++++++-- heat/tests/test_loadbalancer.py | 27 +++++--- heat/tests/test_watch.py | 65 ++++++++++++------- 10 files changed, 158 insertions(+), 63 deletions(-) create mode 100644 heat/db/sqlalchemy/migrate_repo/versions/014_watch_stackid.py diff --git a/heat/api/cloudwatch/watch.py b/heat/api/cloudwatch/watch.py index ac6f3504..299d58f4 100644 --- a/heat/api/cloudwatch/watch.py +++ b/heat/api/cloudwatch/watch.py @@ -96,10 +96,10 @@ class WatchController(object): engine_api.WATCH_THRESHOLD: 'Threshold', engine_api.WATCH_UNIT: 'Unit'} - # AWS doesn't return StackName in the main MetricAlarm - # structure, so we add StackName as a dimension to all responses - a[engine_api.WATCH_DIMENSIONS].append({'StackName': - a[engine_api.WATCH_STACK_NAME]}) + # AWS doesn't return StackId in the main MetricAlarm + # structure, so we add StackId as a dimension to all responses + a[engine_api.WATCH_DIMENSIONS].append({'StackId': + a[engine_api.WATCH_STACK_ID]}) # Reformat dimensions list into AWS API format a[engine_api.WATCH_DIMENSIONS] = self._reformat_dimensions( diff --git a/heat/db/sqlalchemy/migrate_repo/versions/014_watch_stackid.py b/heat/db/sqlalchemy/migrate_repo/versions/014_watch_stackid.py new file mode 100644 index 00000000..6baa8d41 --- /dev/null +++ b/heat/db/sqlalchemy/migrate_repo/versions/014_watch_stackid.py @@ -0,0 +1,25 @@ +from sqlalchemy import * +from migrate import * + + +def upgrade(migrate_engine): + meta = MetaData(bind=migrate_engine) + + stack = Table('stack', meta, autoload=True) + watch_rule = Table('watch_rule', meta, autoload=True) + + Column('stack_id', String(length=36), ForeignKey("stack.id"), + nullable=False).create(watch_rule) + + watch_rule.c.stack_name.drop() + + +def downgrade(migrate_engine): + meta = MetaData(bind=migrate_engine) + + watch_rule = Table('watch_rule', meta, autoload=True) + + watch_rule.c.stack_id.drop() + Column('stack_name', String(length=255), convert_unicode=False, + assert_unicode=None, unicode_error=None, + _warn_on_bytestring=False).create(watch_rule) diff --git a/heat/db/sqlalchemy/models.py b/heat/db/sqlalchemy/models.py index c95df804..2d6d7790 100644 --- a/heat/db/sqlalchemy/models.py +++ b/heat/db/sqlalchemy/models.py @@ -230,9 +230,12 @@ class WatchRule(BASE, HeatBase): name = Column('name', String, nullable=False) rule = Column('rule', Json) state = Column('state', String) - stack_name = Column('stack_name', String) last_evaluated = Column(DateTime, default=timeutils.utcnow) + stack_id = Column(String, ForeignKey('stack.id'), + nullable=False) + stack = relationship(Stack, backref=backref('watch_rule')) + class WatchData(BASE, HeatBase): """Represents a watch_data created by the heat engine.""" diff --git a/heat/engine/api.py b/heat/engine/api.py index 0f1685f7..86ad9984 100644 --- a/heat/engine/api.py +++ b/heat/engine/api.py @@ -192,7 +192,7 @@ WATCH_KEYS = ( WATCH_INSUFFICIENT_ACTIONS, WATCH_METRIC_NAME, WATCH_NAMESPACE, WATCH_OK_ACTIONS, WATCH_PERIOD, WATCH_STATE_REASON, WATCH_STATE_REASON_DATA, WATCH_STATE_UPDATED_TIME, WATCH_STATE_VALUE, - WATCH_STATISTIC, WATCH_THRESHOLD, WATCH_UNIT, WATCH_STACK_NAME + WATCH_STATISTIC, WATCH_THRESHOLD, WATCH_UNIT, WATCH_STACK_ID ) = ( 'actions_enabled', 'actions', 'topic', 'updated_time', 'description', 'name', @@ -200,7 +200,7 @@ WATCH_KEYS = ( 'insufficient_actions', 'metric_name', 'namespace', 'ok_actions', 'period', 'state_reason', 'state_reason_data', 'state_updated_time', 'state_value', - 'statistic', 'threshold', 'unit', 'stack_name') + 'statistic', 'threshold', 'unit', 'stack_id') # Alternate representation of a watch rule to align with DB format @@ -259,7 +259,7 @@ def format_watch(watch): WATCH_STATISTIC: watch.rule.get(RULE_STATISTIC), WATCH_THRESHOLD: watch.rule.get(RULE_THRESHOLD), WATCH_UNIT: watch.rule.get(RULE_UNIT), - WATCH_STACK_NAME: watch.stack_name + WATCH_STACK_ID: watch.stack_id } return result diff --git a/heat/engine/resources/cloud_watch.py b/heat/engine/resources/cloud_watch.py index ac05d642..bc02625a 100644 --- a/heat/engine/resources/cloud_watch.py +++ b/heat/engine/resources/cloud_watch.py @@ -56,7 +56,7 @@ class CloudWatchAlarm(resource.Resource): def handle_create(self): wr = watchrule.WatchRule(context=self.context, watch_name=self.name, rule=self.parsed_template('Properties'), - stack_name=self.stack.name) + stack_id=self.stack.id) wr.store() def handle_update(self): diff --git a/heat/engine/watchrule.py b/heat/engine/watchrule.py index 69b86fa5..3b2863e6 100644 --- a/heat/engine/watchrule.py +++ b/heat/engine/watchrule.py @@ -38,14 +38,15 @@ class WatchRule(object): created_at = timestamp.Timestamp(db_api.watch_rule_get, 'created_at') updated_at = timestamp.Timestamp(db_api.watch_rule_get, 'updated_at') - def __init__(self, context, watch_name, rule, stack_name, state=NORMAL, - wid=None, watch_data=[], last_evaluated=timeutils.utcnow()): + def __init__(self, context, watch_name, rule, stack_id=None, + state=NORMAL, wid=None, watch_data=[], + last_evaluated=timeutils.utcnow()): self.context = context self.now = timeutils.utcnow() self.name = watch_name self.state = state self.rule = rule - self.stack_name = stack_name + self.stack_id = stack_id self.timeperiod = datetime.timedelta(seconds=int(rule['Period'])) self.id = wid self.watch_data = watch_data @@ -68,7 +69,7 @@ class WatchRule(object): return cls(context=context, watch_name=dbwr.name, rule=dbwr.rule, - stack_name=dbwr.stack_name, + stack_id=dbwr.stack_id, state=dbwr.state, wid=dbwr.id, watch_data=dbwr.watch_data, @@ -84,7 +85,7 @@ class WatchRule(object): 'name': self.name, 'rule': self.rule, 'state': self.state, - 'stack_name': self.stack_name + 'stack_id': self.stack_id } if not self.id: @@ -220,7 +221,7 @@ class WatchRule(object): def rule_action(self, new_state): logger.warn('WATCH: stack:%s, watch_name:%s %s', - self.stack_name, self.name, new_state) + self.stack_id, self.name, new_state) actioned = False if not self.ACTION_MAP[new_state] in self.rule: @@ -232,19 +233,19 @@ class WatchRule(object): # scoping, this is the simplest possible solution to the # HA/Autoscaling regression described in bug/1078779 # Further work in-progress here (shardy) as this - # breaks when stack_name is not unique accross tenants + # breaks when stack_id is not unique accross tenants sl = [x for x in db_api.stack_get_all(self.context) - if x.name == self.stack_name] + if x.id == self.stack_id] s = None if len(sl) == 1: s = sl[0] elif len(sl) > 1: - logger.error("stack %s name not unique, " % self.stack_name + logger.error("stack %s not unique, " % self.stack_id + "cannot action watch rule") else: - logger.error("stack %s name could not be found, " % - self.stack_name + "cannot action watch rule") + logger.error("stack %s could not be found, " % + self.stack_id + "cannot action watch rule") if s and s.status in (parser.Stack.CREATE_COMPLETE, parser.Stack.UPDATE_COMPLETE): diff --git a/heat/tests/test_api_cloudwatch.py b/heat/tests/test_api_cloudwatch.py index 1a6fa4b1..a2fe652e 100644 --- a/heat/tests/test_api_cloudwatch.py +++ b/heat/tests/test_api_cloudwatch.py @@ -61,10 +61,11 @@ class WatchControllerTest(unittest.TestCase): # The tests def test_reformat_dimensions(self): - dims = [{'StackName': u'wordpress_ha5', - 'Foo': 'bar'}] + dims = [{'StackId': u'21617058-781e-4262-97ab-5f9df371ee52', + 'Foo': 'bar'}] response = self.controller._reformat_dimensions(dims) - expected = [{'Name': 'StackName', 'Value': u'wordpress_ha5'}, + expected = [{'Name': 'StackId', + 'Value': u'21617058-781e-4262-97ab-5f9df371ee52'}, {'Name': 'Foo', 'Value': 'bar'}] self.assert_(response == expected) @@ -91,7 +92,7 @@ class WatchControllerTest(unittest.TestCase): # Stub out the RPC call to the engine with a pre-canned response engine_resp = [{u'state_updated_time': u'2012-08-30T14:13:21Z', - u'stack_name': u'wordpress_ha5', + u'stack_id': u'21617058-781e-4262-97ab-5f9df371ee52', u'period': u'300', u'actions': [u'WebServerRestartPolicy'], u'topic': None, @@ -149,8 +150,9 @@ class WatchControllerTest(unittest.TestCase): 'MetricName': u'ServiceFailure', 'ActionsEnabled': None, 'Dimensions': [ - {'Name': 'StackName', - 'Value': u'wordpress_ha5'}]}]}}} + {'Name': 'StackId', + 'Value': u'21617058-781e-4262-97ab-5f9df371ee52'} + ]}]}}} self.assert_(response == expected) diff --git a/heat/tests/test_engine_service.py b/heat/tests/test_engine_service.py index 24e8a6e3..7538cf25 100644 --- a/heat/tests/test_engine_service.py +++ b/heat/tests/test_engine_service.py @@ -596,7 +596,8 @@ class stackServiceTest(unittest.TestCase): def test_show_watch(self): # Insert two dummy watch rules into the DB - values = {'stack_name': u'wordpress_ha', 'state': 'NORMAL', + values = {'stack_id': self.stack.id, + 'state': 'NORMAL', 'name': u'HttpFailureAlarm', 'rule': { u'EvaluationPeriods': u'1', @@ -631,8 +632,28 @@ class stackServiceTest(unittest.TestCase): for key in engine_api.WATCH_KEYS: self.assertTrue(key in result[0]) + # Cleanup, delete the dummy rules + db_api.watch_rule_delete(self.ctx, "HttpFailureAlarm") + db_api.watch_rule_delete(self.ctx, "AnotherWatch") + def test_show_watch_metric(self): - # Get one of the watch rules created in test_show_watch + # Insert dummy watch rule into the DB + values = {'stack_id': self.stack.id, + 'state': 'NORMAL', + 'name': u'HttpFailureAlarm', + 'rule': { + u'EvaluationPeriods': u'1', + u'AlarmActions': [u'WebServerRestartPolicy'], + u'AlarmDescription': u'Restart the WikiDatabase', + u'Namespace': u'system/linux', + u'Period': u'300', + u'ComparisonOperator': u'GreaterThanThreshold', + u'Statistic': u'SampleCount', + u'Threshold': u'2', + u'MetricName': u'ServiceFailure'}} + db_ret = db_api.watch_rule_create(self.ctx, values) + self.assertNotEqual(db_ret, None) + # And add a metric datapoint watch = db_api.watch_rule_get_by_name(self.ctx, "HttpFailureAlarm") self.assertNotEqual(watch, None) @@ -654,13 +675,17 @@ class stackServiceTest(unittest.TestCase): metric_name=None) self.assertEqual(2, len(result)) + # Cleanup, delete the dummy rule + db_api.watch_rule_delete(self.ctx, "HttpFailureAlarm") + # Check the response has all keys defined in the engine API for key in engine_api.WATCH_DATA_KEYS: self.assertTrue(key in result[0]) def test_set_watch_state(self): # Insert dummy watch rule into the DB - values = {'stack_name': u'wordpress_ha', 'state': 'NORMAL', + values = {'stack_id': self.stack.id, + 'state': 'NORMAL', 'name': u'OverrideAlarm', 'rule': { u'EvaluationPeriods': u'1', @@ -675,7 +700,19 @@ class stackServiceTest(unittest.TestCase): db_ret = db_api.watch_rule_create(self.ctx, values) self.assertNotEqual(db_ret, None) + class DummyAction: + alarm = "dummyfoo" + + self.m.StubOutWithMock(parser.Stack, '__getitem__') + parser.Stack.__getitem__( + 'WebServerRestartPolicy').AndReturn(DummyAction()) + + self.m.StubOutWithMock(watchrule.greenpool, 'spawn_n') + watchrule.greenpool.spawn_n("dummyfoo").AndReturn(None) + self.m.ReplayAll() + for state in watchrule.WatchRule.WATCH_STATES: + result = self.man.set_watch_state(self.ctx, watch_name="OverrideAlarm", state=state) @@ -687,7 +724,8 @@ class stackServiceTest(unittest.TestCase): def test_set_watch_state_badstate(self): # Insert dummy watch rule into the DB - values = {'stack_name': u'wordpress_ha', 'state': 'NORMAL', + values = {'stack_id': self.stack.id, + 'state': 'NORMAL', 'name': u'OverrideAlarm2', 'rule': { u'EvaluationPeriods': u'1', diff --git a/heat/tests/test_loadbalancer.py b/heat/tests/test_loadbalancer.py index 5f8f12fb..6d3ed25f 100644 --- a/heat/tests/test_loadbalancer.py +++ b/heat/tests/test_loadbalancer.py @@ -25,6 +25,8 @@ import json from nose.plugins.attrib import attr from heat.common import exception +from heat.common import context +from heat.common import config from heat.engine import parser from heat.engine.resources import instance from heat.engine.resources import loadbalancer as lb @@ -33,17 +35,27 @@ from heat.engine.resources import stack from heat.tests.v1_1 import fakes +def create_context(mocks, user='lb_test_user', + tenant='test_admin', ctx=None): + ctx = ctx or context.get_admin_context() + mocks.StubOutWithMock(ctx, 'username') + mocks.StubOutWithMock(ctx, 'tenant_id') + ctx.username = user + ctx.tenant_id = tenant + return ctx + + @attr(tag=['unit', 'resource']) @attr(speed='fast') class LoadBalancerTest(unittest.TestCase): def setUp(self): self.m = mox.Mox() self.fc = fakes.FakeClient() - self.m.StubOutWithMock(parser.Stack, 'store') self.m.StubOutWithMock(lb.LoadBalancer, 'nova') self.m.StubOutWithMock(instance.Instance, 'nova') self.m.StubOutWithMock(self.fc.servers, 'create') self.m.StubOutWithMock(Metadata, '__set__') + config.register_engine_opts() def tearDown(self): self.m.UnsetStubs() @@ -58,16 +70,11 @@ class LoadBalancerTest(unittest.TestCase): return t def parse_stack(self, t): - class DummyContext(): - tenant = 'test_tenant' - tenant_id = '1234abcd' - username = 'test_username' - password = 'password' - auth_url = 'http://localhost:5000/v2.0' template = parser.Template(t) params = parser.Parameters('test_stack', template, {'KeyName': 'test'}) - stack = parser.Stack(DummyContext(), 'test_stack', template, - params, stack_id=-1) + stack = parser.Stack(create_context(self.m), 'test_stack', template, + params, stack_id=None) + stack.store() return stack @@ -82,7 +89,7 @@ class LoadBalancerTest(unittest.TestCase): def test_loadbalancer(self): lb.LoadBalancer.nova().AndReturn(self.fc) - parser.Stack.store(mox.IgnoreArg()).AndReturn('5678') +# parser.Stack.store(mox.IgnoreArg()).AndReturn('5678') instance.Instance.nova().MultipleTimes().AndReturn(self.fc) self.fc.servers.create(flavor=2, image=745, key_name='test', meta=None, name=u'test_stack.LoadBalancer.LB_instance', diff --git a/heat/tests/test_watch.py b/heat/tests/test_watch.py index 3f3a0169..c453d5f4 100644 --- a/heat/tests/test_watch.py +++ b/heat/tests/test_watch.py @@ -45,6 +45,26 @@ class WatchData: @attr(speed='fast') class WatchRuleTest(unittest.TestCase): + @classmethod + def setUpClass(cls): + # Create a dummy stack in the DB as WatchRule instances + # must be associated with a stack + ctx = context.get_admin_context() + tmpl = db_api.raw_template_create(ctx, {'foo': 'bar'}) + dummy_stack = {'id': '6754d843-bed2-40dc-a325-84882bb90a98', + 'name': 'dummystack', + 'raw_template_id': tmpl.id, + 'user_creds_id': 1, + 'username': 'dummyuser', + 'owner_id': None, + 'status': 'CREATE_COMPLETE', + 'status_reason': 'foo status', + 'parameters': {'foo': 'bar'}, + 'timeout': 60, + 'tenant': 123456} + db_ret = db_api.stack_create(ctx, dummy_stack) + cls.stack_id = db_ret.id + def setUp(self): self.username = 'watchrule_test_user' @@ -77,8 +97,8 @@ class WatchRuleTest(unittest.TestCase): watcher = watchrule.WatchRule(context=self.ctx, watch_name="testwatch", rule=rule, - stack_name="teststack", watch_data=data, + stack_id=self.stack_id, last_evaluated=last) new_state = watcher.get_alarm_state() logger.info(new_state) @@ -88,8 +108,8 @@ class WatchRuleTest(unittest.TestCase): watcher = watchrule.WatchRule(context=self.ctx, watch_name="testwatch", rule=rule, - stack_name="teststack", watch_data=data, + stack_id=self.stack_id, last_evaluated=last) new_state = watcher.get_alarm_state() logger.info(new_state) @@ -113,8 +133,8 @@ class WatchRuleTest(unittest.TestCase): watcher = watchrule.WatchRule(context=self.ctx, watch_name="testwatch", rule=rule, - stack_name="teststack", watch_data=data, + stack_id=self.stack_id, last_evaluated=last) watcher.now = now new_state = watcher.get_alarm_state() @@ -125,8 +145,8 @@ class WatchRuleTest(unittest.TestCase): watcher = watchrule.WatchRule(context=self.ctx, watch_name="testwatch", rule=rule, - stack_name="teststack", watch_data=data, + stack_id=self.stack_id, last_evaluated=last) watcher.now = now new_state = watcher.get_alarm_state() @@ -152,8 +172,8 @@ class WatchRuleTest(unittest.TestCase): watcher = watchrule.WatchRule(context=self.ctx, watch_name="testwatch", rule=rule, - stack_name="teststack", watch_data=data, + stack_id=self.stack_id, last_evaluated=last) watcher.now = now new_state = watcher.get_alarm_state() @@ -165,8 +185,8 @@ class WatchRuleTest(unittest.TestCase): watcher = watchrule.WatchRule(context=self.ctx, watch_name="testwatch", rule=rule, - stack_name="teststack", watch_data=data, + stack_id=self.stack_id, last_evaluated=last) watcher.now = now new_state = watcher.get_alarm_state() @@ -179,8 +199,8 @@ class WatchRuleTest(unittest.TestCase): watcher = watchrule.WatchRule(context=self.ctx, watch_name="testwatch", rule=rule, - stack_name="teststack", watch_data=data, + stack_id=self.stack_id, last_evaluated=last) watcher.now = now new_state = watcher.get_alarm_state() @@ -205,8 +225,8 @@ class WatchRuleTest(unittest.TestCase): watcher = watchrule.WatchRule(context=self.ctx, watch_name="testwatch", rule=rule, - stack_name="teststack", watch_data=data, + stack_id=self.stack_id, last_evaluated=last) watcher.now = now new_state = watcher.get_alarm_state() @@ -218,8 +238,8 @@ class WatchRuleTest(unittest.TestCase): watcher = watchrule.WatchRule(context=self.ctx, watch_name="testwatch", rule=rule, - stack_name="teststack", watch_data=data, + stack_id=self.stack_id, last_evaluated=last) watcher.now = now new_state = watcher.get_alarm_state() @@ -243,8 +263,8 @@ class WatchRuleTest(unittest.TestCase): watcher = watchrule.WatchRule(context=self.ctx, watch_name="testwatch", rule=rule, - stack_name="teststack", watch_data=data, + stack_id=self.stack_id, last_evaluated=last) watcher.now = now new_state = watcher.get_alarm_state() @@ -255,8 +275,8 @@ class WatchRuleTest(unittest.TestCase): watcher = watchrule.WatchRule(context=self.ctx, watch_name="testwatch", rule=rule, - stack_name="teststack", watch_data=data, + stack_id=self.stack_id, last_evaluated=last) watcher.now = now new_state = watcher.get_alarm_state() @@ -265,7 +285,8 @@ class WatchRuleTest(unittest.TestCase): def test_load(self): # Insert two dummy watch rules into the DB - values = {'stack_name': u'wordpress_ha', 'state': 'NORMAL', + values = {'stack_id': self.stack_id, + 'state': 'NORMAL', 'name': u'HttpFailureAlarm', 'rule': { u'EvaluationPeriods': u'1', @@ -291,7 +312,6 @@ class WatchRuleTest(unittest.TestCase): self.assertEqual(wr.name, wn) self.assertEqual(wr.state, values['state']) self.assertEqual(wr.rule, values['rule']) - self.assertEqual(wr.stack_name, values['stack_name']) self.assertEqual(wr.timeperiod, datetime.timedelta( seconds=int(values['rule']['Period']))) @@ -301,23 +321,22 @@ class WatchRuleTest(unittest.TestCase): def test_store(self): rule = {u'EvaluationPeriods': u'1', - u'AlarmActions': [u'WebServerRestartPolicy'], - u'AlarmDescription': u'Restart the WikiDatabase', - u'Namespace': u'system/linux', - u'Period': u'300', - u'ComparisonOperator': u'GreaterThanThreshold', - u'Statistic': u'SampleCount', - u'Threshold': u'2', - u'MetricName': u'ServiceFailure'} + u'AlarmActions': [u'WebServerRestartPolicy'], + u'AlarmDescription': u'Restart the WikiDatabase', + u'Namespace': u'system/linux', + u'Period': u'300', + u'ComparisonOperator': u'GreaterThanThreshold', + u'Statistic': u'SampleCount', + u'Threshold': u'2', + u'MetricName': u'ServiceFailure'} wr = watchrule.WatchRule(context=self.ctx, watch_name='storetest', - rule=rule, stack_name='teststack') + stack_id=self.stack_id, rule=rule) wr.store() dbwr = db_api.watch_rule_get_by_name(self.ctx, 'storetest') self.assertNotEqual(dbwr, None) self.assertEqual(dbwr.name, 'storetest') self.assertEqual(dbwr.state, watchrule.WatchRule.NORMAL) - self.assertEqual(dbwr.stack_name, 'teststack') self.assertEqual(dbwr.rule, rule) # Cleanup -- 2.45.2