| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-05-02 | |||
| 19:32:41 | mriedem | melwitt: the attachment_id getting passed to cinder isn't from the bdm, it's from the volume['attachments'] information, which is a thing that existed before the new style attach flow https://github.com/openstack/nova/blob/master/nova/compute/api.py#L4026 | |
| 19:33:32 | efried | arvindn05: example: https://review.openstack.org/#/c/565279/2 is first, https://review.openstack.org/#/c/564351/5 is second, https://review.openstack.org/#/c/517757/33 is third in the series. | |
| 19:34:37 | mriedem | this code is likely all old and busted and should be removed https://github.com/openstack/nova/blob/master/nova/volume/cinder.py#L537-L559 | |
| 19:34:44 | mriedem | since we don't support multiattach volumes with old style attachments | |
| 19:34:58 | openstackgerrit | Merged openstack/nova master: Remove ExactCoreFilter ExactDiskFilter ExactRamFilter https://review.openstack.org/508886 | |
| 19:35:16 | mriedem | that was all prep work written in mitaka https://review.openstack.org/#/c/193134/ | |
| 19:35:41 | melwitt | arvindn05: yeah, with gerrit, you can't have multiple commits in one "review". so to do that, you have to make a stack of gerrit reviews (one review per commit) and then upload them together. you just do all of your commits as usual, then 'git review' will upload all of them as separate reviews that depend on each other | |
| 19:36:18 | mriedem | melwitt: so it looks like we have a bug that we still pass an attachment_id to os-detach even if we don't have an attachment_id | |
| 19:36:46 | melwitt | mriedem: okay, so an empty body os-detach will still do the right thing and detach the volume | |
| 19:37:30 | mriedem | maybe, not really sure, we don't have tests that try to detach a root volume | |
| 19:37:37 | melwitt | I'll try depends-on your patch and see if it makes the tempest test change work (I added an assert that the volume was gone and that failed because the volume couldn't be detached and thus deleted) | |
| 19:38:36 | mriedem | melwitt: ok, but you need this to get ceph working on stable branches? | |
| 19:39:11 | melwitt | mriedem: on master. I don't know why it's failing 100% now over this "volume dependent on snapshot" issue when it didn't use to, but that's what's going on | |
| 19:39:31 | mriedem | ok deps-on my patch and then we'll go from there | |
| 19:39:56 | arvindn05 | melwitt: efried: ahh...k. will keep that in mind. Good workflow to use when all of your commits are complete...if the last one is WIP, we will have to mark it in gerrit review i guess | |
| 19:40:02 | melwitt | but because the test does not first remove the volume snapshot, the second instance can't be deleted. I Don't Know How This Ever Worked(tm) for ceph | |
| 19:40:15 | arvindn05 | learn something new about gerrit everyday :) | |
| 19:40:21 | efried | arvindn05: Happens all the time. Even ones that *aren't* the last one get marked WIPs. | |
| 19:40:35 | efried | arvindn05: And the whole series doesn't merge at once; they merge bottom up like you would expect. | |
| 19:40:42 | efried | each is a dependency of the next. | |
| 19:40:46 | melwitt | arvindn05: yeah, the usual way is to just prepend your commit message subject line on the WIP one with "WIP" | |
| 19:41:30 | mriedem | melwitt: the test wasn't skipped for the ceph job before was it? | |
| 19:41:56 | arvindn05 | thanks! | |
| 19:42:00 | melwitt | mriedem: it might have been. I can't think of any other way this makes sense but I didn't find that yet. tbh I don't remember where to find the skip list for ceph | |
| 19:43:03 | mriedem | i don't think it ever landed, i remember cleaning up something about this recently | |
| 19:43:49 | mriedem | https://review.openstack.org/#/c/550497/ | |
| 19:46:40 | melwitt | so both skip list changes are abandoned ... does that mean there's no skip list? I'm confused | |
| 19:47:08 | melwitt | so that implies that test is new? *looks* | |
| 19:47:18 | mriedem | it's not new | |
| 19:47:23 | mriedem | it's only recently been renamed | |
| 19:47:31 | mriedem | but the idempotent id hasn't changed | |
| 19:47:38 | openstackgerrit | Matt Riedemann proposed openstack/nova master: WIP: Add nova-manage placement heal_allocations CLI https://review.openstack.org/565886 | |
| 19:47:51 | melwitt | hrm | |
| 19:47:54 | mriedem | mgagne: dansmith: ^ here is my start, sans tests and docs and lots of todos/notes | |
| 19:48:16 | mriedem | melwitt: this is the job definition, and no it doesn't have a skip list http://git.openstack.org/cgit/openstack-infra/openstack-zuul-jobs/tree/playbooks/legacy/tempest-dsvm-py35-full-devstack-plugin-ceph/run.yaml#n2 | |
| 19:48:26 | mriedem | jbernard was working on one but as you can see those patches were all abandoned | |
| 19:48:27 | melwitt | doubleyew tee eff | |
| 19:48:31 | mriedem | so for awhile the job was just totally broken | |
| 19:48:52 | melwitt | I don't know how this test could ever pass with ceph | |
| 19:49:45 | melwitt | and the job hasn't been broken for _that_ long, I didn't think | |
| 19:49:52 | melwitt | anywho | |
| 19:50:01 | mriedem | melwitt: could be that it wasn't using DEVSTACK_GATE_TEMPEST_FULL | |
| 19:51:09 | mriedem | tempest tox -e full runs tempest api tests concurrently and then scenario tests serially | |
| 19:51:12 | mriedem | and this is a scenario tes | |
| 19:51:13 | mriedem | *test | |
| 19:51:22 | efried | arvindn05: Do we always have access to the old image metadata during a rebuild? | |
| 19:51:26 | melwitt | okay, I'll take a look for that | |
| 19:51:48 | mriedem | if DEVSTACK_GATE_TEMPEST_* isn't specified, it looks like it should just be running smoke tests | |
| 19:52:05 | arvindn05 | nope...we have the old image ref though....we can load the image meta from that | |
| 19:52:49 | mriedem | efried: arvindn05: you should, it's in the request spec | |
| 19:52:56 | efried | okay | |
| 19:53:12 | mriedem | https://github.com/openstack/nova/blob/master/nova/objects/request_spec.py#L49 | |
| 19:53:33 | mriedem | melwitt: maybe this https://github.com/openstack/devstack-plugin-ceph/commit/430fff8313a364f951423768d9ee8fdba4e79a27 | |
| 19:53:42 | mriedem | nvm that's just manila | |
| 19:54:20 | arvindn05 | for rebuild at the api level a request spec is not created yet i think | |
| 19:54:29 | arvindn05 | compute.api.API#rebuild | |
| 19:54:46 | arvindn05 | i am looking at that method to add the validation...is it the right place? | |
| 19:54:59 | mriedem | the request spec is from when the instance was created | |
| 19:55:53 | mriedem | also, instance.image_meta is the current instance image meta for the instance | |
| 19:56:02 | mriedem | so you don't even need the request spec | |
| 19:56:40 | arvindn05 | great | |
| 19:57:11 | arvindn05 | i would just need to load the new image meta then to compare each other | |
| 19:58:29 | mriedem | you have the new image here https://github.com/openstack/nova/blob/master/nova/compute/api.py#L3013 | |
| 20:00:04 | arvindn05 | btw is there an instance.image_meta? i dont see it under objects.instance.Instance | |
| 20:02:46 | mriedem | https://github.com/openstack/nova/blob/master/nova/objects/instance.py#L238 | |
| 20:03:43 | arvindn05 | ahh..its not a field...but a property got it | |
| 20:07:34 | melwitt | mriedem: indeed, here's a job pass on stable/queens from today http://logs.openstack.org/68/565668/1/check/legacy-tempest-dsvm-full-devstack-plugin-ceph/b6047e4/job-output.txt.gz#_2018-05-02_09_46_32_680582 | |
| 20:08:11 | mriedem | hmm, tempest and devstack-plugin-ceph are branchless, | |
| 20:08:20 | mriedem | so whatever is broken must be in master only for nova or cinder | |
| 20:08:39 | melwitt | yeah, exactly. odd. going to see if I can find what's different | |
| 20:10:13 | mriedem | melwitt: my money is on that schema change in the cinder api which is queens only, merged in march | |
| 20:10:30 | melwitt | you mean rocky only? | |
| 20:10:32 | mriedem | before that, the api would just pass attachment_id=None down to the volume manager which handled it https://github.com/openstack/cinder/blob/master/cinder/volume/manager.py#L1296 | |
| 20:10:40 | mriedem | yeah, i'll never adapt to rocky | |
| 20:10:49 | melwitt | QueensMan | |
| 20:11:00 | melwitt | okay, I think I missed where you pointed out the schema change. *scrolls back* | |
| 20:13:26 | melwitt | ah yep, so there used to be no schema validation I guess? agreed that has to be the reason for the failure now | |
| 20:15:48 | openstack | Launchpad bug 1768650 in Cinder "POST /volumes/{volume_id}/action os-detach incorrectly requires attachment_id is not None" [Undecided,New] | |
| 20:15:48 | mriedem | melwitt: https://bugs.launchpad.net/cinder/+bug/1768650 | |
| 20:15:59 | mriedem | i'll push a patch to cinder | |
| 20:16:37 | melwitt | sounds great | |
| 20:41:41 | mriedem | yeehaw http://paste.openstack.org/show/720250/ | |
| 20:42:32 | melwitt | old school, email it | |
| 20:42:52 | mriedem | heh, i thought about it | |
| 21:29:15 | cfriesen | has anyone ever looked at supporting "-cpu host,migratable=no,+invtsc" in order to get the "nonstop_tsc" flag in the guest? The tricky bit seems to be that nova might want to block attempts to migrate or snapshot the instance if it's enabled since it's going to fail down in qemu. | |
| 22:07:54 | dansmith | melwitt: HEY. This country was built with patch-on-ML! | |
| 22:08:15 | melwitt | ;D | |
| 22:12:45 | lbragstad | melwitt: o/ johnthetubaguy was telling me that you are a quota expert | |
| 22:14:43 | melwitt | lbragstad: o/ an accidental one, maybe | |
| 22:15:35 | lbragstad | :) i was working with john on documenting CERN's usecase for hierarchical quotas/limit with the unified limit approach | |
| 22:15:55 | lbragstad | i think i got it all out on paper now, https://review.openstack.org/#/c/565412/ | |
| 22:16:14 | lbragstad | but figured you'd be pretty good at poking holes in what we have so far | |
| 22:16:38 | lbragstad | (there are a ton of diagrams, but the rendered version isn't bad- http://logs.openstack.org/12/565412/3/check/build-openstack-sphinx-docs/22ea042/html/specs/keystone/rocky/strict-two-level-enforcement-model.html ) | |
| 22:17:10 | melwitt | a-ha, very cool. I will definitely take a look | |
| 22:17:56 | lbragstad | thanks melwitt | |
| 22:18:49 | melwitt | oh, heh, so failed == excluded from the output entirely. that's helpful of it :P | |
| 22:19:37 | melwitt | lbragstad: np, thanks for linking me | |
| 22:20:21 | lbragstad | no problem - latest patch set should render the python example for service to incorporate the limit usage | |
| 22:20:35 | melwitt | excellent | |
| 22:22:46 | openstackgerrit | Eric Fried proposed openstack/nova master: Get anchors for sharing providers https://review.openstack.org/565279 | |