Merge lp:~salvatore-orlando/neutron/quantum-api-alignment into lp:neutron/diablo

Proposed by Salvatore Orlando
Status: Merged
Approved by: Salvatore Orlando
Approved revision: 48
Merged at revision: 51
Proposed branch: lp:~salvatore-orlando/neutron/quantum-api-alignment
Merge into: lp:neutron/diablo
Diff against target: 1449 lines (+501/-259)
21 files modified
etc/quantum.conf (+1/-1)
quantum/api/__init__.py (+9/-3)
quantum/api/api_common.py (+9/-18)
quantum/api/attachments.py (+86/-0)
quantum/api/faults.py (+0/-17)
quantum/api/networks.py (+34/-43)
quantum/api/ports.py (+63/-84)
quantum/api/versions.py (+2/-2)
quantum/api/views/attachments.py (+37/-0)
quantum/api/views/networks.py (+9/-10)
quantum/api/views/ports.py (+6/-14)
quantum/cli.py (+11/-10)
quantum/client.py (+2/-5)
quantum/common/exceptions.py (+0/-10)
quantum/common/wsgi.py (+10/-4)
quantum/db/api.py (+8/-1)
quantum/db/models.py (+2/-1)
quantum/plugins/SamplePlugin.py (+2/-2)
tests/unit/test_api.py (+151/-15)
tests/unit/test_clientlib.py (+1/-1)
tests/unit/testlib_api.py (+58/-18)
To merge this branch: bzr merge lp:~salvatore-orlando/neutron/quantum-api-alignment
Reviewer Review Type Date Requested Status
dan wendlandt Approve
Somik Behera netstack-core Approve
Tyler Smith Approve
Review via email: mp+70788@code.launchpad.net

Description of the change

This is the branch that updates the API code in order to make it 100% compliant with the API specification.

This branch DOES NOT include the status proposal, but includes all feedback received by the community.

API Unit tests have been adapted were necessary(e.g.: attribute names changed)
Client Library Unit tests pass as well.

To post a comment you must log in.
Revision history for this message
dan wendlandt (danwent) wrote :

Hi salvatore, haven't run it yet and only skimmed the test code, but looks good overall.

I do think though that we need to get to the bottom of the "state + status" discussion before pushing this branch, as even this branch had some things that surprised me, particularly that one could not make an attachment to a port in the down state. This seems at odds to my understanding from the email thread that "state" was really akin to "admin state" on a traditional switch. When a switch is "admin down", my understanding is that the common behavior one can still configure the port, its just that no packets will pass.

Also, the spec still seems somewhat unclear on the issue of port "state". For example, the following section, as I interpret it, seems to imply that the state is either "down" or "active" based on whether a VIF is plugged in (i.e., something similar to the operational "link status" we discussed in the last email):

"The VIF will be connected to the network via a logical port. The logical port begins in a “down” state. Logic ports are transient when in the “down” state, by this we mean they don’t have any binding to a physical implementation. The plugin must create this binding as a part of the attachment process. When a VIF is attached to a logical port the plugin creates the binding to the physical hardware required to allow operation of the VIF. This is indicated by the change in port state from “down” to “active”. As a result of the attachment operation, quantum can now knows the capabilities of the VIF as dictated by the physical binding. As a result the logical port may advertise capabilities once it moves to the “active” state."

If you'd prefer to push this code first to avoid merge conflicts and then continue having the discussion that's fine with me, but I want to make sure we clear up this point before finalizing 1.0 (An alternative would be to drop the state/status notion all together for v1.0, and revisit it for a later version).

Revision history for this message
Salvatore Orlando (salvatore-orlando) wrote :

> Hi salvatore, haven't run it yet and only skimmed the test code, but looks
> good overall.
>
> I do think though that we need to get to the bottom of the "state + status"
> discussion before pushing this branch, as even this branch had some things
> that surprised me, particularly that one could not make an attachment to a
> port in the down state. This seems at odds to my understanding from the email
> thread that "state" was really akin to "admin state" on a traditional switch.
> When a switch is "admin down", my understanding is that the common behavior
> one can still configure the port, its just that no packets will pass.
>
> Also, the spec still seems somewhat unclear on the issue of port "state". For
> example, the following section, as I interpret it, seems to imply that the
> state is either "down" or "active" based on whether a VIF is plugged in (i.e.,
> something similar to the operational "link status" we discussed in the last
> email):
>
> "The VIF will be connected to the network via a logical port. The logical port
> begins in a “down” state. Logic ports are transient when in the “down” state,
> by this we mean they don’t have any binding to a physical implementation. The
> plugin must create this binding as a part of the attachment process. When a
> VIF is attached to a logical port the plugin creates the binding to the
> physical hardware required to allow operation of the VIF. This is indicated by
> the change in port state from “down” to “active”. As a result of the
> attachment operation, quantum can now knows the capabilities of the VIF as
> dictated by the physical binding. As a result the logical port may advertise
> capabilities once it moves to the “active” state."
>
> If you'd prefer to push this code first to avoid merge conflicts and then
> continue having the discussion that's fine with me, but I want to make sure we
> clear up this point before finalizing 1.0 (An alternative would be to drop the
> state/status notion all together for v1.0, and revisit it for a later
> version).

I think there are enough comments and questions to drop everything related to "resource state" in any form.
It looks like we all feel the need for some sort of notion of "state", but we are coming at it from rather different angles.
I therefore think it would be better to remove any state concept, including the administrative state for the port, and then resume the discussion post Diablo-4, finalize it at the summit in October, and include the state notion in API v1.1

Revision history for this message
Salvatore Orlando (salvatore-orlando) wrote :

Reverting to WIP to remove all references to resource state.

Revision history for this message
dan wendlandt (danwent) wrote :

One other question for you: as part of nova + quantum integration, I'm prototyping the "admin API" interfaces to communicate information like interface-ownership (for authenticating attachments) and interface-bindings (for knowing where a vif was deployed).

My gut says that we should not hold up the 1.0 version while waiting to include these items, but I wanted to get your thoughts there. We definitely will need to document these eventually though, as there will be other openstack services that want to integrate with Quantum.

review: Needs Information
Revision history for this message
Salvatore Orlando (salvatore-orlando) wrote :

> One other question for you: as part of nova + quantum integration, I'm
> prototyping the "admin API" interfaces to communicate information like
> interface-ownership (for authenticating attachments) and interface-bindings
> (for knowing where a vif was deployed).
>
> My gut says that we should not hold up the 1.0 version while waiting to
> include these items, but I wanted to get your thoughts there. We definitely
> will need to document these eventually though, as there will be other
> openstack services that want to integrate with Quantum.

Good point. I guess the admin API has a different endpoint, as it should be exposed by nova, is that correct?
In that case I'm not entirely sure this should be part of the Quantum API at all (for instance if it going to be an Openstack API extension, why should it be documented in Quantum API). I can't find a blueprint for tracking this piece of work, can you give me a pointer?

My plan to lock down the API for this week is motivated by the fact that we have to lock down features by Aug 25, and then do bug fixing and system testing until diablo release. If you think that we only have two weeks left, and given that code review process usually takes a week, this means that the best way to ensure a timely completion for the API blueprint is to start locking down the spec this week, and then work the week after on the code.

Revision history for this message
dan wendlandt (danwent) wrote :
Download full text (3.2 KiB)

I think this is the right call. We don't necessarily have to wait that long
(or even to stop talking about it at all, but I agree with getting v1.0 out
ASAP).

dan

On Mon, Aug 8, 2011 at 2:47 PM, Salvatore Orlando <
<email address hidden>> wrote:

> > Hi salvatore, haven't run it yet and only skimmed the test code, but
> looks
> > good overall.
> >
> > I do think though that we need to get to the bottom of the "state +
> status"
> > discussion before pushing this branch, as even this branch had some
> things
> > that surprised me, particularly that one could not make an attachment to
> a
> > port in the down state. This seems at odds to my understanding from the
> email
> > thread that "state" was really akin to "admin state" on a traditional
> switch.
> > When a switch is "admin down", my understanding is that the common
> behavior
> > one can still configure the port, its just that no packets will pass.
> >
> > Also, the spec still seems somewhat unclear on the issue of port "state".
> For
> > example, the following section, as I interpret it, seems to imply that
> the
> > state is either "down" or "active" based on whether a VIF is plugged in
> (i.e.,
> > something similar to the operational "link status" we discussed in the
> last
> > email):
> >
> > "The VIF will be connected to the network via a logical port. The logical
> port
> > begins in a “down” state. Logic ports are transient when in the “down”
> state,
> > by this we mean they don’t have any binding to a physical implementation.
> The
> > plugin must create this binding as a part of the attachment process. When
> a
> > VIF is attached to a logical port the plugin creates the binding to the
> > physical hardware required to allow operation of the VIF. This is
> indicated by
> > the change in port state from “down” to “active”. As a result of the
> > attachment operation, quantum can now knows the capabilities of the VIF
> as
> > dictated by the physical binding. As a result the logical port may
> advertise
> > capabilities once it moves to the “active” state."
> >
> > If you'd prefer to push this code first to avoid merge conflicts and then
> > continue having the discussion that's fine with me, but I want to make
> sure we
> > clear up this point before finalizing 1.0 (An alternative would be to
> drop the
> > state/status notion all together for v1.0, and revisit it for a later
> > version).
>
> I think there are enough comments and questions to drop everything related
> to "resource state" in any form.
> It looks like we all feel the need for some sort of notion of "state", but
> we are coming at it from rather different angles.
> I therefore think it would be better to remove any state concept, including
> the administrative state for the port, and then resume the discussion post
> Diablo-4, finalize it at the summit in October, and include the state notion
> in API v1.1
> --
>
> https://code.launchpad.net/~salvatore-orlando/quantum/quantum-api-alignment/+merge/70788
> You are requested to review the proposed merge of
> lp:~salvatore-orlando/quantum/quantum-api-alignment into lp:quantum.
>

--
~~~~~~~~~~~~~~~~~~~~~~~~~~~
Dan Wendlandt
Nicira Networks, Inc.
www...

Read more...

Revision history for this message
dan wendlandt (danwent) wrote :
Download full text (4.0 KiB)

On Mon, Aug 8, 2011 at 3:13 PM, Salvatore Orlando <
<email address hidden>> wrote:

> > One other question for you: as part of nova + quantum integration, I'm
> > prototyping the "admin API" interfaces to communicate information like
> > interface-ownership (for authenticating attachments) and
> interface-bindings
> > (for knowing where a vif was deployed).
> >
> > My gut says that we should not hold up the 1.0 version while waiting to
> > include these items, but I wanted to get your thoughts there. We
> definitely
> > will need to document these eventually though, as there will be other
> > openstack services that want to integrate with Quantum.
>
> Good point. I guess the admin API has a different endpoint, as it should be
> exposed by nova, is that correct?
>

I think one could think of doing it in two ways:

1) exposing an endpoint on nova, with quantum as the client.
2) exposing an endpoints on quantum, with nova as the client.

My mental model had always been #2, but I'm not wedded to it.

#2 seemed natural to me as the "interface service" (e.g., nova) seemed to
need to be able to "push" an interface binding to quantum (e.g., in the case
where a VM migrates, changing the binding). Also, requring "interface
service" to make a JSON call (ideally with the help of the client lib)
seemed easier than requiring each interface service to expose and
authenticate a new API call. This would be particularly true if an interface
service was already acting as a quantum client for other reasons, which is
the case with nova.

