From 1489b2d5fbc1e4ce3622fcff10efcee166394337 Mon Sep 17 00:00:00 2001 From: Thierry Parmentelat Date: Fri, 8 Nov 2013 16:53:00 +0100 Subject: [PATCH] =?utf8?q?the=20generic=20architecture=20had=20a=20serious?= =?utf8?q?=20flaw=20we=20used=20to=20have=20api=20->=20manager=20->=20driv?= =?utf8?q?er=20BUT=20in=20actuality=20api=20and=20driver=20are=20multiple?= =?utf8?q?=20(essentially=20one=20per=20request)=20while=20manager=20is=20?= =?utf8?q?unique=20in=20the=20process=E2=80=A6=20this=20was=20causing=20th?= =?utf8?q?e=20bug=20that=20Jordan=20reported=20with=20simultaneous=20api?= =?utf8?q?=20calls?= MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit From now on we only have api->manager api->driver --- sfa/generic/__init__.py | 8 ++++---- sfa/generic/architecture.txt | 3 +-- sfa/managers/registry_manager.py | 17 +++++++++-------- sfa/methods/Delete.py | 2 +- sfa/methods/Describe.py | 2 +- sfa/methods/Provision.py | 2 +- sfa/methods/Renew.py | 2 +- sfa/methods/Shutdown.py | 2 +- sfa/methods/Status.py | 2 +- sfa/server/sfaapi.py | 5 ++--- 10 files changed, 22 insertions(+), 23 deletions(-) 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', -- 2.47.0