| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-12-17 | |||
| 12:56:10 | Yumeng | hi gibi, good afternoon. | |
| 12:56:16 | gibi | Yumeng: hi | |
| 12:56:35 | Yumeng | are you almost in holiday? :) | |
| 12:57:32 | Yumeng | I saw nova is going to cancel the next two weekly meeting. | |
| 12:57:33 | gibi | I'm off from next week | |
| 12:57:46 | gibi | Yumeng: yes, today is the last meeting this yera | |
| 12:57:48 | gibi | year | |
| 12:58:12 | Yumeng | wow, sounds excited and happy. | |
| 12:58:54 | Yumeng | I'm gonna catch you to discuss vGPU before you off. lol | |
| 12:59:03 | gibi | OK | |
| 13:01:01 | Yumeng | current nova code has this to support get accel_info to resume guest state when a host is booted. https://review.opendev.org/c/openstack/nova/+/767273/1/nova/virt/libvirt/driver.py#3490 | |
| 13:01:45 | Yumeng | and your concern is if cyborg-agent service starts after nova-compute, nova can not get acc_info as expected，how should we solve this issue? | |
| 13:02:47 | sean-k-mooney | we shoudl not in nova, at least not entirely | |
| 13:03:15 | sean-k-mooney | the deployment tools shoudl use systemd before/after to order the service starts | |
| 13:03:30 | gibi | I guess we need to make clear in the doc that we have an service restart ordering dependency on cyborg | |
| 13:04:05 | gibi | other than that nova could simply fail to reboot those VMs during service startup that needs cyborg | |
| 13:04:05 | sean-k-mooney | well ideally the cyborg agent shoudl have a "before: nova-compute.service" requirement | |
| 13:04:10 | sean-k-mooney | not the other way around | |
| 13:04:44 | sean-k-mooney | gibi: well we can and should call cyborgs api and do the async wait for the arq bidnigns | |
| 13:05:13 | sean-k-mooney | if the agent comes online in that tiem and responds it would be fine but yes we coudl skip them if not | |
| 13:05:33 | sean-k-mooney | the resume guest on host reboot feature does not work unless you use system to force the ordering anyway | |
| 13:05:53 | sean-k-mooney | libvirtd and openvswitch for example both need to be started before nova-compute | |
| 13:06:03 | Yumeng | "the resume guest on host reboot feature does not work unless you use system to force the ordering anyway" +1 | |
| 13:06:13 | gibi | Today we don't do async in this code path | |
| 13:06:35 | gibi | at least nova.compute.manager.ComputeManager._get_accel_info does not do that | |
| 13:06:59 | sean-k-mooney | are we just getting the info or rebining on reboot | |
| 13:07:06 | gibi | just getting infor | |
| 13:07:13 | gibi | no rebind as far as I see | |
| 13:07:14 | sean-k-mooney | if its just getting the info there is no depenciy on the agent being running | |
| 13:07:26 | sean-k-mooney | that info shoudl come form the db | |
| 13:07:35 | gibi | yeah, good point | |
| 13:07:46 | sean-k-mooney | if we however need the agent to create mdevs | |
| 13:07:51 | gibi | but I'm not familiar with cyborg internal arch | |
| 13:07:55 | sean-k-mooney | for exampel then we cant use GET | |
| 13:08:35 | sean-k-mooney | or at least if we did we would need to have cyborg reflect the status fo the binding as not active or complete | |
| 13:08:50 | Yumeng | I think we need use GET and also the agent to create mdevs | |
| 13:08:59 | sean-k-mooney | then we coudl poll or better wait for the externa event form the agent to signal cojmplete of its startup | |
| 13:09:07 | sean-k-mooney | like we do for spawn | |
| 13:09:34 | sean-k-mooney | Yumeng: did we reject or at lest strong advise agaisnt makeing GET magic in the cyborg api | |
| 13:10:09 | sean-k-mooney | where it would cause the agent to reporvison the attemnet/device if the host restarted | |
| 13:11:05 | sean-k-mooney | Yumeng: i though we were goign to reflect the status in the db and have the agent automaticaly create the devices on start up and signle that its not finised provioning in the api via a status field | |
| 13:11:44 | sean-k-mooney | gibi: that is why i tought we were doing the async event waiting by the way ^ | |
| 13:12:09 | gibi | ack | |
| 13:13:41 | sean-k-mooney | Yumeng: if we allow GET on the bindings to change state it basically means that cyborg cannot support the READONLY api personces as part of the keyston RBAC/policys effort | |
| 13:14:06 | openstackgerrit | Lee Yarwood proposed openstack/nova master: WIP db: Add machine_type to instance extras https://review.opendev.org/c/openstack/nova/+/767531 | |
| 13:14:06 | openstackgerrit | Lee Yarwood proposed openstack/nova master: WIP objects: Add machine_type to instance https://review.opendev.org/c/openstack/nova/+/767532 | |
| 13:14:07 | openstackgerrit | Lee Yarwood proposed openstack/nova master: WIP libvirt: Record the machine_type of instances during init_host https://review.opendev.org/c/openstack/nova/+/767533 | |
| 13:14:11 | lyarwood | gibi / sean-k-mooney / stephenfin ; http://paste.openstack.org/show/801124/ ^ I'm getting this on instance.save() and can't for the life of me work out why, any ideas? I think I'm missing something basic in the db layer. | |
| 13:15:01 | Yumeng | sean-k-mooney: nope. it is not allowed to change sate in GET | |
| 13:15:02 | lyarwood | I *think* instance_update_and_get_original is trying to update machine_type in the instance db for some reason | |
| 13:15:24 | brinzhang_ | gibi, sean-k-mooney: we found this issue from you comments in vGPU support spec, depends on the "resume_guests_state_on_host_boot=True" config | |
| 13:15:37 | brinzhang_ | https://review.opendev.org/c/openstack/nova-specs/+/750116/9/specs/wallaby/approved/support-vGPU-nova-cyborg-interaction.rst#183 | |
| 13:16:03 | sean-k-mooney | lyarwood: ill take a look but we shoudl not have a machine_type field. it is ment to be img_machine_type in the instance_system_metadata table | |
| 13:16:57 | Yumeng | sean-k-mooney: "the agent automaticaly create the devices on start up" Does this mean cyborg will need another periodic task to sync arq in db with mdevs in the sys path? | |
| 13:16:58 | sean-k-mooney | so "mapper.column_attrs[key], value" looks wrong to me | |
| 13:16:59 | lyarwood | sean-k-mooney: the above series adds machine_type as a field to the instance_extras table etc | |
| 13:17:22 | lyarwood | sean-k-mooney: I've just missed something somewhere leading to this error when I set it in the instance object and try to save | |
| 13:17:48 | sean-k-mooney | lyarwood right but you shoudl not be doing that | |
| 13:18:03 | sean-k-mooney | lyarwood: wasnt the plan to have no db migraiton requireed for this | |
| 13:18:15 | sean-k-mooney | that is what we dicussed at the ptg | |
| 13:19:05 | lyarwood | I think you suggested adding this to system metadata but I then said it may as well go into instance extras | |
| 13:19:17 | lyarwood | it's in the spec as an instance extra | |
| 13:19:22 | openstackgerrit | Elod Illes proposed openstack/nova stable/stein: [stable-only] Cap bandit to 1.6.2 https://review.opendev.org/c/openstack/nova/+/766487 | |
| 13:19:36 | sean-k-mooney | ok i think that is not the right way to do this | |
| 13:19:52 | sean-k-mooney | at least not as a general pattern if we are adding more fileds | |
| 13:20:01 | sean-k-mooney | which is why it wanted it in system metadata to begin with | |
| 13:20:19 | sean-k-mooney | since we will likely want to do this again going forward when we change defaults | |
| 13:20:39 | lyarwood | what's the issue with this being an instance extra? | |
| 13:21:00 | sean-k-mooney | it will require a new column and db migration for every filed for not real value | |
| 13:21:41 | sean-k-mooney | if we use system metadata we dont require either | |
| 13:22:02 | sean-k-mooney | its also where the image metadata is currently sotred | |
| 13:22:28 | sean-k-mooney | so if we recored itn in img_machine_type we dont need to update any code that currently uses it | |
| 13:22:50 | sean-k-mooney | that way we dont miss anythign and it minimsies the code changes | |
| 13:22:51 | lyarwood | oh hell no, I'm not overloading img_machine_type | |
| 13:23:00 | sean-k-mooney | that was the whole point | |
| 13:23:53 | lyarwood | I didn't get that memo tbh, I've always wanted to track the machine type of the instance seperate to anything else | |
| 13:24:27 | lyarwood | so we can move it forward in the future without overwriting the image meta etc | |
| 13:24:30 | sean-k-mooney | we could use a different prefix in the system metadata | |
| 13:25:01 | sean-k-mooney | but im pretty stongly -1 on a new db column | |
| 13:25:44 | lyarwood | kk let me look into using it instead | |
| 13:26:00 | lyarwood | I honestly didn't think a new extra col for this would be a big deal | |
| 13:26:10 | lyarwood | given what's already in there | |
| 13:26:53 | sean-k-mooney | well given we want to evenually recored the value of all image properties and the machine_type propsoal was just a scoped down verion of that i think its a problematic design | |
| 13:27:30 | lyarwood | it wasn't a scoped down version of that at all | |
| 13:27:46 | lyarwood | I think you're crossing wires here tbh | |
| 13:27:49 | sean-k-mooney | it was ment to be | |
| 13:28:16 | lyarwood | it was always just about recording the machine type of existing instances | |
| 13:28:24 | sean-k-mooney | thats what i discussed with stephen when we were deciding if i or you would write the spec | |
| 13:28:55 | lyarwood | the image properties are part of it but not all instances will have the machine type set that way | |
| 13:29:15 | sean-k-mooney | yes i know | |
| 13:29:36 | lyarwood | this was always about capturing the currently used machine type and stashing it somewhere | |
| 13:29:43 | sean-k-mooney | yep | |
| 13:29:45 | lyarwood | allowing the config to change | |
| 13:29:58 | lyarwood | I didn't take from that the need to use the image properties | |
| 13:30:14 | lyarwood | I wasn't even planning to look at the image properties tbh | |
| 13:30:20 | lyarwood | Just the defined domains | |
| 13:30:41 | sean-k-mooney | well i had a very differnet plan for implementing it then | |
| 13:31:53 | sean-k-mooney | i will also point out form a downstream perspecitve we may need to backport this to 16.2 to support 17 upgrades | |
| 13:32:11 | sean-k-mooney | that would depend on how we handel the defualt machine type in 17/wallaby | |
| 13:32:34 | sean-k-mooney | its intended to be q35 for all new installs | |