Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-01-11
16:31:41 mriedem dims: ^? since gcb isn't around.
16:32:21 gibi mriedem: sorry I still not get it. neither flavor.create nor instance.<action> samples shows the flavor.extra_specs but your reference points out that it is loaded. This seems a bug to me
16:32:49 markus_z pkoniszewski: Do you see a chance to have a look at those patches for stable newton? https://review.openstack.org/#/q/status:open+project:openstack/nova+branch:stable/newton+topic:%22bug+1455252%22 When there are concerns about the risks, I'm fully OK with abandoning them.
16:33:22 mriedem gibi: https://review.openstack.org/#/c/413846/10/doc/notification_samples/instance-create-end.json
16:33:28 mriedem that has the extra_specs key in the payload
16:33:41 mriedem it's just empty, because the fake flavor used in those tests don't have any extra specs :)
16:33:59 nik__ cdent, thank you for review! I fixed you comments
16:34:32 cdent cool, will go back and have a look
16:35:12 gibi mriedem: you are right my eyes ticked me
16:35:17 mriedem gibi: https://github.com/openstack/nova/blob/fd91a25324b7b7fed1f3f10c5b56668b20a170f4/nova/tests/functional/notification_sample_tests/notification_sample_base.py#L138
16:35:22 gibi mriedem: instance.<action> has the extra spec
16:35:35 mriedem yeah, so if we added an extra spec to that fake flavor those instance samples would have it
16:35:55 mriedem it might be a nice clarification to add that in a separate change
16:36:05 mriedem hw:watchdog_action=disable or something
16:37:22 artom mriedem, I guess you missed my question earlier about 2 rpc versions bumps but smaller patches vs 1 bump but bigger patch in the tagged attachment series. Any opinions, or shall I plow on with what I think is best and let code review sort it out later?
16:37:44 gibi mriedem: I agree, let's add that in a separate patch
16:39:08 gibi mriedem: so what is left over the existing reno limitation is that the flavor.create doesn't have the projects and the extra_specs field but I assume that the lazy-load bugfix will solve that as well
16:40:13 mriedem gibi: do you feel the need to update that release note to get into that amount of detail?
16:40:31 mriedem gibi: on flavor create i don't think we have access or extra specs anyway, as those are different APIs to update an existing flavor
16:41:06 gibi mriedem: I think the current patch can go in, worst case we add another reno if the bug fix does not solve the flavor.create
16:41:35 mriedem gibi: ok but like i said, flavor.create won't have access or extra specs,
16:41:40 mriedem those aren't available when the flavor is created
16:41:47 mriedem those are later POST actions on an existing flavor
16:41:47 pkoniszewski markus_z: yeah, added series to my TODO list for tomorrow
16:41:50 mriedem so those would be update notifications
16:42:06 mriedem artom: i think i'd prefer separate
16:42:24 mriedem artom: that series is easier to review when we're dealing with bdms and vifs separately
16:42:44 mriedem but that's just me
16:43:06 artom mriedem, I think I agree
16:43:10 gibi mriedem: I agree that the flavor.extra_spec is empty at flavor.create but that empty dict still should be in the emited flavor.create payload
16:43:12 artom mriedem, human time > computer time
16:43:21 artom Or in this case, computer complexity
16:43:41 markus_z pkoniszewski: Thanks
16:44:34 diana_clarke mriedem, sdague, alex_xu: I'm not sure if you saw my last comment here, but I'm not sure this should be shipped as-is. Thoughts? https://review.openstack.org/#/c/398755/
16:45:43 sdague diana_clarke: it would be nice to make that a better error message
16:46:03 dansmith mriedem: sdague: do you know what tox target I should run in tempest to be like what the cells job gets in the gate? if I do -efull I get ~100 more tests than we run
16:46:30 mriedem diana_clarke: hmm, i hadn't seen that
16:46:47 mriedem dansmith: you need to use the rc file in the nova tree
16:47:04 mriedem dansmith: https://github.com/openstack/nova/blob/master/devstack/tempest-dsvm-cells-rc
16:47:05 mriedem sec,
16:48:34 mriedem that is sourced here https://github.com/openstack-infra/devstack-gate/blob/master/devstack-vm-gate-wrap.sh#L601
16:49:07 mriedem dansmith: and then used here https://github.com/openstack-infra/devstack-gate/blob/master/devstack-vm-gate.sh#L789
16:49:16 dansmith mriedem: do I need to re-run devstack for that or can I just run tempest and point it at the regex?
16:49:23 dansmith ah
16:49:34 gibi mriedem: I just cherry-picked the bugfix top of the flavor update patch and the notificaion sample test blows up that the extra_spec field of the flavore in None and the extra_specs field of the falvor payload cannot be none. So I think we have an issue here
16:49:42 mriedem dansmith: are you talking local runs?
16:49:47 dansmith mriedem: yeah
16:49:57 mriedem dansmith: then i think you just source that rc file to get the env variable exported,
16:50:09 mriedem and then tox -eall $DEVSTACK_GATE_TEMPEST_REGEX
16:50:18 mriedem well: tox -e all -- $DEVSTACK_GATE_TEMPEST_REGEX
16:50:27 dansmith oh okay
16:50:30 dansmith confusing
16:51:05 gibi mriedem: worst case I have to bump the version of the falvor payload to make the extra_specs nullable
16:51:24 gibi mriedem: is it still OK as a bugfix?
16:51:25 mriedem dansmith: alternatively, just do what the cellsv1 job does
16:51:25 mriedem http://logs.openstack.org/55/398755/18/check/gate-tempest-dsvm-cells-ubuntu-xenial/22625aa/console.html.gz#_2017-01-04_09_21_05_122446
16:51:45 dansmith it's working with sourcing that thing
16:51:46 dansmith thanks
16:52:29 mriedem diana_clarke: maybe create a bug to track the error message cruft
16:53:06 mriedem gibi: hmm, 'extra_specs': fields.DictOfStringsField(), so that's not nullable in the Flavor object itself
16:53:16 mriedem so i imagine that's why it's not nullable in the flavor payload either
16:53:31 mriedem gibi: is the api sample test setting it to None?
16:53:35 mriedem because that would be an invalid test
16:53:44 diana_clarke mriedem: okie dokie
16:54:47 gibi mriedem: checking..
16:57:48 gibi mriedem: as far as I see all the api sample tests are using flavors that has the extra_specs set to someting. This is possible because flavor.extra_specs are returned by a different api resource than the normal flavor object
16:58:07 gibi mriedem: so in a normal GET os-flavors there is no extra_specs ever
17:01:53 gibi mriedem: and the GET os-flavor-extra-specs will call objects.Flavor.get_by_flavor_id
17:02:24 melwitt dansmith, mriedem: I was just doing like the gate to run locally, just have to put the regex in single quotes: tempest run --regex '<regex stuff>'
17:02:55 gibi mriedem: so that works the same way as the code that emits the instance.<action> notifications
17:03:36 diana_clarke mriedem, sdague, alex_xu: https://bugs.launchpad.net/nova/+bug/1655710
17:03:36 openstack Launchpad bug 1655710 in OpenStack Compute (nova) "Query parameter validation using json schema - error messages" [Undecided,New]
17:05:08 mriedem thanks
17:05:39 mriedem diana_clarke: so do you get a 400 or a 500 now?
17:05:56 mriedem i assume you still get a 400 response
17:06:04 diana_clarke mriedem: not sure off the top of my head, but I can check
17:06:26 gibi mriedem: we can fix flavor.create by adding the extra_specs in the expected_attr here https://github.com/openstack/nova/blob/master/nova/objects/flavor.py#L494
17:07:11 mriedem gibi: wouldn't that already be done because of this? https://github.com/openstack/nova/blob/master/nova/objects/flavor.py#L490
17:07:17 openstackgerrit avnish proposed openstack/python-novaclient: Removed unnecessary utf-8 encoding https://review.openstack.org/418839
17:07:20 mriedem extra_specs is in OPTIONAL_FIELDS
17:07:54 gibi mriedem: I think not because extra_specs is not in updates at https://github.com/openstack/nova/blob/master/nova/objects/flavor.py#L491
17:08:54 mriedem oh right i missed the in updates check
17:10:27 gibi mriedem: Is it OK for you if I just fix this as part of the bugfix https://review.openstack.org/#/c/415857/
17:11:29 mriedem gibi: that's dealing with aggregates, so i'd think it'd be part of a separate change in that series? i guess it's hard for me to say without knowing in detail what the actual change is going to be - i don't have the context in my head right now
17:12:12 mriedem raj_singh: fyi i updated the devstack series that's testing the service user token stuff
17:12:26 mriedem i want to see how that runs before i +2 the bottom change in the nova series
17:12:48 gibi mriedem: right, that patch getting mixed up with different object fixes due to the same root cause
17:12:59 gibi mriedem: I can split it into a series of patches
17:13:18 raj_singh_ mriedem: I am using https://review.openstack.org/#/c/413232/ to test the series, which is dependent on the devstack patch as well. BTW it works
17:13:35 raj_singh_ mriedem: http://logs.openstack.org/32/413232/4/check/gate-tempest-dsvm-neutron-multinode-full-ubuntu-xenial-nv/fcaf67a/console.html
17:13:52 raj_singh_ tempest.api.compute.admin.test_live_migration.LiveBlockMigrationTestJSON.test_live_block_migration [412.934140s] ... ok
17:14:35 raj_singh_ I put a sleep of 400 and this test took more than 400 with service token enabled. I will upadate the commit msg on DNM patches to avoid confusion
17:17:12 diana_clarke mriedem: yup, 400 b/c exceptions.ValidationError inherits from exceptions.Invalid which has 400 for code
17:17:47 mriedem diana_clarke: ok
17:17:51 mriedem raj_singh: yup cool
17:19:11 dansmith melwitt: okay, running with the config as the gate does, I now get fewer tests, and many more fails
17:20:44 mriedem raj_singh: actually that's kind of odd that it's passing in your test patch,
17:20:57 mriedem raj_singh: because we don't currently run volume-backed live migration tests in the live migration job
17:21:04 mriedem those are skipped for now due to bugs
17:21:40 openstackgerrit Sujitha proposed openstack/nova: Add post hook script to run tempest with old n-api https://review.openstack.org/401395

Earlier   Later