| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-03-01 | |||
| 19:31:18 | temka | So I went looking, and realized you were just making it really explicit | |
| 19:31:53 | aspiers | The thing is, a patch series is linear, but a dependency tree is a DAG | |
| 19:32:15 | aspiers | The serialization is lossy, so it is not trivial to infer the tree from the series | |
| 19:32:41 | temka | Heh | |
| 19:32:44 | aspiers | Coincidentally, I've done a lot of work on this topic outside OpenStack: https://github.com/aspiers/git-deps | |
| 19:33:21 | aspiers | But anyway, I welcome all nitpicks, so thanks ;-) | |
| 19:33:54 | temka | aspiers, heh, I can't bring myself to concentrate fully on what I'm trying to do, so instead I nitpick others's work :/ | |
| 19:34:07 | aspiers | LOL I have exactly the same problem ;-) | |
| 19:34:23 | temka | I had no idea of your level of experience with Gerrit, so just wanted to make sure you're not pushing one patch at a time and manually specified the deps | |
| 19:35:00 | aspiers | temka: Yeah thanks, it's definitely worth checking that with unfamiliar faces^Wnicks :-) | |
| 19:35:37 | aspiers | I'm pretty familiar with Gerrit. Attended the last User Summit which was super fun and interesting. | |
| 19:35:57 | temka | But then you started talking about DAGs and I realized I'm *probably* barking up the wrong tree ;) | |
| 19:36:04 | aspiers | ;) | |
| 19:36:31 | temka | Mind you, I've seen people graduate with CS degress and not be able to write a single line of C, so :P | |
| 19:36:38 | aspiers | This is true | |
| 19:36:58 | aspiers | git and its ecosystem is actually one of my specialist areas | |
| 19:38:29 | aspiers | I've hacked on git a bit, and built a plethora of tools. Since it's Friday, you can find more ramblings at https://blog.adamspiers.org/tag/git/ if you're curious | |
| 19:38:50 | temka | "Structured Procrastination" | |
| 19:38:52 | temka | Sounds about right | |
| 19:39:13 | aspiers | Also will finish adding https://ethercalc.openstack.org/github-gerrit to the Contributor's Guide one of these days ... maybe :-o | |
| 19:41:16 | temka | aspiers, wanna add https://github.com/berrange/gerrymander in the Gerrit clients square? | |
| 19:41:31 | aspiers | ooh, that's a new one for me I think | |
| 19:41:35 | aspiers | temka: please feel free | |
| 19:42:05 | temka | Not sure it's still maintained, it's been a few years since last commit | |
| 19:43:21 | aspiers | add it anyway | |
| 19:43:25 | temka | Done | |
| 19:44:47 | aspiers | mriedem: SEV trait successfully detected on real hardware using capabilities :) https://pasteboard.co/I3rPhvJ.png | |
| 19:52:43 | mriedem | get right out of town | |
| 19:52:50 | aspiers | :) | |
| 19:53:25 | aspiers | plus all those new capability traits too of course | |
| 19:57:16 | mriedem | aspiers: so you've already worked around the virConnectGetDomainCapabilities thing? | |
| 19:57:25 | cfriesen | sean-k-mooney: ran into a problem with the cold migration with TPM.../var/lib/libvirt/swtpm and the directories under it are owned by root and nova-compute is not running as root. | |
| 19:57:58 | aspiers | mriedem: yes, https://review.openstack.org/#/c/633855/7/nova/virt/libvirt/host.py@711 | |
| 19:58:57 | aspiers | mriedem: for SEV, this is *almost* good enough for now; I think the only possible issue is that if testing on another distro, qemu-kvm might live in a different place | |
| 19:59:14 | aspiers | I don't have a RHEL / Fedora box handy to check | |
| 20:00:19 | aspiers | mriedem: and the code in comments is pretty darn close to what it will look like when that patch does land | |
| 20:19:55 | temka | Dammit, even something as simple as 2 different hostnames for the 2 compute services is going to be annoying | |
| 20:20:34 | temka | https://github.com/openstack/nova/blob/master/nova/tests/unit/virt/libvirt/fakelibvirt.py#L1131 | |
| 20:20:42 | mriedem | melwitt: i'm confused about your comment here https://review.openstack.org/#/c/633350/7/nova/objects/instance_mapping.py@42 | |
| 20:21:20 | mriedem | temka: ah that's where that comes from...i was wondering because that question came up in a functional test that gibi wrote which was using the fakelibvirt stuff | |
| 20:21:48 | mriedem | can't you modify Connection to pass in a hostname which defaults to 'compute1'? | |
| 20:22:18 | mriedem | note that self.start_service() in the tests will allow you to specify the binary (compute) and hostname which is what our multi-host functional tests do, | |
| 20:22:23 | mriedem | but those aren't using the fakelibvirt stuff | |
| 20:23:41 | temka | mriedem, it's solved in the fake virt driver, but that's different because it's a virt drive | |
| 20:24:00 | temka | fakelibvirt isn't a driver - the real libvirt is the driver | |
| 20:24:17 | temka | fakelibvirt just mocks out the parts that call the libvirt API | |
| 20:24:43 | mriedem | i.e. https://github.com/openstack/nova/blob/master/nova/virt/fake.py#L654 | |
| 20:25:08 | temka | mriedem, right, and virt/fake.py replaces virt/libvirt/driver.py | |
| 20:25:15 | mriedem | yeah i know | |
| 20:25:23 | temka | fakelibvirt.py replaces what virt/libvirt/driver.py calls down to | |
| 20:25:41 | mriedem | right the python-libvirt interface | |
| 20:25:57 | temka | Right | |
| 20:27:04 | mriedem | dansmith: am i correct in my understanding of what you're asking for here? https://review.openstack.org/#/c/633350/7/nova/objects/instance_mapping.py@42 | |
| 20:27:35 | mriedem | on load of instance mapping, of it doesn't have a user_id, try to get it from the request spec, failing that, try the build request, failing *that* try the instance via the cell mapping | |
| 20:27:45 | mriedem | s/of/if/ | |
| 20:28:04 | dansmith | mriedem: right, the nullability has nothing to do with where or how we get the user_id to fill it in | |
| 20:29:01 | temka | I guess I have to mock ComputeManager._get_nodename() | |
| 20:29:14 | temka | ... per service | |
| 20:29:26 | mriedem | dansmith: which means we wouldn't need to update the instance mapping on read of the instance from the api https://review.openstack.org/#/c/638574/4/nova/compute/api.py@2404 | |
| 20:29:42 | mriedem | although, if the request spec doesn't have user_id, and the build request is gone, and the cell the instance is in is down... | |
| 20:30:10 | mriedem | then i guess you're sol? | |
| 20:30:45 | mriedem | temka: you can't override the fakelibvirt.Connection hostname value it uses? | |
| 20:30:46 | aspiers | OK, weekend time. Have a good one everyone o/ | |
| 20:31:22 | temka | mriedem, I guess I could, but I can't make it different depending on which compute service is calling it | |
| 20:32:51 | dansmith | temka: mock service.manager._get_nodename() after it's started to return the value it needs to keep returning, instead of mocking the class | |
| 20:33:15 | temka | dansmith, tried that just now, didn't work | |
| 20:33:25 | temka | I think it needs to be mocked *before* (somehow) it's started | |
| 20:33:35 | dansmith | mriedem: yeah, I mean, making that value nullable for the down-cell-and-not-anywhere-else case is not reasonable I think | |
| 20:33:38 | temka | Because upon startup it reports to placement, and there's a hostname conflict | |
| 20:33:48 | dansmith | temka: you have to mock it both places, | |
| 20:34:00 | dansmith | temka: which is why I said "needs to keep returning" | |
| 20:34:16 | temka | dansmith, which both places? | |
| 20:34:27 | dansmith | the class will catch the startup case, but be invalid once you move to the next service, so you need to mock the resulting instance of the manager class to keep returning the value ongoing | |
| 20:34:50 | temka | Ah, I see | |
| 20:35:08 | dansmith | we used to be able to pass in a different manager class to start_service i think, which would let you just subclass per node.. can we still do that? | |
| 20:35:46 | temka | So mock fakelibvirt's getHostname, start compute0, mock its get_nodename, mock getHostname to the 2nd thing, start compute1, mock its get_nodename() | |
| 20:35:50 | temka | That feels... racy | |
| 20:36:02 | temka | Actually no | |
| 20:36:07 | dansmith | temka: there are no threads here so, no | |
| 20:37:03 | mriedem | heh you still can technically classload a compute manager class it looks like | |
| 20:37:13 | dansmith | mriedem: yeah that's what I was thinking of | |
| 20:38:08 | mriedem | self.start_service('compute', host='fake-host1', manager='artom.special.fake.ComputeManager') | |
| 20:38:28 | dansmith | yeah | |
| 20:38:28 | temka | If anyone says 'dependency injection' there's going to be violence | |
| 20:38:38 | temka | I *still* have no idea what that is | |
| 20:39:01 | temka | Despite university's best attempt to drill it into our heads with massive bloated Java frameworks | |
| 20:39:27 | mriedem | i should have gone to university to learn about hospital | |
| 20:39:38 | temka | dansmith, woot, that worked | |
| 20:39:39 | mriedem | colour flavour | |
| 20:40:44 | mriedem | dansmith: so my one reservation on loading the instance mapping user_id from the turducken is that down cell case because i'm not sure what we'd do, likely 500 from the api, but you can't really online data migrate your way out of that b/c the online data migration relies on, you guessed it, the cells being up | |
| 20:41:02 | dansmith | mriedem: yeah | |
| 20:41:29 | dansmith | you're also migrating from a state where cells had to be up, | |
| 20:41:42 | dansmith | so until all that data is patched up, it's not surprising that...all cells have to be up | |
| 20:44:49 | openstackgerrit | Merged openstack/nova master: Ironic: bump minimum API version to 1.38 https://review.openstack.org/636326 | |
| 20:56:31 | temka | "Allocation for resource provider 'e7fbbec5-7f95-4f4e-895c-a14b691bca6e' that does not exist" | |
| 20:56:40 | temka | Looks like I'm missing a placement-y fixture or something | |
| 20:57:34 | mnaser | does nova cache neutron port info somewhere? | |
| 20:57:50 | mriedem | mnaser: instance_info_caches table | |
| 20:57:51 | mnaser | i had a misconfigured n-ovs-agent which lead to resizes failing with binding_failed on the port | |
| 20:58:00 | mnaser | i've reset the info, but its still grabbing the cached value | |