Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2022-06-03
15:57:21 sean-k-mooney they did not add it
15:57:44 sean-k-mooney that is how you whitelisted the VFs of a pf before we added teh glob and regex support
15:57:58 sean-k-mooney so thats been there since like icehouse
15:58:07 gibi it is pretty remote managed specific https://github.com/openstack/nova/blob/d86916360858daa06164ebc0d012b78d19ae6497/nova/pci/devspec.py#L315-L334
15:58:29 sean-k-mooney no this work for any sriov device
15:58:39 sean-k-mooney they just added a test case for it
15:58:43 gibi I think what you are referring to is the ability to list PF and match VF, but the remote managed extends this to list PF with VF's product_id
15:59:20 sean-k-mooney so you used to be able to use the pf address and vf product id
15:59:22 gibi it has an if self._remote_managed: on top so this does not run for the rest of the caseas
15:59:32 sean-k-mooney and that would allow all the VFs of that pf to be used
15:59:36 sean-k-mooney but not the pf itself
15:59:49 sean-k-mooney just looking at the code you linked now
16:00:31 gibi in the generic case I only see PF address matching for VF devs, but no such logic for vendor / product matching
16:02:14 sean-k-mooney i think its this https://github.com/openstack/nova/blob/d86916360858daa06164ebc0d012b78d19ae6497/nova/pci/devspec.py#L254-L265=
16:02:20 sean-k-mooney but i would have tro look at this carfully
16:02:50 sean-k-mooney we do supprot suign adress=<pf addres> product_id=<vf id>
16:03:11 gibi nope, that is only matching addresses
16:03:20 gibi the PhysicalPciAddress has no info about the vendor / product
16:03:22 sean-k-mooney its match the adress object
16:03:32 gibi yepp
16:03:36 gibi only address matching happens there
16:04:05 gibi vendor / product happens independently https://github.com/openstack/nova/blob/d86916360858daa06164ebc0d012b78d19ae6497/nova/pci/devspec.py#L377-L378
16:04:07 sean-k-mooney right but that is combiend with the vendro id and prodcut id filter elsewhere
16:06:28 sean-k-mooney yes it does so the way it works is we mach on vendor and product id first
16:06:41 sean-k-mooney and then we match on the adrees or the partent adress
16:06:51 sean-k-mooney https://github.com/openstack/nova/blob/d86916360858daa06164ebc0d012b78d19ae6497/nova/pci/devspec.py#L379-L380=
16:07:01 melwitt sean-k-mooney: I was wondering if you might be able to help see what's wrong with this failure, I think it might be related to ansible. I have written about it on L57 https://etherpad.opendev.org/p/nova-stable-branch-ci tl;dr is we're passing executable=/bin/bash with the ansible command args but it's running with /bin/sh on the remote side. I don't understand it
16:07:37 melwitt have you seen something like that before?
16:07:54 sean-k-mooney melwitt: i was talking to fungi about that yesterday on #opentack-infra
16:08:04 gibi sean-k-mooney: hm, OK, I see it now. But they why on earth we needed to re-implement the same (?) logic specifically for remote_managed at https://github.com/openstack/nova/blob/d86916360858daa06164ebc0d012b78d19ae6497/nova/pci/devspec.py#L322-L334 ?
16:08:13 melwitt sean-k-mooney: oh cool. I'll go check the channel log
16:09:21 sean-k-mooney gibi: remote managed does not otherwise allow the PF to be listed
16:09:32 sean-k-mooney if i recall correctly
16:10:18 sean-k-mooney melwitt: they did not come to a conclution either but were speculating it was related to hoave devstrack get invoke it and the #!/bin/bash lines in the hook script
16:10:24 sean-k-mooney but those point to bash too
16:10:45 sean-k-mooney melwitt: i think the fix is to stop usinging the raw module and actully use shell
16:11:15 sean-k-mooney i think the raw moduel is perhaps using a shle script to run your comamdn via the executabel you specify
16:11:17 melwitt sean-k-mooney: I wondered that exact same thing. the raw module seemed the only potentially suspicious thing to me
16:11:30 sean-k-mooney so raw is not ment to be used in normal code
16:11:57 gibi sean-k-mooney: thanks. I think I have not enough brain power for this right now
16:12:02 sean-k-mooney raw is intended to install the deps that are required for command and shell so that you can bootstrap an ansible target when its mising the basic required pacakages for ansibel to work
16:12:26 melwitt sean-k-mooney: yeah. supposedly saying executable=/bin/bash will make it run under bash and it must have used to but for some reason recently the behavior has changed
16:13:05 sean-k-mooney yes so looking at the ansible docs the args you would normally pass in ansibel you set as varibles in the string when you are doing an adhock command
16:13:23 melwitt I had been thinking about trying using the shell module instead to see if it helps
16:13:25 melwitt right
16:13:58 sean-k-mooney i kid of hate that we are invokeing ansibel form bash form ansible by the way
16:14:20 melwitt haha yeah ...
16:14:46 sean-k-mooney it feels like it woudl better just to use ssh and be done with it or a script that we copy and be doen with it
16:14:59 sean-k-mooney but its been working this way for years
16:15:04 sean-k-mooney and now just sudenlly stopped
16:15:51 sean-k-mooney gibi: one thing that is true is that we dont have gret test for all the valid way that you can list thing for the pci module
16:15:54 melwitt yeah. puzzling
16:16:13 sean-k-mooney like not clear human readble ones with comments at anyrate
16:17:11 sean-k-mooney so a lot fo the reasoning why x works is buried in my brain next to memroy of swaring at a terminal
16:19:41 sean-k-mooney gibi: all set for the sumit?
16:20:13 gibi yepp, I'm well set.
16:20:14 sean-k-mooney melwitt: are you goign to push a wip patch by the way to try using shell?
16:20:36 sean-k-mooney gibi: do you have a presination ?
16:21:03 gibi yepp
16:21:09 gibi about the qos work
16:21:17 sean-k-mooney ah nice
16:21:26 melwitt sean-k-mooney: I will unless you wanted to. I have pretty much never used ansible and learning about it now
16:21:51 sean-k-mooney you had one on the bandwith work before with migule right so this is the evolution i guess
16:22:44 gibi sean-k-mooney: yes, it was in berlin too :)
16:22:47 gibi so a nice cycle
16:23:00 sean-k-mooney melwitt: i never use it for adhoc comands its kind of like python that way. sure you can pass a string of python code to python and it will execute it but i only ever use it with a script so im really not familar iwth the adhock syntax
16:23:22 gibi at that time we only had WIP code for booting with bandwidth qos ports. No we have a lot more
16:24:24 melwitt sean-k-mooney: yeah I was just reading the docs yesterday. thought it might be fun to try
16:24:47 sean-k-mooney this one https://docs.ansible.com/ansible/latest/user_guide/intro_adhoc.html
16:25:08 chateaulav gibi: im excited to see it
16:25:23 sean-k-mooney gibi: yep its amazing what can hapne over the course of a pandemic...
16:25:25 chateaulav its my first time to berlin
16:25:44 melwitt sean-k-mooney: thanks
16:26:20 gibi sean-k-mooney: I honestly want to fix the pci spec parsing code before I start adding to it, but it is soo full of edge cases that I start feeling the I cannot clean it up without potentially breaking couple of cases that works somehow today
16:26:43 gibi chateaulav: o/
16:27:19 sean-k-mooney gibi: ya am if i get the vdpa stuff landed perhaps i can help add some more testcsaes to prove out some of those edgecases
16:27:47 sean-k-mooney liek we can test the parsign with the libvirt fuctional tests
16:28:31 sean-k-mooney we shoudl not need that level of testing to parse stings but that would be one way to validate teh edge cacses
16:29:31 sean-k-mooney gibi: in your case you are not really chanign the filtering
16:29:44 sean-k-mooney gibi: you are just addign new tags for resocue class and traits
16:30:05 sean-k-mooney so if you avoid refactoring and just focus on that it shoud eb pretty safe
16:30:44 sean-k-mooney the new tags you are adding will not filter in/out any devices
16:31:14 sean-k-mooney so you can mostly just operate on the objects that are constructed after the parsing is done
16:33:02 sean-k-mooney melwitt: by the way on later branches there are zuul rules to do this
16:33:49 melwitt sean-k-mooney: oh nice
16:33:52 sean-k-mooney https://github.com/SeanMooney/ansible_role_devstack/blob/master/ansible/deploy_multinode_devstack.yaml#L133-L136=
16:33:59 sean-k-mooney sync-controller-ceph-conf-and-keys,
16:34:57 sean-k-mooney https://github.com/openstack/devstack/tree/master/roles/sync-controller-ceph-conf-and-keys
16:35:01 sean-k-mooney actully its in devstack
16:35:57 sean-k-mooney im kind of confused why we are donign this in hte gate hook
16:36:02 gibi sean-k-mooney: I promised to reject the devname case in the new codepath, for that I need to spearate out the devname handling codepath from the current parsing
16:36:49 sean-k-mooney am not really you jus need to check if the whitelist containds devname
16:37:19 sean-k-mooney gibi: if you realy wanted too you could also punt on that for this cycle
16:37:20 gibi yepp that does not allows removing that devname codepath from the complexity picture
16:37:46 sean-k-mooney well devname does not realy change anything for the rest of the spec or code
16:37:48 gibi I just add the guard condition, but the code below will still depend on devname
16:38:06 gibi so the code below will as today, complex, uggly, :/
16:38:08 sean-k-mooney yep but that wont affect tracking the dvices in placment
16:38:15 gibi and I just pile on that with the guard
16:38:29 gibi I don't like piling on hard to comprehend code

Earlier   Later