| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-08-05 | |||
| 12:47:13 | alex_xu | efried: I see, thanks | |
| 12:48:37 | alex_xu | efried: is there any reason we should ignore the addtional inventories and traits when conflict the virt driver managed ones, instead of error out the conflict? | |
| 12:49:47 | efried | We debated this at design time. I can tell you for sure the answer to your question is "yes". But I can't remember exactly why :P | |
| 12:50:41 | alex_xu | efried: ok, so the result is ignore, not the error out, right? I saw the code is error out, not ignore. | |
| 12:51:07 | efried | Oh, whatever the design says is what we decided on. | |
| 12:51:28 | alex_xu | efried: ok, thanks :) I'm not going dump our the history | |
| 12:51:51 | efried | For this issue, I'm reasonably sure whatever is in the design is going to be there because it's what we decided on, not because we accidentally missed it. | |
| 12:52:41 | alex_xu | efried: ok, cool | |
| 12:53:44 | efried | https://specs.openstack.org/openstack/nova-specs/specs/victoria/approved/provider-config-file.html#provider-config-consumption-from-nova says "ignore" under "Provider Tree Merging". | |
| 12:55:42 | alex_xu | efried: yes, that is what I read also | |
| 12:56:00 | alex_xu | I can't thinking of a reason the different between ignore and error out also | |
| 12:56:19 | efried | alex_xu: here's an explanation of that other thing https://review.opendev.org/#/c/693414/3/specs/ussuri/approved/provider-config-file.rst@137 | |
| 12:58:07 | alex_xu | nice | |
| 13:00:18 | efried | alex_xu: https://review.opendev.org/#/c/612497/12/specs/train/approved/provider-config-file.rst@204 | |
| 13:00:18 | efried | I remember now: | |
| 13:00:18 | efried | The conflicts in question should error on startup, but be ignored thereafter. | |
| 13:03:08 | alex_xu | efried: for the case, there are virt driver managed inventory or trait show up later? | |
| 13:26:27 | alex_xu | efried: virt driver's update_provider_tree always overwrited provider tree's inventory. so the conflict will be found at startup, and there won't be any conflict after startup. so the code feel like right | |
| 13:30:01 | efried | Yeah, I thought there was a theoretical edge case where we could come across an error at runtime. The chances were very small, but we wanted to make sure we didn't crater the driver if it did happen. I think that's what led to the design decision as it stands. | |
| 13:35:51 | sean-k-mooney | can there be a conflict if i make a call to placement directly and modify the inventory | |
| 13:36:34 | sean-k-mooney | i know you are not really ment to do that but users be users and they dont always do what we tell them | |
| 13:36:52 | efried | Heh. "You just voided your warranty. You're on your own." | |
| 13:37:57 | sean-k-mooney | have you started getting support request form customer yet | |
| 13:38:02 | sean-k-mooney | they do that alot... | |
| 13:38:12 | sean-k-mooney | then ask us to fix it anyway | |
| 13:39:26 | efried | of course. | |
| 13:40:13 | efried | Unless you really break things, any manual change to the placement inventory ought to be scrubbed back out on the next periodic. | |
| 13:40:36 | sean-k-mooney | we had one customer that for example when they wanted to spawn a vm on specifci core on a host, stoped nova-compute, updated the vcpu_pin_set to only have those cores, booted the vm with --avaiablity-zone <zone>:<host> and then complained that if someone did a concurent operation on another vm on the host it could cause issue with pinning ... | |
| 13:41:03 | efried | You could probably add things that neither the compute nor the config care about, and they would stick around. Probably. But that won't break the code, I don't think. | |
| 13:41:06 | sean-k-mooney | efried: ya it should heal on the next run | |
| 13:41:53 | sean-k-mooney | if you create your own RP it defnitly shoudl be ok | |
| 13:42:24 | sean-k-mooney | if you add inventories to one of nova's RPs via the api well thats not allowed so nova is free to delete it | |
| 13:43:16 | sean-k-mooney | im not sure if we actully will delete the inventory but we are allowed too | |
| 13:44:39 | efried | It's been a hot minute since I looked at update_from_provider_tree, but I think we would delete it, yes. | |
| 13:46:09 | sean-k-mooney | alex_xu: by the way i reworked https://review.opendev.org/#/c/739131/ after your comments in version 5, im hoping stephenfin will get back to it to be the second +2 later today but just an fyi incase you want to look at it before then. | |
| 13:46:49 | sean-k-mooney | efried: this is basicaly why we are providing the provider.yaml i.e. to enable a supported way to do this so ya. | |
| 13:47:19 | sean-k-mooney | efried: hows openshift land going? | |
| 13:49:08 | efried | correct | |
| 13:49:33 | efried | Things are going well. After a couple months of pretty serious culture shock and vertical learning curve, I'm getting my feet under me. | |
| 13:50:44 | efried | I've written an operator (in go) and feel pretty comfortable navigating openshift/kube APIs. | |
| 13:51:44 | sean-k-mooney | cool, i have read some go but havent really written any, never had the need. | |
| 13:52:16 | sean-k-mooney | i am vaguly aware of what operators do but never looked under the cover to see how they are implemented | |
| 13:52:55 | efried | With existing tools (operator-sdk) and any kind of sweng background, they're ridiculously easy to write. | |
| 13:52:57 | sean-k-mooney | i belive there are some frameworks to implement them without go too using declaritive yaml files referencing CRDs too right | |
| 13:53:32 | sean-k-mooney | i know you can go the custom contoler route too in go | |
| 13:53:50 | sean-k-mooney | but again more or less have just read the docs and then done nothing with that info | |
| 13:54:00 | sean-k-mooney | /read/skimmed/ | |
| 13:54:38 | efried | The CRDs are how the operators extend the kube API. Instances of those (CRs) are how you trigger the operator to do its thing. I'm not aware of a way to "declare" an operator with just yaml. | |
| 13:54:38 | efried | And yes, an operator is effectively a custom controller that follows some rules. | |
| 13:56:08 | sean-k-mooney | efried: are you invovled with the current experiments to deploy openstack using operators | |
| 13:56:28 | efried | I don't get anywhere near openstack anymore. | |
| 13:56:55 | sean-k-mooney | there is an experiment to run openstack on openshift the repo is public somehere | |
| 14:00:45 | sean-k-mooney | efried: https://github.com/openstack-k8s-operators | |
| 14:01:14 | efried | cool | |
| 14:01:15 | sean-k-mooney | efried: mdbooth and other have been working on that for a while | |
| 14:03:02 | sean-k-mooney | efried: also this is what i was thining of https://kudo.dev/ | |
| 14:05:46 | sean-k-mooney | efried: i was considring writing an operator for deploying zuul at one point and i was looking at kudo amoung other things as a way to do it | |
| 14:06:48 | sean-k-mooney | well zuul, nodepoll , zookeyper, german and gerrit | |
| 14:07:08 | sean-k-mooney | but i just wrote hte manifest by had instead. | |
| 14:16:06 | openstackgerrit | sean mooney proposed openstack/os-vif master: support pyroute2 0.5.13 https://review.opendev.org/744809 | |
| 14:22:45 | openstackgerrit | Merged openstack/nova master: Document nova in tree virt drivers https://review.opendev.org/740061 | |
| 15:17:04 | openstackgerrit | Lee Yarwood proposed openstack/nova master: WIP zuul: nova-multinode-evacuate https://review.opendev.org/744883 | |
| 15:53:02 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Move equality check into LibvirtConfigGuestInterface https://review.opendev.org/744524 | |
| 15:53:03 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Remove unused vpn param from allocate_for_instance https://review.opendev.org/744933 | |
| 15:54:15 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: [WIP] Support SRIOV interface attach and detach https://review.opendev.org/740995 | |
| 15:54:15 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Only unplug vif after the device is detached from libvirt https://review.opendev.org/744934 | |
| 15:57:53 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: DNM: add logging for device matching https://review.opendev.org/744936 | |
| 16:04:21 | openstackgerrit | Harshavardhan Metla proposed openstack/nova stable/rocky: [stable-only] Moved the quoted section https://review.opendev.org/744681 | |
| 16:05:50 | melwitt | lyarwood: I just barely started looking at ceph job failures on stable/queens, saw this error message "libvirtError: internal error: unable to execute QEMU command 'device_add': Property 'virtio-blk-device.drive' can't find value 'drive-virtio-disk1'" https://zuul.opendev.org/t/openstack/build/fa85e5eeb1f84389b5dd259c8a829552/log/controller/logs/screen-n-cpu.txt#32824 | |
| 16:06:03 | melwitt | does that ring any bells to you? | |
| 16:09:01 | melwitt | hm nvm maybe https://ask.openstack.org/en/question/95328/ceph-cinder-attach-volume-to-running-instance | |
| 16:21:01 | openstackgerrit | Harshavardhan Metla proposed openstack/nova stable/rocky: "[stable-only]" Removed the quoted section https://review.opendev.org/744681 | |
| 16:26:46 | stephenfin | melwitt, bauzas: not sure if you'll know this off the top of your head, but should we be releasing resources before we confirm a resize? | |
| 16:27:49 | stephenfin | Based on https://review.opendev.org/#/c/641806/21, it seems we do (via the resource tracker's periodic task run) | |
| 16:28:01 | melwitt | stephenfin: releasing as in deleting allocations on the source before confirming? AFAIK no, supposed to hold allocations on the source and dest until confirm/revert | |
| 16:31:43 | melwitt | I think that patch's commit message is saying prior to that patch, we were reporting resources held on the source *after* a confirm resize (until the next periodic run) whereas after a confirm, the resources should be shown as released on the source. but only after the confirm, not before | |
| 16:32:33 | stephenfin | ah, that makes sense | |
| 16:32:41 | stephenfin | so now we're freeing those resources immediately | |
| 16:33:12 | melwitt | immediately upon a confirm, yeah | |
| 16:34:06 | melwitt | or rather reporting them immediately after a confirm. the claim has always been dropped at the time of confirm, but apparently the reporting was delayed potentially by one periodic task run | |
| 16:34:12 | stephenfin | okay, cool. There's a bug in it that I'm working on (https://bugs.launchpad.net/nova/+bug/1879878, fwiw); tl;dr: if the periodic runs between the API request and the 'confirm_resize' call in the RT, the ComputeNode.numa_topology gets out of whack | |
| 16:34:12 | openstack | Launchpad bug 1879878 in OpenStack Compute (nova) "VM become Error after confirming resize with Error info CPUUnpinningInvalid on source node " [Medium,Confirmed] - Assigned to Stephen Finucane (stephenfinucane) | |
| 16:34:43 | melwitt | ah, gotcha | |
| 16:36:21 | melwitt | oh interesting, so if the periodic task fires while confirm resize is still processing, it turns the instance to ERROR | |
| 16:37:00 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Move equality check into LibvirtConfigGuestInterface https://review.opendev.org/744524 | |
| 16:37:11 | stephenfin | yeah, the '_update_available_resource' function of the RT seems to regenerate ComputeNode objects from scratch, including the embedded numa_topology | |
| 16:37:47 | stephenfin | and it doesn't seem to be accounting from the still-present allocations on the host once the instance has been confirmed | |
| 16:38:12 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Only unplug vif after the device is detached from libvirt https://review.opendev.org/744934 | |
| 16:39:25 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: [WIP] Support SRIOV interface attach and detach https://review.opendev.org/740995 | |
| 16:39:26 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: DNM: add logging for device matching https://review.opendev.org/744936 | |
| 16:42:01 | melwitt | stephenfin: note that sean-k-mooney's comments are correct though, that if this is in a version older than claims in placement, races are expected (especially since we don't have numa in placement). in your repro, is that with cpu_dedicated_set and on the master branch? | |
| 16:42:38 | gibi | sean-k-mooney: added a patch top of the sriov_attach series with extra logging that can help you figuring out why the macvtap and direct-physical devices are not detached for you | |
| 16:42:42 | gibi | sean-k-mooney: https://review.opendev.org/744936 | |
| 16:42:59 | gibi | also added a patch to move the vif unplug after libvirt detach | |
| 16:43:02 | openstackgerrit | Stephen Finucane proposed openstack/nova master: tests: Add reproducer for bug #1879878 https://review.opendev.org/744950 | |
| 16:43:02 | openstack | bug 1879878 in OpenStack Compute (nova) "VM become Error after confirming resize with Error info CPUUnpinningInvalid on source node " [Medium,Confirmed] https://launchpad.net/bugs/1879878 - Assigned to Stephen Finucane (stephenfinucane) | |
| 16:43:02 | openstackgerrit | Stephen Finucane proposed openstack/nova master: TODO https://review.opendev.org/744951 | |
| 16:43:05 | stephenfin | melwitt: ^ | |
| 16:43:15 | stephenfin | I can reproduce trivially with new-style configuration | |
| 16:43:35 | melwitt | while using cpu_dedicated_set? /me looks | |