One case where #2 is a bit funky is if quantum is deployed after an
interface service like nova is already running (and thus has already spun up
a set of VMs). It would seem that nova would need to support a method where
it "resends" all of its data. Seems possible, but kind of clunky.

Do you have a strong feeling on this one?

> In that case I'm not entirely sure this should be part of the Quantum API
> at all (for instance if it going to be an Openstack API extension, why
> should it be documented in Quantum API). I can't find a blueprint for
> tracking this piece of work, can you give me a pointer?
>

The original blueprint for this work is:
https://blueprints.launchpad.net/quantum/+spec/quantum-interface-binding-api

There's not a whole lot there. I'm hoping to do this as part of the Quantum
Manager work in nova:
https://blueprints.launchpad.net/nova/+spec/implement-network-api

Just last night I started stubbing out out a possible Controller for this
admin API. I've decided my first cut wasn't how I wanted it to work, so I'm
going to tweak it (hopefully tonight), then I will push the branch and see
what feedback people have. Main goals are:

1) transmit "vif ownership" info to quantum, so we can tell what tenant owns
a vif (and thus whether they are allowed to plug it in to one of their
ports).
2) for plugins that need it, have a channel to report interface bindings.

#2 is somewhat trickier (not clear if it should live in the network-manager,
in the vif-plug driver, somewhere else), but #1 seems pretty
straightforward.

>
> My plan to lock down the API for this week is motivated by the fact that we
> ha...

Read more...

Revision history for this message
Salvatore Orlando (salvatore-orlando) wrote :

Re-proposing for merge.
Community feedback has been addressed.

44. By Salvatore Orlando

Removing unused error response codes

Revision history for this message
Tyler Smith (tylesmit) wrote :

A conflict file accidentally got added:
quantum/tests/unit/test_clientlib.py.THIS

Other than that, LGTM.

review: Needs Fixing
45. By Salvatore Orlando

Removing "excess" file

46. By Salvatore Orlando

Merge trunk
Solving conflict in common/exceptions.py
Removing unused error code from client.py

Revision history for this message
Tyler Smith (tylesmit) wrote :

Great work getting a coherent API spec implemented; looks good.

review: Approve
Revision history for this message
Somik Behera (somikbehera) wrote :
Download full text (3.4 KiB)

Hi Salvatore,

This is a good step in the right direction, towards 1.0!

A reviewed, this changeset, the API wiki, quantum_plugin_base that species the data structure that plugins are mandated to use while passing data to the API layer to transform into XML/JSON for external world to consume.

I see a few shortcomings:

1) quantum_plugin_base.py specifies the "interface" for plugin developers, so that plugin developers can follow this file and implement their plugins without looking at any other place. Along those lines, this "interface" file also specifies the data structure/ "format" that plugins are supposed to use while returning data to the API layer.

Currently, the API layer's expectation regarding data "format" are not aligned with what quantum_plugin_base specifies. Therefore, one or other, or both might need fixing.

2) I noticed we upped the version to /v1.0 and I just wanted ask you if we have appropriate bugs on client-side, if any, already filed to ensure that the client is compliant.

3) One thing I have noticed that is not very consistent, within the API, Unit test and documentation implementation is the data format of for the "attachment"

