Stop using scheduler RPC API magic
If a scheduler RPC message isn't handled directly by a SchedulerManager
method, the __getattr__() fallback passes the message to a driver
method in the form of schedule_${method}() and, if that doesn't exist,
instead calls the schedule() method supplying the topic and method
args.
This is pretty bizarre stuff and we appear to only use it in two cases:
  1) live_migration - this is how the schedule_live_migration()
     method in the driver gets called, but the side-effect is that
     we require the client to pass a topic argument which is never
     used. This would be much more sanely handled with an explicit
     SchedulerManager.live_migration() method.
  2) create_volume - the volume API asks the scheduler to pick a
     target host for create_volume() using this method. This would
     be easily handled with an SchedulerManager.create_volume()
     method.
Change-Id: I1047489d85ac51d8d36fea1c4eb858df638ce349
			
			
This commit is contained in:
		@@ -89,6 +89,9 @@ class MultiScheduler(driver.Scheduler):
 | 
				
			|||||||
    def schedule_prep_resize(self, *args, **kwargs):
 | 
					    def schedule_prep_resize(self, *args, **kwargs):
 | 
				
			||||||
        return self.drivers['compute'].schedule_prep_resize(*args, **kwargs)
 | 
					        return self.drivers['compute'].schedule_prep_resize(*args, **kwargs)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    def schedule_create_volume(self, *args, **kwargs):
 | 
				
			||||||
 | 
					        return self.drivers['volume'].schedule_create_volume(*args, **kwargs)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    def update_service_capabilities(self, service_name, host, capabilities):
 | 
					    def update_service_capabilities(self, service_name, host, capabilities):
 | 
				
			||||||
        # Multi scheduler is only a holder of sub-schedulers, so
 | 
					        # Multi scheduler is only a holder of sub-schedulers, so
 | 
				
			||||||
        # pass the capabilities to the schedulers that matter
 | 
					        # pass the capabilities to the schedulers that matter
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -41,6 +41,7 @@ class SchedulerAPI(nova.openstack.common.rpc.proxy.RpcProxy):
 | 
				
			|||||||
        1.4 - Remove update_db from prep_resize
 | 
					        1.4 - Remove update_db from prep_resize
 | 
				
			||||||
        1.5 - Add reservations argument to prep_resize()
 | 
					        1.5 - Add reservations argument to prep_resize()
 | 
				
			||||||
        1.6 - Remove reservations argument to run_instance()
 | 
					        1.6 - Remove reservations argument to run_instance()
 | 
				
			||||||
 | 
					        1.7 - Add create_volume() method, remove topic from live_migration()
 | 
				
			||||||
    '''
 | 
					    '''
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    BASE_RPC_API_VERSION = '1.0'
 | 
					    BASE_RPC_API_VERSION = '1.0'
 | 
				
			||||||
