Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2023-02-07
10:58:43 sean-k-mooney the approch we take really comes down to one choice. does the packed ring format need to be opt-in or automatic
10:59:02 sean-k-mooney if its opt in via a flavor/image property then the prefilter is trivial
10:59:56 sean-k-mooney the request spec has both the image properties and flavor extra spec so you can just check them and add the required trait as the accleror filter does
11:01:07 sean-k-mooney looking at https://github.com/openstack/nova/blob/master/nova/objects/request_spec.py the instance_system_metadata is not part of the request spec currently
11:01:43 sean-k-mooney so to that ahat approch we woul d have ot modify the request spec which im not sure is the right thing to do here.
11:02:19 sean-k-mooney the request spec and instance_system_metadata live in different DBs (api vs cell db)
11:03:54 bauzas gibi: I chose the stestr approach of --until-failure
11:04:01 dvo-plv I check instance_system_metadata table and for me it looks like we will mix different OpenStack's layers ( instance and host) , because there is no type of data for instance like that, we have there some image, project, and user info. And I did not found how this table link with request_spec what we create at the scheduler
11:04:04 sean-k-mooney dvo-plv: so based on that i would suggest we take the opt in/out approch and use flavor/image properties
11:05:05 gibi bauzas: that is independent from increasing the chance of catching it by increasing the number of test case to run that we know can fail due to the issue. you can do both
11:05:10 sean-k-mooney dvo-plv: instance_system_metadata is a generic key value store for storing internal information about the instnace. such as the embeded image properites
11:05:25 sean-k-mooney and its not accesable to the schduler genreally
11:06:04 bauzas gibi: we know that the issue is not on a single test
11:06:31 bauzas so while the testrunner runs, I'm looking at every single failure to see the stacktrace and find a pattern
11:06:41 gibi bauzas: yes, but we can grab a list of test cases run by a failed test worker. That list contains both the test case that leaked and the test case that failed due to the leak
11:06:48 opendevreview Merged openstack/nova master: Fix 6.2 compute RPC version alias https://review.opendev.org/c/openstack/nova/+/872804
11:06:48 gibi bauzas: we know what is the latter
11:07:11 sean-k-mooney dvo-plv: what you would actully need to do is have the conductor populate a filed on the request spec when you do a live migration. i feel like that approch is more complex then requried
11:07:13 gibi bauzas: so we can run the same testcase list
11:07:22 gibi bauzas: as we know it contains both
11:07:36 bauzas gibi: I see your proposal
11:07:41 gibi bauzas: then we can increase the chance by adding more test cases that is in the latter category
11:07:47 bauzas I have the subunits
11:07:54 bauzas so I can generate a list
11:07:58 bauzas of failing tests
11:08:03 bauzas and duplicate that list
11:09:39 sean-k-mooney dvo-plv: if we were to leverage the instance_system_metadta we would likely need to extend the Destination filed to have addtional trait requests or something like that https://github.com/openstack/nova/blob/master/nova/objects/request_spec.py#L1093
11:09:41 gibi originally (in 2021) this way I was able to reproduce https://bugs.launchpad.net/nova/+bug/1946339 but I tried this couple weeks ago again and was not able to reproduce the current occasion after couple hour of --unit-failure run
11:10:47 sean-k-mooney dvo-plv: the destination object is constucted here https://github.com/openstack/nova/blob/1c46c4e9e5ba4b84816f5cadad0674f3a773e739/nova/conductor/tasks/live_migrate.py#L64
11:11:19 sean-k-mooney dvo-plv: but as i said this more complex approch is only relevent if we wanted to automatically enabel this functionality
11:12:31 sean-k-mooney well technially its created here https://github.com/openstack/nova/blob/1c46c4e9e5ba4b84816f5cadad0674f3a773e739/nova/conductor/manager.py#L470
11:13:36 sean-k-mooney this only matters for the live migration case as the feature can be renegociated on cold migration or other move operations
11:14:18 bauzas (functional-py38) [sbauza@sbauza nova]$ stestr load /tmp/zuul-logs.Edb6Rp/testrepository.subunit --subunit | subunit-filter -F | subunit-ls
11:14:18 bauzas nova.tests.functional.libvirt.test_vtpm.VTPMServersTest.test_create_server
11:14:30 bauzas gibi: I'm able to get the failing test
11:14:57 bauzas so given I'm looking at all the fetched logsearch subunits, I could extrapolate a list of usual suspects
11:15:38 dvo-plv So if you think that this way is very complex and can make code not so easy and familiar, maybe we should better use existing approach ( creates a new filter like accelerators_filter and check if the user requested packed option) what already exists and is easy to scale. I already check this approach, this approach also forbids migrating VM to the host without packed ring support and also start VM on the host without packed ring support
11:16:51 sean-k-mooney dvo-plv: yep that is the simpelest approch. we could in a future release enable it by default and add a migration mechaniums too if desired.
11:17:19 sean-k-mooney either by turning it on once we raise our min QEMU/Libvirt versions to one that means it will alwasy be aviable
11:17:39 sean-k-mooney or buy automatically adding the image property if not provided
11:18:09 sean-k-mooney so taking the explict approch now does not prevent use making it implict in the future
11:18:35 sean-k-mooney making it automatic now front loads a bunch of complexity
11:21:54 gibi bauzas: I tried that too. I fetched multiple failed worker test case list and intersected them it resulted in an empty list. probably we have multiple test cases that leaks
11:22:20 gibi bauzas: but you can be lucky
11:23:54 bauzas gibi: I have the uuids from logseearch but I don't have the subunit streams
11:24:11 bauzas gibi: any way to pull them with logsearhc ? maybe using the --file param ?
11:24:28 gibi I pull them manually
11:24:47 gibi I needed only about 5 to see that there is no one common test case in the list
11:25:29 bauzas I can hack download.sh
11:27:54 gibi bauzas: I added
11:28:00 gibi https://bugs.launchpad.net/tempest/+bug/1999893
11:28:03 gibi to the etherpad
11:37:14 bauzas gibi: I'm downloading the subunit file from each of the 159 failing runs
11:37:44 gibi that is maybe overkill as I said 5 example was enough for me to end up in an empty intersect
11:37:59 bauzas we will see
11:38:17 bauzas and then I'll ask stestr run to run 10 times each of the failing tests
11:43:35 sahid_ o/ quick question, regqrding instance.props, we do copy image metadata to the instance right? I don't remember
12:10:53 gibi bauzas: added another bug to the etherpad https://bugs.launchpad.net/nova/+bug/2006467
12:11:38 bauzas ack
12:11:59 bauzas fwiw, looping over 78 different libvirt funct tests
12:12:17 bauzas first pass was saying OK
12:18:47 dvo-plv Sorry, Sean, but I do not get your final think. Do you prefer to use. Honestly, I would like to implement it as I have suggested, all interfaces what I need already exists and I will not extend other methods and classes with one parameter that will not use often. It will be a delicate way to extend the existing bunch of filters with one more filter.
12:50:28 gibi bauzas: added another https://bugs.launchpad.net/glance/+bug/2006473
12:57:06 sean-k-mooney dvo-plv: i was suggeting using the extra specs/image properties for now
12:57:51 sean-k-mooney dvo-plv: and when we raise our min libvirt/qemu version eventually we can enable it by defualt then
12:58:39 sean-k-mooney dvo-plv: that would be my prefence. its simple to add to nova, easy to document and understand and easy to test
12:59:26 sean-k-mooney dvo-plv: we can evenutally turn this on by default when we nolonger supprot qemu/libvirt version that dont have this and there is nolonger an upgade impact
13:01:23 sean-k-mooney dvo-plv: we did the same thing with the virtio random number generator in the past
13:01:53 sean-k-mooney dvo-plv: intially it was opt in and we enabled it by default after a few release after we raised our min libvirt/qemu version
13:14:48 bauzas gibi: after one hour, still none of the 79 tests were having an issue
13:18:17 opendevreview Jorge San Emeterio proposed openstack/nova master: Moving privsep profiles to nova/__init__.py https://review.opendev.org/c/openstack/nova/+/872010
13:18:27 dvo-plv Yes, I would like to have some general pre-approval from you here, before starting to implement and verify this functionality and present it in the blueprint to be sure that it will work okay, and does not waste your time on the blueprint spec file review process 1) User will have the ability to enable/disable this feature via flavor/image. 2) User will have the ability to set trait COMPUTE_NET_VIRTIO_PACKED to the flavor
13:19:10 dvo-plv Sorry, I have interrupt, I will resend my question
13:19:30 dvo-plv Yes, I would like to have some general pre-approval from you here, before starting to implement and verify this functionality and present it in the blueprint to be sure that it will work okay, and does not waste your time on the blueprint spec file review process
13:19:53 dvo-plv 1) User will have the ability to enable/disable this feature via flavor/image. 2) User will have the ability to set trait COMPUTE_NET_VIRTIO_PACKED to the flavor to choose some specific servers. Compute node will set this trait to the resource provider here static_trait.
13:20:11 dvo-plv 3) Scheduler will handle migration and OpenStack cluster update process with automatically understanding which node has this function with extended ALL_REQUEST_FILTERS array with a new filter similarly how it was implemented for accelerators_filter ( get a packed request from flavor ).
13:20:15 sean-k-mooney dvo-plv: yep so requesting the feature via flavor/image shoudl automatically result in the trait request via a pre_fiter like the acclerator filter
13:20:19 dvo-plv 4) As far as Qemu from v4.2 can not be compiled without packed ring support and Libvirt from v6.3, we can get if the current compute node can use this functionality and if it is available for the user.
13:20:25 dvo-plv OR do we need just implement options 1, 2, and 4 without the automatic scheduler handling this feature exists on the compute target node?
13:20:33 sean-k-mooney so they can ask for COMPUTE_NET_VIRTIO_PACKED explictly but it should not be required
13:21:26 sean-k-mooney 2 you get for free we already support arbitry trait request in the flavor/image
13:21:58 sean-k-mooney as part of implementeing 1 you should add a schduler prefilter to request COMPUTE_NET_VIRTIO_PACKED if the extra_spec/image property is set
13:22:23 sean-k-mooney so 1 and 3 are what you need to enable this feature properly
13:23:20 sean-k-mooney dvo-plv: https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L212-L222
13:23:38 sean-k-mooney our current min libvirt is 6.0 and Qemu is 4.2
13:24:58 sean-k-mooney we have not bumped that in a few releases so we will likely go to libvirt 7.0 and qemu 5.2 in the B release
13:25:32 sean-k-mooney although we can technially do that in the A release
13:25:49 dvo-plv Okay, I see, but it in the future, for now Libvirt support packed from 6.3, Should I just update minimum libvirt version, or create my own define for my trait?
13:25:52 sean-k-mooney bauzas: kashyap any reason not to do that in A
13:26:38 sean-k-mooney dvo-plv: we have a speicifc procedure for updating it where we have to annowuch our new min version in advacne for at least 1 cycle
13:27:00 sean-k-mooney we declared 7.0 and 5.2 as our next verion in Wallaby
13:27:31 sean-k-mooney so we could have done that bump some time ago
13:27:46 bauzas sean-k-mooney: we can if you want
13:27:50 sean-k-mooney although we now have new upgrade requirement to test the previous LTS
13:27:54 sean-k-mooney bauzas: i just realsied we cant
13:28:03 sean-k-mooney we need to support focal for A for upgrade reasons
13:28:16 sean-k-mooney bauzas: so we should do this in early B
13:29:00 sean-k-mooney we need to not have 20.04 in our greade job to do this bump
13:29:12 bauzas hmmm ok
13:29:29 sean-k-mooney and the dedicated focal job to go away

Earlier   Later