[openstack-dev] [Keystone] Refactoring the routers, controllers and core for modules

Adam Young ayoung at redhat.com
Wed Dec 12 03:27:33 UTC 2012


Here is a large refactoring I have submitted for code review. 
https://review.openstack.org/#/c/17782/  This was kicked off by a 
problem I found while working on Trusts.

I think we are good on most things, but the sticking point is where to 
put the controllers, and how to name them.  We can see the problem if we 
limit talking about identity and token.  If we solve that, policy and 
catalog will fall into line.

So, if we move the token controllers into keystone/token/ and reference 
them in __init__.py  we get a circular dependency.  Why?


identity.*Controller needs token Manager and Driver.  so it imports
from keystone import token
from keystone.identity import core


token.*Controller needs identity Manager and Driver.
from keystone import identity
from keystone.token import core


so we have the following chain.

identity/controllers.py -> token/__init__.py -> token/controllers.py -> 
identity/__init__.py -> identity/controllers.py

They not only import each other, but they attempt to add the imported 
namespace into their GLobals collection.  So you get


identity.token.identity.token.identity.token.identity.token.identity.token.identity.token.identity.token.identity

Which manifests itself as "Cannot import token..."

The reason we haven't seen this so far is because identity and token 
were set up very differently.  Controllers for Token were in service.py.



Dolph's major objections, if I understand correctly, are best seen in

https://review.openstack.org/#/c/17782/7/keystone/contrib/ec2/core.py

fromkeystone.token importcontrollers astoken_controllers

Which gives us:
token_controller =token_controllers.TokenController()

We would both rather be able to do:

token_controller = token.controllers.Token()

But to enable that would require the __init_.py import that would then 
trigger the circular dependency.

Now, we could put the controllers in their own module.  I had thought 
about having one major package for routers, one for controllers, and one 
for the stuff in the core files.  I'd probably call that last "domain" 
or "data."  But I don't think it gains us much, and it is yet another 
parallel structure.


Here is the general scheme of the refactoring.

1.  Only ComposingRouters are in service.py.  These will only know about 
ComposableRouters.

(for the uninitiated, the ComposingRouters are designed to put in a 
bunch of ComposableRouters to build one big routing table per WSGI App.)


2.  ComposableRouters need to live in a namespace that identifies the 
domain that they solve; identity routers need to be in a namespace 
called identity and so forth.   I have them in individual files 
underneath keystone/routers.  So my import for service.py looks like this:

from keystone.routers import  identity
from keystone.routers import  policy
from keystone.routers import  catalog
from keystone.routers import  token

The code that creates the ComposableRouters only imports the controllers 
for a single module.  Thus the imports for routers/identity.py looks like

from keystone.common import wsgi
from keystone.identity import controllers

The controllers right now are in keystone/<namespace>/controllers.py but 
are not referenced by keystone/<namespace>/__init__.py


So the question is what owns the "good namespace"  for the module. When 
I import keystone/identity, should I get the Manager and Driver, or 
should I get the Controllers.  It can't be both.

I think the current setup is the cleanest separation. I was originally 
going to go the other way, with the controller owning the good 
namespace, but too many things reference the core Classes directly:

1.  Most controllers need to know about multiple core objects.
2.  All of the contrib code accesses the core objects directly
3.  Many of the unit tests import the core objects

It seems like the only thing that needs the controllers are the routers, 
and a little bit of contrib code.  We should probably be writing unit 
tests against the controllers.  We have them for service.py 
(TokenController) but most of the rest of the Controller code is 
exercised via more functional tests.So, to summarize, I propose:

1 service.py contains only ComposingRouters and only imports from 
keystone/routers
2 keystone.routers contains one file permodule, named after that module, 
that imports the controllers file from that module.
3.  Each module directory contains controllers.py which can point to any 
of the modules to reference the core classes.
4.  __init__.py for each module will only from <module>.core import *

Sorry so long.  I'm trying to lay out the whole process that lead to the 
decision.  Hopefully, this at least helps illuminate the Keystone code.




More information about the OpenStack-dev mailing list