Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-04-10
13:51:26 artom I'm kinda leaning towards splitting that in two patches?
13:57:54 sean-k-mooney artom: are you refering to https://review.openstack.org/#/c/648123/4
13:58:05 artom sean-k-mooney, yeah. Just left a revirw
13:58:07 artom *review
13:58:27 sean-k-mooney artom: i did ask myself that question and the anser was "meh" ill see if anyone else cares
13:58:51 sean-k-mooney for backporting would likely be simpler to have them as seperate patches i gues
13:59:53 sean-k-mooney because the test refactor was related to the change i was ok with it but normally i would have prefered them to be split
14:01:06 artom sean-k-mooney, well I have a -1 for other small nits, we'll let others make that call for us :D
14:01:17 artom Since we both appear to be "meh" :)
14:02:42 sean-k-mooney artom: on the api question there are times where this works today and it ends up in the api with the mac but not stats
14:03:00 sean-k-mooney artom: so i dont think its an api change
14:03:26 artom sean-k-mooney, that's what I had understood as well
14:03:38 artom And the test does assert one of the fields is None, so we're probably covered
14:04:36 sean-k-mooney oh i didnt put https://bugzilla.redhat.com/show_bug.cgi?id=1649688#c8 into the upstream bug
14:04:37 openstack bugzilla.redhat.com bug 1649688 in openstack-nova "[osp13] nova diagnostics command is not working with all instances" [High,Assigned] - Assigned to fpalin
14:05:38 sean-k-mooney actully there might be a slight api change in that before nic_details was empty and no i has a mac
14:06:00 sean-k-mooney but i dont think that is versioned but good question
14:07:43 artom I don't think it'd have been empty before, rather an error 50
14:07:45 artom 500
14:08:13 sean-k-mooney no it was empty when i tried to reporduce the bug on my local systlem
14:08:58 sean-k-mooney if the vf has a netdev before its plug libvirt added the target element and we did not raise an error but you had no data at all
14:09:00 artom Hrmm, in any case, None is still allowed in fields
14:09:32 artom So nic_details = None is valid, as is nic_detail = {mac: blah, rest of fields: None}
14:13:14 sean-k-mooney well we dont have any tests assering the api behavior so i think there is some flexibity here. we can either say its fine as it is and this was a bug that was fixed and there does not need a microverion or there is no testing therfor it was broke by default and this fixed it :)
14:13:52 sean-k-mooney but i dont think its being to checky to say it does not need a microverions
14:13:52 artom Well we have an api-ref and a when-to-microversion flow chart
14:14:09 artom And fixing 500 errors is NOT a new microversion
14:14:14 sean-k-mooney we do
14:14:18 sean-k-mooney correct
14:14:22 artom I was just concerned the fields would be *missing* outright
14:14:57 artom Which is moot, because it doesn't appear to be the case, but would have required a new microversion, or more likely an updated patch that didn't have that problem
14:15:38 sean-k-mooney yep well this is why we have code review is a valid concern i think we are ok however.
14:24:27 mriedem boxiang: just a few small things to address in your spec here https://review.openstack.org/#/c/645458/
14:24:51 mriedem but i know it's late so feel free to address those comments tomorrow
14:26:03 openstackgerrit Stephen Finucane proposed openstack/nova master: hacking: Resolve E731 issues https://review.openstack.org/651554
14:26:03 openstackgerrit Stephen Finucane proposed openstack/nova master: Bump to hacking 1.1.0 https://review.openstack.org/651553
14:26:04 openstackgerrit Stephen Finucane proposed openstack/nova master: hacking: Resolve W503 issues https://review.openstack.org/651555
14:31:40 gibi mriedem: hi! when you implemented heal allocation in nova-manage did you thought about adding some kind of end-to-end test for it? I'm wondering how complex it would be to add something to grenade for the heal port allocation but I don't have enough grenade knowledge to see if it is feasible at all
14:33:10 openstackgerrit Boxiang Zhu proposed openstack/nova master: Make evacuation respects anti-affinity rule https://review.openstack.org/649963
14:33:48 mriedem ntegration testing for that command
14:33:48 mriedem when i implemented it (in rocky? queens?) we were only healing basic resource class allocations like VCPU/MEMORY_MB/DISK_GB and only if they didn't exist, which by that time wouldn't be a problem if using the FilterScheduler - the command was really built to get people off the CachingScheduler which didn't use placement, and we didn't have a grenade job that used the CachingScheduler, so no i didn't really put much thought in
14:34:49 gibi mriedem: ack, thanks
14:35:00 mriedem we could have a post-test hook script run it from the nova-next job, but we'd have to fake the missing allocations by creating a server then manually removing it's allocations and then running the command and verify they were healed
14:35:07 mriedem which...we can just do that in functional in-tree tests
14:35:30 gibi mriedem: yeah, I'm doing that in the func test already
14:35:52 mriedem bw provider ports is a whole other ball of wax - can we even test those in the gate?
14:36:11 mriedem i mean we can test that you can create a min bw policy on a network and create ports on that network and make sure the allocations and such are all wired up
14:36:34 mriedem but i'm not sure we can test that the bw is actually limited within the guest in the gate
14:36:55 mriedem just making sure nova+neutron+placement are all doing what they are supposed to (from the gate) would be a good step anyway
14:37:11 gibi mriedem: yeah I think we have such tempest test on review
14:37:15 gibi let me look up the link
14:37:38 gibi mriedem: https://review.openstack.org/#/c/629253/
14:38:29 gibi for the heal port allocation we would need to create a bound port in rocky that would need allocation in stein
14:38:51 gibi unfortunately in rocky only sriov ports supported min bw in neutron, and sriov is no no in the gate
14:39:23 gibi I can trick the system by adding the min bw policy to the port _after_ it was bound but that feels hackis already
14:39:35 mriedem agree
14:39:46 gibi I think I wil stick to the functional test env for heal port allocation
14:40:09 gibi and I might do some manual test in a devstack where I have sriov
14:46:54 mriedem melwitt: are you ok to re-approve the cross-cell resize spec https://review.openstack.org/#/c/642807/ ? I saw you were just +1. I'd like to get the blueprint re-approved so i can start putting the bottom tail of the (huge) series into runways to get some reviews going.
14:53:45 stephenfin mriedem: Fancy revisiting this? https://review.openstack.org/#/c/650018/
14:55:39 mriedem "I've spoken with Lars Kurth (XenProject advisory board chair) and he agrees that at this point in time, this config can be removed."
14:55:46 mriedem so we have no CI for libvirt+xen
14:55:49 mriedem and no plans to
14:55:50 mriedem ok
14:57:53 mriedem stephenfin: back at you https://review.openstack.org/#/q/status:open+project:openstack/nova+branch:master+topic:bug/1823781
14:58:07 stephenfin ack
14:59:17 stephenfin mriedem: Out of curiosity, does this look familiar to you? http://paste.openstack.org/show/P6AlRRpBWnYwFfWWVJFl/ It's a stable/rocky deployment
15:00:44 mriedem yes i think so, sec
15:01:29 mriedem actually no nvm, was thinking of something else pci related
15:02:32 mriedem it's blowing up on this? "n_id = compute_node.id"
15:02:43 mriedem i only see _setup_pci_tracker called with an existing compute node record where id should be set
15:02:47 mriedem unless you guys have a fork
15:04:53 stephenfin Not that I'm aware of. I had been doing some work in that area but none of that is merged yet :/ https://review.openstack.org/#/c/649559/
15:05:54 stephenfin But yeah, looks like that. I don't know if that attribute should be set or not. I can't see why it wouldn't but all things Ironic really aren't my strong suit
15:06:20 melwitt mriedem: I was thinking dansmith would want to sign off on it
15:08:58 mriedem stephenfin: i don't think that error has anything to do with ironic
15:13:21 stephenfin Perhaps, but it's only happening with baremetal nodes so I suspect it's some with how we're initializing the Ironic driver
15:13:25 stephenfin but that's a grasp
15:15:01 mriedem so the driver returns a list of nodenames for that host and those get passed one at a time to RT https://github.com/openstack/nova/blob/stable/rocky/nova/compute/manager.py#L7893
15:16:50 mriedem should eventually call through to here https://github.com/openstack/nova/blob/stable/rocky/nova/compute/resource_tracker.py#L743
15:17:24 mriedem if the compute node already exists in the db you'd get here on first startup https://github.com/openstack/nova/blob/stable/rocky/nova/compute/resource_tracker.py#L567
15:18:08 mriedem this should also be ok https://github.com/openstack/nova/blob/stable/rocky/nova/compute/resource_tracker.py#L530
15:18:35 mriedem otherwise we'd create the node in the db https://github.com/openstack/nova/blob/stable/rocky/nova/compute/resource_tracker.py#L584
15:18:38 mriedem which should set the id on the cn object
15:19:07 mriedem https://github.com/openstack/nova/blob/stable/rocky/nova/objects/compute_node.py#L322
15:19:50 mriedem looks like you've got some debugging to do if you can recreate it
15:32:12 mriedem lyarwood: melwitt: dansmith: bauzas: looks like we can start approving stable/stein backports now so i'll start going through those and removing the WIP on mine
15:32:27 mriedem we need to roll through some of those to get things flushed out down through pike for pike-em
15:32:38 dansmith ack
15:32:40 melwitt ah, yep
15:33:28 melwitt I have a bug fix that needs to go back to pike but it hasn't even merged on master yet and having trouble getting review https://review.openstack.org/611974
15:34:19 cdent mriedem: does a server always get buried to cell0 when it fails to find a host? Is it possible to say "only bury if you requested a build at a microversion that will allow an unbury"?
15:34:30 mriedem melwitt: i definitely don't know that code so would need at least lyarwood and/or mdbooth to sign off
15:34:43 mriedem cdent: "does a server always get buried to cell0 when it fails to find a host?" yes
15:35:05 mriedem "Is it possible to say "only bury if you requested a build at a microversion that will allow an unbury"?" i think that gets really complicated fast
15:35:12 cdent i imagine so
15:35:20 cdent what was the original purpose of burying?
15:35:22 dansmith yeah that's not okay
15:35:27 melwitt there's this one too but people were meh about reviewing it too https://review.openstack.org/582408
15:35:27 mriedem because it's more like (1) only bury if using a microversion to allow unburying except (2) unless you didn't request personality files etc

Earlier   Later