| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-03-12 | |||
| 19:06:59 | cfriesen | dansmith: as per discussion at PTG: https://bugs.launchpad.net/nova/+bug/1754782 I won't have any time to work on it till next week at the earliest. | |
| 19:07:00 | openstack | Launchpad bug 1754782 in OpenStack Compute (nova) "we skip critical scheduler filters when forcing the host on instance boot" [Undecided,New] | |
| 19:13:42 | melwitt | jaypipes: thanks for writing up the PTG summary for placement, good stuff | |
| 19:14:30 | jaypipes | melwitt: no prob. thanks to mriedem and gibi who double-checked stuff on it. | |
| 19:14:57 | melwitt | mriedem++ gibi++ | |
| 19:16:20 | sean-k-mooney | QQ just working on https://bugs.launchpad.net/nova/+bug/1747496 did we remove the old default config values from MTU when not set on the port from nova? i think we did but just setting | |
| 19:16:21 | openstack | Launchpad bug 1747496 in OpenStack Compute (nova) "MTUs are not set for VIFs if using kernel ovs + hybrid plug = false" [Medium,Confirmed] - Assigned to sean mooney (sean-k-mooney) | |
| 19:16:25 | sean-k-mooney | *checking | |
| 19:20:23 | openstackgerrit | Eric Fried proposed openstack/nova-specs master: Allow for merging traits https://review.openstack.org/552122 | |
| 19:20:34 | efried | dansmith, jaypipes, cdent, sean-k-mooney, edleafe: ^ | |
| 19:20:40 | sean-k-mooney | hum it does appare to be in https://github.com/openstack/nova/blob/master/nova/conf/network.py or in .../neutron.py so i guess they are gone and were move to os-vif https://github.com/openstack/os-vif/search?utf8=%E2%9C%93&q=network_device_mtu&type= | |
| 19:20:46 | efried | I didn't make any changes for aggregates. | |
| 19:21:01 | cdent | efried: thanks, in the queue | |
| 19:23:02 | sean-k-mooney | efried: im just heading home but i addem myself to the review and ill leave it open in a tab for tomorow. ill give it a quick skim before i leave however | |
| 19:24:18 | dansmith | efried: looks okay to me | |
| 19:25:46 | sean-k-mooney | efried: why did you start with "Traits are special. Rather than overwriting the entire set of traits" this statement also applies to RPs and Aggregates | |
| 19:26:17 | efried | sean-k-mooney: Because I wrote this before it occurred to me that we should be doing the same for the other bits. | |
| 19:26:21 | sean-k-mooney | efried: the compute node RP is not owned by nova so nova cant override all child RPs of the compute node either | |
| 19:26:29 | sean-k-mooney | efried: ah ok | |
| 19:26:51 | efried | sean-k-mooney: But I also wanted to do this one isolated because I don't think we're done discussing the others. | |
| 19:27:07 | efried | The compute node RP *is* owned by nova. | |
| 19:27:24 | efried | sean-k-mooney: And *some* of its children may also be. | |
| 19:27:51 | dansmith | yeah, the compute node RP is _definitely_ owned by nova :) | |
| 19:28:05 | efried | In general virt needs to be smart enough not to muck with stuff it doesn't own. But originally we *thought* that meant virt owns everything about the RPs it owns. | |
| 19:28:07 | sean-k-mooney | efried: that has issues wich other services want to tag it with traits | |
| 19:28:08 | dansmith | agreed nova can't blow away all the providers underneath, unless it's deleting the compute node | |
| 19:28:28 | sean-k-mooney | dansmith: well event then i dont think it can | |
| 19:28:36 | efried | Which is why we already have ProviderTree methods to add/remove providers in the tree. | |
| 19:28:42 | efried | ...individually. | |
| 19:28:49 | dansmith | sean-k-mooney: it has to be able to delete it and the subtree, IMHO | |
| 19:29:44 | sean-k-mooney | dansmith: if i have a converged deployment where my compute nodes are also cinder storage pool providers the the root node of the tree would be a parent of the nova resources and the cinder resouces | |
| 19:29:52 | efried | Agree. E.g. if neutron created bandwidth RPs under a PF, and compute removes the PF, the bandwidth providers are no longer relevant. | |
| 19:30:05 | sean-k-mooney | which is why i dont think the root node can be owned by nova | |
| 19:30:14 | efried | sean-k-mooney: I don't think that's a model we should support. | |
| 19:30:16 | dansmith | sean-k-mooney: in that case the cinder providers are not servicing other computes, right? then it's okay to delete it | |
| 19:30:28 | dansmith | sean-k-mooney: if they are servicing others, then the cinder pool should not be a child of the compute node | |
| 19:30:45 | sean-k-mooney | dansmith: no the could be and associated with other computes via a sharing aggregate | |
| 19:30:58 | dansmith | sean-k-mooney: if they are, they should not be a child of the compute IMHO | |
| 19:31:20 | jaypipes | efried: I agree with both edleafe and dansmith on that update to the u-p-t spec | |
| 19:31:28 | dansmith | sean-k-mooney: the compute node RP is not the physical computer, it's the nova service and the hypervisor/virt driver underneath | |
| 19:32:00 | sean-k-mooney | dansmith: perhaps i had always just envisioned that the server itself was a resouce provider all services could create childeren under without having to worry about it beeing deleted by e.g. nova | |
| 19:32:24 | dansmith | sean-k-mooney: not, IMHO.. if we need to model that relationship then the compute node RP would be a child to "the computer" I think | |
| 19:32:29 | jaypipes | dansmith: well, in the case of a hypervisor host, yes. :) for ironic, of course, that's different. | |
| 19:32:33 | dansmith | but I hope we don't need to do that | |
| 19:32:48 | sean-k-mooney | dansmith: yes well that has some advantages | |
| 19:32:58 | dansmith | jaypipes: yeah I know, but thats special | |
| 19:33:06 | jaypipes | ack | |
| 19:34:06 | dansmith | jaypipes: ironic nodes wouldn't be cinder providers in the same hierarchy, and even better, the computer running the nova service for those, if it were a cinder pool shared with other nodes, should be modeled peer to the compute service's RP I think | |
| 19:34:26 | sean-k-mooney | dansmith: the only issue i have really with saying that nova owns the root RP is that we may need to have multiple root nodes for the same server depending on the service | |
| 19:34:48 | dansmith | sean-k-mooney: no, nova owns the compute RP.. if you decide to make that the root of your cinder provider, then it is a root :) | |
| 19:34:59 | dansmith | and you get what's coming to you in that case :) | |
| 19:35:06 | dansmith | which is nova may delete the root that it owns | |
| 19:35:30 | sean-k-mooney | dansmith: yes but the we have too trees for the same phyical server and we dont currently have a way to model that they are the same server | |
| 19:35:37 | sean-k-mooney | i guess maybe with an aggregate | |
| 19:35:47 | dansmith | sean-k-mooney: they're not the same server, | |
| 19:36:02 | sean-k-mooney | dansmith: why not? | |
| 19:36:10 | efried | dansmith, jaypipes: Please confirm: you want two separate methods, with *traits args | |
| 19:36:10 | dansmith | sean-k-mooney: they represent services that happen to be on the same box.. if we need to model that they're on the same box (i.e. below a parent provider) then that's a thing | |
| 19:36:17 | dansmith | efried: that's fine | |
| 19:36:18 | jaypipes | efried: yes please | |
| 19:36:26 | efried | ight | |
| 19:36:34 | jaypipes | add_traits() and remove_traits() would be my preference. | |
| 19:36:43 | jaypipes | efried: ^ | |
| 19:36:45 | dansmith | efried: btw, excellent job not having your head explode here. kudos :) | |
| 19:37:19 | efried | jaypipes: Cool, swhere I was gonna go. dansmith: ack, thx; head may yet explode over aggregates thing, but so far so good. | |
| 19:37:21 | dansmith | efried: on friday of PTG, I thought maybe your eyeballs were about to shoot from your head at lethal velocity, which scared me when they were pointed at me | |
| 19:37:43 | sean-k-mooney | dansmith: ok anyway thats a little off topic. i do think we may want to have a parent of the compute node RP at some point then but let cross that bridge only when we need too | |
| 19:38:11 | dansmith | sean-k-mooney: we may and that would solve the problem of this ownership of the tree for deletes problem, that's what I'm saying :) | |
| 19:38:32 | dansmith | until that point, if you create a provider underneath someone else's provider, I think you need to know that you're at the other's mercy | |
| 19:38:35 | efried | dansmith: It ain't my eyeballs that should worry you :P FWIW, I'm still uncomfortable with this, but I don't see a better alternative. | |
| 19:38:46 | dansmith | um. | |
| 19:39:46 | dansmith | oof | |
| 19:40:48 | sean-k-mooney | dansmith: oh yes it would solve that issue. it would raise the question of which tree i shoudl create the resouce other for thinks like bandwith. anyway i better run before the stores close at 8 | |
| 19:41:14 | dansmith | yeah | |
| 19:52:19 | mnaser | hm, does anyone have any idea behind the historical reason why ComputeCapabilitiesFilter is hard-coded to use instancetypes (not taking image properties into consideration?) | |
| 19:52:50 | mnaser | context: customer *needs* instances with a specific cpu flag, i'd like to minimize the number of flavors so i was hoping i can have a image flag requesting that cpu flag | |
| 19:53:00 | mnaser | vs having to create a new instance type for this use case specifically | |
| 19:53:48 | efried | jaypipes: Considering removing ProviderTree.set_traits/set_aggregates entirely. | |
| 20:00:18 | openstackgerrit | Eric Fried proposed openstack/nova-specs master: Allow for merging traits and aggregates https://review.openstack.org/552122 | |
| 20:00:32 | efried | dansmith, jaypipes, cdent, edleafe: ^ with changes as requested, plus for aggregates | |
| 20:03:16 | cdent | ✔ | |
| 20:16:00 | jaypipes | efried: +2 from me. thx | |
| 20:16:09 | efried | thx | |
| 20:21:54 | edleafe | efried: a little late, but added my +1 for good measure :) | |
| 20:22:03 | efried | thx | |
| 20:23:02 | dansmith | efried: consider me suitably thrown under the bus for my method naming | |
| 20:24:57 | efried | dansmith: You're not held accountable for sample code in pastebin. Else I would have griped at you about having the + case first and the - case the default. | |
| 20:25:56 | dansmith | else should have raised | |
| 20:26:37 | mriedem | efried: with ^ how can i integrate that with https://review.openstack.org/#/c/538498/ where i'm not in a virt driver with a provider tree - or will that RT code eventually have the provider tree object so i can call add_traits there? | |
| 20:28:05 | mriedem | alternatively, if the RT isn't the place to do that and the virt driver is, i could add a generic method in the base ComputeDriver class that handles adding traits from the capabilities dict | |
| 20:28:29 | dansmith | mriedem: virt driver can do that from u_p_t he's describing here | |
| 20:28:45 | mriedem | right, but i don't want to copy the same thing in all virt drivers | |
| 20:28:49 | dansmith | but better for compute to do it right where it's calling into the virt driver | |
| 20:28:51 | mriedem | so RT or base class, i'm ok with either | |
| 20:28:52 | dansmith | call the capabilities compute-owned | |
| 20:29:03 | efried | mriedem: I'm glad you asked. I believe you should get the traits from placement and add in those you got from _get_traits (which is the union of those from virt & capabilities). | |
| 20:29:06 | mriedem | the compute isn't calling this, the RT is | |
| 20:29:10 | dansmith | yeah, wherever we call that in the virt driver from | |
| 20:29:22 | mriedem | i don't know what the RT will have the UPT | |
| 20:29:25 | dansmith | efried: eh? | |