From: Thierry Parmentelat Date: Fri, 8 Nov 2013 15:53:00 +0000 (+0100) Subject: the generic architecture had a serious flaw X-Git-Tag: sfa-3.1-1~42^2~13 X-Git-Url: http://git.onelab.eu/?a=commitdiff_plain;h=1489b2d5fbc1e4ce3622fcff10efcee166394337;p=sfa.git the generic architecture had a serious flaw we used to have api -> manager -> driver BUT in actuality api and driver are multiple (essentially one per request) while manager is unique in the process… this was causing the bug that Jordan reported with simultaneous api calls From now on we only have api->manager api->driver --- diff --git a/sfa/generic/__init__.py b/sfa/generic/__init__.py index 2b067ff9..ed2a8dad 100644 --- a/sfa/generic/__init__.py +++ b/sfa/generic/__init__.py @@ -72,10 +72,10 @@ class Generic: # add a manager wrapper manager_wrap = ManagerWrapper(manager_class_or_module,api.interface,api.config) api.manager=manager_wrap - # insert driver in manager - logger.debug("Setting manager.driver, manager=%s"%manager_class_or_module) - # xxx this should go into the object and not the class !?! - manager_class_or_module.driver=driver +# # insert driver in manager +# logger.debug("Setting manager.driver, manager=%s"%manager_class_or_module) +# # xxx this should go into the object and not the class !?! +# manager_class_or_module.driver=driver # add it in api as well for convenience api.driver=driver return api diff --git a/sfa/generic/architecture.txt b/sfa/generic/architecture.txt index cb81c8bd..fe793bff 100644 --- a/sfa/generic/architecture.txt +++ b/sfa/generic/architecture.txt @@ -20,8 +20,7 @@ configurable in a flavour (e.g. sfa.generic.pl.py) following layout: api.manager -manager.driver -api.driver (for convenience) +api.driver driver.api ------ diff --git a/sfa/managers/registry_manager.py b/sfa/managers/registry_manager.py index f55e69bd..958f243d 100644 --- a/sfa/managers/registry_manager.py +++ b/sfa/managers/registry_manager.py @@ -25,7 +25,8 @@ from sqlalchemy.orm.collections import InstrumentedList class RegistryManager: - def __init__ (self, config): pass + def __init__ (self, config): + logger.info("Creating RegistryManager[%s]"%id(self)) # The GENI GetVersion call def GetVersion(self, api, options): @@ -50,7 +51,7 @@ class RegistryManager: # Slivers don't have credentials but users should be able to # specify a sliver xrn and receive the slice's credential if type == 'sliver' or '-' in Xrn(hrn).leaf: - slice_xrn = self.driver.sliver_to_slice_xrn(hrn) + slice_xrn = api.driver.sliver_to_slice_xrn(hrn) hrn = slice_xrn.hrn # Is this a root or sub authority @@ -173,7 +174,7 @@ class RegistryManager: if details: # in details mode we get as much info as we can, which involves contacting the # testbed for getting implementation details about the record - self.driver.augment_records_with_testbed_info(local_dicts) + api.driver.augment_records_with_testbed_info(local_dicts) # also we fill the 'url' field for known authorities # used to be in the driver code, sounds like a poorman thing though def solve_neighbour_url (record): @@ -347,7 +348,7 @@ class RegistryManager: record.reg_keys = [ RegKey (key) for key in record.keys ] # update testbed-specific data if needed - pointer = self.driver.register (record.__dict__, hrn, pub_key) + pointer = api.driver.register (record.__dict__, hrn, pub_key) record.pointer=pointer dbsession.add(record) @@ -412,7 +413,7 @@ class RegistryManager: print "DO NOT REMOVE ME before driver.update, record=%s"%record new_key_pointer = -1 try: - (pointer, new_key_pointer) = self.driver.update (record.__dict__, new_record.__dict__, hrn, new_key) + (pointer, new_key_pointer) = api.driver.update (record.__dict__, new_record.__dict__, hrn, new_key) except: pass if new_key and new_key_pointer: @@ -459,7 +460,7 @@ class RegistryManager: # call testbed callback first # IIUC this is done on the local testbed TOO because of the refreshpeer link - if not self.driver.remove(record.__dict__): + if not api.driver.remove(record.__dict__): logger.warning("driver.remove failed") # delete from sfa db @@ -474,10 +475,10 @@ class RegistryManager: # verify that the callers's ip address exist in the db and is an interface # for a node in the db (ip, port) = api.remote_addr - interfaces = self.driver.shell.GetInterfaces({'ip': ip}, ['node_id']) + interfaces = api.driver.shell.GetInterfaces({'ip': ip}, ['node_id']) if not interfaces: raise NonExistingRecord("no such ip %(ip)s" % locals()) - nodes = self.driver.shell.GetNodes([interfaces[0]['node_id']], ['node_id', 'hostname']) + nodes = api.driver.shell.GetNodes([interfaces[0]['node_id']], ['node_id', 'hostname']) if not nodes: raise NonExistingRecord("no such node using ip %(ip)s" % locals()) node = nodes[0] diff --git a/sfa/methods/Delete.py b/sfa/methods/Delete.py index e8c5128f..94684b97 100644 --- a/sfa/methods/Delete.py +++ b/sfa/methods/Delete.py @@ -25,7 +25,7 @@ class Delete(Method): def call(self, xrns, creds, options): valid_creds = self.api.auth.checkCredentials(creds, 'deletesliver', xrns, - check_sliver_callback = self.api.manager.driver.check_sliver_credentials) + check_sliver_callback = self.api.driver.check_sliver_credentials) #log the call origin_hrn = Credential(cred=valid_creds[0]).get_gid_caller().get_hrn() diff --git a/sfa/methods/Describe.py b/sfa/methods/Describe.py index b66780a4..8485f796 100644 --- a/sfa/methods/Describe.py +++ b/sfa/methods/Describe.py @@ -38,7 +38,7 @@ class Describe(Method): raise SfaInvalidArgument('Must specify an rspec version option. geni_rspec_version cannot be null') valid_creds = self.api.auth.checkCredentials(creds, 'listnodes', urns, \ - check_sliver_callback = self.api.manager.driver.check_sliver_credentials) + check_sliver_callback = self.api.driver.check_sliver_credentials) # get hrn of the original caller origin_hrn = options.get('origin_hrn', None) diff --git a/sfa/methods/Provision.py b/sfa/methods/Provision.py index 74ee350e..a94c1bc3 100644 --- a/sfa/methods/Provision.py +++ b/sfa/methods/Provision.py @@ -33,7 +33,7 @@ class Provision(Method): # Find the valid credentials valid_creds = self.api.auth.checkCredentials(creds, 'createsliver', xrns, - check_sliver_callback = self.api.manager.driver.check_sliver_credentials) + check_sliver_callback = self.api.driver.check_sliver_credentials) origin_hrn = Credential(cred=valid_creds[0]).get_gid_caller().get_hrn() self.api.logger.info("interface: %s\tcaller-hrn: %s\ttarget-hrn: %s\tmethod-name: %s"%(self.api.interface, origin_hrn, xrns, self.name)) result = self.api.manager.Provision(self.api, xrns, creds, options) diff --git a/sfa/methods/Renew.py b/sfa/methods/Renew.py index 288e9700..8f31786b 100644 --- a/sfa/methods/Renew.py +++ b/sfa/methods/Renew.py @@ -34,7 +34,7 @@ class Renew(Method): # Find the valid credentials valid_creds = self.api.auth.checkCredentials(creds, 'renewsliver', urns, - check_sliver_callback = self.api.manager.driver.check_sliver_credentials) + check_sliver_callback = self.api.driver.check_sliver_credentials) # Validate that the time does not go beyond the credential's expiration time requested_time = utcparse(expiration_time) diff --git a/sfa/methods/Shutdown.py b/sfa/methods/Shutdown.py index 8641bd0e..3eee8785 100644 --- a/sfa/methods/Shutdown.py +++ b/sfa/methods/Shutdown.py @@ -20,7 +20,7 @@ class Shutdown(Method): def call(self, xrn, creds): valid_creds = self.api.auth.checkCredentials(creds, 'stopslice', xrn, - check_sliver_callback = self.api.manager.driver.check_sliver_credentials) + check_sliver_callback = self.api.driver.check_sliver_credentials) #log the call origin_hrn = Credential(cred=valid_creds[0]).get_gid_caller().get_hrn() self.api.logger.info("interface: %s\tcaller-hrn: %s\ttarget-hrn: %s\tmethod-name: %s"%(self.api.interface, origin_hrn, xrn, self.name)) diff --git a/sfa/methods/Status.py b/sfa/methods/Status.py index 044e2529..164093e7 100644 --- a/sfa/methods/Status.py +++ b/sfa/methods/Status.py @@ -20,7 +20,7 @@ class Status(Method): def call(self, xrns, creds, options): valid_creds = self.api.auth.checkCredentials(creds, 'sliverstatus', xrns, - check_sliver_callback = self.api.manager.driver.check_sliver_credentials) + check_sliver_callback = self.api.driver.check_sliver_credentials) self.api.logger.info("interface: %s\ttarget-hrn: %s\tmethod-name: %s"%(self.api.interface, xrns, self.name)) return self.api.manager.Status(self.api, xrns, creds, options) diff --git a/sfa/server/sfaapi.py b/sfa/server/sfaapi.py index f1d96a08..9911e466 100644 --- a/sfa/server/sfaapi.py +++ b/sfa/server/sfaapi.py @@ -18,7 +18,6 @@ from sfa.storage.alchemy import alchemy #################### class SfaApi (XmlrpcApi): - """ An SfaApi instance is a basic xmlrcp service augmented with the local cryptographic material and hrn @@ -32,8 +31,8 @@ class SfaApi (XmlrpcApi): It gets augmented by the generic layer with (*) an instance of manager (actually a manager module for now) - (*) which in turn holds an instance of a testbed driver - For convenience api.manager.driver == api.driver + beware that this is shared among all instances of api + (*) an instance of a testbed driver """ def __init__ (self, encoding="utf-8", methods='sfa.methods',