| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-11-11 | |||
| 11:48:10 | gibi | could be | |
| 11:48:19 | owalsh | https://github.com/rdo-packages/neutron-distgit/blob/rpm-master/neutron-metadata-agent.service#L8 | |
| 11:49:03 | gibi | yep, so this rpm config seems to follow what devstack does | |
| 11:49:49 | gibi | compared that to https://github.com/rdo-packages/nova-distgit/blob/rpm-master/openstack-nova-compute.service | |
| 11:50:04 | gibi | I would say the neutron packaging config is better | |
| 11:50:56 | gibi | at least it is more structured and follow what devstack does | |
| 11:51:08 | gibi | so it is more closer what upstream CI tests | |
| 11:51:28 | owalsh | gibi: yea, but nova.conf has been around a lot longer :-) | |
| 11:52:17 | gibi | sure, I can imagine nova is historically used like that | |
| 12:26:18 | zigo | gibi: I don't unserstand your question. Do you mean, is it the same nova.conf on both roles? Yeah, it is. | |
| 12:27:17 | zigo | The nova.conf file is stored in the nova-common package, which is a dependency of all nova services. | |
| 12:27:48 | zigo | It must be the same file anyways, because there's the case were we got all-in-one. | |
| 12:27:54 | zigo | Does this answer your question? | |
| 12:29:01 | gibi | zigo: thanks | |
| 12:29:04 | gibi | zigo: yes it does | |
| 12:29:15 | zigo | Oh, I see, it's about https://review.opendev.org/#/c/762176/ | |
| 12:29:30 | zigo | Well, indeed, -1 for me as it would prevent an all-in-one setup. | |
| 12:29:31 | gibi | zigo: yes | |
| 12:29:40 | gibi | zigo: I guess that patch breaks debian then too | |
| 12:30:26 | gibi | in the all-in-one-case | |
| 12:30:45 | gibi | as nova.conf will have a [database]connection config but | |
| 12:30:50 | zigo | It'd be ok if nova-compute was using a different filename, in which case we should just not add the [database] section for nova-compute. | |
| 12:31:17 | gibi | zigo: owalsh figured out the nova-compute binary reads nova.conf by default | |
| 12:31:49 | gibi | so if there is no --config-file added to the binary then it reads the same file as other nova binaries in an all-in-one deployment | |
| 12:31:50 | zigo | Please don't attempt to use /etc/nova/nova-compute.conf, as this is already in use in Debian. | |
| 12:32:34 | zigo | The Debian package already does: | |
| 12:32:34 | zigo | nova-compute --config-file=/etc/nova/nova.conf --config-file=/etc/nova/nova-compute.conf --log-file=/var/log/nova/nova-compute.log | |
| 12:32:34 | zigo | to startup nova-compute. | |
| 12:33:12 | zigo | Though, feel free to make it use something else, like /etc/nova/nova-compute-service.conf, then nova-compute can get rid of reading /etc/nova/nova.conf reading... | |
| 12:33:26 | zigo | (in fact, whatever, I'll adapt the packaging anyways...) | |
| 12:33:53 | gibi | zigo: thanks for the input | |
| 12:34:11 | gibi | I will try to get oppinions from the rest of the core team before moving forward | |
| 12:43:42 | sean-k-mooney | zigo: nova-compute should not need to read nova.conf today | |
| 12:44:03 | sean-k-mooney | zigo: what you could do is remove all the db creds form nova.conf and put them in nova-db.conf | |
| 12:44:15 | sean-k-mooney | then put common config in nova.conf | |
| 12:44:28 | sean-k-mooney | and start nova compute with -config-file=/etc/nova/nova.conf --config-file=/etc/nova/nova-compute.conf | |
| 12:44:41 | sean-k-mooney | and nova-api with -config-file=/etc/nova/nova.conf --config-file=/etc/nova/nova-db.conf | |
| 12:44:48 | sean-k-mooney | or similar | |
| 12:45:34 | sean-k-mooney | i really dont think we need to add a new file | |
| 12:46:09 | sean-k-mooney | we could but the expectation was for a very long time that nova.conf is used by the contol plane service only | |
| 12:46:31 | openstackgerrit | Takashi Natsume proposed openstack/nova master: Remove six.binary_type/integer_types/string_types https://review.opendev.org/728094 | |
| 12:46:33 | sean-k-mooney | and nova-compute.conf or similar was used for the compute service which would not read nova.conf | |
| 12:46:55 | openstackgerrit | Takashi Natsume proposed openstack/nova master: Remove six.text_type (1/2) https://review.opendev.org/728109 | |
| 12:50:09 | sean-k-mooney | this was hit in kolla in queens https://github.com/openstack/kolla-ansible/commit/11b9eba2a48b793d00ba04d7ace8f644c2a14f97 and after devstack hit this in rocky | |
| 12:52:12 | sean-k-mooney | zigo: i think the fix for debian is to jsut continue to use the nova-compute.conf and not use nova.conf at all | |
| 12:52:36 | openstackgerrit | Lee Yarwood proposed openstack/nova-specs master: WIP - Image and flavor defined ephemeral storage encryption https://review.opendev.org/752284 | |
| 12:54:35 | openstackgerrit | Takashi Natsume proposed openstack/nova master: Update contributor guide for Wallaby https://review.opendev.org/754427 | |
| 12:54:50 | openstackgerrit | Takashi Natsume proposed openstack/nova stable/victoria: Fix a hacking test https://review.opendev.org/758112 | |
| 12:54:58 | sean-k-mooney | zigo: by the way i consider having the db cred in the compute service to be a low severity securtiy issue which is why i have been pushing to get them removed for years | |
| 12:56:19 | sean-k-mooney | its low severity because you have to break out of the guest to get them and if you can get to the config you can get the rabbit creds anyway | |
| 13:02:07 | sean-k-mooney | zigo: just loking at https://salsa.debian.org/openstack-team/services/nova/-/blob/debian/victoria/debian/nova-compute.init.in i dont see nova.conf being included | |
| 13:03:47 | sean-k-mooney | owalsh: by the way we do not need a deprectation cycle for this | |
| 13:04:09 | sean-k-mooney | owalsh: we have not supported having the db cred in the compute agent since grizzly | |
| 13:04:47 | sean-k-mooney | we already had a dprecation cycle for this and it was raised a a relase blocker for our downstrem product for 16.1 | |
| 13:05:00 | owalsh | sean-k-mooney: may have been the intention but that's not what happend in the rpms/debs AFAICT | |
| 13:05:21 | sean-k-mooney | other tools have fixed it | |
| 13:05:30 | owalsh | sean-k-mooney: I can fix tripleo/16.1, it's puppet-nova and rpm installs that I'm concerned about | |
| 13:05:31 | sean-k-mooney | like kolla and devstack | |
| 13:06:00 | tkajinam | what would happen for example in the deployment where they use rpm and some other config management tools ? | |
| 13:06:07 | sean-k-mooney | well that can also be fix by using a nova-compute.conf | |
| 13:06:12 | tkajinam | for example if the deployment has ironic usually nova-compute runs on controller nodes | |
| 13:06:25 | openstackgerrit | Takashi Natsume proposed openstack/nova master: Remove six.text_type (2/2) https://review.opendev.org/728117 | |
| 13:06:32 | sean-k-mooney | tkajinam: you use a spereate file that is all that is required | |
| 13:06:45 | owalsh | sean-k-mooney: the rpm systemd unit does not use a separate file | |
| 13:06:54 | sean-k-mooney | right that is a bug | |
| 13:07:20 | sean-k-mooney | and to be clear i first raised this as a potential security issues a few years ago | |
| 13:07:35 | sean-k-mooney | when i was still at intel | |
| 13:08:26 | sean-k-mooney | we had direct question form some telco partners we were workign with wanting to know if nova compute agtully need direct db access | |
| 13:08:58 | sean-k-mooney | we told them no it just need rabbmq because all db acces form the comptue is mideiated vai the conductor | |
| 13:10:07 | sean-k-mooney | the local conductor support was deprecated in 2015 https://github.com/openstack/nova/blob/master/releasenotes/notes/deprecate-local-conductor-9cb9f45728281eb0.yaml | |
| 13:10:44 | tkajinam | I understood that nova-compute doesn't need db access now, so these settings are indeed unnecessary and unnecessarily causing a security risk | |
| 13:11:02 | sean-k-mooney | tkajinam: it has not needed it for 10 release | |
| 13:11:51 | sean-k-mooney | tkajinam: all that is changing now is a condtional check for service version that happend when the rpc version was auto | |
| 13:11:57 | sean-k-mooney | is not done always | |
| 13:12:08 | sean-k-mooney | that check breaks if the db creds are in the config | |
| 13:12:47 | sean-k-mooney | we have patchs to work around it but we do not want to misslead people by allowing them to store db cred that will never be used in teh compute agent config | |
| 13:12:59 | tkajinam | yeah I see | |
| 13:13:52 | sean-k-mooney | if you had the rpc upgrade level set to auto you have not been able to have the api db creds in the db since i think rocky when cells v2 was added | |
| 13:13:54 | tkajinam | I do agree that we should go forward to remove unnecessary db creds, but at the same time we need some migration period so that we can prepare expected config structure | |
| 13:14:19 | tkajinam | at lease we need some changes in packaging (for default config files) and puppet-nova at least | |
| 13:14:36 | owalsh | and docs I expect | |
| 13:15:12 | owalsh | sean-k-mooney: can we use something other than CONF.api_database_connection to determine the cell scope? | |
| 13:15:36 | sean-k-mooney | not really | |
| 13:15:49 | sean-k-mooney | we could but i dont think we should | |
| 13:16:15 | sean-k-mooney | from a donwstream point of view we shoudl be fixing this in osp 13 as well | |
| 13:16:31 | sean-k-mooney | certenly in 16.1/16.2 | |
| 13:17:01 | owalsh | sean-k-mooney: yea, not concerned about this in tripleo, it's the other use cases | |
| 13:17:33 | sean-k-mooney | we had agreed not to block this in stable by the way | |
| 13:17:40 | tkajinam | my concern here would be that this is now breaking puppet jobs because we need new config file structure to start nova services in a single node | |
| 13:17:44 | sean-k-mooney | we are only going to make it a hard error on master/wallabyt | |
| 13:17:53 | sean-k-mooney | so packages have all of wallaby to update | |
| 13:18:21 | sean-k-mooney | we are going to backprot the patch below it which logs a warning to stable | |
| 13:20:01 | tkajinam | you mean that you will add some fall back mechanism ? | |
| 13:20:06 | sean-k-mooney | tkajinam: we could potentially delay merging the blocker patch a few week to give puppet-nova some time to update it be i think we should still merge it this cycle without a deprecation notice since it has not been supprot for a very long time but we can add an upgrade warning | |
| 13:20:14 | sean-k-mooney | its already proposed | |
| 13:20:50 | sean-k-mooney | https://review.opendev.org/#/c/762175/1/nova/utils.py | |
| 13:21:28 | sean-k-mooney | the bottom ptach catches the db not allowed excption and converts it to a warning. then does the local cell lookup | |
| 13:21:37 | tkajinam | from puppet perspective we need to know how distros change their default file structure | |
| 13:21:43 | sean-k-mooney | owalsh: tkajinam | |
| 13:21:52 | tkajinam | sean-k-mooney, yeah. that's what I exactly expected | |