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 2b067ff..ed2a8da 100644 (file)
--- 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 cb81c8b..fe793bf 100644 (file)
--- 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 f55e69b..958f243 100644 (file)
--- 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 e8c5128..94684b9 100644 (file)
--- 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 b66780a..8485f79 100644 (file)
--- 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 74ee350..a94c1bc 100644 (file)
--- 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 288e970..8f31786 100644 (file)
--- 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 8641bd0..3eee878 100644 (file)
--- 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 044e252..164093e 100644 (file)
--- 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 f1d96a0..9911e46 100644 (file)
--- 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',