Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2020-04-14
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: Pass allocations to virt drivers when reverting resize https://review.opendev.org/712118
21:53:12 openstackgerrit Sylvain Bauza proposed openstack/nova master: Allocate mdevs when resizing or reverting resize https://review.opendev.org/712741
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 sean-k-mooney well stop using gloabals means deviate form how oslo conf is used everywhere else in openstack
21:53:51 melwitt time to start again in 7 minutes!
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 mriedem sean-k-mooney: it's not used globally in placement, intentionally
21:54:45 artom Though I suppose single global config != single global variable
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 bnemec Ugh, really?
21:55:12 artom Smart man
21:55:14 sean-k-mooney mriedem: ya well that was because they had the same proablem and bit the bullet
21:55:16 bnemec Please don't do that.
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: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 artom Ah, the old jump off a bridge debate
21:58:19 bnemec I can tell you it would break parts of the policy generator if you dropped it.
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
22:00:07 bnemec That sounds perfectly reasonable.
22:00:33 bauzas again, I think the easiest is to just stop using global objects as much as we can
22:00:46 bauzas so, bye CONF.host and just check what we need once
22:01:05 bnemec The one danger of not looking it up from the conf object each time is that mutable config won't work correctly, unless you write a hook to update it.
22:01:27 bnemec But that's both solvable and not an issue if that config opt isn't marked mutable.
22:05:51 sean-k-mooney this is a cleaner verion of the same thing by the way http://paste.openstack.org/show/792119/
22:08:11 sean-k-mooney compute-1 and comptue-2 have there own view of the global object that they can modify without affecting each other.
22:09:26 sean-k-mooney this is still rather archane so im not really sure i would want to do this but if i coudl make it into a nice decorator to hide this i could be an option. anyway i just wanted to play with mocking and see what i could do
22:12:32 sean-k-mooney bnemec: we dont really support mutable config for many config options today in nova
22:13:00 sean-k-mooney bnemec: some stuff yes but you cant change the virt dirver while its runing or many of the virt driver options
22:19:43 openstackgerrit Ghanshyam Mann proposed openstack/nova master: Fix servers policy for admin_or_owner https://review.opendev.org/718501
22:19:59 openstackgerrit Ghanshyam Mann proposed openstack/nova master: Add test coverage of existing server policies https://review.opendev.org/718348
22:21:27 gmann stephenfin: can you re approve these, I lost +A due to rebase - https://review.opendev.org/#/c/718348/ https://review.opendev.org/#/c/718501
22:28:38 mriedem bnemec: in this case it's CONF.host which isn't mutable, and if you change the hostname on a compute node you're ... kind of destroying that node
22:48:09 sean-k-mooney mriedem: we often want to change other value that also are not mutable when we do multi node functional test and simple using another value is not always an option
22:48:51 sean-k-mooney im thinking of the cpu_dedicated_set and other values like that where it would be really nice to have a two node functional test with different values.
22:49:43 sean-k-mooney anyway its not relavent to bauzas patch but its frequently a problem we hit.
22:59:39 ianw hi, nodepool testing hit the removal of "user_data" from the nova client https://opendev.org/openstack/python-novaclient/commit/03dca4bc823c82054869dfaf6925d5e1e068ac51
22:59:56 ianw we were testing that the user_data specified in the nodepool config makes it through to nova
23:00:06 ianw is there another suggested way to check that?
23:03:55 melwitt ianw: hm, yeah I wondered if anyone would be affected by that change when I saw it go through. I can't immediately think of anything other than using the novaclient python binding. or calling the API directly
23:05:44 ianw yeah, i think maybe using the binding might work; it doesn't seem like it's generically useful enough to put back in behind a switch or something
23:06:24 ianw unless others think it is ...
23:07:39 melwitt yeah, I wondered the same.
23:09:20 melwitt it's been years but I've looked at it before as an end user, as a "did I do that right?" sanity check
23:11:33 openstackgerrit Merged openstack/nova master: Add test coverage of existing flavor extra spec policies https://review.opendev.org/719372
23:11:43 corvus melwitt: yeah, i think it's worth keeping in some form because of that
23:12:18 corvus but putting it behind a cli flag is fine -- i get that "dump a bunch of base64 by default" isn't optimal ux :)
23:12:33 melwitt heh yeah
23:13:09 melwitt well, it would have been good to have heard this before last thursday :( cause it's in there and locked in for ussuri now

Earlier   Later