| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-10-16 | |||
| 16:56:24 | alaski | not quite | |
| 16:56:35 | mriedem | i'm also pretty sure the foundation people would not allow the usage of the logo on a shirt that makes fun of nova | |
| 16:58:31 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Handle InstanceNotFound when setting password via metadata https://review.openstack.org/472933 | |
| 16:59:38 | openstackgerrit | Matthew Booth proposed openstack/nova master: Remove reboot argument to _create_domain_and_network https://review.openstack.org/512363 | |
| 17:03:29 | dansmith | mriedem: lol | |
| 17:07:30 | openstackgerrit | Eric Fried proposed openstack/nova master: Service user token requested with no auth https://review.openstack.org/512329 | |
| 17:07:42 | openstackgerrit | Dan Smith proposed openstack/nova master: Make migration uuid hold allocations for migrating instances https://review.openstack.org/506420 | |
| 17:07:43 | openstackgerrit | Dan Smith proposed openstack/nova master: Make live migration hold resources with a migration allocation https://review.openstack.org/507638 | |
| 17:08:13 | dtantsur | folks, could you please put spec https://review.openstack.org/#/c/449155/ on your review radar? | |
| 17:08:25 | dtantsur | this should be contained in the ironic driver, and may end up a big win for us | |
| 17:08:27 | dtantsur | thanks! | |
| 17:09:23 | dtantsur | mriedem: ^^ | |
| 17:29:04 | ralonsoh | dansmith: hi, about https://review.openstack.org/#/c/449257/42/nova/objects/instance_pci_requests.py. | |
| 17:29:40 | ralonsoh | dansmith: I modified the obj_make_compatible function to handle this. I don't know why I can't change the type of this field | |
| 17:30:59 | dansmith | ralonsoh: because an older node sending one of these to a newer node won't trigger a backport operation (since the version says it should be compatible), but it won't deserialize it with the right field type | |
| 17:31:18 | dansmith | ralonsoh: let me pull your patch down and make a change to show you, just a sec | |
| 17:31:36 | ralonsoh | dansmith: for sure, and thanks! | |
| 17:38:57 | abhi89 | dansmith, melwitt, sdague: please review https://review.openstack.org/#/c/505546/ .. its been pending since a long time now.. | |
| 17:40:51 | dansmith | ralonsoh: here's a test: https://pastebin.com/LapK0Rnz | |
| 17:41:00 | dansmith | ralonsoh: which gives you this: https://pastebin.com/cXTH5P2i | |
| 17:41:09 | dansmith | ralonsoh: revert your field change and run that and you'll see it passes | |
| 17:43:03 | ralonsoh | dansmith: but in the test, nova_object.version should be 1.1 if sent by an old node | |
| 17:43:22 | ralonsoh | dansmith: shouldn't it? | |
| 17:44:19 | dansmith | ralonsoh: it won't matter :) | |
| 18:01:58 | mriedem | dansmith: want to send this ocata change in? https://review.openstack.org/#/c/482219/ - that's holding up a newton change | |
| 18:02:14 | dansmith | "want" is a little strong | |
| 18:02:37 | mriedem | if you could find it in your heart | |
| 18:09:26 | mriedem | artom: hmm, tonyb raised a concern in https://review.openstack.org/#/c/471356/ about whether or not VIR_DOMAIN_BLOCK_REBASE_COPY_DEV would be in the libvirt version we support in newton | |
| 18:09:34 | mriedem | it looks like VIR_DOMAIN_BLOCK_REBASE_COPY_DEV was added in 1.2.9 | |
| 18:09:46 | mriedem | and min supported version of libvirt in newton (and ocata) is 1.2.1 | |
| 18:10:11 | cdent | dansmith, mriedem, edleafe: in discussion on https://review.openstack.org/#/c/510625/ it was agreed that the GET /allocations/{consumer_uuid} rep should include project_id and user_id since PUT will require it. In your opinions should GET /resource_providers/{uuid}/allocations as well? I think not because the layout is much different (because the cardinality of the output) (see | |
| 18:10:11 | cdent | https://developer.openstack.org/api-ref/placement/#resource-provider-allocations ) | |
| 18:15:54 | dansmith | cdent: I dunno, if there's not a need for it currently I wouldn't personally spend time on it | |
| 18:16:12 | dansmith | ideally I would think it would be best to have that information in each allocation, but.. | |
| 18:17:07 | mriedem | cdent: on GET /resource_providers/{uuid}/allocations you'd have to put project_id and user_id within each consumer-uuid keyed dict entry right? | |
| 18:17:17 | mriedem | i don't think we care about that, | |
| 18:17:41 | mriedem | if you really needed to take the result of GET /resource_providers/{uuid}/allocations and figure out the project/user for one of those allocation cosumers, you could turn around and call GET /allocations/{consumer_uuid} yeah? | |
| 18:17:44 | cdent | right, it would be weird | |
| 18:17:48 | cdent | yes | |
| 18:18:01 | cdent | cool, this is the answer I was hoping for | |
| 18:18:24 | mriedem | artom: so i think we probably need to either revert https://review.openstack.org/#/c/471353/ or put a fix on top | |
| 18:18:34 | mriedem | to not try and use VIR_DOMAIN_BLOCK_REBASE_COPY_DEV if it's not available | |
| 18:18:40 | dansmith | I can see uses for it it but like I say, I don't think there's anything begging for it right now | |
| 18:18:44 | cdent | for the consumer side I had to take jay’s only one join query recent cleanup and add four more to get the project and user inf | |
| 18:23:12 | melwitt | mriedem: yeah, nothing is calling InstanceList.get_count_by_vm_state. it was intended to be used for limiting the number of pending resizes but once we got to it, we decided not to change the existing quota behavior for resizes | |
| 18:33:39 | efried | alex_xu I reread https://review.openstack.org/#/c/497713/ with fresh eyes and commented. | |
| 18:37:18 | efried | alex_xu dansmith cdent edleafe mriedem Pursuant to this morning's discussion about traits-on-shared-with-nested, we should figure out whether there's a way to take aggregates out of the picture until ^ is figured out. | |
| 18:41:10 | cdent | efried: how far out of the picture do you want them to go? | |
| 18:41:56 | efried | cdent Well, the logic in https://review.openstack.org/#/c/479766/ is trying to handle hitting shared RPs with the results. | |
| 18:42:35 | efried | cdent And the logic it's using is at the very least implementing an arbitrary and undiscussed design point. | |
| 18:43:57 | efried | cdent Which is: When you ask for traits X, Y, and Z, I can give you back an allocation request (i.e. one hit in the list of GET /allocation_candidates) that has traits X and Y in the compute host RP but trait Z in a shared RP in the same aggregate as the compute host RP. | |
| 18:45:26 | cdent | presumably that shared RP is also satisfying some inventory requirement, not _just_ a trait? | |
| 18:45:28 | efried | This is going to break down as soon as we have any scenario more complicated than traits X, Y, and Z being mutually exclusive on the compute host and shared RP. | |
| 18:45:44 | artom | mriedem, ah crap | |
| 18:45:46 | efried | cdent Yeah, there's that too. I'm not sure that's being accounted for. | |
| 18:45:50 | mriedem | artom: i'm fixing | |
| 18:45:51 | artom | I thought I checked for min libvirt version | |
| 18:45:54 | artom | Clearly I didn't :( | |
| 18:46:13 | artom | At least not far back enough | |
| 18:46:52 | artom | mriedem, what's the fix? Check for libvirt version in code? | |
| 18:47:25 | efried | cdent I think my main issue is that I don't think we have the semantic codified for the general case in any spec. | |
| 18:47:36 | cdent | efried: my understanding, although I don’t know if this is explicitly expressed in spec or code anywhere, is that if something sets itself up to sharing provider (which requires two things: being in an aggregate with some other stuff, having the MISC_SHARES_VIA_AGGREGATE trait), then it is obliged to only aggregate with something that is a root provider | |
| 18:47:42 | mriedem | artom: hasattr | |
| 18:48:06 | cdent | efried: that is, it is the domain of aggregation that helps avoid some of the problems | |
| 18:48:35 | artom | mriedem, ah. Would we have a way to test that in CI with an old libvirt version? | |
| 18:49:22 | mriedem | artom: in unit tests | |
| 18:49:27 | efried | cdent Is it possible for a compute host to have DISK_GB inventory *and* be in an aggregate with a shared storage RP? | |
| 18:49:36 | mriedem | efried: yes | |
| 18:49:46 | artom | mriedem, you mean with a fake python-libvirt? | |
| 18:50:07 | mriedem | efried: well, maybe :) | |
| 18:50:13 | mriedem | not sure actually | |
| 18:50:54 | efried | mriedem cdent So this is the scenario alex_xu and I discussed earlier. If I ask for storagey traits X and Y; and my local disk has X but not Y; and my shared storage has Y but not X, I *should* not see this compute host in the allocation candidates. | |
| 18:50:59 | cdent | efried, mriedem: that’s something we initialy said “no” and then later changed to “yes” and then later “actually not sure" | |
| 18:51:00 | efried | But I think as currently written, I will. | |
| 18:51:39 | cdent | efried: you are correct, you should not get the compute host | |
| 18:51:41 | efried | cdent mriedem Which probably means we should architect it in such a way that we don't lock ourselves into disallowing it. | |
| 18:51:44 | cdent | because it can’t satisfy itself | |
| 18:51:54 | cdent | nor can it satisfy with the help of an aggregate friend | |
| 18:52:21 | efried | cdent Right; but I think as https://review.openstack.org/#/c/479766/10/nova/objects/resource_provider.py is written, that *will* hit. | |
| 18:53:07 | efried | And once nested is in play, it gets even messier. Because we *are* allowing a certain amount of trait "propagation" within a tree. But only in some directions. | |
| 18:53:10 | cdent | I’ve been unable to fully decode the SQL in that stack, so have mostly just been waiting for some tests along those lines | |
| 18:54:08 | cdent | so what I would think is the right thing here, is making sure, since we already have the concept of shared within the object code, is that there are functional tests in db/test_resource_provider.py for the scenarios you’ve described | |
| 18:54:27 | cdent | and yeah, once nested is in play, it will get even more interesting | |
| 18:54:42 | bauzas | dansmith: thanks for efried's change, will look | |
| 18:54:49 | bauzas | mriedem: ack, will look | |
| 18:56:37 | efried | bauzas Note that there's still work to be done there. The change is kinda blowing up. Hope to have another rev today or tomorrow. | |
| 19:00:28 | openstackgerrit | Merged openstack/python-novaclient master: Update "The nova Shell Utility" in the user guide https://review.openstack.org/512224 | |
| 19:06:01 | openstackgerrit | Merged openstack/nova master: doc: Rework man pages https://review.openstack.org/502105 | |
| 19:10:02 | edleafe | efried: (reading back) so is the issue that the specified traits are not tied to a particular RP? | |
| 19:10:30 | efried | edleafe Yes, that's one aspect of it. | |
| 19:11:09 | efried | edleafe I'm comfortable with the semantics when we're just talking about nested RPs, but I think we have some holes (design points we haven't considered) when aggregates are put in the mix. | |
| 19:11:20 | edleafe | efried: agreed | |
| 19:11:53 | edleafe | efried: just wanted to make sure I followed what the problem was | |
| 19:12:43 | efried | edleafe cdent One suggestion would be to force the aggregate inventory+traits to be specified in its own numbered group (per the 'granular' spec). | |
| 19:12:49 | efried | Then we could greatly simplify the code. | |
| 19:13:10 | efried | Because we could search aggregates separately. | |
| 19:14:17 | efried | The thing I'm still unclear on is how much of a code commitment we've made to aggregates already. Are we reversing/breaking/removing "existing" function if we take aggregates completely out of the picture for Queens? | |
| 19:14:30 | efried | Given that we've said we're not doing shared RPs in Queens. | |
| 19:14:40 | cdent | efried: that kind of busts the point of aggregates, if you’re looking at things from the original shared disk need | |
| 19:14:47 | cdent | efried: as far as I can tell, no, nothing breaks | |