3.1) This is what plugin_base specifies( API can provide a different format to Northbound API client while keeping the specified format on southbound side plugin side):

    @abstractmethod
    def get_port_details(self, tenant_id, net_id, port_id):
        """
        This method allows the user to retrieve a remote interface
        that is attached to this particular port.

        :returns: a mapping sequence with the following signature:
                    {'port-id': uuid representing the port on
                                 specified quantum network
                     'net-id': uuid representing the particular
                                quantum network
                     'attachment': uuid of the virtual interface
                                   bound to the port, None otherwise
                    }

3.2) Here's the attachment format for Northbound API from wiki:

{
 "attachment":
     {
         "id": "test_interface_identifier"
     }
}

3.3) file quantum/api/views/attachments.py looks for "attachment-id" when looking for the identifier representing the attachment, which is inconsistent on the southbound(quantum_plugin_base) and northbound( API wiki) side.

Looking at file history, it looks like its being changed from attachment -> attachment-id

I would prefer that the southbound side( quantum_plugin_base specification) should be left as attachment if there is not good reason to change the naming as it will break compatibility with all the plugins that have been written, and won't provide any enhanced functionality. We can definitely change things northbound as thats our public API.

3.4) Similarly, net-name was changed to "name" on northbound and southbound side, I have similar feelings that we keep the the southbound side( data format specified in quantum_plugin_base) be kept same while changing northbound. Same is try for port-state -> "state" change.

4) Based on final conclusions, we will need to update unit tests and quantum_plugin_base.py doc as well. I am...

Read more...

review: Needs Fixing (netstack-core)
Revision history for this message
Salvatore Orlando (salvatore-orlando) wrote :
Download full text (6.1 KiB)

Hi Somik,

thanks for your review.

I understand your concerns about different the different data structures in northbound/southbound interfaces. After all, those are exactly the concerns I had when we discussed a uniform data model for the API and the plugin interface a few weeks ago, and I thought we concluded that the API and the plugin interface represented two distinct interfaces.

If we decide to unify these interface, I'd like to use the current API spec as a guideline for changing the plugin interface and not vice-versa, as I would like to keep Quantum API as close as possible the ones exposed by Openstack and other services in the ecosystem. However, this is not a "conditio sine qua non" for me.

You did pretty well in summarizing the differences between northbound and southbound interfaces, I would also add the following:
1) net-id --> id
2) port-id --> id
3) port-state --> state
But I have to agree the attachment-id thing you noticed in your comment #2 is actually a bug, which is due to the fact that the FakePlugin does not implement very well the plugin interface. API code, FakePlugin, and unit tests will be fixed accordingly.

If we want the northbound and southbound interface to have the same shape, then having two interfaces (API and quantum_plugin_base) is not a good idea. We can just decide that the plugin interface is "THE" interface, and the only responsibility of the API layer is converting data structures into the appropriate format; or we can do the opposite. Either way, plugins or clients will be affected.

But, at this stage, I don't think we really need to unify these interfaces. Plugin implementors will talk the language of the plugin interface, while API users will talk the language of the API interface. The API layer will "translate" between these two languages (after all it is not a big effort, as it can be seen from the view builders, which are very small modules). Another advantage of separating interfaces is that the API can evolve indepedently of the plugin interface, and vice-versa; for instance, in the future, object ownership information, fetched from the identity service, could be added to API data structures without interfering at all with the plugin interface.

Please let me know what do you think of my proposal.
I'm totally open to unify API and quantum_plugin_base; my proposal of keeping the interfaces separate arises from the considerations reported above.

Thanks again,
Salvatore

> Hi Salvatore,
>
> This is a good step in the right direction, towards 1.0!
>
> A reviewed, this changeset, the API wiki, quantum_plugin_base that species the
> data structure that plugins are mandated to use while passing data to the API
> layer to transform into XML/JSON for external world to consume.
>
> I see a few shortcomings:
>
> 1) quantum_plugin_base.py specifies the "interface" for plugin developers, so
> that plugin developers can follow this file and implement their plugins
> without looking at any other place. Along those lines, this "interface" file
> also specifies the data structure/ "format" that plugins are supposed to use
> while returning data to the API layer.
>
> Currently, the API layer's...

Read more...

Revision history for this message
Somik Behera (somikbehera) wrote :
Download full text (3.6 KiB)

Hi Salvatore,

Overall, I am in line with your thinking and I think thats a good way to
proceed.

Rest inline.

On Mon, Aug 22, 2011 at 12:43 PM, Salvatore Orlando <
<email address hidden>> wrote:

> Hi Somik,
>
> thanks for your review.
>
> I understand your concerns about different the different data structures in
> northbound/southbound interfaces. After all, those are exactly the concerns
> I had when we discussed a uniform data model for the API and the plugin
> interface a few weeks ago, and I thought we concluded that the API and the
> plugin interface represented two distinct interfaces.
>
> If we decide to unify these interface, I'd like to use the current API spec
> as a guideline for changing the plugin interface and not vice-versa, as I
> would like to keep Quantum API as close as possible the ones exposed by
> Openstack and other services in the ecosystem. However, this is not a
> "conditio sine qua non" for me.
>
> You did pretty well in summarizing the differences between northbound and
> southbound interfaces, I would also add the following:
> 1) net-id --> id
> 2) port-id --> id
> 3) port-state --> state
> But I have to agree the attachment-id thing you noticed in your comment #2
> is actually a bug, which is due to the fact that the FakePlugin does not
> implement very well the plugin interface. API code, FakePlugin, and unit
> tests will be fixed accordingly.

> If we want the northbound and southbound interface to have the same shape,
> then having two interfaces (API and quantum_plugin_base) is not a good idea.
> We can just decide that the plugin interface is "THE" interface, and the
> only responsibility of the API layer is converting data structures into the
> appropriate format; or we can do the opposite. Either way, plugins or
> clients will be affected.
>
>
[SB] I agree that API layer can convert to the quantum_plugin_base -> API
format. Since this change is an API change, we can't really prevent API
clients from getting affected. But, if we don't have pressing need to change
quantum_plugin_base then the atleast plugins wont be affected, which would
be good for this late stage change.

> But, at this stage, I don't think we really need to unify these
> interfaces. Plugin implementors will talk the language of the plugin
> interface, while API users will talk the language of the API interface. The
> API layer will "translate" between these two languages (after all it is not
> a big effort, as it can be seen from the view builders, which are very small
> modules). Another advantage of separating interfaces is that the API can
> evolve indepedently of the plugin interface, and vice-versa; for instance,
> in the future, object ownership information, fetched from the identity
> service, could be added to API data structures without interfering at all
> with the plugin interface.
>
>
[SB] I totally agree with this line of thinking and this would be a great
way to proceed. This approach causes least disruption and provides greatest
flexibility.

> Please let me know what do you think of my proposal.
> I'm totally open to unify API and quantum_plugin_base; my proposal of
> keeping the interfaces separate arise...

Read more...

47. By Salvatore Orlando

Fixing issue in view builders concerning attachment identifiers

Revision history for this message
Salvatore Orlando (salvatore-orlando) wrote :

Hi Somik,

thanks for your reply.
It seems we have now an agreement and can finally close down API v1 for Quantum.
I've updated the branch for taking into account the issue with the 'attachment-id' identifier you spotted.

However, prompted by your review, I double-checked consistency with specification in quantum-plugin-base, and I spotted a problem with get_network_details.
Quantum_plugin_base states that id should return a sequence whose elements have the following attributes: 'net-id', 'net-name', 'net-ifaces', with the latter being a list of interface identifiers.

However:
- the API does not look for 'net-ifaces' at all. It instead invoke get_all_ports and get_port_details for each port; the API therefore works, but not in the most efficient way. This could be fixed quite easily;
- however the plugins (FakePlugin, Cisco, and even Openvswitch), return a sequence
whose elements have the following attributes: 'net-id', 'net-name', 'net-ports' (instead of 'net-ifaces').

I think we probably want to sort this out. We can either change plugin interface documentation, or the plugins. Whatever we decide, we should also update the API to ensure it works in an efficient way.

What's the best approach to tackle this problem in your opinion?

Cheers,
Salvatore

48. By Salvatore Orlando

Merge trunk

Revision history for this message
Somik Behera (somikbehera) wrote :

> Hi Somik,
>
> thanks for your reply.
> It seems we have now an agreement and can finally close down API v1 for
> Quantum.
> I've updated the branch for taking into account the issue with the
> 'attachment-id' identifier you spotted.
>
> However, prompted by your review, I double-checked consistency with
> specification in quantum-plugin-base, and I spotted a problem with
> get_network_details.
> Quantum_plugin_base states that id should return a sequence whose elements
> have the following attributes: 'net-id', 'net-name', 'net-ifaces', with the
> latter being a list of interface identifiers.
>
> However:
> - the API does not look for 'net-ifaces' at all. It instead invoke
> get_all_ports and get_port_details for each port; the API therefore works, but
> not in the most efficient way. This could be fixed quite easily;
> - however the plugins (FakePlugin, Cisco, and even Openvswitch), return a
> sequence
> whose elements have the following attributes: 'net-id', 'net-name', 'net-
> ports' (instead of 'net-ifaces').
>

Good find Salvatore, this is an interesting issue where the plugin interface, API interface and the plugin implementation all diverge.

This is what I suggest, we should keep the API working efficiently, having net-ifaces, doesn't add overhead and makes the API efficient. With that in mind, this seems to be a good thing to keep in quantum_plugin_base interface.

For the other issues, I believe we should file bugs, and once the plugins have fixed themselves, the API can then take advantage of the more performant mechanism.

Iterating over all ports, while ok in short term, will be an issue in large scale environments.

> I think we probably want to sort this out. We can either change plugin
> interface documentation, or the plugins. Whatever we decide, we should also
> update the API to ensure it works in an efficient way.
>
> What's the best approach to tackle this problem in your opinion?
>
> Cheers,
> Salvatore

Everything else looks good!

But, I noticed, the somehow some pep8 violations have creeped into the trunk, I'll be filing those bugs soon.

review: Approve (netstack-core)
Revision history for this message
Salvatore Orlando (salvatore-orlando) wrote :

> > Hi Somik,
> >
> > thanks for your reply.
> > It seems we have now an agreement and can finally close down API v1 for
> > Quantum.
> > I've updated the branch for taking into account the issue with the
> > 'attachment-id' identifier you spotted.
> >
> > However, prompted by your review, I double-checked consistency with
> > specification in quantum-plugin-base, and I spotted a problem with
> > get_network_details.
> > Quantum_plugin_base states that id should return a sequence whose elements
> > have the following attributes: 'net-id', 'net-name', 'net-ifaces', with the
> > latter being a list of interface identifiers.
> >
> > However:
> > - the API does not look for 'net-ifaces' at all. It instead invoke
> > get_all_ports and get_port_details for each port; the API therefore works,
> but
> > not in the most efficient way. This could be fixed quite easily;
> > - however the plugins (FakePlugin, Cisco, and even Openvswitch), return a
> > sequence
> > whose elements have the following attributes: 'net-id', 'net-name', 'net-
> > ports' (instead of 'net-ifaces').
> >
>
> Good find Salvatore, this is an interesting issue where the plugin interface,
> API interface and the plugin implementation all diverge.
>
> This is what I suggest, we should keep the API working efficiently, having
> net-ifaces, doesn't add overhead and makes the API efficient. With that in
> mind, this seems to be a good thing to keep in quantum_plugin_base interface.
>

Definetely agree.

> For the other issues, I believe we should file bugs, and once the plugins have
> fixed themselves, the API can then take advantage of the more performant
> mechanism.
>
> Iterating over all ports, while ok in short term, will be an issue in large
> scale environments.

It is not efficient at all, as it will force clients to do a lot of largely unnecessary round trips.
I will file one or more bugs to improve API and client library accordingly (for instance the client library does not currently leverage the 'detail' actions)

>
> > I think we probably want to sort this out. We can either change plugin
> > interface documentation, or the plugins. Whatever we decide, we should also
> > update the API to ensure it works in an efficient way.
> >
> > What's the best approach to tackle this problem in your opinion?
> >
> > Cheers,
> > Salvatore
>
> Everything else looks good!
>

Thanks!

> But, I noticed, the somehow some pep8 violations have creeped into the trunk,
> I'll be filing those bugs soon.

Good thing you spotted this. I do pep8 tests executing run_tests.sh, which does not check .py files in root directory!

Revision history for this message
dan wendlandt (danwent) wrote :

Hi salvatore, thanks for checking with me about this branch. My main concern before was just regarding the issue of state/status, which we've cleared up. Below are some minor comments, but overall things look great and I am switching my review to Approve.

quantum/api/api_common.py

I have a general question about the API validation performed here, based on a problem I saw a while back. The code uses appears to use syntax like "if not param_value and param['required']:" to determine if a parameter exists. This can be dangerous, as the above expression will evaluate to false if the parameter value is zero, or an empty dictionary, a boolean false, etc. which may in fact be valid values. It seems like really the code should either be checking to see if the key exists in the params dictionary, or use an explicit check for None (e.g., if param_value is None).

quantum/api/faults.py

Is this still left in as a TODO?

+ #MUST RETURN 202???

quantum/db/api.py,

definitely feel free to remove the check_duplicate_net_name method, as your comment suggests.

quantum/tools/batch_config.py

Great that you updated the cli.py and tests. Could you also update batch_config.py? Looks like the name change from 'net-name' at least is an issue, though perhaps there are others.

danwent@ubuntu:~/quantum-api-alignment$ PYTHONPATH=. python ./tools/batch_config.py foo net1=foo:net2=bar
nets: {'net2': ['bar'], 'net1': ['foo']}
Traceback (most recent call last):
  File "./tools/batch_config.py", line 111, in <module>
    create_net_with_attachments(client, net_name, iface_ids)
  File "./tools/batch_config.py", line 46, in create_net_with_attachments
    res = client.create_network(data)
  File "/home/danwent/quantum-api-alignment/quantum/client.py", line 53, in with_params
    ret = self.function(instance, *args)
  File "/home/danwent/quantum-api-alignment/quantum/client.py", line 240, in create_network
    return self.do_request("POST", self.networks_path, body=body)
  File "/home/danwent/quantum-api-alignment/quantum/client.py", line 174, in do_request
    raise EXCEPTIONS[res.status]()
quantum.common.exceptions.BadInputError

review: Approve
Revision history for this message
Salvatore Orlando (salvatore-orlando) wrote :
Download full text (3.5 KiB)

> Hi salvatore, thanks for checking with me about this branch. My main concern
> before was just regarding the issue of state/status, which we've cleared up.
> Below are some minor comments, but overall things look great and I am
> switching my review to Approve.
>
> quantum/api/api_common.py
>
> I have a general question about the API validation performed here, based on a
> problem I saw a while back. The code uses appears to use syntax like "if not
> param_value and param['required']:" to determine if a parameter exists. This
> can be dangerous, as the above expression will evaluate to false if the
> parameter value is zero, or an empty dictionary, a boolean false, etc. which
> may in fact be valid values. It seems like really the code should either be
> checking to see if the key exists in the params dictionary, or use an explicit
> check for None (e.g., if param_value is None).

This is a widely used 'pattern' in python programming. It makes me feel a Pythonista although the C++/C#/Java guy which is in me would probably write:

param_value is None and param['required']

possibly adding a bunch of parenthesis as well :)
On this specific issue, given the way our deserializer currently works, param_value can only be a string. As the deserializer can change in the future, or there might be cases in which an empty string is a valid parameter value, it makes sense to file a bug.

>
> quantum/api/faults.py
>
> Is this still left in as a TODO?
>
> + #MUST RETURN 202???

I think we discussed in an email thread or earlier meeting this issue. From the spec the API should return 202, but the implementation returns 200. It turns out the Openstack API has this problem as well. We 'inherited' it by grabbing their wsgi framework. I was planning to file a bug and have this problem resolved before integration freeze. It is not trivial and I did not want to halt progress on the API work for this issue.

>
> quantum/db/api.py,
>
> definitely feel free to remove the check_duplicate_net_name method, as your
> comment suggests.

I'll file a bug for this; low priority should be enough for this work item.

>
> quantum/tools/batch_config.py
>
> Great that you updated the cli.py and tests. Could you also update
> batch_config.py? Looks like the name change from 'net-name' at least is an
> issue, though perhaps there are others.
>

I've updated them, but the tests are not yet there. I have a rather large merge coming through today with tests and some CLI changes (templated output and removal of the direct plugin mode).

I hope to find some time to fix batch_config as well, but I also need to fix XML deserialization on the client side, so I might run short on time!

>
> danwent@ubuntu:~/quantum-api-alignment$ PYTHONPATH=. python
> ./tools/batch_config.py foo net1=foo:net2=bar
> nets: {'net2': ['bar'], 'net1': ['foo']}
> Traceback (most recent call last):
> File "./tools/batch_config.py", line 111, in <module>
> create_net_with_attachments(client, net_name, iface_ids)
> File "./tools/batch_config.py", line 46, in create_net_with_attachments
> res = client.create_network(data)
> File "/home/danwent/quantum-api-alignment/quantum/...

Read more...

Revision history for this message
dan wendlandt (danwent) wrote :
Download full text (4.9 KiB)

On Thu, Aug 25, 2011 at 2:38 AM, Salvatore Orlando <
<email address hidden>> wrote:

> > Hi salvatore, thanks for checking with me about this branch. My main
> concern
> > before was just regarding the issue of state/status, which we've cleared
> up.
> > Below are some minor comments, but overall things look great and I am
> > switching my review to Approve.
> >
> > quantum/api/api_common.py
> >
> > I have a general question about the API validation performed here, based
> on a
> > problem I saw a while back. The code uses appears to use syntax like "if
> not
> > param_value and param['required']:" to determine if a parameter exists.
> This
> > can be dangerous, as the above expression will evaluate to false if the
> > parameter value is zero, or an empty dictionary, a boolean false, etc.
> which
> > may in fact be valid values. It seems like really the code should either
> be
> > checking to see if the key exists in the params dictionary, or use an
> explicit
> > check for None (e.g., if param_value is None).
>
> This is a widely used 'pattern' in python programming.

Hmm... the python experts I know (I am certainly not one myself) all seem to
recommend against using this approach to check for None, exactly due to the
ambiguity we're talking about. PEP-8 states:

'Also, beware of writing "if x" when you really mean "if x is not None" --
e.g. when testing whether a variable or argument that defaults to None was
set to some other value. The other value might have a type (such as a
container) that could be false in a boolean context!'

It makes me feel a Pythonista although the C++/C#/Java guy which is in me
> would probably write:
>
> param_value is None and param['required']
>
> possibly adding a bunch of parenthesis as well :)
> On this specific issue, given the way our deserializer currently works,
> param_value can only be a string.

Is that true? I was under the impression that a value could also be a
nested array or dictionary, in which case an empty array or dictionary would
be interpreted as an invalid value, even if it was valid. If that is the
case, this could impact anyone writing an extension with nested
arrays/dictionary.

> As the deserializer can change in the future, or there might be cases in
> which an empty string is a valid parameter value, it makes sense to file a
> bug.
>
> >
> > quantum/api/faults.py
> >
> > Is this still left in as a TODO?
> >
> > + #MUST RETURN 202???
>
> I think we discussed in an email thread or earlier meeting this issue. From
> the spec the API should return 202, but the implementation returns 200. It
> turns out the Openstack API has this problem as well. We 'inherited' it by
> grabbing their wsgi framework. I was planning to file a bug and have this
> problem resolved before integration freeze. It is not trivial and I did not
> want to halt progress on the API work for this issue.
>
> >
> > quantum/db/api.py,
> >
> > definitely feel free to remove the check_duplicate_net_name method, as
> your
> > comment suggests.
>
> I'll file a bug for this; low priority should be enough for this work item.
>
> >
> > quantum/tools/batch_config.py
> >
> > Great that you updated the cl...

Read more...

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'etc/quantum.conf'
2--- etc/quantum.conf 2011-08-16 23:03:32 +0000
3+++ etc/quantum.conf 2011-08-24 11:55:39 +0000
4@@ -17,7 +17,7 @@
5 [composite:quantum]
6 use = egg:Paste#urlmap
7 /: quantumversions
8-/v0.1: quantumapi
9+/v1.0: quantumapi
10
11 [pipeline:quantumapi]
12 pipeline = extensions quantumapiapp
13
14=== modified file 'quantum/api/__init__.py'
15--- quantum/api/__init__.py 2011-08-01 06:22:39 +0000
16+++ quantum/api/__init__.py 2011-08-24 11:55:39 +0000
17@@ -26,6 +26,7 @@
18
19 from quantum import manager
20 from quantum.api import faults
21+from quantum.api import attachments
22 from quantum.api import networks
23 from quantum.api import ports
24 from quantum.common import flags
25@@ -58,24 +59,29 @@
26 path_prefix=uri_prefix)
27 mapper.resource('port', 'ports',
28 controller=ports.Controller(plugin),
29+ collection={'detail': 'GET'},
30+ member={'detail': 'GET'},
31 parent_resource=dict(member_name='network',
32 collection_name=uri_prefix +\
33 'networks'))
34+
35+ attachments_ctrl = attachments.Controller(plugin)
36+
37 mapper.connect("get_resource",
38 uri_prefix + 'networks/{network_id}/' \
39 'ports/{id}/attachment{.format}',
40- controller=ports.Controller(plugin),
41+ controller=attachments_ctrl,
42 action="get_resource",
43 conditions=dict(method=['GET']))
44 mapper.connect("attach_resource",
45 uri_prefix + 'networks/{network_id}/' \
46 'ports/{id}/attachment{.format}',
47- controller=ports.Controller(plugin),
48+ controller=attachments_ctrl,
49 action="attach_resource",
50 conditions=dict(method=['PUT']))
51 mapper.connect("detach_resource",
52 uri_prefix + 'networks/{network_id}/' \
53 'ports/{id}/attachment{.format}',
54- controller=ports.Controller(plugin),
55+ controller=attachments_ctrl,
56 action="detach_resource",
57 conditions=dict(method=['DELETE']))
58
59=== modified file 'quantum/api/api_common.py'
60--- quantum/api/api_common.py 2011-08-01 14:40:29 +0000
61+++ quantum/api/api_common.py 2011-08-24 11:55:39 +0000
62@@ -38,7 +38,7 @@
63 for param in params:
64 param_name = param['param-name']
65 param_value = None
66- # 1- parse request body
67+ # Parameters are expected to be in request body only
68 if req.body:
69 des_body = self._deserialize(req.body,
70 req.best_match_content_type())
71@@ -50,22 +50,13 @@
72 LOG.error(line)
73 raise exc.HTTPBadRequest(msg)
74 param_value = data.get(param_name, None)
75- if not param_value:
76- # 2- parse request headers
77- # prepend param name with a 'x-' prefix
78- param_value = req.headers.get("x-" + param_name, None)
79- # 3- parse request query parameters
80- if not param_value:
81- try:
82- param_value = req.str_GET[param_name]
83- except KeyError:
84- #param not found
85- pass
86- if not param_value and param['required']:
87- msg = ("Failed to parse request. " +
88- "Parameter: " + param_name + " not specified")
89- for line in msg.split('\n'):
90- LOG.error(line)
91- raise exc.HTTPBadRequest(msg)
92+
93+ # If the parameter wasn't found and it was required, return 400
94+ if not param_value and param['required']:
95+ msg = ("Failed to parse request. " +
96+ "Parameter: " + param_name + " not specified")
97+ for line in msg.split('\n'):
98+ LOG.error(line)
99+ raise exc.HTTPBadRequest(msg)
100 results[param_name] = param_value or param.get('default-value')
101 return results
102
103=== added file 'quantum/api/attachments.py'
104--- quantum/api/attachments.py 1970-01-01 00:00:00 +0000
105+++ quantum/api/attachments.py 2011-08-24 11:55:39 +0000
106@@ -0,0 +1,86 @@
107+# Copyright 2011 Citrix Systems.
108+# All Rights Reserved.
109+#
110+# Licensed under the Apache License, Version 2.0 (the "License"); you may
111+# not use this file except in compliance with the License. You may obtain
112+# a copy of the License at
113+#
114+# http://www.apache.org/licenses/LICENSE-2.0
115+#
116+# Unless required by applicable law or agreed to in writing, software
117+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
118+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
119+# License for the specific language governing permissions and limitations
120+# under the License.
121+
122+import logging
123+
124+from webob import exc
125+
126+from quantum.api import api_common as common
127+from quantum.api import faults
128+from quantum.api.views import attachments as attachments_view
129+from quantum.common import exceptions as exception
130+
131+LOG = logging.getLogger('quantum.api.ports')
132+
133+
134+class Controller(common.QuantumController):
135+ """ Port API controller for Quantum API """
136+
137+ _attachment_ops_param_list = [{
138+ 'param-name': 'id',
139+ 'required': True}, ]
140+
141+ _serialization_metadata = {
142+ "application/xml": {
143+ "attributes": {
144+ "attachment": ["id"], }
145+ },
146+ }
147+
148+ def __init__(self, plugin):
149+ self._resource_name = 'attachment'
150+ super(Controller, self).__init__(plugin)
151+
152+ def get_resource(self, request, tenant_id, network_id, id):
153+ try:
154+ att_data = self._plugin.get_port_details(
155+ tenant_id, network_id, id)
156+ builder = attachments_view.get_view_builder(request)
157+ result = builder.build(att_data)['attachment']
158+ return dict(attachment=result)
159+ except exception.NetworkNotFound as e:
160+ return faults.Fault(faults.NetworkNotFound(e))
161+ except exception.PortNotFound as e:
162+ return faults.Fault(faults.PortNotFound(e))
163+
164+ def attach_resource(self, request, tenant_id, network_id, id):
165+ try:
166+ request_params = \
167+ self._parse_request_params(request,
168+ self._attachment_ops_param_list)
169+ except exc.HTTPError as e:
170+ return faults.Fault(e)
171+ try:
172+ self._plugin.plug_interface(tenant_id, network_id, id,
173+ request_params['id'])
174+ return exc.HTTPNoContent()
175+ except exception.NetworkNotFound as e:
176+ return faults.Fault(faults.NetworkNotFound(e))
177+ except exception.PortNotFound as e:
178+ return faults.Fault(faults.PortNotFound(e))
179+ except exception.PortInUse as e:
180+ return faults.Fault(faults.PortInUse(e))
181+ except exception.AlreadyAttached as e:
182+ return faults.Fault(faults.AlreadyAttached(e))
183+
184+ def detach_resource(self, request, tenant_id, network_id, id):
185+ try:
186+ self._plugin.unplug_interface(tenant_id,
187+ network_id, id)
188+ return exc.HTTPNoContent()
189+ except exception.NetworkNotFound as e:
190+ return faults.Fault(faults.NetworkNotFound(e))
191+ except exception.PortNotFound as e:
192+ return faults.Fault(faults.PortNotFound(e))
193
194=== modified file 'quantum/api/faults.py'
195--- quantum/api/faults.py 2011-08-06 06:12:32 +0000
196+++ quantum/api/faults.py 2011-08-24 11:55:39 +0000
197@@ -31,7 +31,6 @@
198 401: "unauthorized",
199 420: "networkNotFound",
200 421: "networkInUse",
201- 422: "networkNameExists",
202 430: "portNotFound",
203 431: "requestedStateInvalid",
204 432: "portInUse",
205@@ -92,22 +91,6 @@
206 explanation = ('Unable to remove the network: attachments still plugged.')
207
208
209-class NetworkNameExists(webob.exc.HTTPClientError):
210- """
211- subclass of :class:`~HTTPClientError`
212-
213- This indicates that the server could not set the network name to the
214- specified value because another network for the same tenant already has
215- that name.
216-
217- code: 422, title: Network Name Exists
218- """
219- code = 422
220- title = 'Network Name Exists'
221- explanation = ('Unable to set network name: tenant already has network' \
222- ' with same name.')
223-
224-
225 class PortNotFound(webob.exc.HTTPClientError):
226 """
227 subclass of :class:`~HTTPClientError`
228
229=== modified file 'quantum/api/networks.py'
230--- quantum/api/networks.py 2011-08-01 17:11:16 +0000
231+++ quantum/api/networks.py 2011-08-24 11:55:39 +0000
232@@ -29,7 +29,7 @@
233 """ Network API controller for Quantum API """
234
235 _network_ops_param_list = [{
236- 'param-name': 'net-name',
237+ 'param-name': 'name',
238 'required': True}, ]
239
240 _serialization_metadata = {
241@@ -37,38 +37,43 @@
242 "attributes": {
243 "network": ["id", "name"],
244 "port": ["id", "state"],
245- },
246- "plurals": {"networks": "network"}
247- },
248+ "attachment": ["id"]},
249+ "plurals": {"networks": "network",
250+ "ports": "port"}},
251 }
252
253 def __init__(self, plugin):
254 self._resource_name = 'network'
255 super(Controller, self).__init__(plugin)
256
257- def index(self, request, tenant_id):
258- """ Returns a list of network ids """
259- #TODO: this should be for a given tenant!!!
260- return self._items(request, tenant_id)
261-
262 def _item(self, req, tenant_id, network_id,
263 net_details=True, port_details=False):
264 # We expect get_network_details to return information
265 # concerning logical ports as well.
266 network = self._plugin.get_network_details(
267 tenant_id, network_id)
268+ port_list = self._plugin.get_all_ports(
269+ tenant_id, network_id)
270+ ports_data = [self._plugin.get_port_details(
271+ tenant_id, network_id, port['port-id'])
272+ for port in port_list]
273 builder = networks_view.get_view_builder(req)
274- result = builder.build(network, net_details, port_details)['network']
275+ result = builder.build(network, net_details,
276+ ports_data, port_details)['network']
277 return dict(network=result)
278
279- def _items(self, req, tenant_id, net_details=False, port_details=False):
280+ def _items(self, req, tenant_id, net_details=False):
281 """ Returns a list of networks. """
282 networks = self._plugin.get_all_networks(tenant_id)
283 builder = networks_view.get_view_builder(req)
284- result = [builder.build(network, net_details, port_details)['network']
285+ result = [builder.build(network, net_details)['network']
286 for network in networks]
287 return dict(networks=result)
288
289+ def index(self, request, tenant_id):
290+ """ Returns a list of network ids """
291+ return self._items(request, tenant_id)
292+
293 def show(self, request, tenant_id, id):
294 """ Returns network details for the given network id """
295 try:
296@@ -80,23 +85,13 @@
297 def detail(self, request, **kwargs):
298 tenant_id = kwargs.get('tenant_id')
299 network_id = kwargs.get('id')
300- try:
301- if network_id:
302- # show details for a given network
303- return self._item(request, tenant_id, network_id,
304- net_details=True, port_details=True)
305- else:
306- # show details for all networks
307- return self._items(request, tenant_id,
308- net_details=True, port_details=False)
309- network = self._plugin.get_network_details(
310- tenant_id, id)
311- builder = networks_view.get_view_builder(request)
312- #build response with details
313- result = builder.build(network, True)
314- return dict(networks=result)
315- except exception.NetworkNotFound as e:
316- return faults.Fault(faults.NetworkNotFound(e))
317+ if network_id:
318+ # show details for a given network
319+ return self._item(request, tenant_id, network_id,
320+ net_details=True, port_details=True)
321+ else:
322+ # show details for all networks
323+ return self._items(request, tenant_id, net_details=True)
324
325 def create(self, request, tenant_id):
326 """ Creates a new network for a given tenant """
327@@ -107,15 +102,13 @@
328 self._network_ops_param_list)
329 except exc.HTTPError as e:
330 return faults.Fault(e)
331- try:
332- network = self._plugin.\
333- create_network(tenant_id,
334- request_params['net-name'])
335- builder = networks_view.get_view_builder(request)
336- result = builder.build(network)
337- return dict(networks=result)
338- except exception.NetworkNameExists as e:
339- return faults.Fault(faults.NetworkNameExists(e))
340+ network = self._plugin.\
341+ create_network(tenant_id,
342+ request_params['name'])
343+ builder = networks_view.get_view_builder(request)
344+ result = builder.build(network)['network']
345+ #MUST RETURN 202???
346+ return dict(network=result)
347
348 def update(self, request, tenant_id, id):
349 """ Updates the name for the network with the given id """
350@@ -127,18 +120,16 @@
351 return faults.Fault(e)
352 try:
353 self._plugin.rename_network(tenant_id, id,
354- request_params['net-name'])
355- return exc.HTTPAccepted()
356+ request_params['name'])
357+ return exc.HTTPNoContent()
358 except exception.NetworkNotFound as e:
359 return faults.Fault(faults.NetworkNotFound(e))
360- except exception.NetworkNameExists as e:
361- return faults.Fault(faults.NetworkNameExists(e))
362
363 def delete(self, request, tenant_id, id):
364 """ Destroys the network with the given id """
365 try:
366 self._plugin.delete_network(tenant_id, id)
367- return exc.HTTPAccepted()
368+ return exc.HTTPNoContent()
369 except exception.NetworkNotFound as e:
370 return faults.Fault(faults.NetworkNotFound(e))
371 except exception.NetworkInUse as e:
372
373=== modified file 'quantum/api/ports.py'
374--- quantum/api/ports.py 2011-07-22 09:51:22 +0000
375+++ quantum/api/ports.py 2011-08-24 11:55:39 +0000
376@@ -29,55 +29,79 @@
377 """ Port API controller for Quantum API """
378
379 _port_ops_param_list = [{
380- 'param-name': 'port-state',
381+ 'param-name': 'state',
382 'default-value': 'DOWN',
383 'required': False}, ]
384
385- _attachment_ops_param_list = [{
386- 'param-name': 'attachment-id',
387- 'required': True}, ]
388-
389 _serialization_metadata = {
390 "application/xml": {
391 "attributes": {
392- "port": ["id", "state"], },
393- "plurals": {"ports": "port"}
394- },
395+ "port": ["id", "state"],
396+ "attachment": ["id"]},
397+ "plurals": {"ports": "port"}},
398 }
399
400 def __init__(self, plugin):
401 self._resource_name = 'port'
402 super(Controller, self).__init__(plugin)
403
404+ def _items(self, request, tenant_id, network_id,
405+ port_details=False):
406+ """ Returns a list of ports. """
407+ try:
408+ port_list = self._plugin.get_all_ports(tenant_id, network_id)
409+ builder = ports_view.get_view_builder(request)
410+
411+ # Load extra data for ports if required.
412+ if port_details:
413+ port_list_detail = \
414+ [self._plugin.get_port_details(
415+ tenant_id, network_id, port['port-id'])
416+ for port in port_list]
417+ port_list = port_list_detail
418+
419+ result = [builder.build(port, port_details)['port']
420+ for port in port_list]
421+ return dict(ports=result)
422+ except exception.NetworkNotFound as e:
423+ return faults.Fault(faults.NetworkNotFound(e))
424+
425+ def _item(self, request, tenant_id, network_id, port_id,
426+ att_details=False):
427+ """ Returns a specific port. """
428+ port = self._plugin.get_port_details(
429+ tenant_id, network_id, port_id)
430+ builder = ports_view.get_view_builder(request)
431+ result = builder.build(port, port_details=True,
432+ att_details=att_details)['port']
433+ return dict(port=result)
434+
435 def index(self, request, tenant_id, network_id):
436 """ Returns a list of port ids for a given network """
437- return self._items(request, tenant_id, network_id, is_detail=False)
438-
439- def _items(self, request, tenant_id, network_id, is_detail):
440- """ Returns a list of networks. """
441- try:
442- ports = self._plugin.get_all_ports(tenant_id, network_id)
443- builder = ports_view.get_view_builder(request)
444- result = [builder.build(port, is_detail)['port']
445- for port in ports]
446- return dict(ports=result)
447- except exception.NetworkNotFound as e:
448- return faults.Fault(faults.NetworkNotFound(e))
449+ return self._items(request, tenant_id, network_id, port_details=False)
450
451 def show(self, request, tenant_id, network_id, id):
452 """ Returns port details for given port and network """
453 try:
454- port = self._plugin.get_port_details(
455- tenant_id, network_id, id)
456- builder = ports_view.get_view_builder(request)
457- #build response with details
458- result = builder.build(port, True)['port']
459- return dict(port=result)
460+ return self._item(request, tenant_id, network_id, id)
461 except exception.NetworkNotFound as e:
462 return faults.Fault(faults.NetworkNotFound(e))
463 except exception.PortNotFound as e:
464 return faults.Fault(faults.PortNotFound(e))
465
466+ def detail(self, request, **kwargs):
467+ tenant_id = kwargs.get('tenant_id')
468+ network_id = kwargs.get('network_id')
469+ port_id = kwargs.get('id')
470+ if port_id:
471+ # show details for a given network
472+ return self._item(request, tenant_id,
473+ network_id, port_id, att_details=True)
474+ else:
475+ # show details for all port
476+ return self._items(request, tenant_id,
477+ network_id, port_details=True)
478+
479 def create(self, request, tenant_id, network_id):
480 """ Creates a new port for a given network """
481 #look for port state in request
482@@ -89,10 +113,10 @@
483 try:
484 port = self._plugin.create_port(tenant_id,
485 network_id,
486- request_params['port-state'])
487+ request_params['state'])
488 builder = ports_view.get_view_builder(request)
489- result = builder.build(port)
490- return dict(ports=result)
491+ result = builder.build(port)['port']
492+ return dict(port=result)
493 except exception.NetworkNotFound as e:
494 return faults.Fault(faults.NetworkNotFound(e))
495 except exception.StateInvalid as e:
496@@ -107,11 +131,9 @@
497 except exc.HTTPError as e:
498 return faults.Fault(e)
499 try:
500- port = self._plugin.update_port(tenant_id, network_id, id,
501- request_params['port-state'])
502- builder = ports_view.get_view_builder(request)
503- result = builder.build(port, True)
504- return dict(ports=result)
505+ self._plugin.update_port(tenant_id, network_id, id,
506+ request_params['state'])
507+ return exc.HTTPNoContent()
508 except exception.NetworkNotFound as e:
509 return faults.Fault(faults.NetworkNotFound(e))
510 except exception.PortNotFound as e:
511@@ -124,53 +146,10 @@
512 #look for port state in request
513 try:
514 self._plugin.delete_port(tenant_id, network_id, id)
515- return exc.HTTPAccepted()
516- # TODO(salvatore-orlando): Handle portInUse error
517- except exception.NetworkNotFound as e:
518- return faults.Fault(faults.NetworkNotFound(e))
519- except exception.PortNotFound as e:
520- return faults.Fault(faults.PortNotFound(e))
521- except exception.PortInUse as e:
522- return faults.Fault(faults.PortInUse(e))
523-
524- def get_resource(self, request, tenant_id, network_id, id):
525- try:
526- result = self._plugin.get_port_details(
527- tenant_id, network_id, id).get('attachment-id',
528- None)
529- return dict(attachment=result)
530- except exception.NetworkNotFound as e:
531- return faults.Fault(faults.NetworkNotFound(e))
532- except exception.PortNotFound as e:
533- return faults.Fault(faults.PortNotFound(e))
534-
535- def attach_resource(self, request, tenant_id, network_id, id):
536- try:
537- request_params = \
538- self._parse_request_params(request,
539- self._attachment_ops_param_list)
540- except exc.HTTPError as e:
541- return faults.Fault(e)
542- try:
543- self._plugin.plug_interface(tenant_id,
544- network_id, id,
545- request_params['attachment-id'])
546- return exc.HTTPAccepted()
547- except exception.NetworkNotFound as e:
548- return faults.Fault(faults.NetworkNotFound(e))
549- except exception.PortNotFound as e:
550- return faults.Fault(faults.PortNotFound(e))
551- except exception.PortInUse as e:
552- return faults.Fault(faults.PortInUse(e))
553- except exception.AlreadyAttached as e:
554- return faults.Fault(faults.AlreadyAttached(e))
555-
556- def detach_resource(self, request, tenant_id, network_id, id):
557- try:
558- self._plugin.unplug_interface(tenant_id,
559- network_id, id)
560- return exc.HTTPAccepted()
561- except exception.NetworkNotFound as e:
562- return faults.Fault(faults.NetworkNotFound(e))
563- except exception.PortNotFound as e:
564- return faults.Fault(faults.PortNotFound(e))
565+ return exc.HTTPNoContent()
566+ except exception.NetworkNotFound as e:
567+ return faults.Fault(faults.NetworkNotFound(e))
568+ except exception.PortNotFound as e:
569+ return faults.Fault(faults.PortNotFound(e))
570+ except exception.PortInUse as e:
571+ return faults.Fault(faults.PortInUse(e))
572
573=== modified file 'quantum/api/versions.py'
574--- quantum/api/versions.py 2011-05-31 17:15:00 +0000
575+++ quantum/api/versions.py 2011-08-24 11:55:39 +0000
576@@ -31,11 +31,11 @@
577 """Respond to a request for all Quantum API versions."""
578 version_objs = [
579 {
580- "id": "v0.1",
581+ "id": "v1.0",
582 "status": "CURRENT",
583 },
584 {
585- "id": "v1.0",
586+ "id": "v1.1",
587 "status": "FUTURE",
588 },
589 ]
590
591=== added file 'quantum/api/views/attachments.py'
592--- quantum/api/views/attachments.py 1970-01-01 00:00:00 +0000
593+++ quantum/api/views/attachments.py 2011-08-24 11:55:39 +0000
594@@ -0,0 +1,37 @@
595+# vim: tabstop=4 shiftwidth=4 softtabstop=4
596+
597+# Copyright 2011 Citrix Systems
598+# All Rights Reserved.
599+#
600+# Licensed under the Apache License, Version 2.0 (the "License"); you may
601+# not use this file except in compliance with the License. You may obtain
602+# a copy of the License at
603+#
604+# http://www.apache.org/licenses/LICENSE-2.0
605+#
606+# Unless required by applicable law or agreed to in writing, software
607+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
608+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
609+# License for the specific language governing permissions and limitations
610+# under the License.
611+
612+
613+def get_view_builder(req):
614+ base_url = req.application_url
615+ return ViewBuilder(base_url)
616+
617+
618+class ViewBuilder(object):
619+
620+ def __init__(self, base_url):
621+ """
622+ :param base_url: url of the root wsgi application
623+ """
624+ self.base_url = base_url
625+
626+ def build(self, attachment_data):
627+ """Generic method used to generate an attachment entity."""
628+ if attachment_data['attachment']:
629+ return dict(attachment=dict(id=attachment_data['attachment']))
630+ else:
631+ return dict(attachment={})
632
633=== modified file 'quantum/api/views/networks.py'
634--- quantum/api/views/networks.py 2011-08-01 01:21:59 +0000
635+++ quantum/api/views/networks.py 2011-08-24 11:55:39 +0000
636@@ -15,8 +15,6 @@
637 # License for the specific language governing permissions and limitations
638 # under the License.
639
640-from quantum.api.views import ports as ports_view
641-
642
643 def get_view_builder(req):
644 base_url = req.application_url
645@@ -31,17 +29,16 @@
646 """
647 self.base_url = base_url
648
649- def build(self, network_data, net_detail=False, port_detail=False):
650+ def build(self, network_data, net_detail=False,
651+ ports_data=None, port_detail=False):
652 """Generic method used to generate a network entity."""
653 if net_detail:
654 network = self._build_detail(network_data)
655 else:
656 network = self._build_simple(network_data)
657 if port_detail:
658- builder = ports_view.ViewBuilder(self.base_url)
659- ports = [builder.build(port_data, port_detail)['port']
660- for port_data in network_data['net-ports'].values()]
661- network['ports'] = ports
662+ ports = [self._build_port(port_data) for port_data in ports_data]
663+ network['network']['ports'] = ports
664 return network
665
666 def _build_simple(self, network_data):
667@@ -55,6 +52,8 @@
668
669 def _build_port(self, port_data):
670 """Return details about a specific logical port."""
671- return dict(port=dict(id=port_data['port-id'],
672- state=port_data['port-state'],
673- attachment=port_data['attachment']))
674+ port_dict = dict(id=port_data['port-id'],
675+ state=port_data['port-state'])
676+ if port_data['attachment']:
677+ port_dict['attachment'] = dict(id=port_data['attachment'])
678+ return port_dict
679
680=== modified file 'quantum/api/views/ports.py'
681--- quantum/api/views/ports.py 2011-07-22 09:51:22 +0000
682+++ quantum/api/views/ports.py 2011-08-24 11:55:39 +0000
683@@ -29,19 +29,11 @@
684 """
685 self.base_url = base_url
686
687- def build(self, port_data, is_detail=False):
688+ def build(self, port_data, port_details=False, att_details=False):
689 """Generic method used to generate a port entity."""
690- if is_detail:
691- port = self._build_detail(port_data)
692- else:
693- port = self._build_simple(port_data)
694+ port = dict(port=dict(id=port_data['port-id']))
695+ if port_details:
696+ port['port']['state'] = port_data['port-state']
697+ if att_details and port_data['attachment']:
698+ port['port']['attachment'] = dict(id=port_data['attachment'])
699 return port
700-
701- def _build_simple(self, port_data):
702- """Return a simple model of a port."""
703- return dict(port=dict(id=port_data['port-id']))
704-
705- def _build_detail(self, port_data):
706- """Return a simple model of a port (with its state)."""
707- return dict(port=dict(id=port_data['port-id'],
708- state=port_data['port-state']))
709
710=== modified file 'quantum/cli.py'
711--- quantum/cli.py 2011-08-23 19:36:06 +0000
712+++ quantum/cli.py 2011-08-24 11:55:39 +0000
713@@ -69,7 +69,7 @@
714 LOG.debug(res)
715 nid = None
716 try:
717- nid = res["networks"]["network"]["id"]
718+ nid = res["network"]["id"]
719 except Exception, e:
720 print "Failed to create network"
721 # TODO(bgh): grab error details from ws request result
722@@ -104,7 +104,7 @@
723 def api_detail_net(client, *args):
724 tid, nid = args
725 try:
726- res = client.show_network_details(nid)["networks"]["network"]
727+ res = client.show_network_details(nid)["network"]
728 except Exception, e:
729 LOG.error("Failed to get network details: %s" % e)
730 return
731@@ -121,7 +121,7 @@
732 pid = port["id"]
733 res = client.show_port_attachment(nid, pid)
734 LOG.debug(res)
735- remote_iface = res["attachment"]
736+ remote_iface = res["attachment"]["id"]
737 print "\tRemote interface:%s" % remote_iface
738
739
740@@ -133,7 +133,7 @@
741
742 def api_rename_net(client, *args):
743 tid, nid, name = args
744- data = {'network': {'net-name': '%s' % name}}
745+ data = {'network': {'name': '%s' % name}}
746 try:
747 res = client.update_network(nid, data)
748 except Exception, e:
749@@ -179,7 +179,7 @@
750 except Exception, e:
751 LOG.error("Failed to create port: %s" % e)
752 return
753- new_port = res["ports"]["port"]["id"]
754+ new_port = res["port"]["id"]
755 print "Created Virtual Port:%s " \
756 "on Virtual Network:%s" % (new_port, nid)
757
758@@ -214,16 +214,17 @@
759 def api_detail_port(client, *args):
760 tid, nid, pid = args
761 try:
762- port = client.show_port_details(nid, pid)["ports"]["port"]
763+ port = client.show_port_details(nid, pid)["port"]
764+ att = client.show_port_attachment(nid, pid)
765 except Exception, e:
766 LOG.error("Failed to get port details: %s" % e)
767 return
768
769- id = port["id"]
770- attachment = port["attachment"]
771+ id = port['id']
772+ interface_id = att['id']
773 LOG.debug(port)
774 print "Virtual Port:%s on Virtual Network:%s " \
775- "contains remote interface:%s" % (pid, nid, attachment)
776+ "contains remote interface:%s" % (pid, nid, interface_id)
777
778
779 def plug_iface(manager, *args):
780@@ -236,7 +237,7 @@
781 def api_plug_iface(client, *args):
782 tid, nid, pid, vid = args
783 try:
784- data = {'port': {'attachment-id': '%s' % vid}}
785+ data = {'attachment': {'id': '%s' % vid}}
786 res = client.attach_resource(nid, pid, data)
787 except Exception, e:
788 LOG.error("Failed to plug iface \"%s\" to port \"%s\": %s" % (vid,
789
790=== modified file 'quantum/client.py'
791--- quantum/client.py 2011-08-24 10:15:54 +0000
792+++ quantum/client.py 2011-08-24 11:55:39 +0000
793@@ -27,13 +27,10 @@
794 401: exceptions.NotAuthorized,
795 420: exceptions.NetworkNotFound,
796 421: exceptions.NetworkInUse,
797- 422: exceptions.NetworkNameExists,
798 430: exceptions.PortNotFound,
799 431: exceptions.StateInvalid,
800 432: exceptions.PortInUse,
801- 440: exceptions.AlreadyAttached,
802- 441: exceptions.AttachmentNotReady,
803-}
804+ 440: exceptions.AlreadyAttached}
805
806
807 class ApiCall(object):
808@@ -72,7 +69,7 @@
809
810 def __init__(self, host="127.0.0.1", port=9696, use_ssl=False, tenant=None,
811 format="xml", testingStub=None, key_file=None, cert_file=None,
812- logger=None, action_prefix="/v0.1/tenants/{tenant_id}"):
813+ logger=None, action_prefix="/v1.0/tenants/{tenant_id}"):
814 """
815 Creates a new client to some service.
816
817
818=== modified file 'quantum/common/exceptions.py'
819--- quantum/common/exceptions.py 2011-08-18 19:49:20 +0000
820+++ quantum/common/exceptions.py 2011-08-24 11:55:39 +0000
821@@ -111,16 +111,6 @@
822 "already plugged into port %(att_port_id)s")
823
824
825-class AttachmentNotReady(QuantumException):
826- message = _("The attachment %(att_id)s is not ready")
827-
828-
829-class NetworkNameExists(QuantumException):
830- message = _("Unable to set network name to %(net_name). " \
831- "Network with id %(net_id) already has this name for " \
832- "tenant %(tenant_id)")
833-
834-
835 class Duplicate(Error):
836 pass
837
838
839=== modified file 'quantum/common/wsgi.py'
840--- quantum/common/wsgi.py 2011-08-02 04:49:32 +0000
841+++ quantum/common/wsgi.py 2011-08-24 11:55:39 +0000
842@@ -122,6 +122,7 @@
843 Based on the query extension then the Accept header.
844
845 """
846+ # First lookup http request
847 parts = self.path.rsplit('.', 1)
848 LOG.debug("Request parts:%s", parts)
849 if len(parts) > 1:
850@@ -129,21 +130,26 @@
851 if format in ['json', 'xml']:
852 return 'application/{0}'.format(parts[1])
853
854+ #Then look up content header
855+ type_from_header = self.get_content_type()
856+ if type_from_header:
857+ return type_from_header
858 ctypes = ['application/json', 'application/xml']
859+
860+ #Finally search in Accept-* headers
861 bm = self.accept.best_match(ctypes)
862 return bm or 'application/json'
863
864 def get_content_type(self):
865 allowed_types = ("application/xml", "application/json")
866 if not "Content-Type" in self.headers:
867- msg = _("Missing Content-Type")
868- LOG.debug(msg)
869- raise webob.exc.HTTPBadRequest(msg)
870+ LOG.debug(_("Missing Content-Type"))
871+ return None
872 type = self.content_type
873 if type in allowed_types:
874 return type
875 LOG.debug(_("Wrong Content-Type: %s") % type)
876- raise webob.exc.HTTPBadRequest("Invalid content type")
877+ return None
878
879
880 class Application(object):
881
882=== modified file 'quantum/db/api.py'
883--- quantum/db/api.py 2011-08-10 04:58:15 +0000
884+++ quantum/db/api.py 2011-08-24 11:55:39 +0000
885@@ -17,6 +17,8 @@
886 # @author: Brad Hall, Nicira Networks, Inc.
887 # @author: Dan Wendlandt, Nicira Networks, Inc.
888
889+import logging
890+
891 from sqlalchemy import create_engine
892 from sqlalchemy.orm import sessionmaker, exc
893
894@@ -27,6 +29,7 @@
895 _ENGINE = None
896 _MAKER = None
897 BASE = models.BASE
898+LOG = logging.getLogger('quantum.db.api')
899
900
901 def configure_db(options):
902@@ -78,6 +81,11 @@
903
904
905 def _check_duplicate_net_name(tenant_id, net_name):
906+ """Checks whether a network with the same name
907+ already exists for the tenant.
908+ """
909+
910+ #TODO(salvatore-orlando): Not used anymore - candidate for removal
911 session = get_session()
912 try:
913 net = session.query(models.Network).\
914@@ -94,7 +102,6 @@
915 def network_create(tenant_id, name):
916 session = get_session()
917
918- _check_duplicate_net_name(tenant_id, name)
919 with session.begin():
920 net = models.Network(tenant_id, name)
921 session.add(net)
922
923=== modified file 'quantum/db/models.py'
924--- quantum/db/models.py 2011-08-02 04:49:32 +0000
925+++ quantum/db/models.py 2011-08-24 11:55:39 +0000
926@@ -70,13 +70,14 @@
927 uuid = Column(String(255), primary_key=True)
928 network_id = Column(String(255), ForeignKey("networks.uuid"),
929 nullable=False)
930- interface_id = Column(String(255))
931+ interface_id = Column(String(255), nullable=True)
932 # Port state - Hardcoding string value at the moment
933 state = Column(String(8))
934
935 def __init__(self, network_id):
936 self.uuid = str(uuid.uuid4())
937 self.network_id = network_id
938+ self.interface_id = None
939 self.state = "DOWN"
940
941 def __repr__(self):
942
943=== modified file 'quantum/plugins/SamplePlugin.py'
944--- quantum/plugins/SamplePlugin.py 2011-08-10 04:58:15 +0000
945+++ quantum/plugins/SamplePlugin.py 2011-08-24 11:55:39 +0000
946@@ -352,7 +352,7 @@
947 LOG.debug("FakePlugin.get_port_details() called")
948 port = self._get_port(tenant_id, net_id, port_id)
949 return {'port-id': str(port.uuid),
950- 'attachment-id': port.interface_id,
951+ 'attachment': port.interface_id,
952 'port-state': port.state}
953
954 def create_port(self, tenant_id, net_id, port_state=None):
955@@ -407,10 +407,10 @@
956 specified Virtual Network.
957 """
958 LOG.debug("FakePlugin.plug_interface() called")
959+ port = self._get_port(tenant_id, net_id, port_id)
960 # Validate attachment
961 self._validate_attachment(tenant_id, net_id, port_id,
962 remote_interface_id)
963- port = self._get_port(tenant_id, net_id, port_id)
964 if port['interface_id']:
965 raise exc.PortInUse(net_id=net_id, port_id=port_id,
966 att_id=port['interface_id'])
967
968=== modified file 'tests/unit/test_api.py'
969--- tests/unit/test_api.py 2011-08-23 19:36:06 +0000
970+++ tests/unit/test_api.py 2011-08-24 11:55:39 +0000
971@@ -48,7 +48,7 @@
972 if expected_res_status == 200:
973 network_data = Serializer().deserialize(network_res.body,
974 content_type)
975- return network_data['networks']['network']['id']
976+ return network_data['network']['id']
977
978 def _create_port(self, network_id, port_state, format,
979 custom_req_body=None, expected_res_status=200):
980@@ -61,7 +61,7 @@
981 self.assertEqual(port_res.status_int, expected_res_status)
982 if expected_res_status == 200:
983 port_data = Serializer().deserialize(port_res.body, content_type)
984- return port_data['ports']['port']['id']
985+ return port_data['port']['id']
986
987 def _test_create_network(self, format):
988 LOG.debug("_test_create_network - format:%s - START", format)
989@@ -74,8 +74,7 @@
990 self.assertEqual(show_network_res.status_int, 200)
991 network_data = Serializer().deserialize(show_network_res.body,
992 content_type)
993- self.assertEqual(network_id,
994- network_data['network']['id'])
995+ self.assertEqual(network_id, network_data['network']['id'])
996 LOG.debug("_test_create_network - format:%s - END", format)
997
998 def _test_create_network_badrequest(self, format):
999@@ -102,6 +101,25 @@
1000 self.assertEqual(len(network_data['networks']), 2)
1001 LOG.debug("_test_list_networks - format:%s - END", format)
1002
1003+ def _test_list_networks_detail(self, format):
1004+ LOG.debug("_test_list_networks_detail - format:%s - START", format)
1005+ content_type = "application/%s" % format
1006+ self._create_network(format, "net_1")
1007+ self._create_network(format, "net_2")
1008+ list_network_req = testlib.network_list_detail_request(self.tenant_id,
1009+ format)
1010+ list_network_res = list_network_req.get_response(self.api)
1011+ self.assertEqual(list_network_res.status_int, 200)
1012+ network_data = self._net_serializer.deserialize(
1013+ list_network_res.body, content_type)
1014+ # Check network count: should return 2
1015+ self.assertEqual(len(network_data['networks']), 2)
1016+ # Check contents - id & name for each network
1017+ for network in network_data['networks']:
1018+ self.assertTrue('id' in network and 'name' in network)
1019+ self.assertTrue(network['id'] and network['name'])
1020+ LOG.debug("_test_list_networks_detail - format:%s - END", format)
1021+
1022 def _test_show_network(self, format):
1023 LOG.debug("_test_show_network - format:%s - START", format)
1024 content_type = "application/%s" % format
1025@@ -118,6 +136,25 @@
1026 network_data['network'])
1027 LOG.debug("_test_show_network - format:%s - END", format)
1028
1029+ def _test_show_network_detail(self, format):
1030+ LOG.debug("_test_show_network_detail - format:%s - START", format)
1031+ content_type = "application/%s" % format
1032+ # Create a network and a port
1033+ network_id = self._create_network(format)
1034+ port_id = self._create_port(network_id, "ACTIVE", format)
1035+ show_network_req = testlib.show_network_detail_request(
1036+ self.tenant_id, network_id, format)
1037+ show_network_res = show_network_req.get_response(self.api)
1038+ self.assertEqual(show_network_res.status_int, 200)
1039+ network_data = self._net_serializer.deserialize(
1040+ show_network_res.body, content_type)
1041+ self.assertEqual({'id': network_id,
1042+ 'name': self.network_name,
1043+ 'ports': [{'id': port_id,
1044+ 'state': 'ACTIVE'}]},
1045+ network_data['network'])
1046+ LOG.debug("_test_show_network_detail - format:%s - END", format)
1047+
1048 def _test_show_network_not_found(self, format):
1049 LOG.debug("_test_show_network_not_found - format:%s - START", format)
1050 show_network_req = testlib.show_network_request(self.tenant_id,
1051@@ -137,7 +174,7 @@
1052 new_name,
1053 format)
1054 update_network_res = update_network_req.get_response(self.api)
1055- self.assertEqual(update_network_res.status_int, 202)
1056+ self.assertEqual(update_network_res.status_int, 204)
1057 show_network_req = testlib.show_network_request(self.tenant_id,
1058 network_id,
1059 format)
1060@@ -187,7 +224,7 @@
1061 network_id,
1062 format)
1063 delete_network_res = delete_network_req.get_response(self.api)
1064- self.assertEqual(delete_network_res.status_int, 202)
1065+ self.assertEqual(delete_network_res.status_int, 204)
1066 list_network_req = testlib.network_list_request(self.tenant_id,
1067 format)
1068 list_network_res = list_network_req.get_response(self.api)
1069@@ -213,7 +250,7 @@
1070 port_id,
1071 attachment_id)
1072 attachment_res = attachment_req.get_response(self.api)
1073- self.assertEquals(attachment_res.status_int, 202)
1074+ self.assertEquals(attachment_res.status_int, 204)
1075
1076 LOG.debug("Deleting network %(network_id)s"\
1077 " of tenant %(tenant_id)s", locals())
1078@@ -241,6 +278,27 @@
1079 self.assertEqual(len(port_data['ports']), 2)
1080 LOG.debug("_test_list_ports - format:%s - END", format)
1081
1082+ def _test_list_ports_detail(self, format):
1083+ LOG.debug("_test_list_ports_detail - format:%s - START", format)
1084+ content_type = "application/%s" % format
1085+ port_state = "ACTIVE"
1086+ network_id = self._create_network(format)
1087+ self._create_port(network_id, port_state, format)
1088+ self._create_port(network_id, port_state, format)
1089+ list_port_req = testlib.port_list_detail_request(self.tenant_id,
1090+ network_id, format)
1091+ list_port_res = list_port_req.get_response(self.api)
1092+ self.assertEqual(list_port_res.status_int, 200)
1093+ port_data = self._port_serializer.deserialize(
1094+ list_port_res.body, content_type)
1095+ # Check port count: should return 2
1096+ self.assertEqual(len(port_data['ports']), 2)
1097+ # Check contents - id & name for each network
1098+ for port in port_data['ports']:
1099+ self.assertTrue('id' in port and 'state' in port)
1100+ self.assertTrue(port['id'] and port['state'])
1101+ LOG.debug("_test_list_ports_detail - format:%s - END", format)
1102+
1103 def _test_show_port(self, format):
1104 LOG.debug("_test_show_port - format:%s - START", format)
1105 content_type = "application/%s" % format
1106@@ -258,6 +316,44 @@
1107 port_data['port'])
1108 LOG.debug("_test_show_port - format:%s - END", format)
1109
1110+ def _test_show_port_detail(self, format):
1111+ LOG.debug("_test_show_port - format:%s - START", format)
1112+ content_type = "application/%s" % format
1113+ port_state = "ACTIVE"
1114+ network_id = self._create_network(format)
1115+ port_id = self._create_port(network_id, port_state, format)
1116+
1117+ # Part 1 - no attachment
1118+ show_port_req = testlib.show_port_detail_request(self.tenant_id,
1119+ network_id, port_id, format)
1120+ show_port_res = show_port_req.get_response(self.api)
1121+ self.assertEqual(show_port_res.status_int, 200)
1122+ port_data = self._port_serializer.deserialize(
1123+ show_port_res.body, content_type)
1124+ self.assertEqual({'id': port_id, 'state': port_state},
1125+ port_data['port'])
1126+
1127+ # Part 2 - plug attachment into port
1128+ interface_id = "test_interface"
1129+ put_attachment_req = testlib.put_attachment_request(self.tenant_id,
1130+ network_id,
1131+ port_id,
1132+ interface_id,
1133+ format)
1134+ put_attachment_res = put_attachment_req.get_response(self.api)
1135+ self.assertEqual(put_attachment_res.status_int, 204)
1136+ show_port_req = testlib.show_port_detail_request(self.tenant_id,
1137+ network_id, port_id, format)
1138+ show_port_res = show_port_req.get_response(self.api)
1139+ self.assertEqual(show_port_res.status_int, 200)
1140+ port_data = self._port_serializer.deserialize(
1141+ show_port_res.body, content_type)
1142+ self.assertEqual({'id': port_id, 'state': port_state,
1143+ 'attachment': {'id': interface_id}},
1144+ port_data['port'])
1145+
1146+ LOG.debug("_test_show_port_detail - format:%s - END", format)
1147+
1148 def _test_show_port_networknotfound(self, format):
1149 LOG.debug("_test_show_port_networknotfound - format:%s - START",
1150 format)
1151@@ -343,7 +439,7 @@
1152 network_id, port_id,
1153 format)
1154 delete_port_res = delete_port_req.get_response(self.api)
1155- self.assertEqual(delete_port_res.status_int, 202)
1156+ self.assertEqual(delete_port_res.status_int, 204)
1157 list_port_req = testlib.port_list_request(self.tenant_id, network_id,
1158 format)
1159 list_port_res = list_port_req.get_response(self.api)
1160@@ -367,7 +463,7 @@
1161 port_id,
1162 attachment_id)
1163 attachment_res = attachment_req.get_response(self.api)
1164- self.assertEquals(attachment_res.status_int, 202)
1165+ self.assertEquals(attachment_res.status_int, 204)
1166 LOG.debug("Deleting port %(port_id)s for network %(network_id)s"\
1167 " of tenant %(tenant_id)s", locals())
1168 delete_port_req = testlib.port_delete_request(self.tenant_id,
1169@@ -417,7 +513,7 @@
1170 new_port_state,
1171 format)
1172 update_port_res = update_port_req.get_response(self.api)
1173- self.assertEqual(update_port_res.status_int, 200)
1174+ self.assertEqual(update_port_res.status_int, 204)
1175 show_port_req = testlib.show_port_request(self.tenant_id,
1176 network_id, port_id,
1177 format)
1178@@ -427,6 +523,22 @@
1179 show_port_res.body, content_type)
1180 self.assertEqual({'id': port_id, 'state': new_port_state},
1181 port_data['port'])
1182+ # now set it back to the original value
1183+ update_port_req = testlib.update_port_request(self.tenant_id,
1184+ network_id, port_id,
1185+ port_state,
1186+ format)
1187+ update_port_res = update_port_req.get_response(self.api)
1188+ self.assertEqual(update_port_res.status_int, 204)
1189+ show_port_req = testlib.show_port_request(self.tenant_id,
1190+ network_id, port_id,
1191+ format)
1192+ show_port_res = show_port_req.get_response(self.api)
1193+ self.assertEqual(show_port_res.status_int, 200)
1194+ port_data = self._port_serializer.deserialize(
1195+ show_port_res.body, content_type)
1196+ self.assertEqual({'id': port_id, 'state': port_state},
1197+ port_data['port'])
1198 LOG.debug("_test_set_port_state - format:%s - END", format)
1199
1200 def _test_set_port_state_networknotfound(self, format):
1201@@ -491,7 +603,7 @@
1202 interface_id,
1203 format)
1204 put_attachment_res = put_attachment_req.get_response(self.api)
1205- self.assertEqual(put_attachment_res.status_int, 202)
1206+ self.assertEqual(put_attachment_res.status_int, 204)
1207 get_attachment_req = testlib.get_attachment_request(self.tenant_id,
1208 network_id,
1209 port_id,
1210@@ -499,7 +611,7 @@
1211 get_attachment_res = get_attachment_req.get_response(self.api)
1212 attachment_data = Serializer().deserialize(get_attachment_res.body,
1213 content_type)
1214- self.assertEqual(attachment_data['attachment'], interface_id)
1215+ self.assertEqual(attachment_data['attachment']['id'], interface_id)
1216 LOG.debug("_test_show_attachment - format:%s - END", format)
1217
1218 def _test_show_attachment_networknotfound(self, format):
1219@@ -544,7 +656,7 @@
1220 interface_id,
1221 format)
1222 put_attachment_res = put_attachment_req.get_response(self.api)
1223- self.assertEqual(put_attachment_res.status_int, 202)
1224+ self.assertEqual(put_attachment_res.status_int, 204)
1225 LOG.debug("_test_put_attachment - format:%s - END", format)
1226
1227 def _test_put_attachment_networknotfound(self, format):
1228@@ -593,13 +705,13 @@
1229 interface_id,
1230 format)
1231 put_attachment_res = put_attachment_req.get_response(self.api)
1232- self.assertEqual(put_attachment_res.status_int, 202)
1233+ self.assertEqual(put_attachment_res.status_int, 204)
1234 del_attachment_req = testlib.delete_attachment_request(self.tenant_id,
1235 network_id,
1236 port_id,
1237 format)
1238 del_attachment_res = del_attachment_req.get_response(self.api)
1239- self.assertEqual(del_attachment_res.status_int, 202)
1240+ self.assertEqual(del_attachment_res.status_int, 204)
1241 LOG.debug("_test_delete_attachment - format:%s - END", format)
1242
1243 def _test_delete_attachment_networknotfound(self, format):
1244@@ -670,6 +782,12 @@
1245 def test_list_networks_xml(self):
1246 self._test_list_networks('xml')
1247
1248+ def test_list_networks_detail_json(self):
1249+ self._test_list_networks_detail('json')
1250+
1251+ def test_list_networks_detail_xml(self):
1252+ self._test_list_networks_detail('xml')
1253+
1254 def test_create_network_json(self):
1255 self._test_create_network('json')
1256
1257@@ -694,6 +812,12 @@
1258 def test_show_network_xml(self):
1259 self._test_show_network('xml')
1260
1261+ def test_show_network_detail_json(self):
1262+ self._test_show_network_detail('json')
1263+
1264+ def test_show_network_detail_xml(self):
1265+ self._test_show_network_detail('xml')
1266+
1267 def test_delete_network_json(self):
1268 self._test_delete_network('json')
1269
1270@@ -730,12 +854,24 @@
1271 def test_list_ports_xml(self):
1272 self._test_list_ports('xml')
1273
1274+ def test_list_ports_detail_json(self):
1275+ self._test_list_ports_detail('json')
1276+
1277+ def test_list_ports_detail_xml(self):
1278+ self._test_list_ports_detail('xml')
1279+
1280 def test_show_port_json(self):
1281 self._test_show_port('json')
1282
1283 def test_show_port_xml(self):
1284 self._test_show_port('xml')
1285
1286+ def test_show_port_detail_json(self):
1287+ self._test_show_port_detail('json')
1288+
1289+ def test_show_port_detail_xml(self):
1290+ self._test_show_port_detail('xml')
1291+
1292 def test_show_port_networknotfound_json(self):
1293 self._test_show_port_networknotfound('json')
1294
1295
1296=== modified file 'tests/unit/test_clientlib.py'
1297--- tests/unit/test_clientlib.py 2011-08-10 02:15:14 +0000
1298+++ tests/unit/test_clientlib.py 2011-08-24 11:55:39 +0000
1299@@ -43,7 +43,7 @@
1300 return self.content
1301
1302 def status(self):
1303- return status
1304+ return self.status
1305
1306 # To test error codes, set the host to 10.0.0.1, and the port to the code
1307 def __init__(self, host, port=9696, key_file="", cert_file=""):
1308
1309=== modified file 'tests/unit/testlib_api.py'
1310--- tests/unit/testlib_api.py 2011-08-01 14:40:29 +0000
1311+++ tests/unit/testlib_api.py 2011-08-24 11:55:39 +0000
1312@@ -12,26 +12,45 @@
1313 return req
1314
1315
1316+def _network_list_request(tenant_id, format='xml', detail=False):
1317+ method = 'GET'
1318+ detail_str = detail and '/detail' or ''
1319+ path = "/tenants/%(tenant_id)s/networks" \
1320+ "%(detail_str)s.%(format)s" % locals()
1321+ content_type = "application/%s" % format
1322+ return create_request(path, None, content_type, method)
1323+
1324+
1325 def network_list_request(tenant_id, format='xml'):
1326+ return _network_list_request(tenant_id, format)
1327+
1328+
1329+def network_list_detail_request(tenant_id, format='xml'):
1330+ return _network_list_request(tenant_id, format, detail=True)
1331+
1332+
1333+def _show_network_request(tenant_id, network_id, format='xml', detail=False):
1334 method = 'GET'
1335- path = "/tenants/%(tenant_id)s/networks.%(format)s" % locals()
1336+ detail_str = detail and '/detail' or ''
1337+ path = "/tenants/%(tenant_id)s/networks" \
1338+ "/%(network_id)s%(detail_str)s.%(format)s" % locals()
1339 content_type = "application/%s" % format
1340 return create_request(path, None, content_type, method)
1341
1342
1343 def show_network_request(tenant_id, network_id, format='xml'):
1344- method = 'GET'
1345- path = "/tenants/%(tenant_id)s/networks" \
1346- "/%(network_id)s.%(format)s" % locals()
1347- content_type = "application/%s" % format
1348- return create_request(path, None, content_type, method)
1349+ return _show_network_request(tenant_id, network_id, format)
1350+
1351+
1352+def show_network_detail_request(tenant_id, network_id, format='xml'):
1353+ return _show_network_request(tenant_id, network_id, format, detail=True)
1354
1355
1356 def new_network_request(tenant_id, network_name='new_name',
1357 format='xml', custom_req_body=None):
1358 method = 'POST'
1359 path = "/tenants/%(tenant_id)s/networks.%(format)s" % locals()
1360- data = custom_req_body or {'network': {'net-name': '%s' % network_name}}
1361+ data = custom_req_body or {'network': {'name': '%s' % network_name}}
1362 content_type = "application/%s" % format
1363 body = Serializer().serialize(data, content_type)
1364 return create_request(path, body, content_type, method)
1365@@ -42,7 +61,7 @@
1366 method = 'PUT'
1367 path = "/tenants/%(tenant_id)s/networks" \
1368 "/%(network_id)s.%(format)s" % locals()
1369- data = custom_req_body or {'network': {'net-name': '%s' % network_name}}
1370+ data = custom_req_body or {'network': {'name': '%s' % network_name}}
1371 content_type = "application/%s" % format
1372 body = Serializer().serialize(data, content_type)
1373 return create_request(path, body, content_type, method)
1374@@ -56,20 +75,41 @@
1375 return create_request(path, None, content_type, method)
1376
1377
1378+def _port_list_request(tenant_id, network_id, format='xml', detail=False):
1379+ method = 'GET'
1380+ detail_str = detail and '/detail' or ''
1381+ path = "/tenants/%(tenant_id)s/networks/" \
1382+ "%(network_id)s/ports%(detail_str)s.%(format)s" % locals()
1383+ content_type = "application/%s" % format
1384+ return create_request(path, None, content_type, method)
1385+
1386+
1387 def port_list_request(tenant_id, network_id, format='xml'):
1388+ return _port_list_request(tenant_id, network_id, format)
1389+
1390+
1391+def port_list_detail_request(tenant_id, network_id, format='xml'):
1392+ return _port_list_request(tenant_id, network_id,
1393+ format, detail=True)
1394+
1395+
1396+def _show_port_request(tenant_id, network_id, port_id,
1397+ format='xml', detail=False):
1398 method = 'GET'
1399- path = "/tenants/%(tenant_id)s/networks/" \
1400- "%(network_id)s/ports.%(format)s" % locals()
1401+ detail_str = detail and '/detail' or ''
1402+ path = "/tenants/%(tenant_id)s/networks/%(network_id)s" \
1403+ "/ports/%(port_id)s%(detail_str)s.%(format)s" % locals()
1404 content_type = "application/%s" % format
1405 return create_request(path, None, content_type, method)
1406
1407
1408 def show_port_request(tenant_id, network_id, port_id, format='xml'):
1409- method = 'GET'
1410- path = "/tenants/%(tenant_id)s/networks/%(network_id)s" \
1411- "/ports/%(port_id)s.%(format)s" % locals()
1412- content_type = "application/%s" % format
1413- return create_request(path, None, content_type, method)
1414+ return _show_port_request(tenant_id, network_id, port_id, format)
1415+
1416+
1417+def show_port_detail_request(tenant_id, network_id, port_id, format='xml'):
1418+ return _show_port_request(tenant_id, network_id, port_id,
1419+ format, detail=True)
1420
1421
1422 def new_port_request(tenant_id, network_id, port_state,
1423@@ -78,7 +118,7 @@
1424 path = "/tenants/%(tenant_id)s/networks/" \
1425 "%(network_id)s/ports.%(format)s" % locals()
1426 data = custom_req_body or port_state and \
1427- {'port': {'port-state': '%s' % port_state}}
1428+ {'port': {'state': '%s' % port_state}}
1429 content_type = "application/%s" % format
1430 body = data and Serializer().serialize(data, content_type)
1431 return create_request(path, body, content_type, method)
1432@@ -97,7 +137,7 @@
1433 method = 'PUT'
1434 path = "/tenants/%(tenant_id)s/networks" \
1435 "/%(network_id)s/ports/%(port_id)s.%(format)s" % locals()
1436- data = custom_req_body or {'port': {'port-state': '%s' % port_state}}
1437+ data = custom_req_body or {'port': {'state': '%s' % port_state}}
1438 content_type = "application/%s" % format
1439 body = Serializer().serialize(data, content_type)
1440 return create_request(path, body, content_type, method)
1441@@ -116,7 +156,7 @@
1442 method = 'PUT'
1443 path = "/tenants/%(tenant_id)s/networks/" \
1444 "%(network_id)s/ports/%(port_id)s/attachment.%(format)s" % locals()
1445- data = {'port': {'attachment-id': attachment_id}}
1446+ data = {'attachment': {'id': attachment_id}}
1447 content_type = "application/%s" % format
1448 body = Serializer().serialize(data, content_type)
1449 return create_request(path, body, content_type, method)

Subscribers

People subscribed via source and target branches