| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-08-05 | |||
| 12:36:40 | sean-k-mooney | but just doing it once might be fine too | |
| 12:36:43 | lyarwood | stephenfin: LGTM btw | |
| 12:36:51 | sean-k-mooney | at the end | |
| 12:37:25 | sean-k-mooney | gibi: did you confirm it was libvirt by stoping the sriov nic agent? | |
| 12:38:22 | sean-k-mooney | gibi: or did you manage to find a log message | |
| 12:42:47 | gibi | sean-k-mooney: stopping the neutron nic agent did not solved the race so I assumed it is libvirt | |
| 12:42:59 | sean-k-mooney | yep makes sense to me | |
| 12:43:34 | sean-k-mooney | i saw in the libvirtd log that it does set the mac a number of time sbut i dont have the devstack logs to corralte the timestamps | |
| 12:43:42 | alex_xu | efried: do you know what is the usecase for this https://review.opendev.org/#/c/693414/3/specs/ussuri/approved/provider-config-file.rst@247 | |
| 12:45:02 | gibi | sean-k-mooney: I will add a separate patch into the series that moves the unplug | |
| 12:45:15 | efried | alex_xu: Yes. | |
| 12:45:15 | efried | The idea there was that you could define a "default" rule to apply to all your compute nodes, but then override it for specific ones. | |
| 12:47:00 | efried | Used for ironic, but also in cases where you want to have your rules centralized and ansibled out to the hosts. | |
| 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 | |