| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-03-21 | |||
| 17:00:29 | mriedem | that runs on the source host post-live migration | |
| 17:00:35 | gibi | melwitt: yes I can run it | |
| 17:00:38 | mriedem | for the new flow, we just get the connection_info out of the attachment record | |
| 17:00:45 | melwitt | gibi: cool, ty for confirming | |
| 17:00:55 | jgriffith | ahh, yeah | |
| 17:01:26 | mriedem | so the old flow thing there to call init_connectoin was a workaroudn | |
| 17:01:37 | gibi | melwitt: will you update the agenda or shall I do it? | |
| 17:01:37 | mriedem | b/c at this point in the flow, in the nova db, the bdm.connection_info is actually for the dest host | |
| 17:01:53 | melwitt | gibi: I'll update the agenda | |
| 17:02:54 | gibi | melwitt: OK, thanks | |
| 17:02:58 | jgriffith | mriedem: yeah, I get ya | |
| 17:02:58 | mriedem | ok so to summarize, i think i'm hearing the majority of the cinder peeps in here saying doing https://github.com/openstack/nova/blob/2ec8c49f6cb4a0e7dba217e824c20d9c703d2105/nova/virt/libvirt/driver.py#L7749 is a bad workaround and only works for some drivers b/c we're getting lucky | |
| 17:03:08 | melwitt | gibi: no, thank YOU :) | |
| 17:03:10 | mriedem | even though we've always done it....but anyway | |
| 17:03:25 | dansmith | mriedem: well, we've always done it which ended up in them changing a bunch of drivers to tolerate it | |
| 17:03:38 | mriedem | sure, good point | |
| 17:03:48 | mriedem | it's a fun incestual relationship | |
| 17:03:55 | geguileo | rofl | |
| 17:04:08 | dansmith | really? I don't enjoy any of it :) | |
| 17:04:09 | dansmith | but you're into some weird shit, granted | |
| 17:04:22 | mriedem | jgriffith: btw, we should remember to talk about this in vancouver about why the new flow is much better.... | |
| 17:04:32 | jgriffith | mriedem: +1 | |
| 17:04:53 | mriedem | alright i'll move onto reviewing the nova patch then, thanks geguileo jgriffith smcginnis | |
| 17:05:23 | geguileo | no problem, and thanks for fixing that :-) | |
| 17:05:52 | jgriffith | mriedem: just for the record one more time and to hope it sticks in peoples heads... "Never update/modify and attachment, just delete it and create a new one" | |
| 17:06:17 | jgriffith | That way you still have the ability to access the old data if you need it and nothing gets funky behind your back | |
| 17:06:23 | jgriffith | hmmm... well, anyway | |
| 17:06:42 | jgriffith | "funky behind your back" isn't the best phrase I guess | |
| 17:06:46 | mriedem | because that would be like calling initialize_connection again | |
| 17:06:57 | jgriffith | mriedem: exact-u-mundo | |
| 17:11:45 | dansmith | mdbooth: did you follow all that ^ ? | |
| 17:13:12 | mriedem | i put the link into the bug for the irc log and a summary | |
| 17:13:21 | dansmith | cool | |
| 17:14:13 | mriedem | while we're on the topic https://review.openstack.org/#/c/549130/2/nova/compute/manager.py@616 | |
| 17:14:21 | mriedem | in case any cinder people are still here and can direct me | |
| 17:16:09 | dansmith | mriedem: btw, I changed this to fixes and left a snarky remark: https://review.openstack.org/#/c/554600/ | |
| 17:16:17 | dansmith | if you say that's good I'll fix the pike backport | |
| 17:18:41 | dansmith | mriedem: melwitt: tssurya: do we need to have a cells meeting today? I know there are a bunch of tssurya's patches to review, but other than that, I don't know of anything else burning that justifies it | |
| 17:19:02 | tssurya | dansmith: not needed | |
| 17:19:22 | dansmith | tssurya: especially if it means I spend the time reviewing your stuff yeah? :) | |
| 17:19:35 | tssurya | yeaaaaa :D | |
| 17:20:24 | melwitt | +1 to skipping today | |
| 17:20:46 | cdent | the whole day | |
| 17:20:48 | dansmith | woot, even if mriedem really wants a meeting, he's outnumbered | |
| 17:21:44 | ildikov | jgriffith: you need to repeat those that thousands of times so they stick in people's heads :) | |
| 17:22:32 | cfriesen | tssurya: you pinged earlier? | |
| 17:25:21 | mriedem | ildikov: or push a patch to nova to add something to the docstring for the attachment_update method so we don't forget | |
| 17:25:26 | mriedem | i'd +2 that | |
| 17:26:21 | ildikov | mriedem: I can when I'm off booth duty and got to the next hotel today | |
| 17:26:22 | mriedem | dansmith: we've changed 'features' to 'fixes' in release notes in backports for other nova-manage changes for optional things, in order to fix bugs, so i think that's fine | |
| 17:27:07 | tssurya | cfriesen: yea just to ask about the third point in the bug description, since you explicitly stated delete service/compute node, I wanted to just confirm we are still doing a soft delete | |
| 17:27:49 | tssurya | cfriesen: saw you review btw, will change it to cascade, working on some tests and then will update the patch | |
| 17:27:51 | ildikov | mriedem: It won't replace the need of repeating the phrase a thousand times everywhere else | |
| 17:28:48 | cfriesen | tssurya: I think the service/node portion can remain as it is currently. what you have looks reasonable to me with the cascade change | |
| 17:29:54 | tssurya | cfriesen: ack, however I think maybe we need to provide a new command at some point to allow compute node deletions ? like we have osc placement allowing resource provider deletions ? | |
| 17:29:57 | cfriesen | tssurya: dansmith is the go-to guy in this area. :) | |
| 17:30:18 | cfriesen | tssurya: you mean compute node deletions without service deletion? | |
| 17:30:27 | dansmith | mriedem: fine, fine | |
| 17:30:28 | tssurya | cfriesen: yes he is, he just promised he will review ^^ ;) | |
| 17:30:42 | tssurya | cfriesen: yes | |
| 17:31:06 | sean-k-mooney | mriedem: just looked at https://review.openstack.org/#/c/554917 and yes i think that is fine for stable/queens | |
| 17:31:17 | tssurya | I mean deletion of cn, along with its dependencies | |
| 17:32:08 | tssurya | the record as such does not get removed right ? | |
| 17:32:26 | tssurya | or I don't know when its moved to shadow tables | |
| 17:34:08 | dansmith | in the nova database, | |
| 17:34:09 | dansmith | deleting a thing marks it as deleted (deleted=row['id']) | |
| 17:34:11 | mdbooth | dansmith: Thanks for the ping, caught up. | |
| 17:34:20 | dansmith | "nova-manage db archive-deleted rows" moves it to shadow tables | |
| 17:34:26 | dansmith | "nova-manage db purge" removes it from shadow tables | |
| 17:34:27 | dansmith | tssurya: ^ | |
| 17:34:42 | dansmith | mdbooth: ack, I assume we're to expect a review from mriedem forthwith | |
| 17:34:54 | tssurya | ah yes was just checking this, I don't know why for some reason I though only the instance* tables were archived | |
| 17:35:00 | mdbooth | dansmith: Fortunately the above is consistent with my understanding having spoken to Gorka the other week. | |
| 17:35:00 | dansmith | \o/ | |
| 17:35:09 | dansmith | mdbooth: ack, that's good :) | |
| 17:35:14 | mdbooth | dansmith: Heh, I do that, except I tend to google first :) | |
| 17:35:30 | dansmith | mdbooth: this is 'merica.. fire from the hip and ask questions later | |
| 17:35:52 | cfriesen | tssurya: under what scenario would we want to delete only a compute node and not a service? | |
| 17:36:08 | dansmith | cfriesen: ironic nodes outnumber the service(s) they're owned by | |
| 17:36:20 | dansmith | cfriesen: and, ironic nodes balance between services, and could potentially be orphaned | |
| 17:36:41 | tssurya | cfriesen: nope, I was talking about the delete records in the compute node table | |
| 17:36:42 | dansmith | cfriesen: I would guess maybe the same could happen if you change the hostname on a virt host | |
| 17:36:50 | tssurya | but that is done by archive_deleted_rows | |
| 17:36:56 | tssurya | already so taken care of :) | |
| 17:37:01 | tssurya | sorry for the confusion | |
| 17:37:04 | mdbooth | mriedem: Incidentally lyarwood pointed out that we're not running volume tests in the live migration job by default, so on his suggestion I hacked a run based on another patch he pointed me at. Link is in a review comment. | |
| 17:38:44 | openstackgerrit | Stephen Finucane proposed openstack/nova-specs master: Update spec to reflect reality https://review.openstack.org/555000 | |
| 17:39:28 | stephenfin | jaypipes, gibi: Could you stick this on your review queue. Just ensure the spec for a Queens feature matches the actual implementation https://review.openstack.org/555000 | |
| 17:39:31 | stephenfin | *? | |
| 17:40:49 | sean-k-mooney | stephenfin: is that the pci numa policy stuff? | |
| 17:41:26 | sean-k-mooney | stephenfin: i taught we did not want to put the policy in the alias? | |
| 17:43:07 | sean-k-mooney | stephenfin: hum well its in the code here https://github.com/openstack/nova/blob/master/nova/pci/request.py#L90-L92 so i guess we did | |
| 17:43:15 | cfriesen | sean-k-mooney: ah, but we did...that way you can be flexible for one device and strict for another | |
| 17:43:28 | openstackgerrit | Dan Smith proposed openstack/nova stable/pike: Add --by-service to discover_hosts https://review.openstack.org/554603 | |
| 17:45:01 | sean-k-mooney | cfriesen: yes i see that i guess it works but eventually i would hope we can get this kind of stuff out of configs and into openstack server create --device vendor-id=xyz ... | |
| 17:45:58 | sean-k-mooney | cfriesen: with cyborg becoming a thing that seams like the correct route long term | |
| 17:46:07 | cfriesen | sean-k-mooney: I'd be fine with that, but it might affect the instance packing (which dansmith has smacked me with before) | |
| 17:46:30 | sean-k-mooney | instance packing? | |
| 17:46:33 | sean-k-mooney | in placement | |
| 17:46:44 | cfriesen | defining flavors to pack instances neatly onto a compute node with minimal waste | |