Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2020-04-14
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
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
23:13:30 corvus but getting at it from the cli (with a flag) "i put a key in user data, and it didn't show up in the booted vm -- did it actually make it to the cloud?" would be nice
23:13:30 melwitt I guess we could backport
23:14:02 melwitt yeah, I agree
23:14:46 melwitt cc stephenfin
23:15:14 corvus ianw: i guess the -src version of that nodepool job doesn't install the clients from source?
23:15:42 corvus iiuc, we have a -src job to detect pre-release problems with openstacksdk, but maybe it stops there...
23:16:01 ianw coruvs: i guess not; i think it only pulls in nodepool, openstacksdk, dib
23:16:36 corvus that must be how we missed it, since the changed merged in feb, and we've certainly merged nodepool changes which ran that job since then
23:16:41 corvus melwitt: sorry :(
23:17:00 gmann melwitt: and i think (not tried) that patch did not remove that from rebuild operation because there it is present in Rebuild API response as 'user_data' not 'OS-EXT-SRV-ATTR:user_data'.
23:17:18 melwitt gmann: hah, nice
23:18:26 gmann for nodepool case, can it be done via server metadata ?
23:21:50 ianw gmann: we're really just testing that the value in the config file for nodepool made it through to the booted nova instance -- the actual data is not used in the test
23:23:18 gmann ohk i thought it as just tagging the things.
23:56:41 openstackgerrit Merged openstack/nova master: Introduce scope_types in flavor extra spec policy https://review.opendev.org/719375
#openstack-nova - 2020-04-15
00:14:36 openstackgerrit Ghanshyam Mann proposed openstack/nova master: Add test coverage of existing remaining servers policies https://review.opendev.org/720104
00:30:06 openstackgerrit Ghanshyam Mann proposed openstack/nova master: Introduce scope_types in remaining servers Policies https://review.opendev.org/720106
00:34:32 openstackgerrit Ghanshyam Mann proposed openstack/nova master: Introduce scope_types in servers Policies https://review.opendev.org/662968
00:34:43 openstackgerrit Ghanshyam Mann proposed openstack/nova master: Add new default roles in servers policies https://review.opendev.org/662971
00:34:54 openstackgerrit Ghanshyam Mann proposed openstack/nova master: Add test coverage of existing server attributes policies https://review.opendev.org/719726
00:35:05 openstackgerrit Ghanshyam Mann proposed openstack/nova master: Introduce scope_types in servers attributes Policies https://review.opendev.org/719729
00:44:34 openstackgerrit Ghanshyam Mann proposed openstack/nova master: Add new default roles in servers attributes policies https://review.opendev.org/719730
00:45:44 openstackgerrit Ghanshyam Mann proposed openstack/nova master: Add test coverage of existing remaining servers policies https://review.opendev.org/720104
00:45:54 openstackgerrit Ghanshyam Mann proposed openstack/nova master: Introduce scope_types in remaining servers Policies https://review.opendev.org/720106
01:05:03 openstackgerrit Brin Zhang proposed openstack/nova-specs master: Re-proposes 'Proposal for a safer remote console with password authentication' https://review.opendev.org/719720
01:16:12 brinzhang_ gibi, sean-k-mooney: I agree with sean-k-mooney, I think his understand is correct, if we failed to bind, that just leak arq db record in Cyborg, I will check again with Sundar, once I am confirmed, I will respin you again.
01:34:58 openstackgerrit zhufl proposed openstack/nova master: Add missing white spaces between words in log messages https://review.opendev.org/720112
01:58:18 brinzhang_ gibi, sean-k-monney, dansmith: another thing maybe need to concern, please check, see below:
01:58:21 brinzhang_ While we build_instances() we should cleanup arqs if the _create_and_bind_arqs() failed.
01:58:30 brinzhang_ cyborgclinet), right?
01:58:30 brinzhang_ In _cleanup_when_reschedule_fails()[1] we clean the arqs if _create_and_bind_arqs() failed, but I think we just only clean it at L847 if the _create_and_bind_arqs() failed [3]. If we add "compute_utils.delete_arqs_if_needed(context, instance)"[2] in _cleanup_when_reschedule_fails()[1], once we call _cleanup_when_reschedule_fails() it will be cleanup arqs with dp name in instance's flavor, I think that will increase Nova's call to Cyborg (by
01:58:43 brinzhang_ [1]https://opendev.org/openstack/nova/src/branch/master/nova/conductor/manager.py#L582
01:58:51 brinzhang_ [2]https://opendev.org/openstack/nova/src/branch/master/nova/conductor/manager.py#L598
01:58:57 brinzhang_ [3]https://opendev.org/openstack/nova/src/branch/master/nova/conductor/manager.py#L848-L849
01:59:16 openstackgerrit Ghanshyam Mann proposed openstack/nova master: Add new default roles in remaining servers policies https://review.opendev.org/720116

Earlier   Later