| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-04-14 | |||
| 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 | |
| 23:13:30 | melwitt | I guess we could backport | |
| 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: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_ | 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:30 | brinzhang_ | cyborgclinet), right? | |
| 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 | |
| 02:00:24 | brinzhang_ | IMO, if I am correct, I think we should move "compute_utils.delete_arqs_if_needed(context, instance)"[2] to the right place, i.e., when _create_and_bind_arqs() failed | |
| 02:05:00 | gmann | johnthetubaguy: i replied on server policies comments and finished the remaining one. please check. my worried is to allow host attr for project that is why i kept them system-admin for now | |
| 02:20:21 | openstackgerrit | Ghanshyam Mann proposed openstack/nova master: Fix follow up comments on policy work https://review.opendev.org/717835 | |
| 02:23:37 | openstackgerrit | Ghanshyam Mann proposed openstack/nova master: Use oslo policy flag to disable default change warning instead of all https://review.opendev.org/717884 | |
| 03:10:20 | openstackgerrit | Merged openstack/nova master: Introduce scope_types in quota set Policies https://review.opendev.org/719317 | |
| 03:44:48 | openstackgerrit | Ghanshyam Mann proposed openstack/nova master: Fix server actions to be system and project scoped https://review.opendev.org/720123 | |
| 05:56:03 | openstackgerrit | Merged openstack/nova master: Add new default roles in server group policies https://review.opendev.org/717175 | |
| 06:35:39 | openstackgerrit | Merged openstack/nova master: Add new default roles in quota sets policies https://review.opendev.org/719361 | |
| 06:45:13 | openstackgerrit | Ghanshyam Mann proposed openstack/nova master: Add doc for policy new defaults https://review.opendev.org/720129 | |