From fe97e74eea29c990038c0dd36012054d34d704ac Mon Sep 17 00:00:00 2001 From: Donal Lafferty Date: Fri, 6 May 2011 15:57:01 +0100 Subject: Changes to allow a VM to boot from iso image. A blank HD is also attached with a size corresponding to the instance type. --- nova/virt/xenapi/vm_utils.py | 104 +++++++++++++++++++++++++++++++++++++++++-- nova/virt/xenapi/vmops.py | 17 ++++++- 2 files changed, 116 insertions(+), 5 deletions(-) diff --git a/nova/virt/xenapi/vm_utils.py b/nova/virt/xenapi/vm_utils.py index d2045a557..60dddb263 100644 --- a/nova/virt/xenapi/vm_utils.py +++ b/nova/virt/xenapi/vm_utils.py @@ -74,12 +74,14 @@ class ImageType: 2 - raw disk image (local SR, NOT partitioned by plugin) 3 - vhd disk image (local SR, NOT inspected by XS, PV assumed for linux, HVM assumed for Windows) + 4 - ISO disk image (local SR, NOT partitioned by plugin) """ KERNEL_RAMDISK = 0 DISK = 1 DISK_RAW = 2 DISK_VHD = 3 + DISK_ISO = 4 class VMHelper(HelperBase): @@ -204,6 +206,30 @@ class VMHelper(HelperBase): ' VDI %(vdi_ref)s.') % locals()) return vbd_ref + @classmethod + def create_cd_vbd(cls, session, vm_ref, vdi_ref, userdevice, bootable): + """Create a VBD record. Returns a Deferred that gives the new + VBD reference specific to CDRom devices.""" + vbd_rec = {} + vbd_rec['VM'] = vm_ref + vbd_rec['VDI'] = vdi_ref + vbd_rec['userdevice'] = str(userdevice) + vbd_rec['bootable'] = bootable + vbd_rec['mode'] = 'RO' + vbd_rec['type'] = 'CD' + vbd_rec['unpluggable'] = True + vbd_rec['empty'] = False + vbd_rec['other_config'] = {} + vbd_rec['qos_algorithm_type'] = '' + vbd_rec['qos_algorithm_params'] = {} + vbd_rec['qos_supported_algorithms'] = [] + LOG.debug(_('Creating a CDROM-specific VBD for VM %(vm_ref)s,' + ' VDI %(vdi_ref)s ... ') % locals()) + vbd_ref = session.call_xenapi('VBD.create', vbd_rec) + LOG.debug(_('Created a CDROM-specific VBD %(vbd_ref)s ' + ' for VM %(vm_ref)s, VDI %(vdi_ref)s.') % locals()) + return vbd_ref + @classmethod def find_vbd_by_number(cls, session, vm_ref, number): """Get the VBD reference from the device number""" @@ -369,6 +395,23 @@ class VMHelper(HelperBase): task = session.async_call_plugin('glance', 'upload_vhd', kwargs) session.wait_for_task(task, instance.id) + @classmethod + def fetch_blank_disk(cls, session, instance_type_id): + # Size the blank harddrive to suit the machine type: + one_gig = 1024 * 1024 * 1024 + req_type = instance_types.get_instance_type(instance_type_id) + req_size = req_type['local_gb'] + + LOG.debug("Creating blank HD of size %(req_size)d gigs" + % locals()) + vdi_size = one_gig * req_size + + LOG.debug("ISO vm create: Looking for the SR") + sr_ref = safe_find_sr(session) + + vdi_ref = cls.create_vdi(session, sr_ref, 'blank HD', vdi_size, False) + return vdi_ref + @classmethod def fetch_image(cls, session, instance_id, image, user, project, image_type): @@ -437,7 +480,12 @@ class VMHelper(HelperBase): # FIXME(sirp): Since the Glance plugin seems to be required for the # VHD disk, it may be worth using the plugin for both VHD and RAW and # DISK restores - sr_ref = safe_find_sr(session) + sr_ref = None + if image_type == ImageType.DISK_ISO: + sr_ref = safe_find_iso_sr(session) + LOG.debug(_("ISO: Found sr possibly containing the ISO image")) + else: + sr_ref = safe_find_sr(session) client = glance.client.Client(FLAGS.glance_host, FLAGS.glance_port) meta, image_file = client.get_image(image) @@ -452,6 +500,8 @@ class VMHelper(HelperBase): name_label = get_name_label_for_image(image) vdi_ref = cls.create_vdi(session, sr_ref, name_label, vdi_size, False) + LOG.debug(_("Loading image of size %(virtual_size)d. Full set of" + "properties is %(meta)s" % locals())) with_vdi_attached_here(session, vdi_ref, False, lambda dev: _stream_disk(dev, image_type, @@ -490,7 +540,8 @@ class VMHelper(HelperBase): pretty_format = {ImageType.KERNEL_RAMDISK: 'KERNEL_RAMDISK', ImageType.DISK: 'DISK', ImageType.DISK_RAW: 'DISK_RAW', - ImageType.DISK_VHD: 'DISK_VHD'} + ImageType.DISK_VHD: 'DISK_VHD', + ImageType.DISK_ISO: 'DISK_ISO'} disk_format = pretty_format[image_type] image_id = instance.image_id instance_id = instance.id @@ -503,7 +554,8 @@ class VMHelper(HelperBase): 'aki': ImageType.KERNEL_RAMDISK, 'ari': ImageType.KERNEL_RAMDISK, 'raw': ImageType.DISK_RAW, - 'vhd': ImageType.DISK_VHD} + 'vhd': ImageType.DISK_VHD, + 'iso': ImageType.DISK_ISO} client = glance.client.Client(FLAGS.glance_host, FLAGS.glance_port) meta = client.get_image_meta(instance.image_id) disk_format = meta['disk_format'] @@ -579,9 +631,11 @@ class VMHelper(HelperBase): available 4. Glance (DISK): pv is assumed + + 5. Glance (ISO): use 'os_type', raise if not set """ if FLAGS.xenapi_image_service == 'glance': - # 2, 3, 4: Glance + # 2, 3, 4, 5: Glance return cls._determine_is_pv_glance( session, vdi_ref, disk_image_type, os_type) else: @@ -618,6 +672,8 @@ class VMHelper(HelperBase): available 4. Glance (DISK): pv is assumed + + 5. Glance (DISK_ISO): no pv is assumed """ LOG.debug(_("Looking up vdi %s for PV kernel"), vdi_ref) @@ -633,6 +689,9 @@ class VMHelper(HelperBase): elif disk_image_type == ImageType.DISK: # 4. Disk is_pv = True + elif disk_image_type == ImageType.DISK_ISO: + # 5. ISO + is_pv = False else: raise exception.Error(_("Unknown image format %(disk_image_type)s") % locals()) @@ -877,6 +936,43 @@ def find_sr(session): return None +def safe_find_iso_sr(session): + """Same as find_iso_sr except raises a NotFound exception if SR cannot be + determined + """ + sr_ref = find_iso_sr(session) + if sr_ref is None: + raise exception.NotFound(_('Cannot find SR of content-type ISO')) + return sr_ref + + +def find_iso_sr(session): + """Return the storage repository to hold ISO images""" + host = session.get_xenapi_host() + sr_refs = session.get_xenapi().SR.get_all() + for sr_ref in sr_refs: + sr_rec = session.get_xenapi().SR.get_record(sr_ref) + + LOG.debug(_("ISO: looking at SR %(sr_rec)s") % locals()) + #TODO: use ['other_config']['il8n-key-iso-sr'] == 'local-storage' + # or something similar to identify the iso sr + if not (sr_rec['content_type'] == 'iso' and + sr_rec['name_label'] == 'Local ISOs'): + LOG.debug(_("ISO: No match")) + continue + LOG.debug(_("ISO: SR MATCHing our criteria")) + for pbd_ref in sr_rec['PBDs']: + LOG.debug(_("ISO: ISO, looking to see if it is host local")) + pbd_rec = session.get_xenapi().PBD.get_record(pbd_ref) + pbd_rec_host = pbd_rec['host'] + LOG.debug(_("ISO: PBD matching, want %(pbd_rec)s, have %(host)s") % + locals()) + if pbd_rec_host == host: + LOG.debug(_("ISO: SR with local PBD")) + return sr_ref + return None + + def remap_vbd_dev(dev): """Return the appropriate location for a plugged-in VBD device diff --git a/nova/virt/xenapi/vmops.py b/nova/virt/xenapi/vmops.py index 7c7aa8e98..a2ce38c7a 100644 --- a/nova/virt/xenapi/vmops.py +++ b/nova/virt/xenapi/vmops.py @@ -162,7 +162,22 @@ class VMOps(object): vm_ref = VMHelper.create_vm(self._session, instance, kernel, ramdisk, use_pv_kernel) - VMHelper.create_vbd(session=self._session, vm_ref=vm_ref, + # DISK_ISO needs two VBDs: the ISO disk and a blank RW disk + if disk_image_type == ImageType.DISK_ISO: + LOG.debug("detected ISO image type, going to create blank VM for " + "install") + # device 0 reserved for RW disk, so use '1' + cd_vdi_ref = vdi_ref + VMHelper.create_cd_vbd(session=self._session, vm_ref=vm_ref, + vdi_ref=cd_vdi_ref, userdevice=1, bootable=True) + + vdi_ref = VMHelper.fetch_blank_disk(session=self._session, + instance_type_id=instance.instance_type_id) + + VMHelper.create_vbd(session=self._session, vm_ref=vm_ref, + vdi_ref=vdi_ref, userdevice=0, bootable=False) + else: + VMHelper.create_vbd(session=self._session, vm_ref=vm_ref, vdi_ref=vdi_ref, userdevice=0, bootable=True) # TODO(tr3buchet) - check to make sure we have network info, otherwise -- cgit From 11a7736f32b0f26fb2dc496b495aee682bf1bf18 Mon Sep 17 00:00:00 2001 From: Donal Lafferty Date: Fri, 6 May 2011 16:54:57 +0100 Subject: New author in town. --- Authors | 1 + 1 file changed, 1 insertion(+) diff --git a/Authors b/Authors index f4b40a853..427ada3bc 100644 --- a/Authors +++ b/Authors @@ -18,6 +18,7 @@ Dan Prince David Pravec Dean Troyer Devin Carlen +Donal Lafferty Ed Leafe Eldar Nugaev Eric Day -- cgit From 0a3b50d7b754af26b68f81617cab9aa588484362 Mon Sep 17 00:00:00 2001 From: Donal Lafferty Date: Mon, 27 Jun 2011 16:03:14 +0100 Subject: Pulled changes, passed the unit tests. --- .../migrate_repo/versions/027_add_provider_firewall_rules.py | 3 +-- nova/virt/xenapi/vmops.py | 5 +---- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/nova/db/sqlalchemy/migrate_repo/versions/027_add_provider_firewall_rules.py b/nova/db/sqlalchemy/migrate_repo/versions/027_add_provider_firewall_rules.py index 5aa30f7a8..cb3c73170 100644 --- a/nova/db/sqlalchemy/migrate_repo/versions/027_add_provider_firewall_rules.py +++ b/nova/db/sqlalchemy/migrate_repo/versions/027_add_provider_firewall_rules.py @@ -58,8 +58,7 @@ provider_fw_rules = Table('provider_fw_rules', meta, Column('to_port', Integer()), Column('cidr', String(length=255, convert_unicode=False, assert_unicode=None, - unicode_error=None, _warn_on_bytestring=False)) - ) + unicode_error=None, _warn_on_bytestring=False))) def upgrade(migrate_engine): diff --git a/nova/virt/xenapi/vmops.py b/nova/virt/xenapi/vmops.py index 5cf99b9ac..eef354aec 100644 --- a/nova/virt/xenapi/vmops.py +++ b/nova/virt/xenapi/vmops.py @@ -196,7 +196,7 @@ class VMOps(object): kernel, ramdisk, use_pv_kernel) # device 0 reserved for RW disk - userdevice = 0; + userdevice = 0 # DISK_ISO needs two VBDs: the ISO disk and a blank RW disk if disk_image_type == ImageType.DISK_ISO: @@ -214,15 +214,12 @@ class VMOps(object): userdevice = userdevice + 2 VMHelper.create_cd_vbd(session=self._session, vm_ref=vm_ref, vdi_ref=cd_vdi_ref, userdevice=userdevice, bootable=True) - - else: VMHelper.create_vbd(session=self._session, vm_ref=vm_ref, vdi_ref=first_vdi_ref, userdevice=userdevice, bootable=True) # userdevice 1 is reserved for rescue userdevice = userdevice + 1 - # Attach any other disks for vdi in vdis[1:]: # vdi['vdi_type'] is either 'os' or 'swap', but we don't -- cgit From 32b06d654922e08a53c6a4fc49fd2ad40e7a5d20 Mon Sep 17 00:00:00 2001 From: Donal Lafferty Date: Tue, 28 Jun 2011 12:32:23 +0100 Subject: Revise key used to identify the SR used to store ISO images streamed from Glance. --- nova/virt/xenapi/vm_utils.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/nova/virt/xenapi/vm_utils.py b/nova/virt/xenapi/vm_utils.py index 0d08e459c..ede8114c4 100644 --- a/nova/virt/xenapi/vm_utils.py +++ b/nova/virt/xenapi/vm_utils.py @@ -984,12 +984,14 @@ def find_iso_sr(session): sr_rec = session.get_xenapi().SR.get_record(sr_ref) LOG.debug(_("ISO: looking at SR %(sr_rec)s") % locals()) - #TODO: use ['other_config']['il8n-key-iso-sr'] == 'local-storage' - # or something similar to identify the iso sr if not (sr_rec['content_type'] == 'iso' and - sr_rec['name_label'] == 'Local ISOs'): - LOG.debug(_("ISO: No match")) + 'i18n-key' in sr_rec['other_config'] and + sr_rec['other_config']['i18n-key'] == 'local-storage-iso'): + LOG.debug(_("ISO: SR with 'content_type' of 'iso' and" + " 'other-config' key 'i18n-key' of value" + " 'local-storage-iso'")) continue + LOG.debug(_("ISO: SR MATCHing our criteria")) for pbd_ref in sr_rec['PBDs']: LOG.debug(_("ISO: ISO, looking to see if it is host local")) -- cgit From 8878f6433cebcac963ed8789200f38a5ac4dfddd Mon Sep 17 00:00:00 2001 From: Donal Lafferty Date: Tue, 28 Jun 2011 12:33:12 +0100 Subject: Add fake SR with ISO content type. --- nova/virt/xenapi/fake.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/nova/virt/xenapi/fake.py b/nova/virt/xenapi/fake.py index d5ac39473..1aa642e4e 100644 --- a/nova/virt/xenapi/fake.py +++ b/nova/virt/xenapi/fake.py @@ -194,6 +194,7 @@ def create_local_pifs(): Do this one per host.""" for host_ref in _db_content['host'].keys(): _create_local_pif(host_ref) + _create_local_sr_iso(host_ref) def create_local_srs(): @@ -222,6 +223,25 @@ def _create_local_sr(host_ref): return sr_ref +def _create_local_sr_iso(host_ref): + sr_ref = _create_object( + 'SR', + {'name_label': 'Local storage ISO', + 'type': 'lvm', + 'content_type': 'iso', + 'shared': False, + 'physical_size': str(1 << 30), + 'physical_utilisation': str(0), + 'virtual_allocation': str(0), + 'other_config': { + 'i18n-original-value-name_label': 'Local storage ISO', + 'i18n-key': 'local-storage-iso'}, + 'VDIs': []}) + pbd_ref = create_pbd('', host_ref, sr_ref, True) + _db_content['SR'][sr_ref]['PBDs'] = [pbd_ref] + return sr_ref + + def _create_local_pif(host_ref): pif_ref = _create_object('PIF', {'name-label': 'Fake PIF', -- cgit From 6ca1845582334d69474f5f9d661177e77cd769fe Mon Sep 17 00:00:00 2001 From: Donal Lafferty Date: Tue, 28 Jun 2011 12:34:17 +0100 Subject: Add test for spawn from an ISO. --- nova/tests/glance/stubs.py | 6 ++++++ nova/tests/test_xenapi.py | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/nova/tests/glance/stubs.py b/nova/tests/glance/stubs.py index 1e0b90d82..37fb7a9d7 100644 --- a/nova/tests/glance/stubs.py +++ b/nova/tests/glance/stubs.py @@ -32,6 +32,7 @@ class FakeGlance(object): IMAGE_RAMDISK = 3 IMAGE_RAW = 4 IMAGE_VHD = 5 + IMAGE_ISO = 6 IMAGE_FIXTURES = { IMAGE_MACHINE: { @@ -58,6 +59,11 @@ class FakeGlance(object): 'image_meta': {'name': 'fakevhd', 'size': 0, 'disk_format': 'vhd', 'container_format': 'ovf'}, + 'image_data': StringIO.StringIO('')}, + IMAGE_ISO: { + 'image_meta': {'name': 'fakeiso', 'size': 0, + 'disk_format': 'iso', + 'container_format': 'bare'}, 'image_data': StringIO.StringIO('')}} def __init__(self, host, port=None, use_ssl=False): diff --git a/nova/tests/test_xenapi.py b/nova/tests/test_xenapi.py index d9a514745..b8f53b735 100644 --- a/nova/tests/test_xenapi.py +++ b/nova/tests/test_xenapi.py @@ -446,6 +446,12 @@ class XenAPIVMTestCase(test.TestCase): os_type="windows", architecture="i386") self.check_vm_params_for_windows() + def test_spawn_iso_glance(self): + FLAGS.xenapi_image_service = 'glance' + self._test_spawn(glance_stubs.FakeGlance.IMAGE_ISO, None, None, + os_type="windows", architecture="i386") + self.check_vm_params_for_windows() + def test_spawn_glance(self): FLAGS.xenapi_image_service = 'glance' self._test_spawn(glance_stubs.FakeGlance.IMAGE_MACHINE, -- cgit From 3794d8889ed933fc776d7541ef25e2c9583a6cf6 Mon Sep 17 00:00:00 2001 From: Donal Lafferty Date: Wed, 29 Jun 2011 16:17:33 +0100 Subject: clean up logging for iso SR search --- nova/virt/xenapi/vm_utils.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/nova/virt/xenapi/vm_utils.py b/nova/virt/xenapi/vm_utils.py index ede8114c4..4a0e97e2e 100644 --- a/nova/virt/xenapi/vm_utils.py +++ b/nova/virt/xenapi/vm_utils.py @@ -984,12 +984,14 @@ def find_iso_sr(session): sr_rec = session.get_xenapi().SR.get_record(sr_ref) LOG.debug(_("ISO: looking at SR %(sr_rec)s") % locals()) - if not (sr_rec['content_type'] == 'iso' and - 'i18n-key' in sr_rec['other_config'] and - sr_rec['other_config']['i18n-key'] == 'local-storage-iso'): - LOG.debug(_("ISO: SR with 'content_type' of 'iso' and" - " 'other-config' key 'i18n-key' of value" - " 'local-storage-iso'")) + if not sr_rec['content_type'] == 'iso': + LOG.debug(_("ISO: not iso content")) + continue + if not 'i18n-key' in sr_rec['other_config']: + LOG.debug(_("ISO: iso content_type, no 'i18n-key' key")) + continue + if not sr_rec['other_config']['i18n-key'] == 'local-storage-iso': + LOG.debug(_("ISO: iso content_type, i18n-key value not 'local-storage-iso'")) continue LOG.debug(_("ISO: SR MATCHing our criteria")) -- cgit From 848fd99f378976f99fa883fec85f30c4e9f46bca Mon Sep 17 00:00:00 2001 From: Donal Lafferty Date: Wed, 29 Jun 2011 17:38:02 +0100 Subject: pep8 fix --- nova/virt/xenapi/vm_utils.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/nova/virt/xenapi/vm_utils.py b/nova/virt/xenapi/vm_utils.py index 4a0e97e2e..5f6387acc 100644 --- a/nova/virt/xenapi/vm_utils.py +++ b/nova/virt/xenapi/vm_utils.py @@ -991,7 +991,8 @@ def find_iso_sr(session): LOG.debug(_("ISO: iso content_type, no 'i18n-key' key")) continue if not sr_rec['other_config']['i18n-key'] == 'local-storage-iso': - LOG.debug(_("ISO: iso content_type, i18n-key value not 'local-storage-iso'")) + LOG.debug(_("ISO: iso content_type, i18n-key value not " + "'local-storage-iso'")) continue LOG.debug(_("ISO: SR MATCHing our criteria")) -- cgit From c3cdcc1eb0c9fd37f49701d976c7ceae8df44caf Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Fri, 22 Jul 2011 22:41:29 +0200 Subject: This is me being all cocky, thinking I'll make it use ipsets... --- nova/compute/api.py | 18 ++++++++------- nova/db/sqlalchemy/models.py | 6 +++++ nova/network/linux_net.py | 30 +++++++++++++++++++++++++ nova/network/manager.py | 24 +++++++++++++++++++- nova/tests/test_iptables_network.py | 39 ++++++++++++++++++++++++++++++-- nova/virt/libvirt/firewall.py | 44 +++++++++++++++++++++++++++---------- 6 files changed, 139 insertions(+), 22 deletions(-) diff --git a/nova/compute/api.py b/nova/compute/api.py index 432658bbb..65a594d2c 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -305,10 +305,6 @@ class API(base.Base): updates['hostname'] = self.hostname_factory(instance) instance = self.update(context, instance_id, **updates) - - for group_id in security_groups: - self.trigger_security_group_members_refresh(elevated, group_id) - return instance def _ask_scheduler_to_create_instance(self, context, base_options, @@ -464,19 +460,22 @@ class API(base.Base): {"method": "refresh_security_group_rules", "args": {"security_group_id": security_group.id}}) - def trigger_security_group_members_refresh(self, context, group_id): + def trigger_security_group_members_refresh(self, context, group_ids): """Called when a security group gains a new or loses a member. Sends an update request to each compute node for whom this is relevant. """ - # First, we get the security group rules that reference this group as + # First, we get the security group rules that reference these groups as # the grantee.. - security_group_rules = \ + security_group_rules = set() + for group_id in group_ids: + security_group_rules.update( self.db.security_group_rule_get_by_security_group_grantee( context, - group_id) + group_id)) + LOG.info('rules: %r', security_group_rules) # ..then we distill the security groups to which they belong.. security_groups = set() for rule in security_group_rules: @@ -485,12 +484,14 @@ class API(base.Base): rule['parent_group_id']) security_groups.add(security_group) + LOG.info('security_groups: %r', security_groups) # ..then we find the instances that are members of these groups.. instances = set() for security_group in security_groups: for instance in security_group['instances']: instances.add(instance) + LOG.info('instances: %r', instances) # ...then we find the hosts where they live... hosts = set() for instance in instances: @@ -500,6 +501,7 @@ class API(base.Base): # ...and finally we tell these nodes to refresh their view of this # particular security group. for host in hosts: + LOG.info('host: %r', host) rpc.cast(context, self.db.queue_get_for(context, FLAGS.compute_topic, host), {"method": "refresh_security_group_members", diff --git a/nova/db/sqlalchemy/models.py b/nova/db/sqlalchemy/models.py index d29d3d6f1..023821dfb 100644 --- a/nova/db/sqlalchemy/models.py +++ b/nova/db/sqlalchemy/models.py @@ -491,6 +491,12 @@ class SecurityGroupIngressRule(BASE, NovaBase): # Note: This is not the parent SecurityGroup. It's SecurityGroup we're # granting access for. group_id = Column(Integer, ForeignKey('security_groups.id')) + grantee_group = relationship("SecurityGroup", + foreign_keys=group_id, + primaryjoin='and_(' + 'SecurityGroupIngressRule.group_id == SecurityGroup.id,' + 'SecurityGroupIngressRule.deleted == False)') + class ProviderFirewallRule(BASE, NovaBase): diff --git a/nova/network/linux_net.py b/nova/network/linux_net.py index 283a5aca1..0e021a40f 100644 --- a/nova/network/linux_net.py +++ b/nova/network/linux_net.py @@ -96,6 +96,33 @@ class IptablesRule(object): chain = self.chain return '-A %s %s' % (chain, self.rule) +class IpSet(object): + """A class for handling large collections of IPs efficiently""" + + def __init__(self, name, execute=None): + self.name = name + self._ips = set() + if not execute: + self.execute = _execute + else: + self.execute = execute + + def __contains__(self, addr): + return addr in self._ips + + def _set_name(self): + return '%s-%s' % (binary_name, self.name) + + def add_ip(self, addr): + self._ips.add(addr) + self.execute('ipset', '-A', self._set_name(), addr) + + def remove_ip(self, addr): + self._ips.remove(addr) + self.execute('ipset', '-D', self._set_name(), addr) + + def iptables_source_match(self): + return ['-m set --match-set %s src' % (self._set_name(),)] class IptablesTable(object): """An iptables table.""" @@ -281,6 +308,9 @@ class IptablesManager(object): self.ipv4['nat'].add_chain('floating-snat') self.ipv4['nat'].add_rule('snat', '-j $floating-snat') + def ipset_supported(self): + return False + @utils.synchronized('iptables', external=True) def apply(self): """Apply the current in-memory set of iptables rules. diff --git a/nova/network/manager.py b/nova/network/manager.py index 824e8d24d..928cb09f6 100644 --- a/nova/network/manager.py +++ b/nova/network/manager.py @@ -63,6 +63,7 @@ from nova import quota from nova import utils from nova import rpc from nova.network import api as network_api +from nova.compute import api as compute_api import random @@ -297,6 +298,7 @@ class NetworkManager(manager.SchedulerDependentManager): network_driver = FLAGS.network_driver self.driver = utils.import_object(network_driver) self.network_api = network_api.API() + self.compute_api = compute_api.API() super(NetworkManager, self).__init__(service_name='network', *args, **kwargs) @@ -350,6 +352,15 @@ class NetworkManager(manager.SchedulerDependentManager): # return so worker will only grab 1 (to help scale flatter) return self.set_network_host(context, network['id']) + def _do_trigger_security_group_members_refresh_for_instance(self, + context, + instance_id): + instance_ref = db.instance_get(context, instance_id) + groups = instance_ref.security_groups + group_ids = [group.id for group in groups] + self.compute_api.trigger_security_group_members_refresh(context, + group_ids) + def _get_networks_for_instance(self, context, instance_id, project_id): """Determine & return which networks an instance should connect to.""" # TODO(tr3buchet) maybe this needs to be updated in the future if @@ -511,6 +522,9 @@ class NetworkManager(manager.SchedulerDependentManager): address = self.db.fixed_ip_associate_pool(context.elevated(), network['id'], instance_id) + self._do_trigger_security_group_members_refresh_for_instance( + context, + instance_id) vif = self.db.virtual_interface_get_by_instance_and_network(context, instance_id, network['id']) @@ -524,6 +538,12 @@ class NetworkManager(manager.SchedulerDependentManager): self.db.fixed_ip_update(context, address, {'allocated': False, 'virtual_interface_id': None}) + fixed_ip_ref = self.db.fixed_ip_get_by_address(context, address) + instance_ref = fixed_ip_ref['instance'] + instance_id = instance_ref['id'] + self._do_trigger_security_group_members_refresh_for_instance( + context, + instance_id) def lease_fixed_ip(self, context, address): """Called by dhcp-bridge when ip is leased.""" @@ -825,7 +845,9 @@ class VlanManager(RPCAllocateFixedIP, FloatingIP, NetworkManager): address = self.db.fixed_ip_associate_pool(context, network['id'], instance_id) - + self._do_trigger_security_group_members_refresh_for_instance( + context, + instance_id) vif = self.db.virtual_interface_get_by_instance_and_network(context, instance_id, network['id']) diff --git a/nova/tests/test_iptables_network.py b/nova/tests/test_iptables_network.py index 918034269..d0a8c052c 100644 --- a/nova/tests/test_iptables_network.py +++ b/nova/tests/test_iptables_network.py @@ -17,11 +17,46 @@ # under the License. """Unit Tests for network code.""" -import os - from nova import test from nova.network import linux_net +class IpSetTestCase(test.TestCase): + def test_add(self): + """Adding an address""" + ipset = linux_net.IpSet('somename') + + ipset.add_ip('1.2.3.4') + self.assertTrue('1.2.3.4' in ipset) + + + def test_add_remove(self): + """Adding and then removing an address""" + + self.verify_cmd_call_count = 0 + def verify_cmd(*args): + self.assertEquals(args, self.expected_cmd) + self.verify_cmd_call_count += 1 + + self.expected_cmd = ('ipset', '-A', 'run_tests.py-somename', '1.2.3.4') + ipset = linux_net.IpSet('somename',execute=verify_cmd) + ipset.add_ip('1.2.3.4') + self.assertTrue('1.2.3.4' in ipset) + + self.expected_cmd = ('ipset', '-D', 'run_tests.py-somename', '1.2.3.4') + ipset.remove_ip('1.2.3.4') + self.assertTrue('1.2.3.4' not in ipset) + self.assertEquals(self.verify_cmd_call_count, 2) + + + def test_two_adds_one_remove(self): + """Adding the same address twice works. Removing it once removes it entirely.""" + ipset = linux_net.IpSet('somename') + + ipset.add_ip('1.2.3.4') + ipset.add_ip('1.2.3.4') + ipset.remove_ip('1.2.3.4') + self.assertTrue('1.2.3.4' not in ipset) + class IptablesManagerTestCase(test.TestCase): sample_filter = ['#Generated by iptables-save on Fri Feb 18 15:17:05 2011', diff --git a/nova/virt/libvirt/firewall.py b/nova/virt/libvirt/firewall.py index 379197398..aa36e4184 100644 --- a/nova/virt/libvirt/firewall.py +++ b/nova/virt/libvirt/firewall.py @@ -663,11 +663,10 @@ class IptablesFirewallDriver(FirewallDriver): LOG.debug(_('Adding security group rule: %r'), rule) if not rule.cidr: - # Eventually, a mechanism to grant access for security - # groups will turn up here. It'll use ipsets. - continue + version = 4 + else: + version = netutils.get_ip_version(rule.cidr) - version = netutils.get_ip_version(rule.cidr) if version == 4: fw_rules = ipv4_rules else: @@ -677,16 +676,16 @@ class IptablesFirewallDriver(FirewallDriver): if version == 6 and rule.protocol == 'icmp': protocol = 'icmpv6' - args = ['-p', protocol, '-s', rule.cidr] + args = ['-j ACCEPT', '-p', protocol] - if rule.protocol in ['udp', 'tcp']: + if protocol in ['udp', 'tcp']: if rule.from_port == rule.to_port: args += ['--dport', '%s' % (rule.from_port,)] else: args += ['-m', 'multiport', '--dports', '%s:%s' % (rule.from_port, rule.to_port)] - elif rule.protocol == 'icmp': + elif protocol == 'icmp': icmp_type = rule.from_port icmp_code = rule.to_port @@ -705,9 +704,30 @@ class IptablesFirewallDriver(FirewallDriver): args += ['-m', 'icmp6', '--icmpv6-type', icmp_type_arg] - args += ['-j ACCEPT'] - fw_rules += [' '.join(args)] - + if rule.cidr: + LOG.info('Using cidr %r', rule.cidr) + args += ['-s', rule.cidr] + fw_rules += [' '.join(args)] + else: + LOG.info('Not using cidr %r', rule.cidr) + if self.iptables.ipset_supported(): + LOG.info('ipset supported %r', rule.cidr) + ipset = linux_net.IpSet('%s' % rule.group_id) + args += ipset.iptables_source_match() + fw_rules += [' '.join(args)] + else: + LOG.info('ipset unsupported %r', rule.cidr) + LOG.info('rule.grantee_group.instances: %r', rule.grantee_group.instances) + for instance in rule.grantee_group.instances: + LOG.info('instance: %r', instance) + ips = db.instance_get_fixed_addresses(ctxt, + instance['id']) + LOG.info('ips: %r', ips) + for ip in ips: + subrule = args + ['-s %s' % ip] + fw_rules += [' '.join(subrule)] + + LOG.info('Using fw_rules: %r', fw_rules) ipv4_rules += ['-j $sg-fallback'] ipv6_rules += ['-j $sg-fallback'] @@ -718,7 +738,9 @@ class IptablesFirewallDriver(FirewallDriver): return self.nwfilter.instance_filter_exists(instance) def refresh_security_group_members(self, security_group): - pass + if not self.iptables.ipset_supported(): + self.do_refresh_security_group_rules(security_group) + self.iptables.apply() def refresh_security_group_rules(self, security_group, network_info=None): self.do_refresh_security_group_rules(security_group, network_info) -- cgit From 00fcb54769fdbe8828d7bd52a6636ffc5ad6c862 Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Fri, 22 Jul 2011 22:49:16 +0200 Subject: ...and this is me snapping back into reality removing all trace of ipsets. Go me. --- nova/network/linux_net.py | 30 ---------------------------- nova/tests/test_iptables_network.py | 39 ++----------------------------------- nova/virt/libvirt/firewall.py | 30 ++++++++++------------------ 3 files changed, 12 insertions(+), 87 deletions(-) diff --git a/nova/network/linux_net.py b/nova/network/linux_net.py index 0e021a40f..283a5aca1 100644 --- a/nova/network/linux_net.py +++ b/nova/network/linux_net.py @@ -96,33 +96,6 @@ class IptablesRule(object): chain = self.chain return '-A %s %s' % (chain, self.rule) -class IpSet(object): - """A class for handling large collections of IPs efficiently""" - - def __init__(self, name, execute=None): - self.name = name - self._ips = set() - if not execute: - self.execute = _execute - else: - self.execute = execute - - def __contains__(self, addr): - return addr in self._ips - - def _set_name(self): - return '%s-%s' % (binary_name, self.name) - - def add_ip(self, addr): - self._ips.add(addr) - self.execute('ipset', '-A', self._set_name(), addr) - - def remove_ip(self, addr): - self._ips.remove(addr) - self.execute('ipset', '-D', self._set_name(), addr) - - def iptables_source_match(self): - return ['-m set --match-set %s src' % (self._set_name(),)] class IptablesTable(object): """An iptables table.""" @@ -308,9 +281,6 @@ class IptablesManager(object): self.ipv4['nat'].add_chain('floating-snat') self.ipv4['nat'].add_rule('snat', '-j $floating-snat') - def ipset_supported(self): - return False - @utils.synchronized('iptables', external=True) def apply(self): """Apply the current in-memory set of iptables rules. diff --git a/nova/tests/test_iptables_network.py b/nova/tests/test_iptables_network.py index d0a8c052c..918034269 100644 --- a/nova/tests/test_iptables_network.py +++ b/nova/tests/test_iptables_network.py @@ -17,46 +17,11 @@ # under the License. """Unit Tests for network code.""" +import os + from nova import test from nova.network import linux_net -class IpSetTestCase(test.TestCase): - def test_add(self): - """Adding an address""" - ipset = linux_net.IpSet('somename') - - ipset.add_ip('1.2.3.4') - self.assertTrue('1.2.3.4' in ipset) - - - def test_add_remove(self): - """Adding and then removing an address""" - - self.verify_cmd_call_count = 0 - def verify_cmd(*args): - self.assertEquals(args, self.expected_cmd) - self.verify_cmd_call_count += 1 - - self.expected_cmd = ('ipset', '-A', 'run_tests.py-somename', '1.2.3.4') - ipset = linux_net.IpSet('somename',execute=verify_cmd) - ipset.add_ip('1.2.3.4') - self.assertTrue('1.2.3.4' in ipset) - - self.expected_cmd = ('ipset', '-D', 'run_tests.py-somename', '1.2.3.4') - ipset.remove_ip('1.2.3.4') - self.assertTrue('1.2.3.4' not in ipset) - self.assertEquals(self.verify_cmd_call_count, 2) - - - def test_two_adds_one_remove(self): - """Adding the same address twice works. Removing it once removes it entirely.""" - ipset = linux_net.IpSet('somename') - - ipset.add_ip('1.2.3.4') - ipset.add_ip('1.2.3.4') - ipset.remove_ip('1.2.3.4') - self.assertTrue('1.2.3.4' not in ipset) - class IptablesManagerTestCase(test.TestCase): sample_filter = ['#Generated by iptables-save on Fri Feb 18 15:17:05 2011', diff --git a/nova/virt/libvirt/firewall.py b/nova/virt/libvirt/firewall.py index aa36e4184..4d615058b 100644 --- a/nova/virt/libvirt/firewall.py +++ b/nova/virt/libvirt/firewall.py @@ -709,23 +709,14 @@ class IptablesFirewallDriver(FirewallDriver): args += ['-s', rule.cidr] fw_rules += [' '.join(args)] else: - LOG.info('Not using cidr %r', rule.cidr) - if self.iptables.ipset_supported(): - LOG.info('ipset supported %r', rule.cidr) - ipset = linux_net.IpSet('%s' % rule.group_id) - args += ipset.iptables_source_match() - fw_rules += [' '.join(args)] - else: - LOG.info('ipset unsupported %r', rule.cidr) - LOG.info('rule.grantee_group.instances: %r', rule.grantee_group.instances) - for instance in rule.grantee_group.instances: - LOG.info('instance: %r', instance) - ips = db.instance_get_fixed_addresses(ctxt, - instance['id']) - LOG.info('ips: %r', ips) - for ip in ips: - subrule = args + ['-s %s' % ip] - fw_rules += [' '.join(subrule)] + for instance in rule.grantee_group.instances: + LOG.info('instance: %r', instance) + ips = db.instance_get_fixed_addresses(ctxt, + instance['id']) + LOG.info('ips: %r', ips) + for ip in ips: + subrule = args + ['-s %s' % ip] + fw_rules += [' '.join(subrule)] LOG.info('Using fw_rules: %r', fw_rules) ipv4_rules += ['-j $sg-fallback'] @@ -738,9 +729,8 @@ class IptablesFirewallDriver(FirewallDriver): return self.nwfilter.instance_filter_exists(instance) def refresh_security_group_members(self, security_group): - if not self.iptables.ipset_supported(): - self.do_refresh_security_group_rules(security_group) - self.iptables.apply() + self.do_refresh_security_group_rules(security_group) + self.iptables.apply() def refresh_security_group_rules(self, security_group, network_info=None): self.do_refresh_security_group_rules(security_group, network_info) -- cgit From 5961aa33f01db7503beeab4fabafb8e0d9ef6a3e Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Sun, 24 Jul 2011 06:29:43 -0700 Subject: Adjust and re-enable relevant unit tests. --- nova/tests/test_libvirt.py | 50 ++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 44 insertions(+), 6 deletions(-) diff --git a/nova/tests/test_libvirt.py b/nova/tests/test_libvirt.py index f99e1713d..8eec7aada 100644 --- a/nova/tests/test_libvirt.py +++ b/nova/tests/test_libvirt.py @@ -891,7 +891,6 @@ class IptablesFirewallTestCase(test.TestCase): 'project_id': 'fake', 'instance_type_id': 1}) - @test.skip_test("skipping libvirt tests depends on get_network_info shim") def test_static_filters(self): instance_ref = self._create_instance_ref() ip = '10.11.12.13' @@ -907,17 +906,41 @@ class IptablesFirewallTestCase(test.TestCase): fixed_ip = {'address': ip, 'network_id': network_ref['id'], 'virtual_interface_id': vif_ref['id']} + + src_instance_ref = self._create_instance_ref() + src_instance_ip = '10.11.12.14' + src_instance_vif = {'address': '56:12:12:12:12:13', + 'network_id': network_ref['id'], + 'instance_id': src_instance_ref['id']} + src_instance_vif_ref = db.virtual_interface_create(self.context, + src_instance_vif) + src_instance_fixed_ip = {'address': src_instance_ip, + 'network_id': network_ref['id'], + 'virtual_interface_id': + src_instance_vif_ref['id']} + admin_ctxt = context.get_admin_context() db.fixed_ip_create(admin_ctxt, fixed_ip) db.fixed_ip_update(admin_ctxt, ip, {'allocated': True, 'instance_id': instance_ref['id']}) + db.fixed_ip_create(admin_ctxt, src_instance_fixed_ip) + db.fixed_ip_update(admin_ctxt, src_instance_ip, + {'allocated': True, + 'instance_id': src_instance_ref['id']}) + secgroup = db.security_group_create(admin_ctxt, {'user_id': 'fake', 'project_id': 'fake', 'name': 'testgroup', 'description': 'test group'}) + src_secgroup = db.security_group_create(admin_ctxt, + {'user_id': 'fake', + 'project_id': 'fake', + 'name': 'testsourcegroup', + 'description': 'src group'}) + db.security_group_rule_create(admin_ctxt, {'parent_group_id': secgroup['id'], 'protocol': 'icmp', @@ -939,9 +962,19 @@ class IptablesFirewallTestCase(test.TestCase): 'to_port': 81, 'cidr': '192.168.10.0/24'}) + db.security_group_rule_create(admin_ctxt, + {'parent_group_id': secgroup['id'], + 'protocol': 'tcp', + 'from_port': 80, + 'to_port': 81, + 'group_id': src_secgroup['id']}) + db.instance_add_security_group(admin_ctxt, instance_ref['id'], secgroup['id']) + db.instance_add_security_group(admin_ctxt, src_instance_ref['id'], + src_secgroup['id']) instance_ref = db.instance_get(admin_ctxt, instance_ref['id']) + src_instance_ref = db.instance_get(admin_ctxt, src_instance_ref['id']) # self.fw.add_instance(instance_ref) def fake_iptables_execute(*cmd, **kwargs): @@ -994,17 +1027,22 @@ class IptablesFirewallTestCase(test.TestCase): self.assertTrue(security_group_chain, "The security group chain wasn't added") - regex = re.compile('-A .* -p icmp -s 192.168.11.0/24 -j ACCEPT') + regex = re.compile('-A .* -j ACCEPT -p icmp -s 192.168.11.0/24') self.assertTrue(len(filter(regex.match, self.out_rules)) > 0, "ICMP acceptance rule wasn't added") - regex = re.compile('-A .* -p icmp -s 192.168.11.0/24 -m icmp ' - '--icmp-type 8 -j ACCEPT') + regex = re.compile('-A .* -j ACCEPT -p icmp -m icmp --icmp-type 8' + ' -s 192.168.11.0/24') self.assertTrue(len(filter(regex.match, self.out_rules)) > 0, "ICMP Echo Request acceptance rule wasn't added") - regex = re.compile('-A .* -p tcp -s 192.168.10.0/24 -m multiport ' - '--dports 80:81 -j ACCEPT') + regex = re.compile('-A .* -j ACCEPT -p tcp -m multiport ' + '--dports 80:81 -s %s' % (src_instance_ip,)) + self.assertTrue(len(filter(regex.match, self.out_rules)) > 0, + "TCP port 80/81 acceptance rule wasn't added") + + regex = re.compile('-A .* -j ACCEPT -p tcp ' + '-m multiport --dports 80:81 -s 192.168.10.0/24') self.assertTrue(len(filter(regex.match, self.out_rules)) > 0, "TCP port 80/81 acceptance rule wasn't added") db.instance_destroy(admin_ctxt, instance_ref['id']) -- cgit From a943c01dd56169270e1986ce62ae99f16ee4abe3 Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Sun, 24 Jul 2011 06:30:59 -0700 Subject: Make IP allocation test work again. --- nova/tests/test_network.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/nova/tests/test_network.py b/nova/tests/test_network.py index b09021e13..9e021feea 100644 --- a/nova/tests/test_network.py +++ b/nova/tests/test_network.py @@ -216,7 +216,11 @@ class VlanNetworkTestCase(test.TestCase): self.mox.StubOutWithMock(db, 'fixed_ip_update') self.mox.StubOutWithMock(db, 'virtual_interface_get_by_instance_and_network') + self.mox.StubOutWithMock(db, 'instance_get') + db.instance_get(mox.IgnoreArg(), + mox.IgnoreArg()).AndReturn({ 'security_groups': + [ { 'id': 0 } ] }) db.fixed_ip_associate_pool(mox.IgnoreArg(), mox.IgnoreArg(), mox.IgnoreArg()).AndReturn('192.168.0.1') -- cgit From 6ddc49298f87fc20c6daff994495d745dc82b6e3 Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Sun, 24 Jul 2011 06:33:57 -0700 Subject: Use subscript rather than attribute. --- nova/virt/libvirt/firewall.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/nova/virt/libvirt/firewall.py b/nova/virt/libvirt/firewall.py index 4d615058b..27056255c 100644 --- a/nova/virt/libvirt/firewall.py +++ b/nova/virt/libvirt/firewall.py @@ -709,14 +709,15 @@ class IptablesFirewallDriver(FirewallDriver): args += ['-s', rule.cidr] fw_rules += [' '.join(args)] else: - for instance in rule.grantee_group.instances: - LOG.info('instance: %r', instance) - ips = db.instance_get_fixed_addresses(ctxt, - instance['id']) - LOG.info('ips: %r', ips) - for ip in ips: - subrule = args + ['-s %s' % ip] - fw_rules += [' '.join(subrule)] + if rule['grantee_group']: + for instance in rule['grantee_group']['instances']: + LOG.info('instance: %r', instance) + ips = db.instance_get_fixed_addresses(ctxt, + instance['id']) + LOG.info('ips: %r', ips) + for ip in ips: + subrule = args + ['-s %s' % ip] + fw_rules += [' '.join(subrule)] LOG.info('Using fw_rules: %r', fw_rules) ipv4_rules += ['-j $sg-fallback'] -- cgit From a88f7ab3b7469af70c74ed5962abf867e62d768f Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Sun, 24 Jul 2011 06:35:38 -0700 Subject: Use admin context when fetching instances. --- nova/network/manager.py | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/nova/network/manager.py b/nova/network/manager.py index 928cb09f6..ac37bb885 100644 --- a/nova/network/manager.py +++ b/nova/network/manager.py @@ -353,12 +353,12 @@ class NetworkManager(manager.SchedulerDependentManager): return self.set_network_host(context, network['id']) def _do_trigger_security_group_members_refresh_for_instance(self, - context, instance_id): - instance_ref = db.instance_get(context, instance_id) - groups = instance_ref.security_groups - group_ids = [group.id for group in groups] - self.compute_api.trigger_security_group_members_refresh(context, + admin_context = context.get_admin_context() + instance_ref = self.db.instance_get(admin_context, instance_id) + groups = instance_ref['security_groups'] + group_ids = [group['id'] for group in groups] + self.compute_api.trigger_security_group_members_refresh(admin_context, group_ids) def _get_networks_for_instance(self, context, instance_id, project_id): @@ -523,7 +523,6 @@ class NetworkManager(manager.SchedulerDependentManager): network['id'], instance_id) self._do_trigger_security_group_members_refresh_for_instance( - context, instance_id) vif = self.db.virtual_interface_get_by_instance_and_network(context, instance_id, @@ -542,7 +541,6 @@ class NetworkManager(manager.SchedulerDependentManager): instance_ref = fixed_ip_ref['instance'] instance_id = instance_ref['id'] self._do_trigger_security_group_members_refresh_for_instance( - context, instance_id) def lease_fixed_ip(self, context, address): @@ -846,7 +844,6 @@ class VlanManager(RPCAllocateFixedIP, FloatingIP, NetworkManager): network['id'], instance_id) self._do_trigger_security_group_members_refresh_for_instance( - context, instance_id) vif = self.db.virtual_interface_get_by_instance_and_network(context, instance_id, -- cgit -- cgit From 3841a5515807b42e2e74e3119f76cdb2ef0f5575 Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Tue, 26 Jul 2011 11:04:53 -0700 Subject: Remove debugging code. --- nova/compute/api.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/nova/compute/api.py b/nova/compute/api.py index 65a594d2c..ad8886f23 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -475,7 +475,6 @@ class API(base.Base): context, group_id)) - LOG.info('rules: %r', security_group_rules) # ..then we distill the security groups to which they belong.. security_groups = set() for rule in security_group_rules: @@ -484,14 +483,12 @@ class API(base.Base): rule['parent_group_id']) security_groups.add(security_group) - LOG.info('security_groups: %r', security_groups) # ..then we find the instances that are members of these groups.. instances = set() for security_group in security_groups: for instance in security_group['instances']: instances.add(instance) - LOG.info('instances: %r', instances) # ...then we find the hosts where they live... hosts = set() for instance in instances: @@ -501,7 +498,6 @@ class API(base.Base): # ...and finally we tell these nodes to refresh their view of this # particular security group. for host in hosts: - LOG.info('host: %r', host) rpc.cast(context, self.db.queue_get_for(context, FLAGS.compute_topic, host), {"method": "refresh_security_group_members", -- cgit From 00171c3f50d333a1771efc048b064e1fd73614b0 Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Tue, 26 Jul 2011 14:10:26 -0700 Subject: pep8 --- nova/db/sqlalchemy/models.py | 3 +-- nova/tests/test_network.py | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/nova/db/sqlalchemy/models.py b/nova/db/sqlalchemy/models.py index 4ecf80c8f..9f1967d69 100644 --- a/nova/db/sqlalchemy/models.py +++ b/nova/db/sqlalchemy/models.py @@ -494,14 +494,13 @@ class SecurityGroupIngressRule(BASE, NovaBase): # Note: This is not the parent SecurityGroup. It's SecurityGroup we're # granting access for. group_id = Column(Integer, ForeignKey('security_groups.id')) - grantee_group = relationship("SecurityGroup", + grantee_group = relationship("SecurityGroup", foreign_keys=group_id, primaryjoin='and_(' 'SecurityGroupIngressRule.group_id == SecurityGroup.id,' 'SecurityGroupIngressRule.deleted == False)') - class ProviderFirewallRule(BASE, NovaBase): """Represents a rule in a security group.""" __tablename__ = 'provider_fw_rules' diff --git a/nova/tests/test_network.py b/nova/tests/test_network.py index e6cd5858a..4119953f2 100644 --- a/nova/tests/test_network.py +++ b/nova/tests/test_network.py @@ -215,8 +215,8 @@ class VlanNetworkTestCase(test.TestCase): self.mox.StubOutWithMock(db, 'instance_get') db.instance_get(mox.IgnoreArg(), - mox.IgnoreArg()).AndReturn({ 'security_groups': - [ { 'id': 0 } ] }) + mox.IgnoreArg()).AndReturn({'security_groups': + [{'id': 0}]}) db.fixed_ip_associate_pool(mox.IgnoreArg(), mox.IgnoreArg(), mox.IgnoreArg()).AndReturn('192.168.0.1') -- cgit From 151ecddc0227ff9e779712532971fac8a2c79c7b Mon Sep 17 00:00:00 2001 From: Donal Lafferty Date: Wed, 27 Jul 2011 15:40:13 +0100 Subject: Address merge review concerns. --- nova/virt/xenapi/vm_utils.py | 2 +- nova/virt/xenapi/vmops.py | 28 +++++++++++++++++----------- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/nova/virt/xenapi/vm_utils.py b/nova/virt/xenapi/vm_utils.py index d0ffb13da..5d6fe0825 100644 --- a/nova/virt/xenapi/vm_utils.py +++ b/nova/virt/xenapi/vm_utils.py @@ -520,7 +520,7 @@ class VMHelper(HelperBase): # DISK restores LOG.debug(_("Fetching image %(image)s") % locals()) LOG.debug(_("Image Type: %s"), ImageType.to_string(image_type)) - sr_ref = None + if image_type == ImageType.DISK_ISO: sr_ref = safe_find_iso_sr(session) LOG.debug(_("ISO: Found sr possibly containing the ISO image")) diff --git a/nova/virt/xenapi/vmops.py b/nova/virt/xenapi/vmops.py index 1830ce18d..7ac923933 100644 --- a/nova/virt/xenapi/vmops.py +++ b/nova/virt/xenapi/vmops.py @@ -238,6 +238,21 @@ class VMOps(object): raise vm_create_error + # Add disks to VM + self._attach_disks(instance, disk_image_type, vm_ref, first_vdi_ref, + vdis) + + # Alter the image before VM start for, e.g. network injection + if FLAGS.xenapi_inject_image: + VMHelper.preconfigure_instance(self._session, instance, + first_vdi_ref, network_info) + + self.create_vifs(vm_ref, instance, network_info) + self.inject_network_info(instance, network_info, vm_ref) + return vm_ref + + def _attach_disks(self, instance, disk_image_type, vm_ref, first_vdi_ref, + vdis): # device 0 reserved for RW disk userdevice = 0 @@ -253,7 +268,7 @@ class VMOps(object): VMHelper.create_vbd(session=self._session, vm_ref=vm_ref, vdi_ref=first_vdi_ref, userdevice=userdevice, bootable=False) - # device 1 reserved for rescue disk so use '2', we've used '0' + # device 1 reserved for rescue disk and we've used '0' userdevice = 2 VMHelper.create_cd_vbd(session=self._session, vm_ref=vm_ref, vdi_ref=cd_vdi_ref, userdevice=userdevice, bootable=True) @@ -264,7 +279,7 @@ class VMOps(object): VMHelper.create_vbd(session=self._session, vm_ref=vm_ref, vdi_ref=first_vdi_ref, userdevice=userdevice, bootable=True) # set user device to next free value - # userdevice 1 is reserved for rescue, we've used '0' + # userdevice 1 is reserved for rescue and we've used '0' userdevice = 2 # Attach any other disks @@ -278,15 +293,6 @@ class VMOps(object): bootable=False) userdevice += 1 - # Alter the image before VM start for, e.g. network injection - if FLAGS.xenapi_inject_image: - VMHelper.preconfigure_instance(self._session, instance, - first_vdi_ref, network_info) - - self.create_vifs(vm_ref, instance, network_info) - self.inject_network_info(instance, network_info, vm_ref) - return vm_ref - def _spawn(self, instance, vm_ref): """Spawn a new instance.""" LOG.debug(_('Starting VM %s...'), vm_ref) -- cgit From 8c54cfdabad3b3c102bae05283dd8484da38c557 Mon Sep 17 00:00:00 2001 From: Donal Lafferty Date: Mon, 1 Aug 2011 09:30:13 +0100 Subject: Remove copy/paste error. --- nova/virt/xenapi/vmops.py | 9 --------- 1 file changed, 9 deletions(-) diff --git a/nova/virt/xenapi/vmops.py b/nova/virt/xenapi/vmops.py index f2258bde3..5c0b3020f 100644 --- a/nova/virt/xenapi/vmops.py +++ b/nova/virt/xenapi/vmops.py @@ -293,15 +293,6 @@ class VMOps(object): bootable=False) userdevice += 1 - # Alter the image before VM start for, e.g. network injection - if FLAGS.flat_injected: - VMHelper.preconfigure_instance(self._session, instance, - first_vdi_ref, network_info) - - self.create_vifs(vm_ref, instance, network_info) - self.inject_network_info(instance, network_info, vm_ref) - return vm_ref - def _spawn(self, instance, vm_ref): """Spawn a new instance.""" LOG.debug(_('Starting VM %s...'), vm_ref) -- cgit From e72fafbf76ed456039426a96dd65d2c148dffa29 Mon Sep 17 00:00:00 2001 From: John Tran Date: Mon, 1 Aug 2011 12:37:12 -0700 Subject: adding a function with logic to make the creation of networks validation a bit smarter: - detects if the cidr is already in use - when specifying a supernet to be split into smaller subnets via num_networks && network_size, ensures none of the returned subnets are in use by either a subnet of the same size and range, nor a SMALLER size within the same range. - detects if splitting a supernet into # of num_networks && network_size will fit - detects if the supernet/cidr specified is conflicting with a network cidr that currently exists that may be a larger supernet already encompassing the specified cidr. " --- nova/network/manager.py | 48 +++++++++++++++++++++ nova/tests/test_network.py | 102 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 150 insertions(+) diff --git a/nova/network/manager.py b/nova/network/manager.py index 4a3791d8a..fafb75c9e 100644 --- a/nova/network/manager.py +++ b/nova/network/manager.py @@ -609,6 +609,54 @@ class NetworkManager(manager.SchedulerDependentManager): network_ref = self.db.fixed_ip_get_network(context, address) self._setup_network(context, network_ref) + def _validate_cidrs(self, context, cidr, num_networks, network_size): + significant_bits = 32 - int(math.log(network_size, 2)) + req_net = netaddr.IPNetwork(cidr) + req_net_ip = str(req_net.ip) + req_size = network_size * num_networks + if req_size > req_net.size: + raise ValueError(_("network_size * num_networks exceeds cidr size")) + adjusted_cidr = netaddr.IPNetwork(req_net_ip+'/'+str(significant_bits)) + all_req_nets = [adjusted_cidr] + try: + used_nets = self.db.network_get_all(context) + except exception.NoNetworksFound: + used_nets = [] + used_cidrs = [netaddr.IPNetwork(net['cidr']) for net in used_nets] + if adjusted_cidr in used_cidrs: + raise ValueError(_("cidr already in use")) + for adjusted_cidr_supernet in adjusted_cidr.supernet(): + if adjusted_cidr_supernet in used_cidrs: + raise ValueError(_("requested cidr (%s) conflicts with existing supernet (%s)" % (str(adjusted_cidr), str(adjusted_cidr_supernet)))) + # split supernet into subnets + if num_networks >= 2: + next_cidr = adjusted_cidr + for used_cidr in used_cidrs: + # watch for smaller subnets conflicting + if used_cidr.size < next_cidr.size: + for ucsupernet in used_cidr.supernet(): + if ucsupernet.size == next_cidr.size: + used_cidrs.append(ucsupernet) + for index in range(1, num_networks): + while True: + next_cidr = next_cidr.next() + if next_cidr in used_cidrs: + continue + else: + all_req_nets.append(next_cidr) + break + all_req_nets = list(set(all_req_nets)) + if not used_nets: + return all_req_nets + # after splitting ensure there were enough to satisfy the num_networks + if len(all_req_nets) < num_networks: + raise ValueError(_("Not enough subnets avail to satisfy requested num_networks")) + # if one of the split subnets were already defined, remove from create list + for req_cidr in all_req_nets: + if req_cidr in used_cidrs: + all_req_nets.remove(req_cidr) + return all_req_nets + def create_networks(self, context, label, cidr, multi_host, num_networks, network_size, cidr_v6, gateway_v6, bridge, bridge_interface, dns1=None, dns2=None, **kwargs): diff --git a/nova/tests/test_network.py b/nova/tests/test_network.py index 28f50d328..8a851cf76 100644 --- a/nova/tests/test_network.py +++ b/nova/tests/test_network.py @@ -15,6 +15,7 @@ # License for the specific language governing permissions and limitations # under the License. +from nova import context from nova import db from nova import exception from nova import flags @@ -249,6 +250,15 @@ class CommonNetworkTestCase(test.TestCase): return [dict(address='10.0.0.0'), dict(address='10.0.0.1'), dict(address='10.0.0.2')] + def network_get_by_cidr(self, context, cidr): + raise exception.NetworkNotFoundForCidr() + + def network_create_safe(self, context, net): + return {'foo': 'bar'} + + def network_get_all(self, context): + raise exception.NoNetworksFound() + def __init__(self): self.db = self.FakeDB() self.deallocate_called = None @@ -267,3 +277,95 @@ class CommonNetworkTestCase(test.TestCase): self.assertRaises(exception.FixedIpNotFoundForSpecificInstance, manager.remove_fixed_ip_from_instance, None, 99, 'bad input') + + def test__validate_cidrs(self): + manager = self.FakeNetworkManager() + nets = manager._validate_cidrs(None, '192.168.0.0/24', 1, 256) + self.assertEqual(1, len(nets)) + cidrs = [str(net) for net in nets] + self.assertTrue('192.168.0.0/24' in cidrs) + + def test__validate_cidrs_split_exact_in_half(self): + manager = self.FakeNetworkManager() + nets = manager._validate_cidrs(None, '192.168.0.0/24', 2, 128) + self.assertEqual(2, len(nets)) + cidrs = [str(net) for net in nets] + self.assertTrue('192.168.0.0/25' in cidrs) + self.assertTrue('192.168.0.128/25' in cidrs) + + def test__validate_cidrs_split_cidr_in_use_middle_of_range(self): + manager = self.FakeNetworkManager() + self.mox.StubOutWithMock(manager.db, 'network_get_all') + ctxt = mox.IgnoreArg() + manager.db.network_get_all(ctxt).AndReturn([{'id': 1, 'cidr': '192.168.2.0/24'}]) + self.mox.ReplayAll() + nets = manager._validate_cidrs(None, '192.168.0.0/16', 4, 256) + self.assertEqual(4, len(nets)) + cidrs = [str(net) for net in nets] + exp_cidrs = ['192.168.0.0', '192.168.1.0', '192.168.3.0', '192.168.4.0'] + for exp_cidr in exp_cidrs: + self.assertTrue(exp_cidr+'/24' in cidrs) + self.assertFalse('192.168.2.0/24' in cidrs) + + def test__validate_cidrs_split_cidr_smaller_subnet_in_use_middle_of_range(self): + manager = self.FakeNetworkManager() + self.mox.StubOutWithMock(manager.db, 'network_get_all') + ctxt = mox.IgnoreArg() + manager.db.network_get_all(ctxt).AndReturn([{'id': 1, 'cidr': '192.168.2.0/25'}]) + self.mox.ReplayAll() + nets = manager._validate_cidrs(None, '192.168.0.0/16', 4, 256) + self.assertEqual(4, len(nets)) + cidrs = [str(net) for net in nets] + exp_cidrs = ['192.168.0.0', '192.168.1.0', '192.168.3.0', '192.168.4.0'] + for exp_cidr in exp_cidrs: + self.assertTrue(exp_cidr+'/24' in cidrs) + self.assertFalse('192.168.2.0/24' in cidrs) + + def test__validate_cidrs_one_in_use(self): + manager = self.FakeNetworkManager() + args = [None, '192.168.0.0/24', 2, 256] + # ValueError: network_size * num_networks exceeds cidr size + self.assertRaises(ValueError, manager._validate_cidrs, *args) + + def test__validate_cidrs_already_used(self): + manager = self.FakeNetworkManager() + self.mox.StubOutWithMock(manager.db, 'network_get_all') + ctxt = mox.IgnoreArg() + manager.db.network_get_all(ctxt).AndReturn([{'id': 1, 'cidr': '192.168.0.0/24'}]) + self.mox.ReplayAll() + # ValueError: cidr already in use + args = [None, '192.168.0.0/24', 1, 256] + self.assertRaises(ValueError, manager._validate_cidrs, *args) + + def test__validate_cidrs_too_many(self): + manager = self.FakeNetworkManager() + args = [None, '192.168.0.0/24', 200, 256] + # ValueError: Not enough subnets avail to satisfy requested num_networks + self.assertRaises(ValueError, manager._validate_cidrs, *args) + + def test__validate_cidrs_split_partial(self): + manager = self.FakeNetworkManager() + nets = manager._validate_cidrs(None, '192.168.0.0/16', 2, 256) + returned_cidrs = [str(net) for net in nets] + print returned_cidrs + self.assertTrue('192.168.0.0/24' in returned_cidrs) + self.assertTrue('192.168.1.0/24' in returned_cidrs) + + def test__validate_cidrs_conflict_existing_supernet(self): + manager = self.FakeNetworkManager() + self.mox.StubOutWithMock(manager.db, 'network_get_all') + ctxt = mox.IgnoreArg() + manager.db.network_get_all(ctxt).AndReturn([{'id': 1, 'cidr': '192.168.0.0/8'}]) + self.mox.ReplayAll() + args = [None, '192.168.0.0/24', 1, 256] + # ValueError: requested cidr (192.168.0.0/24) conflicts with existing supernet (192.0.0.0/8) + self.assertRaises(ValueError, manager._validate_cidrs, *args) +# +# def test_create_networks_cidr_already_used(self): +# mockany = self.mox.CreateMockAnything() +# manager = self.FakeNetworkManager() +# self.mox.StubOutWithMock(manager.db, 'network_get_by_cidr') +# manager.db.network_get_by_cidr(mox.IgnoreArg(), '192.168.0.0/24').AndReturn(mockany) +# self.mox.ReplayAll() +# args = [None, 'foo', '192.168.0.0/24', None, 1, 256, 'fd00::/48', None, None, None] +# self.assertRaises(ValueError, manager.create_networks, *args) -- cgit From 951114be20065044e7f12e37188eb30e859ff2cb Mon Sep 17 00:00:00 2001 From: John Tran Date: Mon, 1 Aug 2011 12:47:41 -0700 Subject: removed redundant logic --- nova/network/manager.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/nova/network/manager.py b/nova/network/manager.py index fafb75c9e..f6c7e35e0 100644 --- a/nova/network/manager.py +++ b/nova/network/manager.py @@ -646,15 +646,9 @@ class NetworkManager(manager.SchedulerDependentManager): all_req_nets.append(next_cidr) break all_req_nets = list(set(all_req_nets)) - if not used_nets: - return all_req_nets # after splitting ensure there were enough to satisfy the num_networks if len(all_req_nets) < num_networks: raise ValueError(_("Not enough subnets avail to satisfy requested num_networks")) - # if one of the split subnets were already defined, remove from create list - for req_cidr in all_req_nets: - if req_cidr in used_cidrs: - all_req_nets.remove(req_cidr) return all_req_nets def create_networks(self, context, label, cidr, multi_host, num_networks, -- cgit From ab4bfcf6c458ab6bf6ead126a91413b92aa543b8 Mon Sep 17 00:00:00 2001 From: John Tran Date: Mon, 1 Aug 2011 16:27:17 -0700 Subject: pep8 fixes --- nova/network/manager.py | 12 ++++++++---- nova/tests/test_network.py | 36 +++++++++++++++++++++++------------- 2 files changed, 31 insertions(+), 17 deletions(-) diff --git a/nova/network/manager.py b/nova/network/manager.py index f6c7e35e0..9d0f080ba 100644 --- a/nova/network/manager.py +++ b/nova/network/manager.py @@ -615,8 +615,10 @@ class NetworkManager(manager.SchedulerDependentManager): req_net_ip = str(req_net.ip) req_size = network_size * num_networks if req_size > req_net.size: - raise ValueError(_("network_size * num_networks exceeds cidr size")) - adjusted_cidr = netaddr.IPNetwork(req_net_ip+'/'+str(significant_bits)) + msg = "network_size * num_networks exceeds cidr size" + raise ValueError(_(msg)) + adjusted_cidr_str = req_net_ip + '/' + str(significant_bits) + adjusted_cidr = netaddr.IPNetwork(adjusted_cidr_str) all_req_nets = [adjusted_cidr] try: used_nets = self.db.network_get_all(context) @@ -627,7 +629,8 @@ class NetworkManager(manager.SchedulerDependentManager): raise ValueError(_("cidr already in use")) for adjusted_cidr_supernet in adjusted_cidr.supernet(): if adjusted_cidr_supernet in used_cidrs: - raise ValueError(_("requested cidr (%s) conflicts with existing supernet (%s)" % (str(adjusted_cidr), str(adjusted_cidr_supernet)))) + msg = "requested cidr (%s) conflicts with existing supernet" + raise ValueError(_(msg % str(adjusted_cidr))) # split supernet into subnets if num_networks >= 2: next_cidr = adjusted_cidr @@ -648,7 +651,8 @@ class NetworkManager(manager.SchedulerDependentManager): all_req_nets = list(set(all_req_nets)) # after splitting ensure there were enough to satisfy the num_networks if len(all_req_nets) < num_networks: - raise ValueError(_("Not enough subnets avail to satisfy requested num_networks")) + msg = "Not enough subnets avail to satisfy requested num_networks" + raise ValueError(_(msg)) return all_req_nets def create_networks(self, context, label, cidr, multi_host, num_networks, diff --git a/nova/tests/test_network.py b/nova/tests/test_network.py index 8a851cf76..1f5305b1d 100644 --- a/nova/tests/test_network.py +++ b/nova/tests/test_network.py @@ -297,28 +297,32 @@ class CommonNetworkTestCase(test.TestCase): manager = self.FakeNetworkManager() self.mox.StubOutWithMock(manager.db, 'network_get_all') ctxt = mox.IgnoreArg() - manager.db.network_get_all(ctxt).AndReturn([{'id': 1, 'cidr': '192.168.2.0/24'}]) + manager.db.network_get_all(ctxt).AndReturn([{'id': 1, + 'cidr': '192.168.2.0/24'}]) self.mox.ReplayAll() nets = manager._validate_cidrs(None, '192.168.0.0/16', 4, 256) self.assertEqual(4, len(nets)) cidrs = [str(net) for net in nets] - exp_cidrs = ['192.168.0.0', '192.168.1.0', '192.168.3.0', '192.168.4.0'] + exp_cidrs = ['192.168.0.0', '192.168.1.0', '192.168.3.0', + '192.168.4.0'] for exp_cidr in exp_cidrs: - self.assertTrue(exp_cidr+'/24' in cidrs) + self.assertTrue(exp_cidr + '/24' in cidrs) self.assertFalse('192.168.2.0/24' in cidrs) - def test__validate_cidrs_split_cidr_smaller_subnet_in_use_middle_of_range(self): + def test__validate_cidrs_split_cidr_smaller_subnet_in_use(self): manager = self.FakeNetworkManager() self.mox.StubOutWithMock(manager.db, 'network_get_all') ctxt = mox.IgnoreArg() - manager.db.network_get_all(ctxt).AndReturn([{'id': 1, 'cidr': '192.168.2.0/25'}]) + manager.db.network_get_all(ctxt).AndReturn([{'id': 1, + 'cidr': '192.168.2.0/25'}]) self.mox.ReplayAll() nets = manager._validate_cidrs(None, '192.168.0.0/16', 4, 256) self.assertEqual(4, len(nets)) cidrs = [str(net) for net in nets] - exp_cidrs = ['192.168.0.0', '192.168.1.0', '192.168.3.0', '192.168.4.0'] + exp_cidrs = ['192.168.0.0', '192.168.1.0', '192.168.3.0', + '192.168.4.0'] for exp_cidr in exp_cidrs: - self.assertTrue(exp_cidr+'/24' in cidrs) + self.assertTrue(exp_cidr + '/24' in cidrs) self.assertFalse('192.168.2.0/24' in cidrs) def test__validate_cidrs_one_in_use(self): @@ -331,7 +335,8 @@ class CommonNetworkTestCase(test.TestCase): manager = self.FakeNetworkManager() self.mox.StubOutWithMock(manager.db, 'network_get_all') ctxt = mox.IgnoreArg() - manager.db.network_get_all(ctxt).AndReturn([{'id': 1, 'cidr': '192.168.0.0/24'}]) + manager.db.network_get_all(ctxt).AndReturn([{'id': 1, + 'cidr': '192.168.0.0/24'}]) self.mox.ReplayAll() # ValueError: cidr already in use args = [None, '192.168.0.0/24', 1, 256] @@ -340,7 +345,8 @@ class CommonNetworkTestCase(test.TestCase): def test__validate_cidrs_too_many(self): manager = self.FakeNetworkManager() args = [None, '192.168.0.0/24', 200, 256] - # ValueError: Not enough subnets avail to satisfy requested num_networks + # ValueError: Not enough subnets avail to satisfy requested + # num_networks self.assertRaises(ValueError, manager._validate_cidrs, *args) def test__validate_cidrs_split_partial(self): @@ -355,17 +361,21 @@ class CommonNetworkTestCase(test.TestCase): manager = self.FakeNetworkManager() self.mox.StubOutWithMock(manager.db, 'network_get_all') ctxt = mox.IgnoreArg() - manager.db.network_get_all(ctxt).AndReturn([{'id': 1, 'cidr': '192.168.0.0/8'}]) + manager.db.network_get_all(ctxt).AndReturn([{'id': 1, + 'cidr': '192.168.0.0/8'}]) self.mox.ReplayAll() args = [None, '192.168.0.0/24', 1, 256] - # ValueError: requested cidr (192.168.0.0/24) conflicts with existing supernet (192.0.0.0/8) + # ValueError: requested cidr (192.168.0.0/24) conflicts + # with existing supernet self.assertRaises(ValueError, manager._validate_cidrs, *args) # # def test_create_networks_cidr_already_used(self): # mockany = self.mox.CreateMockAnything() # manager = self.FakeNetworkManager() # self.mox.StubOutWithMock(manager.db, 'network_get_by_cidr') -# manager.db.network_get_by_cidr(mox.IgnoreArg(), '192.168.0.0/24').AndReturn(mockany) +# manager.db.network_get_by_cidr(mox.IgnoreArg(), '192.168.0.0/24')\ +# .AndReturn(mockany) # self.mox.ReplayAll() -# args = [None, 'foo', '192.168.0.0/24', None, 1, 256, 'fd00::/48', None, None, None] +# args = [None, 'foo', '192.168.0.0/24', None, 1, 256, +# 'fd00::/48', None, None, None] # self.assertRaises(ValueError, manager.create_networks, *args) -- cgit From bcfd8f5e1e0c3b53a2ad4a5bb533d94dcf5ef18c Mon Sep 17 00:00:00 2001 From: John Tran Date: Mon, 1 Aug 2011 21:34:43 -0700 Subject: added some tests for network create & moved the ipv6 logic back into the function --- nova/network/manager.py | 43 ++++++++++++++++++------------------------- nova/tests/test_network.py | 41 ++++++++++++++++++++++++++++------------- 2 files changed, 46 insertions(+), 38 deletions(-) diff --git a/nova/network/manager.py b/nova/network/manager.py index 9d0f080ba..8b86bc036 100644 --- a/nova/network/manager.py +++ b/nova/network/manager.py @@ -659,71 +659,64 @@ class NetworkManager(manager.SchedulerDependentManager): network_size, cidr_v6, gateway_v6, bridge, bridge_interface, dns1=None, dns2=None, **kwargs): """Create networks based on parameters.""" - fixed_net = netaddr.IPNetwork(cidr) + req_cidrs = self._validate_cidrs(context, cidr, num_networks, network_size) + if FLAGS.use_ipv6: fixed_net_v6 = netaddr.IPNetwork(cidr_v6) significant_bits_v6 = 64 network_size_v6 = 1 << 64 - for index in range(num_networks): - start = index * network_size - significant_bits = 32 - int(math.log(network_size, 2)) - cidr = '%s/%s' % (fixed_net[start], significant_bits) - project_net = netaddr.IPNetwork(cidr) + count = 0 + for req_cidr in req_cidrs: + count = count + 1 net = {} net['bridge'] = bridge net['bridge_interface'] = bridge_interface net['dns1'] = dns1 net['dns2'] = dns2 - net['cidr'] = cidr - net['multi_host'] = multi_host - net['netmask'] = str(project_net.netmask) - net['gateway'] = str(project_net[1]) - net['broadcast'] = str(project_net.broadcast) - net['dhcp_start'] = str(project_net[2]) + net['cidr'] = str(req_cidr) + net['netmask'] = str(req_cidr.netmask) + net['gateway'] = str(req_cidr[1]) + net['broadcast'] = str(req_cidr.broadcast) + net['dhcp_start'] = str(req_cidr[2]) if num_networks > 1: - net['label'] = '%s_%d' % (label, index) + net['label'] = '%s_%d' % (label, count) else: net['label'] = label if FLAGS.use_ipv6: - start_v6 = index * network_size_v6 + start_v6 = count * network_size_v6 cidr_v6 = '%s/%s' % (fixed_net_v6[start_v6], significant_bits_v6) net['cidr_v6'] = cidr_v6 - project_net_v6 = netaddr.IPNetwork(cidr_v6) - if gateway_v6: # use a pre-defined gateway if one is provided net['gateway_v6'] = str(gateway_v6) else: net['gateway_v6'] = str(project_net_v6[1]) - net['netmask_v6'] = str(project_net_v6._prefixlen) if kwargs.get('vpn', False): # this bit here is for vlan-manager del net['dns1'] del net['dns2'] - vlan = kwargs['vlan_start'] + index - net['vpn_private_address'] = str(project_net[2]) - net['dhcp_start'] = str(project_net[3]) + vlan = kwargs['vlan_start'] + count + net['vpn_private_address'] = str(req_cidr[2]) + net['dhcp_start'] = str(req_cidr[3]) net['vlan'] = vlan net['bridge'] = 'br%s' % vlan # NOTE(vish): This makes ports unique accross the cloud, a more # robust solution would be to make them uniq per ip - net['vpn_public_port'] = kwargs['vpn_start'] + index + net['vpn_public_port'] = kwargs['vpn_start'] + count - # None if network with cidr or cidr_v6 already exists network = self.db.network_create_safe(context, net) - if network: self._create_fixed_ips(context, network['id']) else: - raise ValueError(_('Network with cidr %s already exists') % - cidr) + msg = "Error creating cidr %s, see log" + raise ValueError(_(msg) % str(cidr)) @property def _bottom_reserved_ips(self): # pylint: disable=R0201 diff --git a/nova/tests/test_network.py b/nova/tests/test_network.py index 1f5305b1d..424b5f098 100644 --- a/nova/tests/test_network.py +++ b/nova/tests/test_network.py @@ -361,21 +361,36 @@ class CommonNetworkTestCase(test.TestCase): manager = self.FakeNetworkManager() self.mox.StubOutWithMock(manager.db, 'network_get_all') ctxt = mox.IgnoreArg() - manager.db.network_get_all(ctxt).AndReturn([{'id': 1, - 'cidr': '192.168.0.0/8'}]) + fakecidr= [{'id': 1, 'cidr': '192.168.0.0/8'}] + manager.db.network_get_all(ctxt).AndReturn(fakecidr) self.mox.ReplayAll() args = [None, '192.168.0.0/24', 1, 256] # ValueError: requested cidr (192.168.0.0/24) conflicts # with existing supernet self.assertRaises(ValueError, manager._validate_cidrs, *args) -# -# def test_create_networks_cidr_already_used(self): -# mockany = self.mox.CreateMockAnything() -# manager = self.FakeNetworkManager() -# self.mox.StubOutWithMock(manager.db, 'network_get_by_cidr') -# manager.db.network_get_by_cidr(mox.IgnoreArg(), '192.168.0.0/24')\ -# .AndReturn(mockany) -# self.mox.ReplayAll() -# args = [None, 'foo', '192.168.0.0/24', None, 1, 256, -# 'fd00::/48', None, None, None] -# self.assertRaises(ValueError, manager.create_networks, *args) + + def test_create_networks(self): + cidr = '192.168.0.0/24' + manager = self.FakeNetworkManager() + self.mox.StubOutWithMock(manager.db, 'network_create_safe') + ignore = mox.IgnoreArg() + fakecidr= {'id': 1, 'cidr': cidr} + manager.db.network_create_safe(ignore, ignore).AndReturn(fakecidr) + self.mox.StubOutWithMock(manager, '_create_fixed_ips') + manager._create_fixed_ips(ignore, ignore).AndReturn('foo') + self.mox.ReplayAll() + args = [None, 'foo', cidr, None, 1, 256, 'fd00::/48', None, None, + None] + # making it to the end, without err, is enough validation + self.assertEqual(manager.create_networks(*args), None) + + def test_create_networks_cidr_already_used(self): + manager = self.FakeNetworkManager() + self.mox.StubOutWithMock(manager.db, 'network_get_all') + ctxt = mox.IgnoreArg() + fakecidr= [{'id': 1, 'cidr': '192.168.0.0/24'}] + manager.db.network_get_all(ctxt).AndReturn(fakecidr) + self.mox.ReplayAll() + args = [None, 'foo', '192.168.0.0/24', None, 1, 256, + 'fd00::/48', None, None, None] + self.assertRaises(ValueError, manager.create_networks, *args) -- cgit From 51f0cbf9221b461eb92beae2497e871bf2a7f45f Mon Sep 17 00:00:00 2001 From: John Tran Date: Tue, 2 Aug 2011 10:06:22 -0700 Subject: refactored tests --- nova/network/manager.py | 13 ++++++------- nova/tests/test_network.py | 27 ++++++++++++++++++--------- 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/nova/network/manager.py b/nova/network/manager.py index 8b86bc036..b21667def 100644 --- a/nova/network/manager.py +++ b/nova/network/manager.py @@ -666,9 +666,8 @@ class NetworkManager(manager.SchedulerDependentManager): significant_bits_v6 = 64 network_size_v6 = 1 << 64 - count = 0 - for req_cidr in req_cidrs: - count = count + 1 + for index in range(len(req_cidrs)): + req_cidr = req_cidrs[index] net = {} net['bridge'] = bridge net['bridge_interface'] = bridge_interface @@ -680,12 +679,12 @@ class NetworkManager(manager.SchedulerDependentManager): net['broadcast'] = str(req_cidr.broadcast) net['dhcp_start'] = str(req_cidr[2]) if num_networks > 1: - net['label'] = '%s_%d' % (label, count) + net['label'] = '%s_%d' % (label, index) else: net['label'] = label if FLAGS.use_ipv6: - start_v6 = count * network_size_v6 + start_v6 = index * network_size_v6 cidr_v6 = '%s/%s' % (fixed_net_v6[start_v6], significant_bits_v6) net['cidr_v6'] = cidr_v6 @@ -701,7 +700,7 @@ class NetworkManager(manager.SchedulerDependentManager): # this bit here is for vlan-manager del net['dns1'] del net['dns2'] - vlan = kwargs['vlan_start'] + count + vlan = kwargs['vlan_start'] + index net['vpn_private_address'] = str(req_cidr[2]) net['dhcp_start'] = str(req_cidr[3]) net['vlan'] = vlan @@ -709,7 +708,7 @@ class NetworkManager(manager.SchedulerDependentManager): # NOTE(vish): This makes ports unique accross the cloud, a more # robust solution would be to make them uniq per ip - net['vpn_public_port'] = kwargs['vpn_start'] + count + net['vpn_public_port'] = kwargs['vpn_start'] + index network = self.db.network_create_safe(context, net) if network: diff --git a/nova/tests/test_network.py b/nova/tests/test_network.py index 424b5f098..9c808c6db 100644 --- a/nova/tests/test_network.py +++ b/nova/tests/test_network.py @@ -254,7 +254,9 @@ class CommonNetworkTestCase(test.TestCase): raise exception.NetworkNotFoundForCidr() def network_create_safe(self, context, net): - return {'foo': 'bar'} + fakenet = {} + fakenet['id'] = 999 + return fakenet def network_get_all(self, context): raise exception.NoNetworksFound() @@ -266,6 +268,9 @@ class CommonNetworkTestCase(test.TestCase): def deallocate_fixed_ip(self, context, address): self.deallocate_called = address + def fake_create_fixed_ips(self, context, network_id): + return None + def test_remove_fixed_ip_from_instance(self): manager = self.FakeNetworkManager() manager.remove_fixed_ip_from_instance(None, 99, '10.0.0.1') @@ -372,16 +377,11 @@ class CommonNetworkTestCase(test.TestCase): def test_create_networks(self): cidr = '192.168.0.0/24' manager = self.FakeNetworkManager() - self.mox.StubOutWithMock(manager.db, 'network_create_safe') - ignore = mox.IgnoreArg() - fakecidr= {'id': 1, 'cidr': cidr} - manager.db.network_create_safe(ignore, ignore).AndReturn(fakecidr) - self.mox.StubOutWithMock(manager, '_create_fixed_ips') - manager._create_fixed_ips(ignore, ignore).AndReturn('foo') - self.mox.ReplayAll() + self.stubs.Set(manager, '_create_fixed_ips', + self.fake_create_fixed_ips) args = [None, 'foo', cidr, None, 1, 256, 'fd00::/48', None, None, None] - # making it to the end, without err, is enough validation + result = manager.create_networks(*args) self.assertEqual(manager.create_networks(*args), None) def test_create_networks_cidr_already_used(self): @@ -394,3 +394,12 @@ class CommonNetworkTestCase(test.TestCase): args = [None, 'foo', '192.168.0.0/24', None, 1, 256, 'fd00::/48', None, None, None] self.assertRaises(ValueError, manager.create_networks, *args) + + def test_create_networks_many(self): + cidr = '192.168.0.0/16' + manager = self.FakeNetworkManager() + self.stubs.Set(manager, '_create_fixed_ips', + self.fake_create_fixed_ips) + args = [None, 'foo', cidr, None, 10, 256, 'fd00::/48', None, None, + None] + self.assertEqual(manager.create_networks(*args), None) -- cgit From 0c19e26cddb50bf6808670d550d71ab435df37c5 Mon Sep 17 00:00:00 2001 From: Donal Lafferty Date: Thu, 4 Aug 2011 18:48:31 +0100 Subject: Fix comments. --- nova/virt/xenapi/vm_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nova/virt/xenapi/vm_utils.py b/nova/virt/xenapi/vm_utils.py index a723c5e22..666b84b76 100644 --- a/nova/virt/xenapi/vm_utils.py +++ b/nova/virt/xenapi/vm_utils.py @@ -644,7 +644,7 @@ class VMHelper(HelperBase): # 3. Disk is_pv = True elif disk_image_type == ImageType.DISK_ISO: - # 5. ISO + # 4. ISO is_pv = False else: raise exception.Error(_("Unknown image format %(disk_image_type)s") -- cgit From 8c7b71f65e54d67615e52927591e12a43b8b3991 Mon Sep 17 00:00:00 2001 From: John Tran Date: Thu, 4 Aug 2011 16:05:08 -0700 Subject: re-integrated my changes after merging trunk. fixed some pep8 issues. sorting the list of cidrs to create, so that it will create x.x.0.0 with a lower 'id' than x.x.1.0 (as an example). <- was causing libvirtd test to fail --- nova/network/manager.py | 54 +++++++++++++++++++++++++++++++++++++++++----- nova/tests/test_network.py | 6 +++--- 2 files changed, 52 insertions(+), 8 deletions(-) diff --git a/nova/network/manager.py b/nova/network/manager.py index 8fc6a295f..3c29417d7 100644 --- a/nova/network/manager.py +++ b/nova/network/manager.py @@ -614,6 +614,52 @@ class NetworkManager(manager.SchedulerDependentManager): network_ref = self.db.fixed_ip_get_network(context, address) self._setup_network(context, network_ref) + def _validate_cidrs(self, context, cidr, num_networks, network_size): + significant_bits = 32 - int(math.log(network_size, 2)) + req_net = netaddr.IPNetwork(cidr) + req_net_ip = str(req_net.ip) + req_size = network_size * num_networks + if req_size > req_net.size: + msg = "network_size * num_networks exceeds cidr size" + raise ValueError(_(msg)) + adjusted_cidr_str = req_net_ip + '/' + str(significant_bits) + adjusted_cidr = netaddr.IPNetwork(adjusted_cidr_str) + all_req_nets = [adjusted_cidr] + try: + used_nets = self.db.network_get_all(context) + except exception.NoNetworksFound: + used_nets = [] + used_cidrs = [netaddr.IPNetwork(net['cidr']) for net in used_nets] + if adjusted_cidr in used_cidrs: + raise ValueError(_("cidr already in use")) + for adjusted_cidr_supernet in adjusted_cidr.supernet(): + if adjusted_cidr_supernet in used_cidrs: + msg = "requested cidr (%s) conflicts with existing supernet" + raise ValueError(_(msg % str(adjusted_cidr))) + # split supernet into subnets + if num_networks >= 2: + next_cidr = adjusted_cidr + for used_cidr in used_cidrs: + # watch for smaller subnets conflicting + if used_cidr.size < next_cidr.size: + for ucsupernet in used_cidr.supernet(): + if ucsupernet.size == next_cidr.size: + used_cidrs.append(ucsupernet) + for index in range(1, num_networks): + while True: + next_cidr = next_cidr.next() + if next_cidr in used_cidrs: + continue + else: + all_req_nets.append(next_cidr) + break + all_req_nets = sorted(list(set(all_req_nets))) + # after splitting ensure there were enough to satisfy the num_networks + if len(all_req_nets) < num_networks: + msg = "Not enough subnets avail to satisfy requested num_networks" + raise ValueError(_(msg)) + return all_req_nets + def create_networks(self, context, label, cidr, multi_host, num_networks, network_size, cidr_v6, gateway_v6, bridge, bridge_interface, dns1=None, dns2=None, **kwargs): @@ -624,8 +670,8 @@ class NetworkManager(manager.SchedulerDependentManager): network_size_v6 = 1 << 64 if cidr: - fixed_net = netaddr.IPNetwork(cidr) - significant_bits = 32 - int(math.log(network_size, 2)) + req_cidrs = self._validate_cidrs(context, cidr, num_networks, + network_size) for index in range(num_networks): net = {} @@ -635,9 +681,7 @@ class NetworkManager(manager.SchedulerDependentManager): net['dns2'] = dns2 if cidr: - start = index * network_size - project_net = netaddr.IPNetwork('%s/%s' % (fixed_net[start], - significant_bits)) + project_net = req_cidrs[index] net['cidr'] = str(project_net) net['multi_host'] = multi_host net['netmask'] = str(project_net.netmask) diff --git a/nova/tests/test_network.py b/nova/tests/test_network.py index 9c808c6db..ceb12e890 100644 --- a/nova/tests/test_network.py +++ b/nova/tests/test_network.py @@ -269,7 +269,7 @@ class CommonNetworkTestCase(test.TestCase): self.deallocate_called = address def fake_create_fixed_ips(self, context, network_id): - return None + return None def test_remove_fixed_ip_from_instance(self): manager = self.FakeNetworkManager() @@ -366,7 +366,7 @@ class CommonNetworkTestCase(test.TestCase): manager = self.FakeNetworkManager() self.mox.StubOutWithMock(manager.db, 'network_get_all') ctxt = mox.IgnoreArg() - fakecidr= [{'id': 1, 'cidr': '192.168.0.0/8'}] + fakecidr = [{'id': 1, 'cidr': '192.168.0.0/8'}] manager.db.network_get_all(ctxt).AndReturn(fakecidr) self.mox.ReplayAll() args = [None, '192.168.0.0/24', 1, 256] @@ -388,7 +388,7 @@ class CommonNetworkTestCase(test.TestCase): manager = self.FakeNetworkManager() self.mox.StubOutWithMock(manager.db, 'network_get_all') ctxt = mox.IgnoreArg() - fakecidr= [{'id': 1, 'cidr': '192.168.0.0/24'}] + fakecidr = [{'id': 1, 'cidr': '192.168.0.0/24'}] manager.db.network_get_all(ctxt).AndReturn(fakecidr) self.mox.ReplayAll() args = [None, 'foo', '192.168.0.0/24', None, 1, 256, -- cgit From f58d441b55e143de35aefd039b80e0b27dad9ce2 Mon Sep 17 00:00:00 2001 From: John Tran Date: Thu, 4 Aug 2011 16:27:55 -0700 Subject: removed unnecessary context from test I had left there from prior --- nova/tests/test_network.py | 1 - 1 file changed, 1 deletion(-) diff --git a/nova/tests/test_network.py b/nova/tests/test_network.py index ceb12e890..1ff3f0c01 100644 --- a/nova/tests/test_network.py +++ b/nova/tests/test_network.py @@ -15,7 +15,6 @@ # License for the specific language governing permissions and limitations # under the License. -from nova import context from nova import db from nova import exception from nova import flags -- cgit From 89ec28c70d7795d427ecd4242cb1856eabdca104 Mon Sep 17 00:00:00 2001 From: John Tran Date: Thu, 4 Aug 2011 18:01:07 -0700 Subject: fixed bug, wasn't detecting smaller subnet conflict properly added test for it --- nova/network/manager.py | 35 ++++++++++++++++++++++++----------- nova/tests/test_network.py | 30 +++++++++++++++++++++++++++++- 2 files changed, 53 insertions(+), 12 deletions(-) diff --git a/nova/network/manager.py b/nova/network/manager.py index 3c29417d7..873fcadf5 100644 --- a/nova/network/manager.py +++ b/nova/network/manager.py @@ -624,7 +624,6 @@ class NetworkManager(manager.SchedulerDependentManager): raise ValueError(_(msg)) adjusted_cidr_str = req_net_ip + '/' + str(significant_bits) adjusted_cidr = netaddr.IPNetwork(adjusted_cidr_str) - all_req_nets = [adjusted_cidr] try: used_nets = self.db.network_get_all(context) except exception.NoNetworksFound: @@ -636,22 +635,36 @@ class NetworkManager(manager.SchedulerDependentManager): if adjusted_cidr_supernet in used_cidrs: msg = "requested cidr (%s) conflicts with existing supernet" raise ValueError(_(msg % str(adjusted_cidr))) - # split supernet into subnets - if num_networks >= 2: + # watch for smaller subnets conflicting + used_supernets = [] + for used_cidr in used_cidrs: + if not used_cidr: + continue + if used_cidr.size < network_size: + for ucsupernet in used_cidr.supernet(): + if ucsupernet.size == network_size: + used_supernets.append(ucsupernet) + all_req_nets = [] + if num_networks == 1: + if adjusted_cidr in used_supernets: + msg = "requested cidr (%s) conflicts with existing smaller cidr" + raise ValueError(_(msg % str(adjusted_cidr))) + else: + all_req_nets.append(adjusted_cidr) + elif num_networks >= 2: + # split supernet into subnets next_cidr = adjusted_cidr - for used_cidr in used_cidrs: - # watch for smaller subnets conflicting - if used_cidr.size < next_cidr.size: - for ucsupernet in used_cidr.supernet(): - if ucsupernet.size == next_cidr.size: - used_cidrs.append(ucsupernet) - for index in range(1, num_networks): + for index in range(num_networks): while True: - next_cidr = next_cidr.next() if next_cidr in used_cidrs: + next_cidr = next_cidr.next() + continue + elif next_cidr in used_supernets: + next_cidr = next_cidr.next() continue else: all_req_nets.append(next_cidr) + next_cidr = next_cidr.next() break all_req_nets = sorted(list(set(all_req_nets))) # after splitting ensure there were enough to satisfy the num_networks diff --git a/nova/tests/test_network.py b/nova/tests/test_network.py index 1ff3f0c01..e3a677c97 100644 --- a/nova/tests/test_network.py +++ b/nova/tests/test_network.py @@ -313,7 +313,19 @@ class CommonNetworkTestCase(test.TestCase): self.assertTrue(exp_cidr + '/24' in cidrs) self.assertFalse('192.168.2.0/24' in cidrs) - def test__validate_cidrs_split_cidr_smaller_subnet_in_use(self): + def test__validate_cidrs_smaller_subnet_in_use(self): + manager = self.FakeNetworkManager() + self.mox.StubOutWithMock(manager.db, 'network_get_all') + ctxt = mox.IgnoreArg() + manager.db.network_get_all(ctxt).AndReturn([{'id': 1, + 'cidr': '192.168.2.9/25'}]) + self.mox.ReplayAll() + # ValueError: requested cidr (192.168.2.0/24) conflicts with + # existing smaller cidr + args = [None, '192.168.2.0/24', 1, 256] + self.assertRaises(ValueError, manager._validate_cidrs, *args) + + def test__validate_cidrs_split_smaller_cidr_in_use(self): manager = self.FakeNetworkManager() self.mox.StubOutWithMock(manager.db, 'network_get_all') ctxt = mox.IgnoreArg() @@ -329,6 +341,22 @@ class CommonNetworkTestCase(test.TestCase): self.assertTrue(exp_cidr + '/24' in cidrs) self.assertFalse('192.168.2.0/24' in cidrs) + def test__validate_cidrs_split_smaller_cidr_in_use2(self): + manager = self.FakeNetworkManager() + self.mox.StubOutWithMock(manager.db, 'network_get_all') + ctxt = mox.IgnoreArg() + manager.db.network_get_all(ctxt).AndReturn([{'id': 1, + 'cidr': '192.168.2.9/29'}]) + self.mox.ReplayAll() + nets = manager._validate_cidrs(None, '192.168.2.0/24', 3, 32) + self.assertEqual(3, len(nets)) + cidrs = [str(net) for net in nets] + print cidrs + exp_cidrs = ['192.168.2.32', '192.168.2.64', '192.168.2.96'] + for exp_cidr in exp_cidrs: + self.assertTrue(exp_cidr + '/27' in cidrs) + self.assertFalse('192.168.2.0/27' in cidrs) + def test__validate_cidrs_one_in_use(self): manager = self.FakeNetworkManager() args = [None, '192.168.0.0/24', 2, 256] -- cgit From 38eb72be5f15731ba34a7dc0f8a28aa0fb63ea90 Mon Sep 17 00:00:00 2001 From: John Tran Date: Thu, 4 Aug 2011 18:37:36 -0700 Subject: fixed bug , when logic searched for next avail cidr it would return cidrs that were out of range of original requested cidr block. added test for it --- nova/network/manager.py | 4 ++++ nova/tests/test_network.py | 17 +++++++++++++++-- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/nova/network/manager.py b/nova/network/manager.py index 873fcadf5..0bb107268 100644 --- a/nova/network/manager.py +++ b/nova/network/manager.py @@ -655,6 +655,10 @@ class NetworkManager(manager.SchedulerDependentManager): # split supernet into subnets next_cidr = adjusted_cidr for index in range(num_networks): + if next_cidr.first > req_net.last: + msg = "Not enough subnets avail to satisfy requested num_net" \ + "works - some subnets in requested range already in use" + raise ValueError(_(msg)) while True: if next_cidr in used_cidrs: next_cidr = next_cidr.next() diff --git a/nova/tests/test_network.py b/nova/tests/test_network.py index e3a677c97..b4c5a7584 100644 --- a/nova/tests/test_network.py +++ b/nova/tests/test_network.py @@ -351,12 +351,26 @@ class CommonNetworkTestCase(test.TestCase): nets = manager._validate_cidrs(None, '192.168.2.0/24', 3, 32) self.assertEqual(3, len(nets)) cidrs = [str(net) for net in nets] - print cidrs exp_cidrs = ['192.168.2.32', '192.168.2.64', '192.168.2.96'] for exp_cidr in exp_cidrs: self.assertTrue(exp_cidr + '/27' in cidrs) self.assertFalse('192.168.2.0/27' in cidrs) + def test__validate_cidrs_split_all_in_use(self): + manager = self.FakeNetworkManager() + self.mox.StubOutWithMock(manager.db, 'network_get_all') + ctxt = mox.IgnoreArg() + in_use = [{'id': 1, 'cidr': '192.168.2.9/29'}, + {'id': 2, 'cidr': '192.168.2.64/26'}, + {'id': 3, 'cidr': '192.168.2.128/26'} + ] + manager.db.network_get_all(ctxt).AndReturn(in_use) + self.mox.ReplayAll() + args = [None, '192.168.2.0/24', 3, 64] + # ValueError: Not enough subnets avail to satisfy requested num_networks + # - some subnets in requested range already in use + self.assertRaises(ValueError, manager._validate_cidrs, *args) + def test__validate_cidrs_one_in_use(self): manager = self.FakeNetworkManager() args = [None, '192.168.0.0/24', 2, 256] @@ -385,7 +399,6 @@ class CommonNetworkTestCase(test.TestCase): manager = self.FakeNetworkManager() nets = manager._validate_cidrs(None, '192.168.0.0/16', 2, 256) returned_cidrs = [str(net) for net in nets] - print returned_cidrs self.assertTrue('192.168.0.0/24' in returned_cidrs) self.assertTrue('192.168.1.0/24' in returned_cidrs) -- cgit From b557b6366b21a0d3795369785037ee29c8cef377 Mon Sep 17 00:00:00 2001 From: John Tran Date: Thu, 4 Aug 2011 18:52:15 -0700 Subject: fix pep8 issues --- nova/network/manager.py | 14 +++++++++----- nova/tests/test_network.py | 5 +++-- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/nova/network/manager.py b/nova/network/manager.py index 0bb107268..1b92effb2 100644 --- a/nova/network/manager.py +++ b/nova/network/manager.py @@ -647,7 +647,8 @@ class NetworkManager(manager.SchedulerDependentManager): all_req_nets = [] if num_networks == 1: if adjusted_cidr in used_supernets: - msg = "requested cidr (%s) conflicts with existing smaller cidr" + msg = "requested cidr (%s) conflicts with existing smaller" \ + " cidr" raise ValueError(_(msg % str(adjusted_cidr))) else: all_req_nets.append(adjusted_cidr) @@ -656,8 +657,9 @@ class NetworkManager(manager.SchedulerDependentManager): next_cidr = adjusted_cidr for index in range(num_networks): if next_cidr.first > req_net.last: - msg = "Not enough subnets avail to satisfy requested num_net" \ - "works - some subnets in requested range already in use" + msg = "Not enough subnets avail to satisfy requested " \ + "num_net works - some subnets in requested range" \ + " already in use" raise ValueError(_(msg)) while True: if next_cidr in used_cidrs: @@ -671,9 +673,11 @@ class NetworkManager(manager.SchedulerDependentManager): next_cidr = next_cidr.next() break all_req_nets = sorted(list(set(all_req_nets))) - # after splitting ensure there were enough to satisfy the num_networks + # after splitting ensure there were enough to satisfy the + # num_networks if len(all_req_nets) < num_networks: - msg = "Not enough subnets avail to satisfy requested num_networks" + msg = "Not enough subnets avail to satisfy requested " \ + "num_networks" raise ValueError(_(msg)) return all_req_nets diff --git a/nova/tests/test_network.py b/nova/tests/test_network.py index b4c5a7584..8c84ea938 100644 --- a/nova/tests/test_network.py +++ b/nova/tests/test_network.py @@ -367,8 +367,9 @@ class CommonNetworkTestCase(test.TestCase): manager.db.network_get_all(ctxt).AndReturn(in_use) self.mox.ReplayAll() args = [None, '192.168.2.0/24', 3, 64] - # ValueError: Not enough subnets avail to satisfy requested num_networks - # - some subnets in requested range already in use + # ValueError: Not enough subnets avail to satisfy requested num_ + # networks - some subnets in requested range already + # in use self.assertRaises(ValueError, manager._validate_cidrs, *args) def test__validate_cidrs_one_in_use(self): -- cgit From b7167b21d615f8617d588a1656aa341fd226ded9 Mon Sep 17 00:00:00 2001 From: John Tran Date: Thu, 4 Aug 2011 20:13:23 -0700 Subject: removed redundant logic --- nova/network/manager.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/nova/network/manager.py b/nova/network/manager.py index 1b92effb2..3d915423f 100644 --- a/nova/network/manager.py +++ b/nova/network/manager.py @@ -673,12 +673,6 @@ class NetworkManager(manager.SchedulerDependentManager): next_cidr = next_cidr.next() break all_req_nets = sorted(list(set(all_req_nets))) - # after splitting ensure there were enough to satisfy the - # num_networks - if len(all_req_nets) < num_networks: - msg = "Not enough subnets avail to satisfy requested " \ - "num_networks" - raise ValueError(_(msg)) return all_req_nets def create_networks(self, context, label, cidr, multi_host, num_networks, -- cgit From 2935bebd718e770d0f2c9d1ab5dca76cc7d5f76a Mon Sep 17 00:00:00 2001 From: John Tran Date: Fri, 5 Aug 2011 09:50:11 -0700 Subject: fix typo --- nova/tests/test_network.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/nova/tests/test_network.py b/nova/tests/test_network.py index 40cc96147..1f27643e5 100644 --- a/nova/tests/test_network.py +++ b/nova/tests/test_network.py @@ -360,8 +360,7 @@ class CommonNetworkTestCase(test.TestCase): ctxt = mox.IgnoreArg() in_use = [{'id': 1, 'cidr': '192.168.2.9/29'}, {'id': 2, 'cidr': '192.168.2.64/26'}, - {'id': 3, 'cidr': '192.168.2.128/26'} - ] + {'id': 3, 'cidr': '192.168.2.128/26'}] manager.db.network_get_all(ctxt).AndReturn(in_use) self.mox.ReplayAll() args = [None, '192.168.2.0/24', 3, 64] -- cgit From 3812d22b7a6f5d74418a7a99dc69c68a5b9f9046 Mon Sep 17 00:00:00 2001 From: John Tran Date: Fri, 5 Aug 2011 11:36:00 -0700 Subject: fixed per peer review --- nova/network/manager.py | 27 ++++++++++++--------------- nova/tests/test_network.py | 40 ++++++++++++++++++++-------------------- 2 files changed, 32 insertions(+), 35 deletions(-) diff --git a/nova/network/manager.py b/nova/network/manager.py index 3d915423f..5677df670 100644 --- a/nova/network/manager.py +++ b/nova/network/manager.py @@ -620,8 +620,8 @@ class NetworkManager(manager.SchedulerDependentManager): req_net_ip = str(req_net.ip) req_size = network_size * num_networks if req_size > req_net.size: - msg = "network_size * num_networks exceeds cidr size" - raise ValueError(_(msg)) + msg = _("network_size * num_networks exceeds cidr size") + raise ValueError(msg) adjusted_cidr_str = req_net_ip + '/' + str(significant_bits) adjusted_cidr = netaddr.IPNetwork(adjusted_cidr_str) try: @@ -633,8 +633,8 @@ class NetworkManager(manager.SchedulerDependentManager): raise ValueError(_("cidr already in use")) for adjusted_cidr_supernet in adjusted_cidr.supernet(): if adjusted_cidr_supernet in used_cidrs: - msg = "requested cidr (%s) conflicts with existing supernet" - raise ValueError(_(msg % str(adjusted_cidr))) + msg = _("requested cidr (%s) conflicts with existing supernet") + raise ValueError(msg % str(adjusted_cidr)) # watch for smaller subnets conflicting used_supernets = [] for used_cidr in used_cidrs: @@ -647,9 +647,9 @@ class NetworkManager(manager.SchedulerDependentManager): all_req_nets = [] if num_networks == 1: if adjusted_cidr in used_supernets: - msg = "requested cidr (%s) conflicts with existing smaller" \ - " cidr" - raise ValueError(_(msg % str(adjusted_cidr))) + msg = _("requested cidr (%s) conflicts with existing smaller" \ + " cidr") + raise ValueError(msg % str(adjusted_cidr)) else: all_req_nets.append(adjusted_cidr) elif num_networks >= 2: @@ -657,17 +657,14 @@ class NetworkManager(manager.SchedulerDependentManager): next_cidr = adjusted_cidr for index in range(num_networks): if next_cidr.first > req_net.last: - msg = "Not enough subnets avail to satisfy requested " \ + msg = ("Not enough subnets avail to satisfy requested " \ "num_net works - some subnets in requested range" \ - " already in use" - raise ValueError(_(msg)) + " already in use") + raise ValueError(msg) while True: - if next_cidr in used_cidrs: + used_values = used_cidrs + used_supernets + if next_cidr in used_values: next_cidr = next_cidr.next() - continue - elif next_cidr in used_supernets: - next_cidr = next_cidr.next() - continue else: all_req_nets.append(next_cidr) next_cidr = next_cidr.next() diff --git a/nova/tests/test_network.py b/nova/tests/test_network.py index 1f27643e5..547a7a1fa 100644 --- a/nova/tests/test_network.py +++ b/nova/tests/test_network.py @@ -280,14 +280,14 @@ class CommonNetworkTestCase(test.TestCase): manager.remove_fixed_ip_from_instance, None, 99, 'bad input') - def test__validate_cidrs(self): + def test_validate_cidrs(self): manager = self.FakeNetworkManager() nets = manager._validate_cidrs(None, '192.168.0.0/24', 1, 256) self.assertEqual(1, len(nets)) cidrs = [str(net) for net in nets] self.assertTrue('192.168.0.0/24' in cidrs) - def test__validate_cidrs_split_exact_in_half(self): + def test_validate_cidrs_split_exact_in_half(self): manager = self.FakeNetworkManager() nets = manager._validate_cidrs(None, '192.168.0.0/24', 2, 128) self.assertEqual(2, len(nets)) @@ -295,7 +295,7 @@ class CommonNetworkTestCase(test.TestCase): self.assertTrue('192.168.0.0/25' in cidrs) self.assertTrue('192.168.0.128/25' in cidrs) - def test__validate_cidrs_split_cidr_in_use_middle_of_range(self): + def test_validate_cidrs_split_cidr_in_use_middle_of_range(self): manager = self.FakeNetworkManager() self.mox.StubOutWithMock(manager.db, 'network_get_all') ctxt = mox.IgnoreArg() @@ -305,13 +305,13 @@ class CommonNetworkTestCase(test.TestCase): nets = manager._validate_cidrs(None, '192.168.0.0/16', 4, 256) self.assertEqual(4, len(nets)) cidrs = [str(net) for net in nets] - exp_cidrs = ['192.168.0.0', '192.168.1.0', '192.168.3.0', - '192.168.4.0'] + exp_cidrs = ['192.168.0.0/24', '192.168.1.0/24', '192.168.3.0/24', + '192.168.4.0/24'] for exp_cidr in exp_cidrs: - self.assertTrue(exp_cidr + '/24' in cidrs) + self.assertTrue(exp_cidr in cidrs) self.assertFalse('192.168.2.0/24' in cidrs) - def test__validate_cidrs_smaller_subnet_in_use(self): + def test_validate_cidrs_smaller_subnet_in_use(self): manager = self.FakeNetworkManager() self.mox.StubOutWithMock(manager.db, 'network_get_all') ctxt = mox.IgnoreArg() @@ -323,7 +323,7 @@ class CommonNetworkTestCase(test.TestCase): args = [None, '192.168.2.0/24', 1, 256] self.assertRaises(ValueError, manager._validate_cidrs, *args) - def test__validate_cidrs_split_smaller_cidr_in_use(self): + def test_validate_cidrs_split_smaller_cidr_in_use(self): manager = self.FakeNetworkManager() self.mox.StubOutWithMock(manager.db, 'network_get_all') ctxt = mox.IgnoreArg() @@ -333,13 +333,13 @@ class CommonNetworkTestCase(test.TestCase): nets = manager._validate_cidrs(None, '192.168.0.0/16', 4, 256) self.assertEqual(4, len(nets)) cidrs = [str(net) for net in nets] - exp_cidrs = ['192.168.0.0', '192.168.1.0', '192.168.3.0', - '192.168.4.0'] + exp_cidrs = ['192.168.0.0/24', '192.168.1.0/24', '192.168.3.0/24', + '192.168.4.0/24'] for exp_cidr in exp_cidrs: - self.assertTrue(exp_cidr + '/24' in cidrs) + self.assertTrue(exp_cidr in cidrs) self.assertFalse('192.168.2.0/24' in cidrs) - def test__validate_cidrs_split_smaller_cidr_in_use2(self): + def test_validate_cidrs_split_smaller_cidr_in_use2(self): manager = self.FakeNetworkManager() self.mox.StubOutWithMock(manager.db, 'network_get_all') ctxt = mox.IgnoreArg() @@ -349,12 +349,12 @@ class CommonNetworkTestCase(test.TestCase): nets = manager._validate_cidrs(None, '192.168.2.0/24', 3, 32) self.assertEqual(3, len(nets)) cidrs = [str(net) for net in nets] - exp_cidrs = ['192.168.2.32', '192.168.2.64', '192.168.2.96'] + exp_cidrs = ['192.168.2.32/27', '192.168.2.64/27', '192.168.2.96/27'] for exp_cidr in exp_cidrs: - self.assertTrue(exp_cidr + '/27' in cidrs) + self.assertTrue(exp_cidr in cidrs) self.assertFalse('192.168.2.0/27' in cidrs) - def test__validate_cidrs_split_all_in_use(self): + def test_validate_cidrs_split_all_in_use(self): manager = self.FakeNetworkManager() self.mox.StubOutWithMock(manager.db, 'network_get_all') ctxt = mox.IgnoreArg() @@ -369,13 +369,13 @@ class CommonNetworkTestCase(test.TestCase): # in use self.assertRaises(ValueError, manager._validate_cidrs, *args) - def test__validate_cidrs_one_in_use(self): + def test_validate_cidrs_one_in_use(self): manager = self.FakeNetworkManager() args = [None, '192.168.0.0/24', 2, 256] # ValueError: network_size * num_networks exceeds cidr size self.assertRaises(ValueError, manager._validate_cidrs, *args) - def test__validate_cidrs_already_used(self): + def test_validate_cidrs_already_used(self): manager = self.FakeNetworkManager() self.mox.StubOutWithMock(manager.db, 'network_get_all') ctxt = mox.IgnoreArg() @@ -386,21 +386,21 @@ class CommonNetworkTestCase(test.TestCase): args = [None, '192.168.0.0/24', 1, 256] self.assertRaises(ValueError, manager._validate_cidrs, *args) - def test__validate_cidrs_too_many(self): + def test_validate_cidrs_too_many(self): manager = self.FakeNetworkManager() args = [None, '192.168.0.0/24', 200, 256] # ValueError: Not enough subnets avail to satisfy requested # num_networks self.assertRaises(ValueError, manager._validate_cidrs, *args) - def test__validate_cidrs_split_partial(self): + def test_validate_cidrs_split_partial(self): manager = self.FakeNetworkManager() nets = manager._validate_cidrs(None, '192.168.0.0/16', 2, 256) returned_cidrs = [str(net) for net in nets] self.assertTrue('192.168.0.0/24' in returned_cidrs) self.assertTrue('192.168.1.0/24' in returned_cidrs) - def test__validate_cidrs_conflict_existing_supernet(self): + def test_validate_cidrs_conflict_existing_supernet(self): manager = self.FakeNetworkManager() self.mox.StubOutWithMock(manager.db, 'network_get_all') ctxt = mox.IgnoreArg() -- cgit From 353fa4871069cf0b926f09aa00496002f65584cb Mon Sep 17 00:00:00 2001 From: John Tran Date: Fri, 5 Aug 2011 11:48:06 -0700 Subject: fixed per peer review --- nova/network/manager.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/nova/network/manager.py b/nova/network/manager.py index 5677df670..402049d44 100644 --- a/nova/network/manager.py +++ b/nova/network/manager.py @@ -647,8 +647,8 @@ class NetworkManager(manager.SchedulerDependentManager): all_req_nets = [] if num_networks == 1: if adjusted_cidr in used_supernets: - msg = _("requested cidr (%s) conflicts with existing smaller" \ - " cidr") + msg = _("requested cidr (%s) conflicts with existing smaller" + " cidr") raise ValueError(msg % str(adjusted_cidr)) else: all_req_nets.append(adjusted_cidr) @@ -657,9 +657,9 @@ class NetworkManager(manager.SchedulerDependentManager): next_cidr = adjusted_cidr for index in range(num_networks): if next_cidr.first > req_net.last: - msg = ("Not enough subnets avail to satisfy requested " \ - "num_net works - some subnets in requested range" \ - " already in use") + msg = _("Not enough subnets avail to satisfy requested " + "num_net works - some subnets in requested range" + " already in use") raise ValueError(msg) while True: used_values = used_cidrs + used_supernets -- cgit From 6548ce754984f2eb5e72612392a8a3392c2a21a2 Mon Sep 17 00:00:00 2001 From: John Tran Date: Tue, 9 Aug 2011 18:43:18 -0700 Subject: fix so that the exception shows up in euca2ools instead of UnknownError --- nova/api/ec2/__init__.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/nova/api/ec2/__init__.py b/nova/api/ec2/__init__.py index 8b6e47cfb..ffd5382bf 100644 --- a/nova/api/ec2/__init__.py +++ b/nova/api/ec2/__init__.py @@ -358,6 +358,10 @@ class Executor(wsgi.Application): LOG.debug(_('InvalidParameterValue raised: %s'), unicode(ex), context=context) return self._error(req, context, type(ex).__name__, unicode(ex)) + except exception.InvalidPortRange as ex:$ + LOG.debug(_('InvalidPortRange raised: %s'), unicode(ex),$ + context=context)$ + return self._error(req, context, type(ex).__name__, unicode(ex))$ except Exception as ex: extra = {'environment': req.environ} LOG.exception(_('Unexpected error raised: %s'), unicode(ex), -- cgit From 7b72972cbc9fbd267160d8d3282e1d0ec888de98 Mon Sep 17 00:00:00 2001 From: John Tran Date: Wed, 10 Aug 2011 16:19:21 -0700 Subject: removed typos, end of line chars --- nova/api/ec2/__init__.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/nova/api/ec2/__init__.py b/nova/api/ec2/__init__.py index ffd5382bf..96df97393 100644 --- a/nova/api/ec2/__init__.py +++ b/nova/api/ec2/__init__.py @@ -358,10 +358,10 @@ class Executor(wsgi.Application): LOG.debug(_('InvalidParameterValue raised: %s'), unicode(ex), context=context) return self._error(req, context, type(ex).__name__, unicode(ex)) - except exception.InvalidPortRange as ex:$ - LOG.debug(_('InvalidPortRange raised: %s'), unicode(ex),$ - context=context)$ - return self._error(req, context, type(ex).__name__, unicode(ex))$ + except exception.InvalidPortRange as ex: + LOG.debug(_('InvalidPortRange raised: %s'), unicode(ex), + context=context) + return self._error(req, context, type(ex).__name__, unicode(ex)) except Exception as ex: extra = {'environment': req.environ} LOG.exception(_('Unexpected error raised: %s'), unicode(ex), -- cgit From 0a543d4f8ff31733c32cbd9063e461ca41a0b076 Mon Sep 17 00:00:00 2001 From: Alex Meade Date: Wed, 10 Aug 2011 21:27:40 -0400 Subject: Changed bad server actions requests to raise an HTTP 400 --- nova/api/openstack/servers.py | 10 +++++++--- nova/tests/api/openstack/test_extensions.py | 2 +- nova/tests/api/openstack/test_server_actions.py | 20 +++++++++++++++++++- 3 files changed, 27 insertions(+), 5 deletions(-) diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index 736fdf6ce..c7d17a5bc 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -210,11 +210,15 @@ class Controller(object): } self.actions.update(admin_actions) - for key in self.actions.keys(): - if key in body: + for key in body.keys(): + if key in self.actions: return self.actions[key](body, req, id) + else: + msg = _('There is no such server action: %s' % key) + raise exc.HTTPBadRequest(explanation=msg) - raise exc.HTTPNotImplemented() + msg = _('Invalid request body') + raise exc.HTTPBadRequest(explanation=msg) def _action_create_backup(self, input_dict, req, instance_id): """Backup a server instance. diff --git a/nova/tests/api/openstack/test_extensions.py b/nova/tests/api/openstack/test_extensions.py index 8b7e11a5b..12eee3367 100644 --- a/nova/tests/api/openstack/test_extensions.py +++ b/nova/tests/api/openstack/test_extensions.py @@ -280,7 +280,7 @@ class ActionExtensionTest(test.TestCase): def test_invalid_action_body(self): body = dict(blah=dict(name="test")) # Doesn't exist response = self._send_server_action_request("/servers/1/action", body) - self.assertEqual(501, response.status_int) + self.assertEqual(400, response.status_int) def test_invalid_action(self): body = dict(blah=dict(name="test")) diff --git a/nova/tests/api/openstack/test_server_actions.py b/nova/tests/api/openstack/test_server_actions.py index 717e11c00..687a19390 100644 --- a/nova/tests/api/openstack/test_server_actions.py +++ b/nova/tests/api/openstack/test_server_actions.py @@ -352,7 +352,7 @@ class ServerActionsTest(test.TestCase): req.body = json.dumps(body) req.headers["content-type"] = "application/json" response = req.get_response(fakes.wsgi_app()) - self.assertEqual(501, response.status_int) + self.assertEqual(400, response.status_int) def test_create_backup_with_metadata(self): self.flags(allow_admin_api=True) @@ -487,6 +487,24 @@ class ServerActionsTestV11(test.TestCase): def tearDown(self): self.stubs.UnsetAll() + def test_server_bad_body(self): + body = {} + req = webob.Request.blank('/v1.1/servers/1/action') + req.method = 'POST' + req.content_type = 'application/json' + req.body = json.dumps(body) + res = req.get_response(fakes.wsgi_app()) + self.assertEqual(res.status_int, 400) + + def test_server_unknown_action(self): + body = {'sockTheFox': {'fakekey': '1234'}} + req = webob.Request.blank('/v1.1/servers/1/action') + req.method = 'POST' + req.content_type = 'application/json' + req.body = json.dumps(body) + res = req.get_response(fakes.wsgi_app()) + self.assertEqual(res.status_int, 400) + def test_server_change_password(self): mock_method = MockSetAdminPassword() self.stubs.Set(nova.compute.api.API, 'set_admin_password', mock_method) -- cgit From 8517d9563191b635669032e8364d8fa64876b977 Mon Sep 17 00:00:00 2001 From: Alex Meade Date: Thu, 11 Aug 2011 10:53:40 -0400 Subject: Fixed per HACKING --- nova/api/openstack/servers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index c7d17a5bc..c3bb23adb 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -214,10 +214,10 @@ class Controller(object): if key in self.actions: return self.actions[key](body, req, id) else: - msg = _('There is no such server action: %s' % key) + msg = _("There is no such server action: %s") % (key,)) raise exc.HTTPBadRequest(explanation=msg) - msg = _('Invalid request body') + msg = _("Invalid request body") raise exc.HTTPBadRequest(explanation=msg) def _action_create_backup(self, input_dict, req, instance_id): -- cgit From 7ae64a0b7e1db7e46d183bfa8a2fe1be5d47f1cc Mon Sep 17 00:00:00 2001 From: Alex Meade Date: Thu, 11 Aug 2011 11:57:16 -0400 Subject: removed extra paren --- nova/api/openstack/servers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index c3bb23adb..4c56539a4 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -214,7 +214,7 @@ class Controller(object): if key in self.actions: return self.actions[key](body, req, id) else: - msg = _("There is no such server action: %s") % (key,)) + msg = _("There is no such server action: %s") % (key,) raise exc.HTTPBadRequest(explanation=msg) msg = _("Invalid request body") -- cgit From 24869338aad2dfd36db9d466820325d1a3ed1adb Mon Sep 17 00:00:00 2001 From: Johannes Erdfelt Date: Thu, 11 Aug 2011 18:01:37 +0000 Subject: Make PUT /servers/ follow the API specs and return a 200 status --- nova/api/openstack/servers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index 736fdf6ce..0f8e8e461 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -185,7 +185,7 @@ class Controller(object): except exception.NotFound: raise exc.HTTPNotFound() - return exc.HTTPNoContent() + return webob.Response() # 200 response def _parse_update(self, context, id, inst_dict, update_dict): pass -- cgit From 2ccec88a5a5c85ce7776b4b70d490189d63d3098 Mon Sep 17 00:00:00 2001 From: Tushar Patil Date: Thu, 11 Aug 2011 11:15:14 -0700 Subject: Added availability zone support to the Create Server API --- bin/nova-manage | 14 +++++++++++--- nova/api/openstack/create_instance_helper.py | 4 +++- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/bin/nova-manage b/bin/nova-manage index 077a89d6f..02591a49c 100755 --- a/bin/nova-manage +++ b/bin/nova-manage @@ -882,6 +882,14 @@ class ServiceCommands(object): services = [s for s in services if s['host'] == host] if service: services = [s for s in services if s['binary'] == service] + print_format = "%-16s %-36s %-16s %-10s %-5s %-10s" + print print_format % ( + _('Binary'), + _('Host'), + _('Zone'), + _('Status'), + _('State'), + _('Updated_At')) for svc in services: delta = now - (svc['updated_at'] or svc['created_at']) alive = (delta.seconds <= 15) @@ -889,9 +897,9 @@ class ServiceCommands(object): active = 'enabled' if svc['disabled']: active = 'disabled' - print "%-10s %-10s %-8s %s %s" % (svc['host'], svc['binary'], - active, art, - svc['updated_at']) + print print_format % (svc['binary'], svc['host'], + svc['availability_zone'], active, art, + svc['updated_at']) @args('--host', dest='host', metavar='', help='Host') @args('--service', dest='service', metavar='', diff --git a/nova/api/openstack/create_instance_helper.py b/nova/api/openstack/create_instance_helper.py index 1425521a9..4e1da549e 100644 --- a/nova/api/openstack/create_instance_helper.py +++ b/nova/api/openstack/create_instance_helper.py @@ -122,6 +122,7 @@ class CreateInstanceHelper(object): raise exc.HTTPBadRequest(explanation=msg) zone_blob = server_dict.get('blob') + availability_zone = server_dict.get('availability_zone') name = server_dict['name'] self._validate_server_name(name) name = name.strip() @@ -161,7 +162,8 @@ class CreateInstanceHelper(object): zone_blob=zone_blob, reservation_id=reservation_id, min_count=min_count, - max_count=max_count)) + max_count=max_count, + availability_zone=availability_zone)) except quota.QuotaError as error: self._handle_quota_error(error) except exception.ImageNotFound as error: -- cgit From 5dd39df596f7038cffde5079822ae4b747b92b72 Mon Sep 17 00:00:00 2001 From: Alex Meade Date: Thu, 11 Aug 2011 14:20:18 -0400 Subject: minor cleanup --- nova/api/openstack/servers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index 4c56539a4..c22a5a2a6 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -210,7 +210,7 @@ class Controller(object): } self.actions.update(admin_actions) - for key in body.keys(): + for key in body: if key in self.actions: return self.actions[key](body, req, id) else: -- cgit From 3bfaf0a0720fc8713fb77fddd8f1b2dffa0eabfc Mon Sep 17 00:00:00 2001 From: Johannes Erdfelt Date: Thu, 11 Aug 2011 18:28:15 +0000 Subject: v1.0 and v1.1 API differs for PUT, so split them out Update tests to match API --- nova/api/openstack/servers.py | 72 ++++++++++++++++++++------------ nova/tests/api/openstack/test_servers.py | 4 +- 2 files changed, 48 insertions(+), 28 deletions(-) diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index 0f8e8e461..90b6e684b 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -161,32 +161,6 @@ class Controller(object): server['server']['adminPass'] = extra_values['password'] return server - @scheduler_api.redirect_handler - def update(self, req, id, body): - """ Updates the server name or password """ - if len(req.body) == 0: - raise exc.HTTPUnprocessableEntity() - - if not body: - raise exc.HTTPUnprocessableEntity() - - ctxt = req.environ['nova.context'] - update_dict = {} - - if 'name' in body['server']: - name = body['server']['name'] - self.helper._validate_server_name(name) - update_dict['display_name'] = name.strip() - - self._parse_update(ctxt, id, body, update_dict) - - try: - self.compute_api.update(ctxt, id, **update_dict) - except exception.NotFound: - raise exc.HTTPNotFound() - - return webob.Response() # 200 response - def _parse_update(self, context, id, inst_dict, update_dict): pass @@ -545,6 +519,29 @@ class Controller(object): class ControllerV10(Controller): """v1.0 OpenStack API controller""" + @scheduler_api.redirect_handler + def update(self, req, id, body): + """ Updates the server name or password """ + if len(req.body) == 0 or not body: + raise exc.HTTPUnprocessableEntity() + + ctxt = req.environ['nova.context'] + update_dict = {} + + if 'name' in body['server']: + name = body['server']['name'] + self.helper._validate_server_name(name) + update_dict['display_name'] = name.strip() + + self._parse_update(ctxt, id, body, update_dict) + + try: + self.compute_api.update(ctxt, id, **update_dict) + except exception.NotFound: + raise exc.HTTPNotFound() + + return exc.HTTPNoContent() + @scheduler_api.redirect_handler def delete(self, req, id): """ Destroys a server """ @@ -614,6 +611,29 @@ class ControllerV10(Controller): class ControllerV11(Controller): """v1.1 OpenStack API controller""" + @scheduler_api.redirect_handler + def update(self, req, id, body): + """ Updates the server name or password """ + if len(req.body) == 0 or not body: + raise exc.HTTPUnprocessableEntity() + + ctxt = req.environ['nova.context'] + update_dict = {} + + if 'name' in body['server']: + name = body['server']['name'] + self.helper._validate_server_name(name) + update_dict['display_name'] = name.strip() + + self._parse_update(ctxt, id, body, update_dict) + + try: + self.compute_api.update(ctxt, id, **update_dict) + except exception.NotFound: + raise exc.HTTPNotFound() + + # v1.1 API returns 200, which differs from v1.0 + @scheduler_api.redirect_handler def delete(self, req, id): """ Destroys a server """ diff --git a/nova/tests/api/openstack/test_servers.py b/nova/tests/api/openstack/test_servers.py index b6342ae2f..b732d2972 100644 --- a/nova/tests/api/openstack/test_servers.py +++ b/nova/tests/api/openstack/test_servers.py @@ -1869,7 +1869,7 @@ class ServersTest(test.TestCase): req.content_type = 'application/json' req.body = json.dumps({'server': {'name': 'new-name'}}) res = req.get_response(fakes.wsgi_app()) - self.assertEqual(res.status_int, 204) + self.assertEqual(res.status_int, 200) self.assertEqual(res.body, '') def test_update_server_adminPass_ignored_v1_1(self): @@ -1889,7 +1889,7 @@ class ServersTest(test.TestCase): req.content_type = "application/json" req.body = self.body res = req.get_response(fakes.wsgi_app()) - self.assertEqual(res.status_int, 204) + self.assertEqual(res.status_int, 200) self.assertEqual(res.body, '') def test_create_backup_schedules(self): -- cgit From 684c41e7a4aa5fdab78f2e1aac1d309c3bb16412 Mon Sep 17 00:00:00 2001 From: "vladimir.p" Date: Thu, 11 Aug 2011 13:18:30 -0700 Subject: lp824780: fixed typo in update_service_capabilities --- nova/scheduler/manager.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nova/scheduler/manager.py b/nova/scheduler/manager.py index c8b16b622..294de62e4 100644 --- a/nova/scheduler/manager.py +++ b/nova/scheduler/manager.py @@ -71,8 +71,8 @@ class SchedulerManager(manager.Manager): def update_service_capabilities(self, context=None, service_name=None, host=None, capabilities=None): """Process a capability update from a service node.""" - if not capability: - capability = {} + if not capabilities: + capabilities = {} self.zone_manager.update_service_capabilities(service_name, host, capabilities) -- cgit From 26d96b80fdc07d8bb9453112cd33ee12143c6f46 Mon Sep 17 00:00:00 2001 From: Johannes Erdfelt Date: Thu, 11 Aug 2011 20:48:16 +0000 Subject: v1.1 API also requires the server be returned in the body --- nova/api/openstack/servers.py | 83 +++++++++++++------------------- nova/tests/api/openstack/test_servers.py | 6 ++- 2 files changed, 37 insertions(+), 52 deletions(-) diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index 90b6e684b..f19befd6f 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -161,8 +161,32 @@ class Controller(object): server['server']['adminPass'] = extra_values['password'] return server - def _parse_update(self, context, id, inst_dict, update_dict): - pass + @scheduler_api.redirect_handler + def update(self, req, id, body): + """ Updates the server name or password """ + if len(req.body) == 0: + raise exc.HTTPUnprocessableEntity() + + if not body: + raise exc.HTTPUnprocessableEntity() + + ctxt = req.environ['nova.context'] + update_dict = {} + + if 'name' in body['server']: + name = body['server']['name'] + self.helper._validate_server_name(name) + update_dict['display_name'] = name.strip() + + try: + self.compute_api.update(ctxt, id, **update_dict) + except exception.NotFound: + raise exc.HTTPNotFound() + + return self._update(ctxt, req, id, body) + + def _update(self, context, req, id, inst_dict): + return exc.HTTPNotImplemented() @scheduler_api.redirect_handler def action(self, req, id, body): @@ -519,29 +543,6 @@ class Controller(object): class ControllerV10(Controller): """v1.0 OpenStack API controller""" - @scheduler_api.redirect_handler - def update(self, req, id, body): - """ Updates the server name or password """ - if len(req.body) == 0 or not body: - raise exc.HTTPUnprocessableEntity() - - ctxt = req.environ['nova.context'] - update_dict = {} - - if 'name' in body['server']: - name = body['server']['name'] - self.helper._validate_server_name(name) - update_dict['display_name'] = name.strip() - - self._parse_update(ctxt, id, body, update_dict) - - try: - self.compute_api.update(ctxt, id, **update_dict) - except exception.NotFound: - raise exc.HTTPNotFound() - - return exc.HTTPNoContent() - @scheduler_api.redirect_handler def delete(self, req, id): """ Destroys a server """ @@ -565,10 +566,11 @@ class ControllerV10(Controller): def _limit_items(self, items, req): return common.limited(items, req) - def _parse_update(self, context, server_id, inst_dict, update_dict): + def _update(self, context, req, id, inst_dict): if 'adminPass' in inst_dict['server']: - self.compute_api.set_admin_password(context, server_id, + self.compute_api.set_admin_password(context, id, inst_dict['server']['adminPass']) + return exc.HTTPNoContent() def _action_resize(self, input_dict, req, id): """ Resizes a given instance to the flavor size requested """ @@ -611,29 +613,6 @@ class ControllerV10(Controller): class ControllerV11(Controller): """v1.1 OpenStack API controller""" - @scheduler_api.redirect_handler - def update(self, req, id, body): - """ Updates the server name or password """ - if len(req.body) == 0 or not body: - raise exc.HTTPUnprocessableEntity() - - ctxt = req.environ['nova.context'] - update_dict = {} - - if 'name' in body['server']: - name = body['server']['name'] - self.helper._validate_server_name(name) - update_dict['display_name'] = name.strip() - - self._parse_update(ctxt, id, body, update_dict) - - try: - self.compute_api.update(ctxt, id, **update_dict) - except exception.NotFound: - raise exc.HTTPNotFound() - - # v1.1 API returns 200, which differs from v1.0 - @scheduler_api.redirect_handler def delete(self, req, id): """ Destroys a server """ @@ -713,6 +692,10 @@ class ControllerV11(Controller): LOG.info(msg) raise exc.HTTPBadRequest(explanation=msg) + def _update(self, context, req, id, inst_dict): + instance = self.compute_api.routing_get(context, id) + return self._build_view(req, instance, is_detail=True) + def _action_resize(self, input_dict, req, id): """ Resizes a given instance to the flavor size requested """ try: diff --git a/nova/tests/api/openstack/test_servers.py b/nova/tests/api/openstack/test_servers.py index b732d2972..457ce5aec 100644 --- a/nova/tests/api/openstack/test_servers.py +++ b/nova/tests/api/openstack/test_servers.py @@ -1870,7 +1870,8 @@ class ServersTest(test.TestCase): req.body = json.dumps({'server': {'name': 'new-name'}}) res = req.get_response(fakes.wsgi_app()) self.assertEqual(res.status_int, 200) - self.assertEqual(res.body, '') + res_dict = json.loads(res.body) + self.assertEqual(res_dict['server']['id'], 1) def test_update_server_adminPass_ignored_v1_1(self): inst_dict = dict(name='server_test', adminPass='bacon') @@ -1890,7 +1891,8 @@ class ServersTest(test.TestCase): req.body = self.body res = req.get_response(fakes.wsgi_app()) self.assertEqual(res.status_int, 200) - self.assertEqual(res.body, '') + res_dict = json.loads(res.body) + self.assertEqual(res_dict['server']['id'], 1) def test_create_backup_schedules(self): req = webob.Request.blank('/v1.0/servers/1/backup_schedule') -- cgit From 500bd3218906e2575467467700f80f5b31e0a87e Mon Sep 17 00:00:00 2001 From: termie Date: Thu, 11 Aug 2011 16:26:26 -0700 Subject: allow scheduling topics to multiple drivers --- nova/scheduler/manager.py | 5 ++- nova/scheduler/multi.py | 73 ++++++++++++++++++++++++++++++++++ nova/tests/scheduler/test_scheduler.py | 24 ++++++++++- 3 files changed, 98 insertions(+), 4 deletions(-) create mode 100644 nova/scheduler/multi.py diff --git a/nova/scheduler/manager.py b/nova/scheduler/manager.py index c8b16b622..13c0bf22a 100644 --- a/nova/scheduler/manager.py +++ b/nova/scheduler/manager.py @@ -34,12 +34,13 @@ from nova.scheduler import zone_manager LOG = logging.getLogger('nova.scheduler.manager') FLAGS = flags.FLAGS flags.DEFINE_string('scheduler_driver', - 'nova.scheduler.chance.ChanceScheduler', - 'Driver to use for the scheduler') + 'nova.scheduler.multi.MultiScheduler', + 'Default driver to use for the scheduler') class SchedulerManager(manager.Manager): """Chooses a host to run instances on.""" + def __init__(self, scheduler_driver=None, *args, **kwargs): self.zone_manager = zone_manager.ZoneManager() if not scheduler_driver: diff --git a/nova/scheduler/multi.py b/nova/scheduler/multi.py new file mode 100644 index 000000000..b1578033c --- /dev/null +++ b/nova/scheduler/multi.py @@ -0,0 +1,73 @@ +# vim: tabstop=4 shiftwidth=4 softtabstop=4 + +# Copyright (c) 2010 Openstack, LLC. +# Copyright 2010 United States Government as represented by the +# Administrator of the National Aeronautics and Space Administration. +# All Rights Reserved. +# +# 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. + +""" +Scheduler that allows routing some calls to one driver and others to another. +""" + +from nova import flags +from nova import utils +from nova.scheduler import driver + + +FLAGS = flags.FLAGS +flags.DEFINE_string('compute_scheduler_driver', + 'nova.scheduler.chance.ChanceScheduler', + 'Driver to use for scheduling compute calls') +flags.DEFINE_string('volume_scheduler_driver', + 'nova.scheduler.chance.ChanceScheduler', + 'Driver to use for scheduling volume calls') + + +# A mapping of methods to topics so we can figure out which driver to use. +_METHOD_MAP = {'run_instance': 'compute', + 'start_instance': 'compute', + 'create_volume': 'volume'} + + +class MultiScheduler(driver.Scheduler): + """A scheduler that holds multiple sub-schedulers. + + This exists to allow flag-driven composibility of schedulers, allowing + third parties to integrate custom schedulers more easily. + + """ + + def __init__(self): + super(MultiScheduler, self).__init__() + compute_driver = utils.import_object(FLAGS.compute_scheduler_driver) + volume_driver = utils.import_object(FLAGS.volume_scheduler_driver) + + self.drivers = {'compute': compute_driver, + 'volume': volume_driver} + + def __getattr__(self, key): + if not key.startswith('schedule_'): + raise AttributeError(key) + method = key[len('schedule_'):] + if method not in _METHOD_MAP: + raise AttributeError(key) + return getattr(self.drivers[_METHOD_MAP[method]], key) + + def set_zone_manager(self, zone_manager): + for k, v in self.drivers.iteritems(): + v.set_zone_manager(zone_manager) + + def schedule(self, context, topic, *_args, **_kwargs): + return self.drivers[topic].schedule(context, topic, *_args, **_kwargs) diff --git a/nova/tests/scheduler/test_scheduler.py b/nova/tests/scheduler/test_scheduler.py index 7a26fd1bb..d70a6779f 100644 --- a/nova/tests/scheduler/test_scheduler.py +++ b/nova/tests/scheduler/test_scheduler.py @@ -36,8 +36,9 @@ from nova import test from nova import rpc from nova import utils from nova.scheduler import api -from nova.scheduler import manager from nova.scheduler import driver +from nova.scheduler import manager +from nova.scheduler import multi from nova.compute import power_state @@ -391,7 +392,7 @@ class SimpleDriverTestCase(test.TestCase): compute1.kill() compute2.kill() - def test_wont_sechedule_if_specified_host_is_down_no_queue(self): + def test_wont_schedule_if_specified_host_is_down_no_queue(self): compute1 = service.Service('host1', 'nova-compute', 'compute', @@ -903,6 +904,25 @@ class SimpleDriverTestCase(test.TestCase): db.service_destroy(self.context, s_ref2['id']) +class MultiDriverTestCase(SimpleDriverTestCase): + """Test case for multi driver.""" + + def setUp(self): + super(MultiDriverTestCase, self).setUp() + self.flags(connection_type='fake', + stub_network=True, + max_cores=4, + max_gigabytes=4, + network_manager='nova.network.manager.FlatManager', + volume_driver='nova.volume.driver.FakeISCSIDriver', + compute_scheduler_driver=('nova.scheduler.simple' + '.SimpleScheduler'), + volume_scheduler_driver=('nova.scheduler.simple' + '.SimpleScheduler'), + scheduler_driver='nova.scheduler.multi.MultiScheduler') + self.scheduler = manager.SchedulerManager() + + class FakeZone(object): def __init__(self, id, api_url, username, password): self.id = id -- cgit From 4ce5c65e4002f1c3cca02bb06d892a6d270a0149 Mon Sep 17 00:00:00 2001 From: "vladimir.p" Date: Thu, 11 Aug 2011 16:32:51 -0700 Subject: Author added --- Authors | 1 + 1 file changed, 1 insertion(+) diff --git a/Authors b/Authors index e639cbf76..ccd70baaf 100644 --- a/Authors +++ b/Authors @@ -103,6 +103,7 @@ Tushar Patil Vasiliy Shlykov Vishvananda Ishaya Vivek Y S +Vladimir Popovski William Wolf Yoshiaki Tamura Youcef Laribi -- cgit From 258e169a60d3551e789022ec23d6ae040c1f981e Mon Sep 17 00:00:00 2001 From: Johannes Erdfelt Date: Fri, 12 Aug 2011 20:18:47 +0000 Subject: Stub out instance_get as well so we can show the results of the name change --- nova/api/openstack/servers.py | 2 +- nova/tests/api/openstack/test_servers.py | 19 ++++++++++++------- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/nova/api/openstack/servers.py b/nova/api/openstack/servers.py index f19befd6f..42e46a94a 100644 --- a/nova/api/openstack/servers.py +++ b/nova/api/openstack/servers.py @@ -163,7 +163,7 @@ class Controller(object): @scheduler_api.redirect_handler def update(self, req, id, body): - """ Updates the server name or password """ + """Update server name then pass on to version-specific controller""" if len(req.body) == 0: raise exc.HTTPUnprocessableEntity() diff --git a/nova/tests/api/openstack/test_servers.py b/nova/tests/api/openstack/test_servers.py index d43a40070..1a4288ae7 100644 --- a/nova/tests/api/openstack/test_servers.py +++ b/nova/tests/api/openstack/test_servers.py @@ -134,8 +134,8 @@ def return_security_group(context, instance_id, security_group_id): pass -def instance_update(context, instance_id, kwargs): - return stub_instance(instance_id) +def instance_update(context, instance_id, values): + return stub_instance(instance_id, name=values.get('display_name')) def instance_addresses(context, instance_id): @@ -145,7 +145,7 @@ def instance_addresses(context, instance_id): def stub_instance(id, user_id='fake', project_id='fake', private_address=None, public_addresses=None, host=None, power_state=0, reservation_id="", uuid=FAKE_UUID, image_ref="10", - flavor_id="1", interfaces=None): + flavor_id="1", interfaces=None, name=None): metadata = [] metadata.append(InstanceMetadata(key='seq', value=id)) @@ -161,7 +161,7 @@ def stub_instance(id, user_id='fake', project_id='fake', private_address=None, host = str(host) # ReservationID isn't sent back, hack it in there. - server_name = "server%s" % id + server_name = name or "server%s" % id if reservation_id != "": server_name = "reservation_%s" % (reservation_id, ) @@ -1880,14 +1880,17 @@ class ServersTest(test.TestCase): self.assertEqual(res.status_int, 400) def test_update_server_name_v1_1(self): + self.stubs.Set(nova.db.api, 'instance_get', + return_server_with_attributes(name='server_test')) req = webob.Request.blank('/v1.1/servers/1') req.method = 'PUT' req.content_type = 'application/json' - req.body = json.dumps({'server': {'name': 'new-name'}}) + req.body = json.dumps({'server': {'name': 'server_test'}}) res = req.get_response(fakes.wsgi_app()) self.assertEqual(res.status_int, 200) res_dict = json.loads(res.body) self.assertEqual(res_dict['server']['id'], 1) + self.assertEqual(res_dict['server']['name'], 'server_test') def test_update_server_adminPass_ignored_v1_1(self): inst_dict = dict(name='server_test', adminPass='bacon') @@ -1898,8 +1901,9 @@ class ServersTest(test.TestCase): self.assertEqual(params, filtered_dict) return filtered_dict - self.stubs.Set(nova.db.api, 'instance_update', - server_update) + self.stubs.Set(nova.db.api, 'instance_update', server_update) + self.stubs.Set(nova.db.api, 'instance_get', + return_server_with_attributes(name='server_test')) req = webob.Request.blank('/v1.1/servers/1') req.method = 'PUT' @@ -1909,6 +1913,7 @@ class ServersTest(test.TestCase): self.assertEqual(res.status_int, 200) res_dict = json.loads(res.body) self.assertEqual(res_dict['server']['id'], 1) + self.assertEqual(res_dict['server']['name'], 'server_test') def test_create_backup_schedules(self): req = webob.Request.blank('/v1.0/servers/1/backup_schedule') -- cgit