On Fri, Sep 23, 2011 at 8:51 AM, Dan Wendlandt <d...@nicira.com> wrote:
> > 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). > I take it back, this WAS already mentioned in the API doc: "If no attachment is currently plugged into the port, the operation does not return any attachment identifier in the response. The response will contain an empty attachment element." I will add a test case for this in test_api.py. As discussed in the review thread, I think it makes sense to keep the implementation + document as is for Diablo and view this as a bug in QuantumManager. Common use cases of QuantumManager will still work, so I am not too worried about it. Thanks, Dan > > > > 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 > ~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > -- ~~~~~~~~~~~~~~~~~~~~~~~~~~~ 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
