Move API V1 root controller logic from __init__ to the dedicated file

[Max Lobur]

1. Currently there is inconsistency in controller class location. F.e. this is the root controller class, placed in root.py :
https://github.com/openstack/ironic/blob/master/ironic/api/controllers/root.py

And this is the V1 root controller placed in __init__.py
https://github.com/openstack/ironic/blob/master/ironic/api/controllers/v1/__init__.py

Tests from V1 root placed in https://github.com/openstack/ironic/blob/master/ironic/tests/api/test_root.py

2. Root controller (api/controllers/root.py) inherits base from v1 (api/controllers/v1/base.py)

Need to make this stuff consistent, as an option move V1 to /v1/root.py, make v1 dir in tests and move V1 tests there and so on

[aeva black (tenbrae)]

You raise some good points.

ironic.api.controllers.root is the root '/' controller imported by Pecan. This must import and instantiate a controller for each version of API that we support, and expose them in the root.RootController class. However, it shouldn't depend on any content defined within any particular version for its basic functionality. For example, in ironic/api/controllers/root.py:

 26 from ironic.api.controllers.v1 import base
 27 from ironic.api.controllers.v1 import link
 ...
 30 class Version(base.APIBase):
 ...
 36 links = [link.Link]
 ...
 48 class Root(base.APIBase):

I think this is flawed. The top-level root controller should not depend on a class defined in the v1 api.

I think the tests in ironic.tests.api.test_root are fine -- these are ensuring that the RootController exposes the /v1/ API, which is distinct from all the test modules that run against the /v1/ API itself. That being said, I agree that several test modules should be moved from ironic/tests/api/ to ironic/tests/api/v1/

I see at least two patches needed to address these issues:
- one to just move the test classes to a 'v1/' directory
- another to move api.controllers.v1.base and ...v1.link out of v1

[Matt Wagner (matt-wagner)]

You read my mind! I'd picked this up expecting a fairly basic introductory task as I get up to speed on Ironic, but was running into some challenges making sense of the organization and resolving circular imports. Talking to Lucas today, we reached the same conclusion -- the root really shouldn't be pulling in classes from v1/.

I'm going to carry on with the separation of controllers. Please let me know if my doing this this while learning is starting to delay implementation, but for now I see it's marked as low-priority and will carry on, hopefully with a patch soon.

[Max Lobur (max-lobur)]

Hi Deva and Matt,

I agree with both of you. Thank you for your time!

[aeva black (tenbrae)]

Hi Matt, any movement on this? If you're not going to pick it up after the holidays, can you unassign yourself? Thanks! -Deva

[Matt Wagner (matt-wagner)]

Sorry about that. I'd intended to revisit this, but obviously haven't done so yet. Since I've been sitting on it for a while now, I've just unassigned it.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/78912

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/78918

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/78918
Committed: https://git.openstack.org/cgit/openstack/ironic/commit/?id=0c11c3a5bcd1bb8611d34657988dc2c54a8fde03
Submitter: Jenkins
Branch: master

commit 0c11c3a5bcd1bb8611d34657988dc2c54a8fde03
Author: Ramakrishnan G <email address hidden>
Date: Fri Mar 7 02:24:28 2014 +0530

    Move v1 API tests to separate v1 directory

    This commit adds a new v1 directory within tests/api
    and moves the tests related to v1 API inside it.

    Change-Id: Ia4cf177a6aed676a42ab9bd4e4420c290df546c9
    Partial-Bug: #1255954

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/78912
Committed: https://git.openstack.org/cgit/openstack/ironic/commit/?id=a62fe2b833129a5818c3a083eb2034443b003ee4
Submitter: Jenkins
Branch: master

commit a62fe2b833129a5818c3a083eb2034443b003ee4
Author: Ramakrishnan G <email address hidden>
Date: Fri Mar 7 01:30:11 2014 +0530

    Avoid API root controller dependency on v1 dir

    This commit fixes the issue that pecan root controller
    is dependent on files in v1 directory.

    Change-Id: Id26db27b61094356ee02dbc7c5baa7e551a3cc5c
    Partial-Bug: #1255954