| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-03-20 | |||
| 20:54:49 | melwitt | lbragstad: sure, I'll reply | |
| 20:54:54 | lbragstad | thanks | |
| 21:30:07 | mriedem | welp, this doesn't look good http://logs.openstack.org/63/642863/1/gate/nova-tox-functional/6573680/job-output.txt.gz#_2019-03-20_15_58_21_993250 | |
| 21:30:10 | mriedem | cdent: efried: ^ | |
| 21:30:56 | mriedem | this is the test that uses the PlacementFixture as a context manager | |
| 21:31:07 | cdent | ah, that might explain why we don't register log opts | |
| 21:31:13 | cdent | in register_opts | |
| 21:31:32 | mriedem | what i can't explain is why this would just start failing all of a sudden | |
| 21:31:49 | dansmith | mriedem: okay, since the list of disk formats is such a restricted and static list, it doesn't seem unreasonable to just use capabilities for this, but I don't care that much | |
| 21:31:51 | cdent | that merged only in the last few ours | |
| 21:32:00 | efried | mriedem: It's because we landed https://review.openstack.org/#/c/644591/ in placement | |
| 21:32:01 | _pewp_ | [ Gerrit Code Review ] - review.openstack.org | |
| 21:32:02 | cdent | well, some times today (the register opts change) | |
| 21:32:24 | efried | hemna_: Not loving _pewp_ - what value is it supposed to add? | |
| 21:32:45 | hemna_ | arg | |
| 21:32:47 | cdent | mriedem: there's the revert https://review.openstack.org/#/c/645014/ | |
| 21:32:48 | _pewp_ | [ Gerrit Code Review ] - review.openstack.org | |
| 21:32:56 | cdent | but I think there's likely a better fix | |
| 21:32:59 | hemna_ | I'll remove the github portion, I thought I already had | |
| 21:33:01 | cdent | (on the nova side) | |
| 21:33:54 | mriedem | opening a bug | |
| 21:34:31 | mriedem | https://bugs.launchpad.net/nova/+bug/1821092 | |
| 21:34:32 | openstack | Launchpad bug 1821092 in OpenStack Compute (nova) "test_local_delete_removes_allocations_after_compute_restart failing since https://review.openstack.org/#/c/644591/" [Critical,Confirmed] | |
| 21:35:19 | mriedem | dansmith: so you were thinking like a capability boolean for each known disk_format? | |
| 21:36:05 | dansmith | mriedem: yeah, I mean, I know that there would be more of those than existing capabilities, but... I don't think the cost is any higher, and we just have to invent a new place to keep that list | |
| 21:36:30 | klindgren | sean-k-mooney, mriedem , dansmith, I assume based upon earlier discussions that you guys would be ok with a patch on live-migration to regenerate vs's copy an iso9660 config drive under the correct conditions? | |
| 21:36:51 | efried | cdent: You're just making a blank nova patch that depends-on the revert? | |
| 21:37:07 | dansmith | klindgren: meaning regenerate if we know there aren't files we'd be missing? not sure we can know that currently | |
| 21:37:35 | mriedem | i think we can assume there wouldn't be any personality files if inject_partition is -2 | |
| 21:37:37 | mriedem | i.e. disabled | |
| 21:38:00 | dansmith | mriedem: they could be in the config drive just not injected into the root image right? | |
| 21:38:04 | cdent | efried: no, I was hoping someone else would make the thing that matt has seen fail depend-on the revert patch. I'm looking into fixing the nova side so we don't have to have the revert on the placement side. It's happening because of a misuse of conf, which would be good to remove | |
| 21:38:13 | klindgren | I mean - I *know* that on my clouds it's not there, because of the layers that are built onto of it, don't use it. | |
| 21:38:15 | mriedem | dansmith: oh maybe, i'd have to dig into that code | |
| 21:38:40 | dansmith | mriedem: pretty sure they end up in config drive if provided, and then also in the root disk if you inject, but I could be wrong | |
| 21:38:51 | klindgren | So I was thinking maybe a config option for scp/rsync to add regenerate? | |
| 21:38:53 | efried | cdent: ack. | |
| 21:38:56 | dansmith | mriedem: but deciding whether or not to regen based on the config of the (current) source it's coming from seems too loose a restriction | |
| 21:39:28 | efried | mriedem: you want me to update https://review.openstack.org/#/c/642863/ ? | |
| 21:39:44 | mriedem | efried: to depend on the fix for this blocker bug? | |
| 21:39:55 | mriedem | dansmith: i don't disagree, it would require thought, | |
| 21:40:13 | mriedem | so i think the answer to klindgren's question is it's going to require thought, not just a simple single change | |
| 21:40:18 | klindgren | I just want to avoid a patch that we can never get merged upstream | |
| 21:40:34 | mriedem | i suspect there would be MigrateData object RPC changes and such to communicate between the dest and source compute | |
| 21:40:34 | efried | mriedem: yes. or create a new blank patch. | |
| 21:40:56 | dansmith | mriedem: agree it's non trivial | |
| 21:41:27 | mriedem | klindgren: are you or your coworker buddies going to be at the ptg? | |
| 21:41:34 | efried | oo, cdent do tell | |
| 21:41:52 | klindgren | mriedem, yes we will have 2 people there | |
| 21:41:53 | mriedem | cdent: did someone put potato skins down the drain again? | |
| 21:41:55 | klindgren | I won't be there | |
| 21:42:03 | klindgren | afaik | |
| 21:42:03 | cdent | mriedem: twice | |
| 21:42:08 | mriedem | klindgren: at least throw something on here https://etherpad.openstack.org/p/nova-ptg-train | |
| 21:42:23 | cdent | efried: I'm relatively certain the same config is being used twice and it should be. need to confirm | |
| 21:42:28 | mriedem | and if you start coming up with design ideas and have thought through the corner cases and such, it'd be good to dump those thoughts in the ML | |
| 21:42:33 | dansmith | files=injected_files, | |
| 21:42:33 | dansmith | mriedem: injection_info = InjectionInfo(network_info=network_info, | |
| 21:42:44 | cdent | and I'm not clear why this only matters for logging opts | |
| 21:42:48 | cdent | but we'll see | |
| 21:42:52 | dansmith | mriedem: ^ is where we pass in the injected files to be written to the configdrive | |
| 21:43:22 | dansmith | mriedem: so I don't think host config disabling injection tells us anything about what is in the configdrive image | |
| 21:43:34 | mriedem | ok | |
| 21:43:38 | mriedem | RequestSpec to the rescue! | |
| 21:43:52 | dansmith | or system_metadata, a count of injected files we assert to be nonzero or something | |
| 21:44:00 | dansmith | but still, it's going to be confusing for admins, | |
| 21:44:15 | dansmith | wondering why some instances are migratable and/or don't depend on ssh keys and some do | |
| 21:44:18 | dansmith | but..yeah. | |
| 21:45:15 | dansmith | if we had the http service integrated into compute for the health check, we could do something better, like allowing one compute to fetch the image via one-time token from the source | |
| 21:46:11 | dansmith | i.e. fetch http://sourcehost/migration_assets/$uuid | |
| 21:46:18 | dansmith | they have to be able to talk to each other anyway | |
| 21:46:28 | dansmith | and just make that self-destruct as soon as the GET is performed | |
| 21:47:44 | mriedem | have you changed topics? or in the wrong channel perhaps? :) | |
| 21:47:50 | dansmith | no? | |
| 21:47:53 | mriedem | oh | |
| 21:47:55 | mriedem | heh | |
| 21:48:09 | mriedem | i'm not sure where "if we had the http service integrated into compute for the health check" came from | |
| 21:48:32 | dansmith | thinking of better ways to move bulk data between compute nodes than requiring ssh keys and allowing logins | |
| 21:48:44 | dansmith | because that _is_ totally nuts from a security perspective, | |
| 21:49:06 | dansmith | and fixing that with something more secure would be better than just adding a different path (regen) which only works in some cases anyway | |
| 21:49:55 | dansmith | such an approach would work for and fix the security holes in cold migration as well | |
| 21:50:26 | mriedem | well, it's essentially what i'm doing for cross-cell resize with glance | |
| 21:50:40 | dansmith | using glance as the intermediary, but yeah | |
| 21:50:59 | mriedem | noting now that your config drive will have to get rebuilt on the other side... | |
| 21:51:11 | mriedem | does config driver already get rebuilt on resize? | |
| 21:51:15 | mriedem | *drive | |
| 21:51:20 | dansmith | I thought it gets copied | |
| 21:51:34 | dansmith | along with the root disk | |
| 21:51:57 | mriedem | i'm not sure https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L8882 | |
| 21:52:00 | dansmith | if we had this http endpoint, your cross-cell resize stuff could be much more like regular resize and not have to go through glance | |
| 21:52:34 | dansmith | mriedem: that doesn't run if it already exists | |
| 21:52:44 | dansmith | so yeah, maybe that means it's regen'd but if it's copied then it won't | |
| 21:53:03 | dansmith | https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L3626 | |
| 21:54:17 | mriedem | hmm yeah so i suppose disk.config gets copied here https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L8811 | |
| 21:55:39 | mriedem | huh, i guess that's something that needs to be at least documented for the cross-cell stuff as a limitation right now because it's not doing anything with the config drive, so it's going to get regenerated on the dest and you'd lose any personality files | |
| 21:56:40 | dansmith | you're welcome. | |
| 21:58:23 | mriedem | duly noted in my re-proposed spec https://review.openstack.org/#/c/642807/1 | |
| 21:58:41 | mriedem | i don't know what the answer is, upload a snapshot of the disk.config to glance? | |
| 22:00:04 | dansmith | no the answer is implement an http server :D | |
| 22:03:38 | cdent | efried, mriedem : the gist of the problem is that you can register other opts multiple times, but not cli opts, and adding oslo_log.log.register_opts() to the common path brings CLI options into the scene of test_local_delete_removes_allocations_after_compute_restart, where config is registered multiple times (3 in fact) | |