@@ -73,14 +74,21 @@ class SchedulerAPI(nova.openstack.common.rpc.proxy.RpcProxy):
 | 
				
			|||||||
        return self.call(ctxt, self.make_msg('show_host_resources', host=host))
 | 
					        return self.call(ctxt, self.make_msg('show_host_resources', host=host))
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    def live_migration(self, ctxt, block_migration, disk_over_commit,
 | 
					    def live_migration(self, ctxt, block_migration, disk_over_commit,
 | 
				
			||||||
            instance, dest, topic):
 | 
					            instance, dest):
 | 
				
			||||||
        # NOTE(comstud): Call vs cast so we can get exceptions back, otherwise
 | 
					        # NOTE(comstud): Call vs cast so we can get exceptions back, otherwise
 | 
				
			||||||
        # this call in the scheduler driver doesn't return anything.
 | 
					        # this call in the scheduler driver doesn't return anything.
 | 
				
			||||||
        instance_p = jsonutils.to_primitive(instance)
 | 
					        instance_p = jsonutils.to_primitive(instance)
 | 
				
			||||||
        return self.call(ctxt, self.make_msg('live_migration',
 | 
					        return self.call(ctxt, self.make_msg('live_migration',
 | 
				
			||||||
                block_migration=block_migration,
 | 
					                block_migration=block_migration,
 | 
				
			||||||
                disk_over_commit=disk_over_commit, instance=instance_p,
 | 
					                disk_over_commit=disk_over_commit, instance=instance_p,
 | 
				
			||||||
                dest=dest, topic=topic), version='1.3')
 | 
					                dest=dest), version='1.7')
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    def create_volume(self, ctxt, volume_id, snapshot_id, reservations):
 | 
				
			||||||
 | 
					        self.cast(ctxt,
 | 
				
			||||||
 | 
					                  self.make_msg('create_volume',
 | 
				
			||||||
 | 
					                                volume_id=volume_id, snapshot_id=snapshot_id,
 | 
				
			||||||
 | 
					                                reservations=reservations),
 | 
				
			||||||
 | 
					                  version='1.7')
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    def update_service_capabilities(self, ctxt, service_name, host,
 | 
					    def update_service_capabilities(self, ctxt, service_name, host,
 | 
				
			||||||
            capabilities):
 | 
					            capabilities):
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -54,7 +54,8 @@ class SimpleScheduler(chance.ChanceScheduler):
 | 
				
			|||||||
        super(SimpleScheduler, self).schedule_run_instance(context,
 | 
					        super(SimpleScheduler, self).schedule_run_instance(context,
 | 
				
			||||||
                request_spec, reservations, *_args, **_kwargs)
 | 
					                request_spec, reservations, *_args, **_kwargs)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    def schedule_create_volume(self, context, volume_id, *_args, **_kwargs):
 | 
					    def schedule_create_volume(self, context, volume_id, snapshot_id,
 | 
				
			||||||
 | 
					                               reservations):
 | 
				
			||||||
        """Picks a host that is up and has the fewest volumes."""
 | 
					        """Picks a host that is up and has the fewest volumes."""
 | 
				
			||||||
        deprecated.warn(_('nova-volume functionality is deprecated in Folsom '
 | 
					        deprecated.warn(_('nova-volume functionality is deprecated in Folsom '
 | 
				
			||||||
                'and will be removed in Grizzly.  Volumes are now handled '
 | 
					                'and will be removed in Grizzly.  Volumes are now handled '
 | 
				
			||||||
@@ -72,7 +73,8 @@ class SimpleScheduler(chance.ChanceScheduler):
 | 
				
			|||||||
            if not utils.service_is_up(service):
 | 
					            if not utils.service_is_up(service):
 | 
				
			||||||
                raise exception.WillNotSchedule(host=host)
 | 
					                raise exception.WillNotSchedule(host=host)
 | 
				
			||||||
            driver.cast_to_volume_host(context, host, 'create_volume',
 | 
					            driver.cast_to_volume_host(context, host, 'create_volume',
 | 
				
			||||||
                    volume_id=volume_id, **_kwargs)
 | 
					                    volume_id=volume_id, snapshot_id=snapshot_id,
 | 
				
			||||||
 | 
					                    reservations=reservations)
 | 
				
			||||||
            return None
 | 
					            return None
 | 
				
			||||||
 | 
					
 | 
				
			||||||
        results = db.service_get_all_volume_sorted(elevated)
 | 
					        results = db.service_get_all_volume_sorted(elevated)
 | 
				
			||||||
@@ -86,7 +88,8 @@ class SimpleScheduler(chance.ChanceScheduler):
 | 
				
			|||||||
                raise exception.NoValidHost(reason=msg)
 | 
					                raise exception.NoValidHost(reason=msg)
 | 
				
			||||||
            if utils.service_is_up(service) and not service['disabled']:
 | 
					            if utils.service_is_up(service) and not service['disabled']:
 | 
				
			||||||
                driver.cast_to_volume_host(context, service['host'],
 | 
					                driver.cast_to_volume_host(context, service['host'],
 | 
				
			||||||
                        'create_volume', volume_id=volume_id, **_kwargs)
 | 
					                        'create_volume', volume_id=volume_id,
 | 
				
			||||||
 | 
					                        snapshot_id=snapshot_id, reservations=reservations)
 | 
				
			||||||
                return None
 | 
					                return None
 | 
				
			||||||
        msg = _("Is the appropriate service running?")
 | 
					        msg = _("Is the appropriate service running?")
 | 
				
			||||||
        raise exception.NoValidHost(reason=msg)
 | 
					        raise exception.NoValidHost(reason=msg)
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -46,9 +46,6 @@ class FakeVolumeScheduler(driver.Scheduler):
 | 
				
			|||||||
        super(FakeVolumeScheduler, self).__init__()
 | 
					        super(FakeVolumeScheduler, self).__init__()
 | 
				
			||||||
        self.is_update_caps_called = False
 | 
					        self.is_update_caps_called = False
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    def schedule_create_volume(self, *args, **kwargs):
 | 
					 | 
				
			||||||
        pass
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
    def schedule_create_volumes(self, *args, **kwargs):
 | 
					    def schedule_create_volumes(self, *args, **kwargs):
 | 
				
			||||||
        pass
 | 
					        pass
 | 
				
			||||||
 | 
					
 | 
				
			||||||
@@ -97,7 +94,7 @@ class MultiDriverTestCase(test_scheduler.SchedulerTestCase):
 | 
				
			|||||||
 | 
					
 | 
				
			||||||
        #no compute methods are proxied at this time
 | 
					        #no compute methods are proxied at this time
 | 
				
			||||||
        test_methods = {compute_driver: [],
 | 
					        test_methods = {compute_driver: [],
 | 
				
			||||||
                        volume_driver: ['create_volume', 'create_volumes']}
 | 
					                        volume_driver: ['create_volumes']}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
        for driver, methods in test_methods.iteritems():
 | 
					        for driver, methods in test_methods.iteritems():
 | 
				
			||||||
            for method in methods:
 | 
					            for method in methods:
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -86,10 +86,16 @@ class SchedulerRpcAPITestCase(test.TestCase):
 | 
				
			|||||||
        self._test_scheduler_api('live_migration', rpc_method='call',
 | 
					        self._test_scheduler_api('live_migration', rpc_method='call',
 | 
				
			||||||
                block_migration='fake_block_migration',
 | 
					                block_migration='fake_block_migration',
 | 
				
			||||||
                disk_over_commit='fake_disk_over_commit',
 | 
					                disk_over_commit='fake_disk_over_commit',
 | 
				
			||||||
                instance='fake_instance', dest='fake_dest', topic='fake_topic',
 | 
					                instance='fake_instance', dest='fake_dest',
 | 
				
			||||||
                version='1.3')
 | 
					                version='1.7')
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    def test_update_service_capabilities(self):
 | 
					    def test_update_service_capabilities(self):
 | 
				
			||||||
        self._test_scheduler_api('update_service_capabilities',
 | 
					        self._test_scheduler_api('update_service_capabilities',
 | 
				
			||||||
                rpc_method='fanout_cast', service_name='fake_name',
 | 
					                rpc_method='fanout_cast', service_name='fake_name',
 | 
				
			||||||
                host='fake_host', capabilities='fake_capabilities')
 | 
					                host='fake_host', capabilities='fake_capabilities')
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    def test_create_volume(self):
 | 
				
			||||||
 | 
					        self._test_scheduler_api('create_volume',
 | 
				
			||||||
 | 
					                rpc_method='cast', volume_id="fake_volume",
 | 
				
			||||||
 | 
					                snapshot_id="fake_snapshots", reservations=list('fake_res'),
 | 
				
			||||||
 | 
					                version='1.7')
 | 
				
			||||||
 
 | 
				
			|||||||
		Reference in New Issue
	
	Block a user