| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-04-14 | |||
| 19:40:17 | bauzas | erm, do folks here have any ideas on how to get a right CONF.host value within a functional test ? | |
| 19:40:51 | bauzas | I mean, I could self.flags it but given it's a global object, all the created computes in the functest will get the same valur | |
| 19:40:53 | bauzas | value* | |
| 20:29:45 | melwitt | bauzas: yes, you do that by passing the 'host' kwarg when you start_service https://github.com/openstack/nova/blob/9001d7e3459d97f507e8ce638d1fc3935401252d/nova/test.py#L381-L391 | |
| 20:30:23 | bauzas | melwitt: sure, I saw it | |
| 20:30:37 | bauzas | melwitt: but for example, I start two computes in a functest | |
| 20:30:42 | bauzas | (using this helper) | |
| 20:31:04 | melwitt | artom: yeah ... the 2 Ts version is owned by someone else and I was uncreative so I just tacked on another t. probably should have picked something else, in hindsight | |
| 20:31:50 | bauzas | melwitt: but then, when I want to confirm resize an instance, the global CONF.host is only having the last host value | |
| 20:31:51 | bauzas | https://github.com/openstack/nova/blob/9001d7e3459d97f507e8ce638d1fc3935401252d/nova/virt/libvirt/driver.py#L1519 | |
| 20:32:47 | melwitt | bauzas: oh, I see. I don't know of a way around that. you would probably need the ghost of mriedem. or maybe gibi might have ideas | |
| 20:33:12 | bauzas | melwitt: so, for example, if I create an instance for host1, resize it to host2 and then confirm the resize, it won't remove the guest in host1 because instance.host (host2) == CONF.host (host2) even I checked that it was calling host1 | |
| 20:33:38 | bauzas | melwitt: no worries, I'll just then modify the CONF before calling confirm_resize | |
| 20:36:04 | mriedem | isn't there another handle within the libvirt driver to the host value used when the service was started? | |
| 20:36:06 | mriedem | the compute service i mean | |
| 20:36:16 | mriedem | if so, you can change that code to use that and not deal with global config problems | |
| 20:38:11 | mriedem | the compute driver has a handle to the virtapi which has a handle to the compute manager which has a self.host value | |
| 20:39:09 | bauzas | mriedem: yeah, no worries, I can provide a change for it | |
| 20:39:25 | mriedem | so i think it's just `self.virtapi._compute.host` | |
| 20:39:40 | melwitt | oh yeah, self.host, that will be different even in func tests. good call | |
| 20:40:06 | mriedem | you could add a "host" property getter method to the virtapi so that the driver doesn't need to know about self._compute | |
| 20:40:23 | mriedem | self.virtapi.host | |
| 20:40:25 | mriedem | badabing | |
| 20:41:09 | bauzas | thanks | |
| 20:41:28 | bauzas | fwiw, we have a lot of CONF.host values in libvirt | |
| 20:41:54 | bauzas | so I'll just create a change for all of them | |
| 20:45:03 | artom | bauzas, yeah, there's not much you can do with the global CONF.host... | |
| 20:45:19 | artom | Although the computes should keep their hostnames... | |
| 20:45:24 | artom | So maybe there's a bug somewhere? | |
| 20:45:48 | bauzas | artom: that's not really a bug | |
| 20:45:51 | artom | Oh, wait, melwitt linked the "wrong" helper | |
| 20:45:55 | bauzas | artom: it's just a problem for tests | |
| 20:46:15 | artom | bauzas, are you using the fake driver, or the libvirt driver? | |
| 20:46:22 | bauzas | the latter | |
| 20:46:31 | bauzas | hence the problem | |
| 20:46:58 | bauzas | anyway, I found the solution | |
| 20:47:00 | melwitt | artom: what do you mean "wrong" helper? | |
| 20:47:12 | artom | bauzas, https://github.com/openstack/nova/blob/9001d7e3459d97f507e8ce638d1fc3935401252d/nova/tests/functional/libvirt/base.py#L117 | |
| 20:47:32 | bauzas | I all know about it | |
| 20:47:58 | bauzas | but again, CONF.host will be overrided by the last value | |
| 20:48:16 | bauzas | given it's a global variable | |
| 20:48:21 | artom | bauzas, no way around it, then | |
| 20:48:26 | artom | CONF is global per process | |
| 20:48:37 | bauzas | we only run by a single process fwiw | |
| 20:48:43 | artom | Yep | |
| 20:48:44 | bauzas | (for testing) | |
| 20:48:59 | artom | OK, my kids are impatiently calling me to literally go fly a kite | |
| 20:49:04 | artom | So I'm off again | |
| 20:49:06 | mriedem | if the driver code uses the host set on the compute manager (via virtapi) then you get whatever host name was used when the service was started | |
| 20:49:12 | mriedem | so, ween off global conf | |
| 20:49:16 | bauzas | anyway, I got the workaround (changing the value before calling the post api), and I'll create a new change for libvirt | |
| 20:49:54 | bauzas | I mean, I'll propose the functional test first, and then another change for just using virtapi in libvirt | |
| 20:49:58 | sean-k-mooney | i think you could mock it to have different values in different greenthread/coroutiens but ya the currnet constution makes having different values chalanging | |
| 20:50:02 | bauzas | done. | |
| 20:50:48 | bauzas | sean-k-mooney: oh yeah of course, we could start the services by each greenthread... and then we would see problems :p | |
| 20:50:56 | bauzas | or I dunno | |
| 20:51:02 | bauzas | anyway | |
| 20:51:06 | bauzas | I'm done | |
| 21:51:13 | sean-k-mooney | bauzas: mriedem melwitt this is how we can mock the globals i think http://paste.openstack.org/show/792118/ | |
| 21:51:41 | melwitt | sean-k-mooney: ahhh mine eyes! | |
| 21:52:43 | sean-k-mooney | hehe i have to create an event loop to get it to interleave and emulate two compute services so there is a bit of boiler plate | |
| 21:53:03 | mriedem | or just stop using globals | |
| 21:53:05 | bauzas | sean-k-mooney: lol | |
| 21:53:11 | openstackgerrit | Sylvain Bauza proposed openstack/nova master: Pass allocations to virt drivers when resizing https://review.opendev.org/589085 | |
| 21:53:12 | openstackgerrit | Sylvain Bauza proposed openstack/nova master: Allocate mdevs when resizing or reverting resize https://review.opendev.org/712741 | |
| 21:53:12 | openstackgerrit | Sylvain Bauza proposed openstack/nova master: Pass allocations to virt drivers when reverting resize https://review.opendev.org/712118 | |
| 21:53:19 | bauzas | sean-k-mooney: I'm just uploading the series ^ | |
| 21:53:41 | bauzas | aaaaaand I'm done today (well, tomorrow is in 7 mins) | |
| 21:53:51 | melwitt | time to start again in 7 minutes! | |
| 21:53:51 | sean-k-mooney | well stop using gloabals means deviate form how oslo conf is used everywhere else in openstack | |
| 21:53:52 | artom | mriedem, well, a global per-process CONF makes sense | |
| 21:54:07 | artom | It's not like different bits of a single nova process can have different configs | |
| 21:54:08 | sean-k-mooney | not that im saying we should not stop using globals | |
| 21:54:43 | bauzas | sean-k-mooney: artom: melwitt: honestly, let's discuss it on the change I'll provide tomorrow morning my time (ie. using virtapi.host field instead of CONF.host) | |
| 21:54:45 | artom | Though I suppose single global config != single global variable | |
| 21:54:45 | mriedem | sean-k-mooney: it's not used globally in placement, intentionally | |
| 21:54:48 | sean-k-mooney | i just wanted to point out we cauld create muliple patchers for the config an activate them as needed | |
| 21:54:49 | mriedem | cdent burned all of that out | |
| 21:55:12 | artom | Smart man | |
| 21:55:12 | bnemec | Ugh, really? | |
| 21:55:14 | sean-k-mooney | mriedem: ya well that was because they had the same proablem and bit the bullet | |
| 21:55:16 | mriedem | and it's not meant to be used globally, the oslo docs even say that i think, but that's what's happened | |
| 21:55:16 | bnemec | Please don't do that. | |
| 21:55:46 | artom | Anyways, I have to make supper | |
| 21:55:53 | sean-k-mooney | bnemec: stop using global or my fun with mock | |
| 21:55:55 | artom | I do enjoy these drive-by discussions :P | |
| 21:56:25 | mriedem | https://docs.openstack.org/oslo.config/latest/reference/faq.html#why-does-oslo-config-have-a-conf-object-global-objects-suck | |
| 21:56:44 | mriedem | artom: i was summoned earlier | |
| 21:57:11 | artom | mriedem, summoned eh? You're an Eldrich God? | |
| 21:57:18 | bauzas | again, I agree with mriedem, I think the simpliest would be to just stopping to use CONF.host everytime in libvirt | |
| 21:57:20 | artom | At least you don't talk of yourself in the third person ;) | |
| 21:57:21 | bnemec | mriedem: That's saying you _should_ use the global object because it's what everyone else is doing. | |
| 21:57:27 | mriedem | more like a pit fiend | |
| 21:57:43 | mriedem | bnemec: it concedes | |
| 21:57:50 | mriedem | "yes it sucks but everyone is doing it so sure" | |
| 21:58:19 | bnemec | I can tell you it would break parts of the policy generator if you dropped it. | |
| 21:58:19 | artom | Ah, the old jump off a bridge debate | |
| 21:58:29 | artom | mriedem, dammit, jinx! | |
| 21:59:09 | mriedem | bnemec: tbc, the suggestion was to stop using the global config to access one variable in one very specific place in the libvirt driver so that functional tests weren't mocking or hacking global config to workaround its global-ness | |
| 21:59:28 | mriedem | bnemec: nova is never going to fully move off global config usage, it's just that one spot i was talking about | |