Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-07-10
13:59:03 aarents Hi there, it will be great if someone can have a look to this reproducible bug and related fix: https://review.opendev.org/#/c/670000
13:59:10 yonglihe fake-db does had this behavior for deleted instance, and i also saw while deleted, we save instance.id to instance.delete filed.
13:59:18 sean-k-mooney there is a test isolation analasys flag in stestr to figure out what test cause issues
13:59:35 sean-k-mooney so im hoping i can just let that run and get a list of the broken tests
14:01:55 yonglihe sean-k-mooney: I did not got time to test that on machine. I gonna do smoke test tomorrow to make sure it won't delete active instance. Other comments seems reasonable and good, thanks.
14:03:06 sean-k-mooney ok i didnt run it locally to test. but that was the main reason for the -1
14:03:36 yonglihe and here is another wired thing i running into. for the unit test
14:03:43 yonglihe https://www.irccloud.com/pastebin/Fep61BPV/
14:04:12 yonglihe the assert_calls say 'Calls not found.'
14:04:49 sean-k-mooney by the way an instance not having id set does not mean its deleted it means it has not be created in the db
14:05:05 yonglihe but the instance in expect and actual actually just same (but might had wrong order).
14:05:07 sean-k-mooney or nolonger exists in the db
14:06:26 yonglihe sean-k-mooney: Is it possible only database know what is deleted? and we can not get that information from the instance ?
14:06:34 sean-k-mooney anyway that sinip is a little hard to read but you can proably do an order independ check if that is not important
14:07:09 yonglihe sean-k-mooney : i don't find one to ignore the order.
14:07:56 sean-k-mooney you would have to write it. just asser the call object is in the mock.calls array
14:08:08 sean-k-mooney but you would have to construct a mock call object to do that
14:08:10 yonglihe replace wiht assert_called_with also does not work, assert_called_with still checking the order, that's so wired for me.
14:08:32 sean-k-mooney normally we want the order to be checked
14:09:00 sean-k-mooney we have some examples of doing order independ dict comparisons i think
14:09:05 yonglihe seems i had to assert the call is in that array.
14:10:43 sean-k-mooney https://docs.python.org/3/library/unittest.mock.html#unittest.mock.Mock.assert_has_calls
14:10:50 yonglihe I would like to find the example, that's should work. but checking if instance is deleted seems hard? any other condition i can check?
14:11:04 sean-k-mooney you can do mock.assert_has_calls(calls, any_order=True)
14:11:11 yonglihe sean-k-mooney: thanks, got it.
14:12:35 sean-k-mooney if you use assert_has_calls you have to create teh mock calls manually like this https://github.com/openstack/nova/blob/de31466fdb09dd367b23660f71b4ca06f83271a2/nova/tests/unit/utils.py#L323-L336
14:14:42 sean-k-mooney here are some example you can follow https://github.com/openstack/nova/search?q=assert_has_calls+any_order%3DTrue&unscoped_q=assert_has_calls+any_order%3DTrue
14:17:26 yonglihe thanks. for the 'delete' states, i think the 'deleted_at' filed been set seems could filter out the instance is creating at that moment.
14:18:20 yonglihe 'no id attr' and 'deleted_at' is set before now.
14:18:39 sean-k-mooney perhap but again i would use the instance status personally unless you have an example where that does not work
14:19:48 yonglihe not work in the unite test, that's why i checking other stuff. but the unit test just call the destroy, not set the states right.
14:20:24 sean-k-mooney yonglihe: then why not fix the unit tests
14:20:51 sean-k-mooney anyway ill leave it up to you
14:21:14 yonglihe Sure. do we need checking other stuff if we depend on he vm_state filed?
14:21:19 sean-k-mooney leave a comment in the next version based on your testing
14:21:33 yonglihe Sure.
14:21:44 sean-k-mooney e.g. why you are doing whatever test you come up with
14:23:11 sean-k-mooney im not sure but i need to go work on somehting before a meeting soon
14:23:29 yonglihe I gonna to verify if that code working on real stack. thanks. I got to run also.
14:23:30 sean-k-mooney i think the task state + checking for soft delete would be enough
14:23:56 yonglihe Ok , i gonna checking that,
14:24:14 yonglihe Have a good day.
14:32:19 sean-k-mooney johnthetubaguy: by the way i -1 the unifed limits spec but if just declare the discoverablity as out of scope to be resolved in a futrue version of the sdk or in a follow up spec then im fine to swap to +1 on that spec
14:32:48 sean-k-mooney once the nits inline are adressed that is
14:33:09 johnthetubaguy sean-k-mooney: cool, I will respin
14:34:34 sean-k-mooney is it safe to assume that the keysone limits api wont have limits for compute resouces if we have not enabled it for nova
14:35:23 sean-k-mooney if so the sdk could check that once an cache it or something like that to determin if it should use keystone limits or nova
14:36:54 sean-k-mooney so the actully check would be. 1.) is keystone version new enough to support limtis 2.) are instance limits reported in keysonte limits api, if yes to both use keystone if the either are false use nova
14:37:56 sean-k-mooney johnthetubaguy: would somthing like ^ work
14:40:00 johnthetubaguy sean-k-mooney: Its not a smooth transition that way... nova might still be ignoring keystone, but it might be good enough (assuming default keystone policy means you can list the limits of your project)
14:40:20 johnthetubaguy it gets worse with hierarchy mind, as everything seems to
14:41:18 sean-k-mooney well we dont need to encode that in the spec i was just wondering if there was a huristic the sdk could resonably use
14:42:27 sean-k-mooney it could be as simple as stick it in a config option for now
14:43:45 sean-k-mooney the clouds.yaml has profiles for clouds that encode there knonw behavior for non discoverable things
14:43:56 sean-k-mooney so that is always an option for limits
14:44:41 sean-k-mooney at least i think nodepool gets that stuff from clouds.yaml for the sdk
14:52:06 openstackgerrit Dakshina Ilangovan proposed openstack/nova-specs master: Spec: Provider config YAML file https://review.opendev.org/612497
14:59:56 spatel sean-k-mooney: did you ever see this error http://paste.openstack.org/show/754267/
15:07:05 stephenfin johnthetubaguy: Seeing as you've been reviewing docs, could you have a look at https://review.opendev.org/#/c/667164/ ?
15:07:29 johnthetubaguy stephenfin: sure
15:15:37 openstackgerrit John Garbutt proposed openstack/nova-specs master: Add Unified Limits Spec https://review.opendev.org/602201
15:15:48 openstackgerrit Stephen Finucane proposed openstack/nova master: ec2: Remove unused functions from 'ec2utils' https://review.opendev.org/662501
15:15:49 openstackgerrit Stephen Finucane proposed openstack/nova master: objects: Remove unused ec2 objects https://review.opendev.org/662502
15:15:49 openstackgerrit Stephen Finucane proposed openstack/nova master: ec2: Remove ec2.CloudController https://review.opendev.org/662503
15:15:49 openstackgerrit Stephen Finucane proposed openstack/nova master: ec2: Pre-move cleanup of utils https://review.opendev.org/662504
15:15:50 openstackgerrit Stephen Finucane proposed openstack/nova master: ec2: Move ec2utils functions to their callers https://review.opendev.org/662505
15:15:51 openstackgerrit Stephen Finucane proposed openstack/nova master: api: Remove 'Debug' middleware https://review.opendev.org/662506
15:19:07 openstackgerrit Stephen Finucane proposed openstack/nova master: vif: Stop using getattr for VIF lookups https://review.opendev.org/666630
15:19:07 openstackgerrit Stephen Finucane proposed openstack/nova master: vif: Resolve a TODO and update another https://review.opendev.org/666631
15:19:08 openstackgerrit Stephen Finucane proposed openstack/nova master: vif: Remove dead minimum libvirt checks https://review.opendev.org/666814
15:20:35 Nick_A We are trying to offload the _base nova image cache using cephfs. our idea was to do a read-only mirror of glance's image pool and mount that to _base on compute. Would nova have problems with that setup?
15:21:04 openstackgerrit Merged openstack/nova master: Perf: Use dicts for ProviderTree roots https://review.opendev.org/637225
15:21:35 openstackgerrit Stephen Finucane proposed openstack/nova master: Handle libvirt reporting incorrect 4k page quantities https://review.opendev.org/631038
15:21:35 openstackgerrit Stephen Finucane proposed openstack/nova master: libvirt: Split '_get_host_numa_topology' function https://review.opendev.org/631237
15:21:36 openstackgerrit Stephen Finucane proposed openstack/nova master: Make overcommit check for pinned instance pagesize aware https://review.opendev.org/631053
15:22:30 openstackgerrit Shilpa Devharakar proposed openstack/nova master: Support filtering of hosts by forbidden aggregates https://review.opendev.org/667952
15:25:54 openstackgerrit Stephen Finucane proposed openstack/nova stable/stein: doc: Correct issues with 'openstack quota set' commands https://review.opendev.org/670096
15:26:03 openstackgerrit Stephen Finucane proposed openstack/nova stable/rocky: doc: Correct issues with 'openstack quota set' commands https://review.opendev.org/670097
15:28:05 openstackgerrit Stephen Finucane proposed openstack/nova stable/stein: doc: Correct issues with 'openstack quota set' commands https://review.opendev.org/670096
15:28:37 openstackgerrit Stephen Finucane proposed openstack/nova stable/rocky: doc: Correct issues with 'openstack quota set' commands https://review.opendev.org/670097
15:29:21 openstackgerrit Stephen Finucane proposed openstack/nova stable/queens: doc: Correct issues with 'openstack quota set' commands https://review.opendev.org/670100
15:35:33 mriedem stephenfin: i see https://review.opendev.org/#/c/667164/2 is already approved and you've started backports, but some issues inline
15:35:45 stephenfin looking
15:36:43 openstackgerrit sean mooney proposed openstack/nova master: WIP: coverage job changes https://review.opendev.org/669912
15:36:59 stephenfin mriedem: Those all look fair. Want me to pull it out of the gate and address those?
15:37:15 mriedem that would be nice yes
15:37:20 artom There's a thing to re-run just the tests that failed in the last run... right?
15:37:28 mriedem artom: not in the gate
15:37:32 mriedem locally,
15:37:33 artom mriedem, locally
15:37:36 mriedem tox -e <whatever> -- --failing
15:37:41 artom mriedem++
15:38:59 openstackgerrit Stephen Finucane proposed openstack/nova master: doc: Correct issues with 'openstack quota set' commands https://review.opendev.org/667164
15:40:07 stephenfin mriedem: 'openstack quota list --compute' won't show the _default_ quotas though, will it?
15:40:13 stephenfin (RE: https://review.opendev.org/#/c/667164/2/doc/source/admin/quotas.rst@68)
15:41:13 mriedem stephenfin: honestly i don't know how that command works under the covers,
15:41:25 mriedem "default" doesn't really mean anything for the compute API if you're not looking at the default class
15:41:31 mriedem neutron has an api to see 'defaults'
15:41:39 mriedem but i don't think cinder does and nova doesn't besides the class
15:43:49 stephenfin looks like 'quota list' is no good to us

Earlier   Later