| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-10-10 | |||
| 13:43:01 | sean-k-mooney | dansmith: that is part of my argument as to why we should version for compoltion and inheritenc but since the has does not change for compostion versiosn its not enforece by our tests | |
| 13:43:05 | dansmith | sean-k-mooney: yeah that was a total disaster | |
| 13:43:14 | stephenfin | which wouldn't be the case normally because this line would be false for direct https://github.com/openstack/nova/blob/6bf11e1dc14afad78b11d980c2544a3dc41579ff/nova/virt/libvirt/vif.py#L137 | |
| 13:43:26 | dansmith | sean-k-mooney: it doesn't need to change | |
| 13:43:29 | sean-k-mooney | dansmith: the lockstep stuff. it was a pain but it worked | |
| 13:43:39 | dansmith | sean-k-mooney: it was unnecessary for us | |
| 13:43:49 | stephenfin | moshele: If that makes sense? I'm just thinking it's a latent bug. I'll test myself but doing so requires me setting up an environment :) | |
| 13:43:56 | dansmith | maybe for you because you don't have the same communication we have between nodes | |
| 13:44:48 | sean-k-mooney | dansmith: it maye if version 1 of my containing clase used version 1 of a composed calss and version 2 of the conatine uses version 5 then downgrading the containing class to 1 results in it having an object filed of 5 | |
| 13:44:52 | moshele | stephenfin: not sure, but you maybe right here | |
| 13:45:26 | dansmith | sean-k-mooney: yep, which is fine, because the objects are forward compatible | |
| 13:45:45 | mriedem | gibi: i'm late to this party but a few comments inline https://review.openstack.org/#/c/609330/ | |
| 13:45:47 | dansmith | sean-k-mooney: and if there's some reason, the containing class can downlevel the version it has | |
| 13:45:49 | mriedem | i haven't read the ML thread yet | |
| 13:46:00 | mriedem | pooja_jadhav: go ahead | |
| 13:46:53 | moshele | stephenfin: please update me with your result | |
| 13:46:53 | sean-k-mooney | dansmith: today we dont track that it should down level the object filed to 2 when it downlevels to 1 which is the gap | |
| 13:47:08 | stephenfin | moshele: Yup, will open a bug if it's an issue | |
| 13:47:42 | dansmith | sean-k-mooney: we don't backlevel it if and only if the requesting party says that they support version 5 of the sub object | |
| 13:47:43 | sean-k-mooney | dansmith: its totally solveable in the make_compatible function just we dont enforce it today and we should when we start passing os-vif objects over the api | |
| 13:47:50 | dansmith | sean-k-mooney: which wouldn't happen, and thus we'd backport both | |
| 13:47:53 | gibi | mriedem: you are not late at all | |
| 13:48:07 | dansmith | sean-k-mooney: sure we don't enforce it | |
| 13:48:10 | moshele | stephenfin: is 'hw_vif_type=virtio' in image metadata per network interface? | |
| 13:48:20 | dansmith | sean-k-mooney: that's why I'm saying if you are making objects where that matters, then you can backlevel it | |
| 13:48:25 | stephenfin | moshele: I don't think so. I don't see how it could be | |
| 13:48:40 | sean-k-mooney | dansmith: this isnt in relation to RPC by they way. this is relation to passing os-vif object betwen nova an neutron via the rest api | |
| 13:48:51 | stephenfin | moshele: Assuming you mean can it be configured per interface. I assume it affects all network interfaces | |
| 13:49:01 | moshele | stephenfin: I see, so we need to ignore it or reject it | |
| 13:49:06 | dansmith | sean-k-mooney: I know that, which is why I said you may have less flexibility because you don't control the communication in the same way | |
| 13:49:12 | stephenfin | For direct SR-IOV, I would imagine | |
| 13:49:32 | sean-k-mooney | dansmith: sure. we have added code in the past to do this and taken it out as part of code review because it did not go over rpc | |
| 13:49:34 | dansmith | sean-k-mooney: fwiw, the relation mapping stuff is all still in o.vo because it's a library, so you can use it if you really think it's necessary | |
| 13:49:46 | moshele | stephenfin: for VNIC_TYPES_DIRECT_PASSTHROUGH | |
| 13:49:52 | stephenfin | moshele: yeah | |
| 13:50:14 | sean-k-mooney | dansmith: ya we can but its also easy to do this in the make compatible fuction without that | |
| 13:50:40 | sean-k-mooney | jangutter: does any of this help? | |
| 13:51:10 | openstackgerrit | sean mooney proposed openstack/os-vif master: add support for generic tap device plug https://review.openstack.org/602384 | |
| 13:51:11 | openstackgerrit | sean mooney proposed openstack/os-vif master: clean up ip_command interface https://review.openstack.org/609414 | |
| 13:51:36 | jangutter | sean-k-mooney: I'm still downleveling the conversation to the simple version I can understand :-p | |
| 13:52:03 | gibi | mriedem: good point about the pre 2.29 and pre 2.30 microversion. I have to think that through. | |
| 13:52:41 | mriedem | gibi: i don't see operators on this thread either in the ML...has anyone reached out to the ops community to see if they are cool with dropping support for the force parameter? and if not, why not. | |
| 13:53:06 | gibi | mriedem: bauzas forwarded it to the ops I think | |
| 13:53:07 | mriedem | although as noted you can still use the force flag with the older microversion, or use 2.1 | |
| 13:53:11 | dansmith | jangutter: so you might be assuming there's more magic going on here than there is | |
| 13:53:19 | mriedem | this spec is really just about signaling | |
| 13:53:25 | dansmith | jangutter: you can't define that a field is an object at (or below or above) a particular version | |
| 13:53:25 | mriedem | like when we deprecated personality files | |
| 13:53:26 | bauzas | gibi: mriedem: yup but no answers yet | |
| 13:53:42 | bauzas | gibi: and I asked you about two separate microversions in the change | |
| 13:53:45 | jangutter | dansmith: right, this comes into play when serializing | |
| 13:53:55 | dansmith | jangutter: within a scope, it's assumed that you support a set of object schemas (by name) at a particular version, with compatibility for older versions | |
| 13:53:57 | mriedem | bauzas: no one but you wants 2 microversions for this | |
| 13:54:07 | gibi | bauzas: it seems to me that others more like having a single microversion instead | |
| 13:54:08 | mriedem | 2 microversions for the same thing is excessive | |
| 13:54:36 | mriedem | as efried noted in the spec review, we don't do multiple microversions for something just because it touches multiple APIs | |
| 13:54:38 | dansmith | jaypipes: so when we serialize today, we have the object and a list of versions that the other side supports, and we backlevel everything in the tree to the versions they support, as needed | |
| 13:54:41 | dansmith | er jangutter ^ | |
| 13:54:47 | mriedem | otherwise i would have had multiple microversions for volume multiattach and removing personality files | |
| 13:55:07 | dansmith | jangutter: if you don't have that manifest of supported versions, then you'd have to make inferences based on the version of the parent you're pinned to, which is what we used to do | |
| 13:55:40 | dansmith | jangutter: like this: https://github.com/openstack/nova/blob/kilo-eol/nova/objects/instance.py#L257 | |
| 13:55:51 | dansmith | jangutter: if you set those up on your object then o.vo will still respect them | |
| 13:55:58 | jangutter | dansmith: yep. the trick comes when downlevelling an object that has members containing other objects which are versioned. | |
| 13:56:15 | dansmith | jangutter: right, but that's what this obj_relationships is for | |
| 13:56:17 | sean-k-mooney | dansmith: that is assume that the latest version of each object both side knows about is the desired version correct | |
| 13:56:38 | dansmith | jangutter: note that it specifies which versions the contains-another-object fields refer to | |
| 13:56:44 | bauzas | mriedem: gibi: okay, just the fact that we had 2 different versions previously (2.29 and 2.30) | |
| 13:56:47 | jangutter | dansmith: I've seen that mapping, the question is, do we need/want it in os-vif? | |
| 13:57:11 | dansmith | sean-k-mooney: it means within a scope you don't randomly upgrade one object and not another yeah | |
| 13:57:34 | dansmith | sean-k-mooney: which is what this was designed to do, not for "I loaded a new plugin which loaded one new object into a sea of older ones" | |
| 13:58:15 | sean-k-mooney | dansmith: right... so we might have to deal with the lather in os-vif but perhaps we dont | |
| 13:58:39 | dansmith | yeah I dunno | |
| 13:58:44 | mriedem | bauzas: we had 2 different versions previously because of all the related plumbing those required | |
| 13:59:04 | mriedem | this is a simple schema change | |
| 13:59:09 | sean-k-mooney | dansmith: did any of this impact out of tree virt drivers on the nova side? or are all the OVOs managed above the virt dirver level | |
| 13:59:16 | bauzas | mriedem: okay, fair then | |
| 13:59:27 | dansmith | sean-k-mooney: we don't support out of tree virt drivers | |
| 14:00:18 | sean-k-mooney | dansmith: :) well thats one answer. but i think we are still fine. plugins are not allowed to provide vif types or profiles they can only consume them so i think we wont have the mixed case | |
| 14:00:38 | dansmith | oh okay I thought that was the point of os-vif | |
| 14:00:39 | dansmith | but cool if not | |
| 14:01:05 | sean-k-mooney | plugins can consume the datamodels but not extend them | |
| 14:02:40 | sean-k-mooney | dansmith: that said kuryr-kubernetes are violating that contract https://github.com/openstack/kuryr-kubernetes/blob/master/kuryr_kubernetes/objects/vif.py | |
| 14:03:08 | mriedem | bauzas: i think you wanted to copy the ops list but didn't | |
| 14:03:09 | mriedem | "adding openstack-operators@ accordingly." | |
| 14:03:12 | mriedem | the ops list isn't on copy | |
| 14:03:35 | sean-k-mooney | dansmith: i told them that this was not be supported a few cycles ago and we will be moving those vif types before nova and neutron start useing os-vif objects to negociate bindings | |
| 14:03:37 | mriedem | oh i see, "Shit, I forgot to add openstack-operators@..." | |
| 14:03:40 | bauzas | mriedem: I had a problem with my other gmail address | |
| 14:03:53 | bauzas | hence the three emails | |
| 14:04:24 | bauzas | hopefully the single ML address will simplify it | |
| 14:04:28 | dansmith | sean-k-mooney: ack | |
| 14:06:57 | jangutter | dansmith, sean-k-mooney, jaypipes: thanks very much for this discussion, I think it might be a good time to explicitly fix os-vif into the simpler model rather than allowing a possible version explosion. | |
| 14:07:59 | sean-k-mooney | jangutter: im not sure how you would do that | |
| 14:08:07 | jaypipes | jangutter: well, since nothing yet uses the object-over-http I wouldn't mind keeping all the versions at 1.0 at this point. | |
| 14:08:14 | sean-k-mooney | i dont think its partacally complex | |
| 14:08:25 | jaypipes | but I need to run away now for a half hour. back soon. | |
| 14:09:07 | sean-k-mooney | jaypipes: we proably can make the all 1 but i would be fine to bump them all to 2.0 or something wehn we use them over http | |
| 14:09:37 | jaypipes | sean-k-mooney: ++ | |
| 14:09:48 | jaypipes | sean-k-mooney: and after the version manifest stuff is added in... | |
| 14:11:30 | sean-k-mooney | jaypipes: yes assuming that is needed | |