Earlier  
Posted Nick Remark
#openstack-nova - 2018-06-06
13:43:39 mriedem dansmith: it adds a new ip_addresses field to the response which is a list of fixed ips rather than the first one from the list
13:43:47 mriedem we only take the first fixed ip today
13:43:53 mriedem which is the ip_address field
13:43:58 sahid jangutter: hum... i don't understand the note
13:43:59 mriedem this exposes the list of fixed ips
13:44:14 mriedem and still maintains the single ip_address field - which arguably we don't need to do since it's versioned
13:44:41 mriedem i don't know how sensitive apps are to doing version discovery with metadata
13:44:53 dansmith mriedem: this certainly looks like we're asserting multiple interfaces and ips in the test no? https://github.com/openstack/nova/blob/4b333b989dfc778a8b61db4a1b8552e988a10471/nova/tests/unit/network/test_network_info.py
13:44:59 dansmith er hang on
13:45:04 dansmith https://github.com/openstack/nova/blob/4b333b989dfc778a8b61db4a1b8552e988a10471/nova/tests/unit/network/test_network_info.py#L1118
13:45:30 mriedem https://github.com/openstack/nova/blob/4b333b989dfc778a8b61db4a1b8552e988a10471/nova/tests/unit/network/test_network_info.py#L1121
13:45:30 mriedem dansmith: how so?
13:45:35 mriedem that's a single ipv4 fixed ip
13:45:43 mriedem but you can have multiple fixed ips per port in neutron
13:45:53 mriedem which is what this spec is trying to expose via network_data.json
13:46:04 dansmith oh,
13:46:10 dansmith I thought the spec was saying it was the subsequent networks and the ips there that it needed to add
13:46:12 jangutter sahid: Back in the day, when I wrote that, my rationale was something like: the VIF is used to generate the xml for libvirt, the port_profile is used to find the representor and plug it into the bridge.
13:46:16 mriedem there is also something in the spec about multiple subnet routes which i didn't fully follow
13:46:51 mriedem i believe the networks list is per-vif (port)
13:46:53 dansmith I see, multiple fixed per port
13:47:00 dansmith is what they're enabling
13:47:01 mriedem similar to how the server "addresses" field works in the compute api
13:47:08 mriedem dansmith: yeah
13:47:11 dansmith sorry, I just skimmed when you sent and haven't really read it
13:47:32 mriedem so you didn't see the dan smith easter egg in there yet
13:48:17 jangutter sahid: Also, the symmetry will break either way, there's only so far you can take the abstraction, I'm agnostic on this, but if the pattern is continued, then at least the os-vif plugins look similar.
13:49:09 dansmith mriedem: well now I can't approve until I find something I think qualifies
13:52:15 mriedem melwitt: ack, and thanks for hitting those placement policy changes
13:53:52 kashyap lyarwood: Seems like your recheck has resulted in a success for the 'legacy-tempest-dsvm-neutron-full'?
13:54:02 kashyap (For this: https://review.openstack.org/#/c/571433/)
13:54:20 kashyap But there's other failures, though
13:57:01 kashyap (Like this one 'test_volume_extend_when_volume_has_snapshot')
14:01:26 openstackgerrit Merged openstack/nova-specs master: Multiple Fixed-IPs support in network information https://review.openstack.org/312626
14:04:11 mriedem mdbooth: artom: replied in the ML, but if we're going to block retype/live migration of multiattach volumes with >1 read/write attachment, that should happen in the cinder API
14:04:34 mriedem failing in the swap volume api in nova is about 3/4 of the way through the operatoin which is a real shitty place to be failing
14:05:36 mdbooth mriedem: Reading. Was just responding to jaypipes understandable horror at this messy interface.
14:06:12 artom mriedem, unless it's called directly, no? Which apparently some people are doing
14:07:05 mriedem mgagne: https://blueprints.launchpad.net/nova/+spec/multiple-fixed-ips-network-information is now approved so you can post code
14:07:15 mriedem artom: who is doing that?
14:07:29 artom mriedem, see mdbooth's reply :)
14:07:31 mriedem swap volume in nova is an admin-only api
14:07:42 mdbooth mriedem: We really do have customers who call swap volume directly.
14:07:42 artom We also got a downstream but about it
14:07:48 mdbooth More than one.
14:08:03 mriedem why?
14:08:29 kashyap Presumably to move storage from one type (local) to another (NFS)?
14:08:33 mdbooth I still haven't nailed it down, but I *think* it's a workaround related to BFV.
14:08:41 artom mriedem, I mean, I would agree, it's a weird thing to do, but we can't honestly put an API out there and get defensive when people use it in unexpected ways
14:08:42 mriedem kashyap: that would be a retype
14:08:47 mriedem which you should initiate through cinder
14:08:54 kashyap Ah, damned if I ever remember the names
14:09:13 mdbooth mriedem: But regardless, even if we think it's dumb, it's a public api and they're using it.
14:10:04 mriedem awesome
14:10:16 mdbooth Indeed
14:10:18 mriedem i made it 70 minutes today before hating my life
14:10:25 mdbooth \o/
14:10:26 kashyap This morning we chatted here about mdbooth's e-mail (which I haven't pulled down yet). At least in context of what libvirt APi does by default
14:10:38 jaypipes mdbooth: my annoyance is that this is a libvirt/QEMU-specific behaviour/implementation that is leaking out of the Compute API.
14:10:59 kashyap jaypipes: By _default_ libvirt doesn't copy shared disks
14:11:02 mdbooth jaypipes: That was the thrust of my reply. It's not libvirt/qemu specific at all.
14:11:09 kashyap But if you _ask_ for it, it will migrate it
14:11:09 jaypipes mdbooth: BTW, where exactly does "swap volume" imply or denote that there is a copy operation going on? :(
14:11:16 mriedem mdbooth: the only virt driver that implements this is libvirt
14:11:21 mdbooth jaypipes: It could be implemented by any hypervisor, and they'd have to do the copy.
14:11:22 jaypipes mdbooth: yes, it is.
14:11:32 mriedem same with guest-assisted volume snapshot
14:11:42 mdbooth jaypipes: I replied in depth on the ML, anyway.
14:11:50 jaypipes yes, I read your reply.
14:11:57 mriedem i have no idea how non-libvirt drivers handle volume retype/live migration either
14:12:07 jaypipes mriedem: they don't. wisely.
14:12:10 mdbooth But Cinder can't do this, regardless of which hypervisor is attached.
14:12:27 jaypipes mdbooth: it's the "live" part that Cinder can't do, yes?
14:12:28 mriedem i must be missing something but i don't see any reply to jaypipes from mdbooth in the ML
14:12:38 mdbooth jaypipes: No, that's just an optimisation.
14:12:49 mdbooth jaypipes: If anything is attached, Cinder needs to ask Nova for help.
14:12:58 mdbooth Nova could just pause, copy, unpause
14:12:59 kashyap jaypipes: To your "swap volume" denoting copy operation, you wouldn't know it, unless you look at the Nova _swap_volume() method
14:13:01 jaypipes mdbooth: then I don't understand what is different between the pause VM, volume migrate, start VM.
14:13:10 mriedem i wonder when k8s is going to implement swap volume for cinder to use
14:13:14 mriedem that will be awesome
14:13:23 kashyap jaypipes: And then, map it to libvirt's blockRebase() API, which is called by _swap_volume().
14:13:25 mdbooth jaypipes: How would you prevent unpause during the volume migrate?
14:13:29 jaypipes kashyap: no... unless you look at the LIBVIRT driver _swap_volume() method.
14:13:33 jaypipes kashyap: which is my point.
14:13:40 kashyap jaypipes: That's what I said :-)
14:13:44 openstackgerrit Balazs Gibizer proposed openstack/nova master: Transfer port.resource_request to the scheduler https://review.openstack.org/567268
14:14:00 jaypipes kashyap: no, you said "the Nova _swap_volume() method".
14:14:21 kashyap jaypipes: Yes, you're right - Nova's libvirt driver.
14:14:43 jaypipes mdbooth: it's an administrative action that involves multiple orchestrated pieces.
14:15:40 mdbooth jaypipes: Note that we're redefining swap_volume here, which is a bit out of scope. My concern relates to multiattach.
14:15:45 jaypipes mdbooth: and just because libvirt/QEMU has some magic foo to do all that orchestration in a (sometimes) live fashion, we've made the Compute API look like the libvirt/QEMU API and embedded expectations of full coordination/orchestration into our API. and that's wrong IMHO.
14:16:17 mdbooth However, there are 2 parts to it.
14:16:41 MultipleCrashes Please have a look at this review and leave a feedback : https://review.openstack.org/#/c/570370/
14:16:45 jaypipes mdbooth: yes, I understand your concern about multiattach and artom's suggestion of just barfing back a 400 Bad Request when any of the source or dest volumes in the swap operation are multiattach is a perfectly reasonable "solution" to that.
14:16:49 mdbooth If we removed swap volume and implemented this with pause/resume, we would still need an additional api in nova to 'lock' the pause.
14:17:08 jaypipes mdbooth: I'm just annoyed at everything, sorry.
14:17:09 mdbooth Otherwise, orchestration or not, a concurrent operation could unpause it.
14:17:26 artom jaypipes, would you like a lawn and some kids to yell at? ;)
14:17:35 jaypipes mdbooth: precisely. which is why it belongs in an orchestration layer, not the virt driver layer :)

Earlier   Later