Great work Salvatore. We'll get that branch reviewed ASAP, so it is ready if the rest of the community agrees that (b) is the right approach.
Brad did some testing of reverting the patch last night too and he likewise found that it was sufficient to get things working. We could merge that change into quantum/trunk, then I can merge it into quantum/diablo. I was thinking that if this change goes in, we should also update the API doc to indicate that for v1.0 of the API we return 200 for create operations? We can decide if for v1.1 and beyond we want to change it to 202 in order to be more consistent. Thanks, Dan p.s. the second issue that you found (where id is not included in the dictionary if an attachment is not set) highlights a gap in our API doc, as the doc does not currently indicate what to expect if an attachment is not yet set. I'm also going to add a test case around this, as I didn't see one (though may have missed it). On Fri, Sep 23, 2011 at 5:14 AM, Salvatore Orlando < salvatore.orla...@eu.citrix.com> wrote: > I too vote for option b, as IMHO it is the only viable one. > > In order to partially make up for not having completed the API > documentation, I have pushed a branch to revert the problem causing the > client library in nova to not deserialize responses. > > Merge prop: > https://code.launchpad.net/~salvatore-orlando/quantum/quantum-reverted/+merge/76719 > Bug report: https://bugs.launchpad.net/quantum/+bug/857315 > > That branch also slightly changes the way in which the response for show > attachment is generated. I make sure than an 'id' attribute is always added, > even if there is no attachment. > In that case, the value would be "None". > > This is because of the following iteration in get_port_by_attachment ( > http://bazaar.launchpad.net/~hudson-openstack/nova/trunk/view/head:/nova/network/quantum/quantum_connection.py#L101 > ) > > for p in port_list_resdict["ports"]: > port_id = p["id"] > port_get_resdict = self.client.show_port_attachment(net_id, > port_id, tenant=tenant_id) > if attachment_id == port_get_resdict["attachment"]["id"]: > return (net_id, port_id) > > I think that the right part of the comparison in the if statement will fail > if the API does not return attachments object with id attribute. This > happened to me as I had create a few ports on the network before launching > instances. > > I tested the branch against nova and I have been able to successfully > launch instances, as well as create and manage networks and ports, both > through nova-manage and the dashboard. > > Cheers, > Salvatore > > > -----Original Message----- > > From: Brad Hall [mailto:b...@nicira.com] > > Sent: 23 September 2011 05:07 > > To: Dan Wendlandt > > Cc: Salvatore Orlando; netstack@lists.launchpad.net > > Subject: Re: [Netstack] Running Nova with Quantum (OVS Plugin) > > > > Hey Dan, > > > > Replies below.. > > > > On Thu, Sep 22, 2011 at 6:33 PM, Dan Wendlandt <d...@nicira.com> wrote: > > > > > > > > > On Thu, Sep 22, 2011 at 5:25 PM, Brad Hall <b...@nicira.com> wrote: > > >> > > >> > > > >> > o QuantumManager.allocate_for_instance was failing because the > > >> > JSON response from Quantum was not being serialized. I then looked > > >> > at network.quantum.client.py and found that if the response code > > >> > was 202 the response was not deserialized. 202 is exactly the code > > >> > returned by the API for create ops (there was a bug fixed before > > >> > rbp on this). I believe the client shipped with quantum avoids > > >> > deserialization for error code 204. > > >> > To > > >> > work around the issue, I changed the code in client.py. > > >> > > >> This is fixed in a branch proposed here: > > >> https://code.launchpad.net/~bgh/nova/qmanager-rbp-trunk > > >> > > >> Unfortunately, it didn't get enough reviews and didn't make it into > > >> nova in the diablo timeframe. > > > > > > > > > That's really unfortunate. I'm guessing this is due to the changes in > > > this branch? > > > https://code.launchpad.net/~salvatore- > > orlando/quantum/bug834013/+merge > > > /73788 > > > It likes like this branch came in to Quantum just a day after the > > > QuantumManager code went in to Nova :( My understanding is that this > > > branch changes the return code for API create operations from 200 > > > (will be deserialized by client.py in Nova) to 202 (will not be > > > deserialized by client.py in Nova). > > > It would be unfortunate to release a version of Quantum for diablo > > > that doesn't work with the Quantum integration in the Nova diablo > > > release, especially given that documentation for how to run Quantum is > > > built around running it with the QuantumManager. Given that a lot of > > > people install nova from packages, it does not seem wise to have them > > > patch the client.py in nova or install from an alternative branch. > > > Since Brad's branch with the fix didn't get into nova, our two options > > > seem to be: > > > a) live with it. > > > b) tweak Quantum to work with diablo nova. Looking at the patch it > > > seems that this would be pretty simple, however, I believe the > > > motivation for changing the code from returning 200 to 202 was that > > > 202 was more inline with other OpenStack APIs, which seems like a good > > > goal. Hence, we would also need to update the 1.0 API doc to specify > > > 200, then switch to using 202 in v1.1 of the API. > > > > I vote for option b too. > > > > > I think we can reasonably delay the release a day to get people's > > > input on this. > > > My bias is toward having working code to minimize the problems folks > > > have when first trying to play with Nova + Quantum, even if it comes > > > at the expense of API purity, but others may not agree. Thoughts? > > > Please try to get your thoughts in by noon tomorrow, so we can make a > > > decision and release by 5pm Pacific tomorrow. If necessary, we may > > > want to hop on IRC and have a chat, but by default we'll just have the > > > conversation on the email list. > > > > I think one day is a small cost to make sure we have working software out > in > > the wild. If people hit errors quickly when running it they'll just stop > using it > > and that isn't what we want. > > > > > Thanks! > > > Dan > > > p.s. and to think: some people thought we weren't going to have a > > > last minute release emergency :) > > > > Pfft.. That never happens :) > > > > Thanks, > > Brad > -- ~~~~~~~~~~~~~~~~~~~~~~~~~~~ Dan Wendlandt Nicira Networks, Inc. www.nicira.com | www.openvswitch.org Sr. Product Manager cell: 650-906-2650 ~~~~~~~~~~~~~~~~~~~~~~~~~~~
-- Mailing list: https://launchpad.net/~netstack Post to : netstack@lists.launchpad.net Unsubscribe : https://launchpad.net/~netstack More help : https://help.launchpad.net/ListHelp
