From 43a2e9ddc051d5af964c80c5ae2b7f78d1b09a8e Mon Sep 17 00:00:00 2001 From: Joe Gordon Date: Fri, 30 Nov 2012 13:57:00 -0800 Subject: [PATCH] Access DB values as dict not as attributes. Part 3 We cannot assume nova.db.api will be returning sqlalchemy objects attributes, instead treat return values as dicts Part of blueprint db-api-cleanup Change-Id: Icf45e5475b9d626f7e07efe953615dd82bb69ade --- nova/tests/test_xenapi.py | 87 +++++++++++++++++------------------- nova/virt/firewall.py | 4 +- nova/virt/xenapi/firewall.py | 8 ++-- nova/virt/xenapi/pool.py | 54 +++++++++++----------- 4 files changed, 74 insertions(+), 79 deletions(-) diff --git a/nova/tests/test_xenapi.py b/nova/tests/test_xenapi.py index c23e4257087c..5f6bdf0f0332 100644 --- a/nova/tests/test_xenapi.py +++ b/nova/tests/test_xenapi.py @@ -242,9 +242,9 @@ class XenAPIVolumeTestCase(stubs.XenAPITestBase): stubs.stubout_session(self.stubs, stubs.FakeSessionForVolumeTests) conn = xenapi_conn.XenAPIDriver(fake.FakeVirtAPI(), False) instance = db.instance_create(self.context, self.instance_values) - vm = xenapi_fake.create_vm(instance.name, 'Running') + vm = xenapi_fake.create_vm(instance['name'], 'Running') result = conn.attach_volume(self._make_connection_info(), - instance.name, '/dev/sdc') + instance['name'], '/dev/sdc') # check that the VM has a VBD attached to it # Get XenAPI record for VBD @@ -259,11 +259,11 @@ class XenAPIVolumeTestCase(stubs.XenAPITestBase): stubs.FakeSessionForVolumeFailedTests) conn = xenapi_conn.XenAPIDriver(fake.FakeVirtAPI(), False) instance = db.instance_create(self.context, self.instance_values) - xenapi_fake.create_vm(instance.name, 'Running') + xenapi_fake.create_vm(instance['name'], 'Running') self.assertRaises(exception.VolumeDriverNotFound, conn.attach_volume, {'driver_volume_type': 'nonexist'}, - instance.name, + instance['name'], '/dev/sdc') @@ -409,7 +409,7 @@ class XenAPIVMTestCase(stubs.XenAPITestBase): if not vm_rec["is_control_domain"]: vm_labels.append(vm_rec["name_label"]) - self.assertEquals(vm_labels, [instance.name]) + self.assertEquals(vm_labels, [instance['name']]) # Ensure VBDs were torn down vbd_labels = [] @@ -417,7 +417,7 @@ class XenAPIVMTestCase(stubs.XenAPITestBase): vbd_rec = xenapi_fake.get_record('VBD', vbd_ref) vbd_labels.append(vbd_rec["vm_name_label"]) - self.assertEquals(vbd_labels, [instance.name]) + self.assertEquals(vbd_labels, [instance['name']]) # Ensure VDIs were torn down for vdi_ref in xenapi_fake.get_all('VDI'): @@ -587,8 +587,8 @@ class XenAPIVMTestCase(stubs.XenAPITestBase): 'herp', network_info) self.create_vm_record(self.conn, os_type, instance['name']) self.check_vm_record(self.conn, check_injection) - self.assertTrue(instance.os_type) - self.assertTrue(instance.architecture) + self.assertTrue(instance['os_type']) + self.assertTrue(instance['architecture']) def test_spawn_empty_dns(self): """Test spawning with an empty dns list""" @@ -825,7 +825,7 @@ class XenAPIVMTestCase(stubs.XenAPITestBase): instance = self._create_instance() session = xenapi_conn.XenAPISession('test_url', 'root', 'test_pass', fake.FakeVirtAPI()) - vm_ref = vm_utils.lookup(session, instance.name) + vm_ref = vm_utils.lookup(session, instance['name']) swap_vdi_ref = xenapi_fake.create_vdi('swap', None) root_vdi_ref = xenapi_fake.create_vdi('root', None) @@ -853,7 +853,8 @@ class XenAPIVMTestCase(stubs.XenAPITestBase): conn = xenapi_conn.XenAPIDriver(fake.FakeVirtAPI(), False) # Unrescue expects the original instance to be powered off conn.power_off(instance) - rescue_vm = xenapi_fake.create_vm(instance.name + '-rescue', 'Running') + rescue_vm = xenapi_fake.create_vm(instance['name'] + '-rescue', + 'Running') conn.unrescue(instance, None) def test_unrescue_not_in_rescue(self): @@ -894,16 +895,16 @@ class XenAPIVMTestCase(stubs.XenAPITestBase): fake.FakeVirtAPI()) instance = self._create_instance(spawn=False) conn = xenapi_conn.XenAPIDriver(fake.FakeVirtAPI(), False) - xenapi_fake.create_vm(instance.name, 'Halted') + xenapi_fake.create_vm(instance['name'], 'Halted') conn.reboot(instance, None, "SOFT") - vm_ref = vm_utils.lookup(session, instance.name) + vm_ref = vm_utils.lookup(session, instance['name']) vm = xenapi_fake.get_record('VM', vm_ref) self.assertEquals(vm['power_state'], 'Running') def test_reboot_unknown_state(self): instance = self._create_instance(spawn=False) conn = xenapi_conn.XenAPIDriver(fake.FakeVirtAPI(), False) - xenapi_fake.create_vm(instance.name, 'Unknown') + xenapi_fake.create_vm(instance['name'], 'Unknown') self.assertRaises(xenapi_fake.Failure, conn.reboot, instance, None, "SOFT") @@ -1128,7 +1129,7 @@ class XenAPIMigrateInstance(stubs.XenAPITestBase): def test_migrate_disk_and_power_off(self): instance = db.instance_create(self.context, self.instance_values) - xenapi_fake.create_vm(instance.name, 'Running') + xenapi_fake.create_vm(instance['name'], 'Running') instance_type = db.instance_type_get_by_name(self.context, 'm1.large') conn = xenapi_conn.XenAPIDriver(fake.FakeVirtAPI(), False) conn.migrate_disk_and_power_off(self.context, instance, @@ -1136,7 +1137,7 @@ class XenAPIMigrateInstance(stubs.XenAPITestBase): def test_migrate_disk_and_power_off_passes_exceptions(self): instance = db.instance_create(self.context, self.instance_values) - xenapi_fake.create_vm(instance.name, 'Running') + xenapi_fake.create_vm(instance['name'], 'Running') instance_type = db.instance_type_get_by_name(self.context, 'm1.large') def fake_raise(*args, **kwargs): @@ -1176,7 +1177,7 @@ class XenAPIMigrateInstance(stubs.XenAPITestBase): conn = xenapi_conn.XenAPIDriver(fake.FakeVirtAPI(), False) network_info = fake_network.fake_get_instance_nw_info(self.stubs, spectacular=True) - image_meta = {'id': instance.image_ref, 'disk_format': 'vhd'} + image_meta = {'id': instance['image_ref'], 'disk_format': 'vhd'} base = xenapi_fake.create_vdi('hurr', 'fake') base_uuid = xenapi_fake.get_record('VDI', base)['uuid'] cow = xenapi_fake.create_vdi('durr', 'fake') @@ -1211,7 +1212,7 @@ class XenAPIMigrateInstance(stubs.XenAPITestBase): conn = xenapi_conn.XenAPIDriver(fake.FakeVirtAPI(), False) network_info = fake_network.fake_get_instance_nw_info(self.stubs, spectacular=True) - image_meta = {'id': instance.image_ref, 'disk_format': 'vhd'} + image_meta = {'id': instance['image_ref'], 'disk_format': 'vhd'} conn.finish_migration(self.context, self.migration, instance, dict(base_copy='hurr', cow='durr'), network_info, image_meta, resize_instance=True) @@ -1233,7 +1234,7 @@ class XenAPIMigrateInstance(stubs.XenAPITestBase): conn = xenapi_conn.XenAPIDriver(fake.FakeVirtAPI(), False) network_info = fake_network.fake_get_instance_nw_info(self.stubs, spectacular=True) - image_meta = {'id': instance.image_ref, 'disk_format': 'vhd'} + image_meta = {'id': instance['image_ref'], 'disk_format': 'vhd'} conn.finish_migration(self.context, self.migration, instance, dict(base_copy='hurr', cow='durr'), network_info, image_meta, resize_instance=True) @@ -1250,7 +1251,7 @@ class XenAPIMigrateInstance(stubs.XenAPITestBase): network_info = fake_network.fake_get_instance_nw_info(self.stubs, spectacular=True) # Resize instance would be determined by the compute call - image_meta = {'id': instance.image_ref, 'disk_format': 'vhd'} + image_meta = {'id': instance['image_ref'], 'disk_format': 'vhd'} conn.finish_migration(self.context, self.migration, instance, dict(base_copy='hurr', cow='durr'), network_info, image_meta, resize_instance=False) @@ -1261,7 +1262,7 @@ class XenAPIMigrateInstance(stubs.XenAPITestBase): instance_values['root_gb'] = 40 instance_values['auto_disk_config'] = False instance = db.instance_create(self.context, instance_values) - xenapi_fake.create_vm(instance.name, 'Running') + xenapi_fake.create_vm(instance['name'], 'Running') instance_type = db.instance_type_get_by_name(self.context, 'm1.small') conn = xenapi_conn.XenAPIDriver(fake.FakeVirtAPI(), False) self.assertRaises(exception.ResizeError, @@ -2159,10 +2160,10 @@ class XenAPIAggregateTestCase(stubs.XenAPITestBase): aggregate = self._aggregate_setup() self.conn._pool.add_to_aggregate(self.context, aggregate, "host") - result = db.aggregate_get(self.context, aggregate.id) + result = db.aggregate_get(self.context, aggregate['id']) self.assertTrue(fake_init_pool.called) self.assertThat(self.fake_metadata, - matchers.DictMatches(result.metadetails)) + matchers.DictMatches(result['metadetails'])) def test_join_slave(self): """Ensure join_slave gets called when the request gets to master.""" @@ -2192,12 +2193,12 @@ class XenAPIAggregateTestCase(stubs.XenAPITestBase): result = db.aggregate_create(self.context, values) metadata = {pool_states.POOL_FLAG: "XenAPI", pool_states.KEY: pool_states.CREATED} - db.aggregate_metadata_add(self.context, result.id, metadata) + db.aggregate_metadata_add(self.context, result['id'], metadata) - db.aggregate_host_add(self.context, result.id, "host") - aggregate = db.aggregate_get(self.context, result.id) - self.assertEqual(["host"], aggregate.hosts) - self.assertEqual(metadata, aggregate.metadetails) + db.aggregate_host_add(self.context, result['id'], "host") + aggregate = db.aggregate_get(self.context, result['id']) + self.assertEqual(["host"], aggregate['hosts']) + self.assertEqual(metadata, aggregate['metadetails']) self.conn._pool.add_to_aggregate(self.context, aggregate, "host") self.assertTrue(fake_pool_set_name_label.called) @@ -2238,11 +2239,11 @@ class XenAPIAggregateTestCase(stubs.XenAPITestBase): aggregate = self._aggregate_setup(metadata=self.fake_metadata) self.conn._pool.remove_from_aggregate(self.context, aggregate, "host") - result = db.aggregate_get(self.context, aggregate.id) + result = db.aggregate_get(self.context, aggregate['id']) self.assertTrue(fake_clear_pool.called) self.assertThat({pool_states.POOL_FLAG: 'XenAPI', pool_states.KEY: pool_states.ACTIVE}, - matchers.DictMatches(result.metadetails)) + matchers.DictMatches(result['metadetails'])) def test_remote_master_non_empty_pool(self): """Ensure AggregateError is raised if removing the master.""" @@ -2262,13 +2263,13 @@ class XenAPIAggregateTestCase(stubs.XenAPITestBase): result = db.aggregate_create(self.context, values) pool_flag = {pool_states.POOL_FLAG: "XenAPI", pool_states.KEY: aggr_state} - db.aggregate_metadata_add(self.context, result.id, pool_flag) + db.aggregate_metadata_add(self.context, result['id'], pool_flag) for host in hosts: - db.aggregate_host_add(self.context, result.id, host) + db.aggregate_host_add(self.context, result['id'], host) if metadata: - db.aggregate_metadata_add(self.context, result.id, metadata) - return db.aggregate_get(self.context, result.id) + db.aggregate_metadata_add(self.context, result['id'], metadata) + return db.aggregate_get(self.context, result['id']) def test_add_host_to_aggregate_invalid_changing_status(self): """Ensure InvalidAggregateAction is raised when adding host while @@ -2341,23 +2342,17 @@ class XenAPIAggregateTestCase(stubs.XenAPITestBase): fake_driver_add_to_aggregate) metadata = {pool_states.POOL_FLAG: "XenAPI", pool_states.KEY: pool_states.ACTIVE} - db.aggregate_metadata_add(self.context, self.aggr.id, metadata) - db.aggregate_host_add(self.context, self.aggr.id, 'fake_host') + db.aggregate_metadata_add(self.context, self.aggr['id'], metadata) + db.aggregate_host_add(self.context, self.aggr['id'], 'fake_host') self.assertRaises(exception.AggregateError, self.compute.add_aggregate_host, self.context, "fake_host", aggregate=jsonutils.to_primitive(self.aggr)) - excepted = db.aggregate_get(self.context, self.aggr.id) - self.assertEqual(excepted.metadetails[pool_states.KEY], + excepted = db.aggregate_get(self.context, self.aggr['id']) + self.assertEqual(excepted['metadetails'][pool_states.KEY], pool_states.ERROR) - self.assertEqual(excepted.hosts, []) - - -class Aggregate(object): - def __init__(self, id=None, hosts=None): - self.id = id - self.hosts = hosts or [] + self.assertEqual(excepted['hosts'], []) class MockComputeAPI(object): @@ -2404,7 +2399,7 @@ class HypervisorPoolTestCase(test.TestCase): def test_slave_asks_master_to_add_slave_to_pool(self): slave = ResourcePoolWithStubs() - aggregate = Aggregate(id=98, hosts=[]) + aggregate = {'id': 98, 'hosts': []} slave.add_to_aggregate("CONTEXT", aggregate, "slave") @@ -2416,7 +2411,7 @@ class HypervisorPoolTestCase(test.TestCase): def test_slave_asks_master_to_remove_slave_from_pool(self): slave = ResourcePoolWithStubs() - aggregate = Aggregate(id=98, hosts=[]) + aggregate = {'id': 98, 'hosts': []} slave.remove_from_aggregate("CONTEXT", aggregate, "slave") diff --git a/nova/virt/firewall.py b/nova/virt/firewall.py index ee39b0b8ca6e..035c38080862 100644 --- a/nova/virt/firewall.py +++ b/nova/virt/firewall.py @@ -297,8 +297,8 @@ class IptablesFirewallDriver(FirewallDriver): '-s %s/128 -p icmpv6 -j ACCEPT' % (gateway_v6,)) def _build_icmp_rule(self, rule, version): - icmp_type = rule.from_port - icmp_code = rule.to_port + icmp_type = rule['from_port'] + icmp_code = rule['to_port'] if icmp_type == -1: icmp_type_arg = None diff --git a/nova/virt/xenapi/firewall.py b/nova/virt/xenapi/firewall.py index a3935583095b..e30465741ce1 100644 --- a/nova/virt/xenapi/firewall.py +++ b/nova/virt/xenapi/firewall.py @@ -55,12 +55,12 @@ class Dom0IptablesFirewallDriver(firewall.IptablesFirewallDriver): self.iptables.ipv6['filter'].add_rule('sg-fallback', '-j DROP') def _build_tcp_udp_rule(self, rule, version): - if rule.from_port == rule.to_port: - return ['--dport', '%s' % (rule.from_port,)] + if rule['from_port'] == rule['to_port']: + return ['--dport', '%s' % (rule['from_port'],)] else: # No multiport needed for XS! - return ['--dport', '%s:%s' % (rule.from_port, - rule.to_port)] + return ['--dport', '%s:%s' % (rule['from_port'], + rule['to_port'])] def _provider_rules(self): """Generate a list of rules from provider for IP4 & IP6. diff --git a/nova/virt/xenapi/pool.py b/nova/virt/xenapi/pool.py index e76bdbe1c3c2..0be6fad12796 100644 --- a/nova/virt/xenapi/pool.py +++ b/nova/virt/xenapi/pool.py @@ -80,51 +80,51 @@ class ResourcePool(object): def add_to_aggregate(self, context, aggregate, host, slave_info=None): """Add a compute host to an aggregate.""" - if not self._is_hv_pool(context, aggregate.id): + if not self._is_hv_pool(context, aggregate['id']): return invalid = {pool_states.CHANGING: 'setup in progress', pool_states.DISMISSED: 'aggregate deleted', pool_states.ERROR: 'aggregate in error'} - if (self._get_metadata(context, aggregate.id)[pool_states.KEY] + if (self._get_metadata(context, aggregate['id'])[pool_states.KEY] in invalid.keys()): raise exception.InvalidAggregateAction( action='add host', - aggregate_id=aggregate.id, + aggregate_id=aggregate['id'], reason=invalid[self._get_metadata(context, - aggregate.id) + aggregate['id']) [pool_states.KEY]]) - if (self._get_metadata(context, aggregate.id)[pool_states.KEY] + if (self._get_metadata(context, aggregate['id'])[pool_states.KEY] == pool_states.CREATED): - self._virtapi.aggregate_metadata_add(context, aggregate.id, + self._virtapi.aggregate_metadata_add(context, aggregate['id'], {pool_states.KEY: pool_states.CHANGING}) - if len(aggregate.hosts) == 1: + if len(aggregate['hosts']) == 1: # this is the first host of the pool -> make it master - self._init_pool(aggregate.id, aggregate.name) + self._init_pool(aggregate['id'], aggregate['name']) # save metadata so that we can find the master again metadata = {'master_compute': host, host: self._host_uuid, pool_states.KEY: pool_states.ACTIVE} - self._virtapi.aggregate_metadata_add(context, aggregate.id, + self._virtapi.aggregate_metadata_add(context, aggregate['id'], metadata) else: # the pool is already up and running, we need to figure out # whether we can serve the request from this host or not. master_compute = self._get_metadata(context, - aggregate.id)['master_compute'] + aggregate['id'])['master_compute'] if master_compute == CONF.host and master_compute != host: # this is the master -> do a pool-join # To this aim, nova compute on the slave has to go down. # NOTE: it is assumed that ONLY nova compute is running now - self._join_slave(aggregate.id, host, + self._join_slave(aggregate['id'], host, slave_info.get('compute_uuid'), slave_info.get('url'), slave_info.get('user'), slave_info.get('passwd')) metadata = {host: slave_info.get('xenhost_uuid'), } - self._virtapi.aggregate_metadata_add(context, aggregate.id, + self._virtapi.aggregate_metadata_add(context, aggregate['id'], metadata) elif master_compute and master_compute != host: # send rpc cast to master, asking to add the following @@ -137,55 +137,55 @@ class ResourcePool(object): def remove_from_aggregate(self, context, aggregate, host, slave_info=None): """Remove a compute host from an aggregate.""" slave_info = slave_info or dict() - if not self._is_hv_pool(context, aggregate.id): + if not self._is_hv_pool(context, aggregate['id']): return invalid = {pool_states.CREATED: 'no hosts to remove', pool_states.CHANGING: 'setup in progress', pool_states.DISMISSED: 'aggregate deleted', } - if (self._get_metadata(context, aggregate.id)[pool_states.KEY] + if (self._get_metadata(context, aggregate['id'])[pool_states.KEY] in invalid.keys()): raise exception.InvalidAggregateAction( action='remove host', - aggregate_id=aggregate.id, + aggregate_id=aggregate['id'], reason=invalid[self._get_metadata(context, - aggregate.id)[pool_states.KEY]]) + aggregate['id'])[pool_states.KEY]]) master_compute = self._get_metadata(context, - aggregate.id)['master_compute'] + aggregate['id'])['master_compute'] if master_compute == CONF.host and master_compute != host: # this is the master -> instruct it to eject a host from the pool - host_uuid = self._get_metadata(context, aggregate.id)[host] - self._eject_slave(aggregate.id, + host_uuid = self._get_metadata(context, aggregate['id'])[host] + self._eject_slave(aggregate['id'], slave_info.get('compute_uuid'), host_uuid) - self._virtapi.aggregate_metadata_delete(context, aggregate.id, + self._virtapi.aggregate_metadata_delete(context, aggregate['id'], host) elif master_compute == host: # Remove master from its own pool -> destroy pool only if the # master is on its own, otherwise raise fault. Destroying a # pool made only by master is fictional - if len(aggregate.hosts) > 1: + if len(aggregate['hosts']) > 1: # NOTE: this could be avoided by doing a master # re-election, but this is simpler for now. raise exception.InvalidAggregateAction( - aggregate_id=aggregate.id, + aggregate_id=aggregate['id'], action='remove_from_aggregate', reason=_('Unable to eject %(host)s ' 'from the pool; pool not empty') % locals()) - self._clear_pool(aggregate.id) + self._clear_pool(aggregate['id']) for key in ['master_compute', host]: - self._virtapi.aggregate_metadata_delete(context, aggregate.id, - key) + self._virtapi.aggregate_metadata_delete(context, + aggregate['id'], key) elif master_compute and master_compute != host: # A master exists -> forward pool-eject request to master slave_info = self._create_slave_info() self.compute_rpcapi.remove_aggregate_host( - context, aggregate.id, host, master_compute, slave_info) + context, aggregate['id'], host, master_compute, slave_info) else: # this shouldn't have happened - raise exception.AggregateError(aggregate_id=aggregate.id, + raise exception.AggregateError(aggregate_id=aggregate['id'], action='remove_from_aggregate', reason=_('Unable to eject %(host)s ' 'from the pool; No master found')