| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-07-25 | |||
| 18:35:04 | mriedem | having said that, | |
| 18:35:16 | mriedem | i have a func test in my cross-cell resize series that does db archive on all 3 cells in the test https://review.opendev.org/#/c/651650/22/nova/tests/functional/test_cross_cell_migrate.py | |
| 18:35:20 | dansmith | but the real code sends untargeted stuff to the default db in the config, which is why the fixture does | |
| 18:35:36 | melwitt | mriedem: I did figure it out, it was because of the auto-targeting by "last targeted database" if untargeted | |
| 18:36:04 | dansmith | mriedem: yeah, even still, I'd be happy with just a unit test to make sure that archive is calling archive on all the cell mappings.. it | |
| 18:36:09 | dansmith | is such a trivial op I don't really know that we need much more than that, | |
| 18:36:17 | mriedem | ok i guess i mean "i'd rather figure out an easier way to make the new tests work with the existing fixture" | |
| 18:36:24 | melwitt | dansmith: but the fixture also sends untargeted stuff to the "last targeted db" first. I think it should only send untargeted stuff to the default db | |
| 18:36:24 | dansmith | and we run archive in tempest jobs, which should hit cell0 and cell1 if we make it run all cells | |
| 18:36:25 | mriedem | also, | |
| 18:36:29 | melwitt | (which is what my patch is doing) | |
| 18:36:48 | mriedem | i was going to say - nova-next runs archive_deleted_rows, we can and should make it run on all cells | |
| 18:36:54 | mriedem | which will hit cell0 and cell1 as dansmith said | |
| 18:37:03 | mriedem | so....then we'd have real integration test coverage | |
| 18:37:04 | dansmith | melwitt: everything in the compute node is untargeted though, which is why I don't really know how you can change that globally and have it match the real world | |
| 18:37:09 | dansmith | but.. I haven't read it so I dunno | |
| 18:37:11 | mriedem | of the cli, which is better than the functional stuff anyway | |
| 18:37:18 | dansmith | mriedem: agreed | |
| 18:38:03 | melwitt | dansmith: I think maybe things are getting confused. in the fixture we have two ways of targeting untargeted stuff. one is sending it to the default db (good) and one is sending it to the last db that was targeted (bad IMHO) | |
| 18:38:38 | mriedem | i have run afoul of that latter behavior | |
| 18:38:40 | dansmith | I understand | |
| 18:38:41 | mriedem | agree it's not fun | |
| 18:38:53 | mriedem | but i think i've found ways around that in my multi-cell func testing | |
| 18:39:16 | melwitt | so y'all actually like the last targeted db thing? I'm open to that, just didn't think anyone would think they wanted to keep it | |
| 18:39:25 | mriedem | this one https://review.opendev.org/#/c/641179/ | |
| 18:39:36 | mriedem | i remember i was hitting weirdness because of that 'last targeted context' thing as well | |
| 18:39:43 | mriedem | i'm not saying i like it | |
| 18:39:52 | mriedem | but i also don't like redoing the whole thing per se | |
| 18:39:56 | mriedem | when there are maybe other ways | |
| 18:40:05 | melwitt | I'm +1 on the real integration testing, that's fine by me. but I just thought it would make the fixture a lot simpler and less big hiding to remove that bit about "last targeted db" | |
| 18:40:12 | mriedem | it's like touching the old quotas code - i can, but don't want to if i can help it | |
| 18:40:40 | mriedem | w/o looking deep into your change idk | |
| 18:40:45 | mriedem | i wouldn't abandon it, | |
| 18:40:50 | mriedem | but i wouldn't block the other thing on it either | |
| 18:40:57 | mriedem | i'd get the integration testing in nova-next working | |
| 18:41:09 | melwitt | that's fair. it's less like redoing and more like "delete all the self._last_ctxt_mgr" but yeah, when you get around to it, take a look and see if you hate it | |
| 18:41:09 | mriedem | which should be like, 1 line | |
| 18:42:22 | melwitt | don't get me wrong, I'm totally fine with that. as long as it gets tested, I'm happy. I was honed in on trying to make the func test work 100% | |
| 18:43:50 | melwitt | and thought people might be happy to see all the _last_ctxt_mgr stuff deleted from the fixture, no more global state changing, much simpler | |
| 18:45:06 | melwitt | I'll rebase the multi-cell archive patch on top of a different change to add --all-cells to nova-next | |
| 19:04:30 | mriedem | efried: ha http://lists.openstack.org/pipermail/openstack-discuss/2019-July/008037.html | |
| 19:25:29 | openstackgerrit | Merged openstack/nova master: api-ref: touch up the os-services docs https://review.opendev.org/672571 | |
| 19:33:45 | artom | That actually went pretty well. | |
| 19:45:30 | openstackgerrit | Matt Riedemann proposed openstack/nova-specs master: Support delete_on_termination in volume attach api https://review.opendev.org/612949 | |
| 19:45:39 | mriedem | i cleaned this up ^ it's pretty straight-forward | |
| 20:02:28 | efried | mriedem: approved that. Seems like an easy win. | |
| 20:03:19 | mriedem | ack, i'm cleaning up https://review.opendev.org/#/c/667894/ now | |
| 20:04:55 | efried | mriedem: make all the names match up too if you please | |
| 20:05:40 | efried | mriedem: the bp is at https://blueprints.launchpad.net/nova/+spec/add-user-id-field-to-the-migrations-table | |
| 20:06:48 | efried | (so the path at the top of the spec is right, but the file path, commit message, and topic are wrong) | |
| 20:09:42 | openstackgerrit | Merged openstack/nova-specs master: Support delete_on_termination in volume attach api https://review.opendev.org/612949 | |
| 20:10:25 | efried | sean-k-mooney, stephenfin: Did we ever figure out whether/how you could tell from within a guest which / how many CPUs are pinned? Was that going to be via "metadata"? | |
| 20:10:46 | sean-k-mooney | efried: that is what the latest spec say yes | |
| 20:11:24 | efried | okay, I was about to dig into it. Unfortunately, I don't see me being competent to update it such as to get it approved today. | |
| 20:12:02 | sean-k-mooney | https://review.opendev.org/#/c/668656/5/specs/train/approved/use-pcpu-vcpu-in-one-instance.rst@173 | |
| 20:12:19 | sean-k-mooney | The metadata API will be extended to dedicated cpu info with new version. | |
| 20:12:21 | sean-k-mooney | The new field will be added to the `meta_data.json`:: | |
| 20:12:23 | sean-k-mooney | dedicated_cpu=<cpuset string> | |
| 20:12:25 | sean-k-mooney | The ``cpuset string`` indicated the instance cpus which are running on | |
| 20:12:27 | efried | ah nice | |
| 20:12:27 | sean-k-mooney | dedicated pCPU. | |
| 20:13:32 | sean-k-mooney | you could alos use that new numa toploty api if we approved that i guess | |
| 20:13:45 | sean-k-mooney | although that is not really from within the guest | |
| 20:14:45 | efried | I would want stephenfin to be a +2 on that spec anyway, so I guess we'll see if alex_xu et al can polish it up and request sfe, since we seem to have general agreement with caveats as noted. | |
| 20:15:16 | efried | I'll sent a note on sfe process (which apparently I'll be making up) either tomorrow or Monday. | |
| 20:15:21 | sean-k-mooney | does the timeline i set out make sesne to you | |
| 20:15:28 | efried | oh, totally. | |
| 20:28:47 | openstackgerrit | Matt Riedemann proposed openstack/nova-specs master: Add user_id to the migrations https://review.opendev.org/667894 | |
| 20:29:28 | openstackgerrit | Matt Riedemann proposed openstack/nova-specs master: Add user_id to the migrations https://review.opendev.org/667894 | |
| 20:29:39 | mriedem | first is the diff, second is the file rename | |
| 20:31:23 | mriedem | only open question on ^ is if we should add user_id/project_id as request filter params to GET /os-migrations | |
| 20:31:33 | mriedem | i think we might as well | |
| 20:35:22 | efried | sean-k-mooney: please vet that I represented you correctly in the whiteboard https://blueprints.launchpad.net/nova/+spec/use-pcpu-and-vcpu-in-one-instance | |
| 20:37:01 | sean-k-mooney | sure | |
| 20:38:03 | sean-k-mooney | yep | |
| 20:38:14 | efried | thx | |
| 20:38:23 | sean-k-mooney | without all the typos in my original comment :) | |
| 20:38:26 | gmann | efried: melwitt sean-k-mooney ack I will check the spec. it was in my list but did not get time to review. | |
| 20:38:43 | efried | thanks gmann | |
| 20:39:11 | melwitt | cool gmann | |
| 20:41:45 | efried | mriedem: where are request filter params for GET /os-migrations documented in the api-ref? | |
| 20:42:26 | efried | oh, are they the top-level fields hidden, host, instance_uuid etc? | |
| 20:42:28 | efried | uck | |
| 20:43:26 | efried | mriedem: but yeah, I think it makes sense to add this in there. | |
| 20:43:46 | mriedem | yeah | |
| 20:44:26 | mriedem | i frequently filter on migration_type and instance_uuid in functional tests since GET /servers/{server_id}/migrations is hard-coded to only be in-progress live migrations | |
| 20:54:56 | efried | mriedem: chuck that filter field in there and I'm +2 | |
| 20:55:09 | mriedem | alright then | |
| 20:55:13 | efried | not sure if you're still comfortable being the other +2 | |
| 20:55:21 | mriedem | o | |
| 20:55:28 | mriedem | *i'm like a 1.5 | |
| 20:56:08 | efried | sean-k-mooney: want to throw a +1 on there to push us over the edge? | |
| 20:56:13 | efried | ...once mriedem has updated | |
| 20:56:23 | sean-k-mooney | efried: link? | |
| 20:56:31 | efried | https://review.opendev.org/#/c/667894/ -- stand by for PS4 | |
| 20:57:06 | sean-k-mooney | oh i have looked at that before sure ill re review | |
| 21:05:12 | mriedem | build specs docs takes awhile | |
| 21:05:26 | mriedem | *building | |
| 21:05:47 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: [WIP-until-series-is-ready] Introduce live_migration_claim() https://review.opendev.org/635669 | |
| 21:05:47 | sean-k-mooney | im reviewing v3 in the mean time and then ill revew the delta | |