| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-10-18 | |||
| 09:49:17 | sean-k-mooney | if so it might be nice to trigger it on a restited set of os-vif files also | |
| 09:49:26 | jangutter | sean-k-mooney: heh, in theory yes, in practice it's been busted for ages. | |
| 09:49:37 | sean-k-mooney | ok :) | |
| 09:50:02 | jangutter | sean-k-mooney: (on expanding the tests, is this more or less in the right direction for "future os-vif"? https://review.openstack.org/#/c/610636/ ) | |
| 09:54:40 | sean-k-mooney | i was just opening that :) | |
| 09:57:48 | sean-k-mooney | jangutter: so one change to that interface that i think we will need to do is be able to pass in the tree and then filter it | |
| 09:58:31 | jangutter | sean-k-mooney: you mean something like https://github.com/openstack/nova/blob/kilo-eol/nova/objects/instance.py#L257 | |
| 09:59:00 | sean-k-mooney | no | |
| 09:59:43 | jangutter | sean-k-mooney: ah, I see what you mean. Rather than self-editing the object, return a new one with the filtered fields? | |
| 10:00:23 | sean-k-mooney | yes or rather host_info would retrun an unfiltered tree and then we would have a sperate filter function | |
| 10:01:03 | sean-k-mooney | the reason for this is i think we will want to have nova call host info and then filter by what the hypervior can support then serialser it and send it to neutron | |
| 10:01:09 | jangutter | sean-k-mooney: I had the same thought, it feels dirty to self-edit. | |
| 10:01:39 | sean-k-mooney | then neutron will want to filter it again and select a vif type from what is left | |
| 10:01:53 | jangutter | sean-k-mooney: filter should be a static function, not a member function. | |
| 10:03:25 | sean-k-mooney | ya proably it could be a class function that take steh tree as the second argument but a static fucntion would work just as well | |
| 10:03:28 | openstackgerrit | Takashi NATSUME proposed openstack/nova master: Remove mox in unit/network/test_neutronv2.py (3) https://review.openstack.org/574104 | |
| 10:03:47 | openstackgerrit | Takashi NATSUME proposed openstack/nova master: Remove mox in unit/network/test_neutronv2.py (4) https://review.openstack.org/574106 | |
| 10:04:02 | openstackgerrit | Takashi NATSUME proposed openstack/nova master: Remove mox in unit/network/test_neutronv2.py (5) https://review.openstack.org/574110 | |
| 10:05:15 | jangutter | sean-k-mooney: cool, will refactor the refilter. | |
| 10:06:12 | sean-k-mooney | :) cool the main issue is this part of the api was actully just a poc that was cerated without really thinking about how it would be used | |
| 10:09:28 | jangutter | sean-k-mooney: yep, and I realised while doing Jay's proposal for the offload metadata that if I don't work on this, it's going to result in another poor idiot thinking about this later. | |
| 10:12:09 | sean-k-mooney | jangutter: ya so this is all laying the ground work for doing the negciation likely in T | |
| 10:12:44 | sean-k-mooney | we could try and start it in Stein but i think that would be too tight to get the nova and neutron work done also | |
| 10:29:46 | openstackgerrit | Takashi NATSUME proposed openstack/nova master: Use oslo_db.sqlalchemy.test_fixtures https://review.openstack.org/609352 | |
| 11:36:01 | pooja_jadhav | hi team, I have some doubt in the https://github.com/openstack/nova/blob/master/nova/tests/unit/api/openstack/compute/test_simple_tenant_usage.py#L135-L143.. In this they are trying to create 10 instances but when I have debug the code, I can see 20 istances. (1 instances repeated again). Can anyone guide me for the same? | |
| 11:54:03 | openstackgerrit | Martin Midolesov proposed openstack/nova master: Implementing graceful shutdown. https://review.openstack.org/608704 | |
| 12:23:31 | pooja_jadhav | sean-k-mooney: Hi | |
| 12:23:49 | sean-k-mooney | jangutter: fyi your patch is still in the gate which is why the next patach has the -2 since its not merged yet | |
| 12:23:53 | sean-k-mooney | pooja_jadhav: hi | |
| 12:24:33 | pooja_jadhav | sean-k-mooney: I have some doubt in the https://github.com/openstack/nova/blob/master/nova/tests/unit/api/openstack/compute/test_simple_tenant_usage.py#L135-L143.. In this they are trying to create 10 instances but when I have debug the code, I can see 20 instances. (1 instances repeated again). Can you guide me for the same? | |
| 12:26:51 | jangutter | sean-k-mooney: I tried looking for any os-vif patches in the gate and it seemed to have silently disappeared. | |
| 12:27:07 | sean-k-mooney | looking at it. depending on what _fake_instance does it looks like this should create tenahts*servers fake instance objecst | |
| 12:27:55 | sean-k-mooney | jangutter: ok well its there now http://zuul.openstack.org/status if you put 611017 in the filter | |
| 12:28:26 | sean-k-mooney | sometimes it takes a minute for it to post back after it leaves the queue | |
| 12:29:32 | sean-k-mooney | pooja_jadhav: was there somthing in partcaller that you were confusted by regarding https://github.com/openstack/nova/blob/master/nova/tests/unit/api/openstack/compute/test_simple_tenant_usage.py#L135-L143 | |
| 12:31:24 | pooja_jadhav | sean-k-mooney: sorry, I am not getting what u mean to say | |
| 12:32:09 | sean-k-mooney | pooja_jadhav: https://github.com/openstack/nova/blob/master/nova/tests/unit/api/openstack/compute/test_simple_tenant_usage.py#L136-L143 is is construcint an InstanceList by initalisint the objects filed with a list comprehention | |
| 12:32:59 | sean-k-mooney | it is first calulating a rag by multipliying tenants by server then for each x in that range it is calling _fake_instance | |
| 12:33:07 | pooja_jadhav | sean-k-mooney: the issue i am facing like, I am creating 2 instances then in the simple tenant usage api should give me usgaes for that 2 instances only. but I can see 4 instance instead | |
| 12:34:01 | pooja_jadhav | correct | |
| 12:34:34 | pooja_jadhav | SERVERS = 5 and TENANTS = 2 (it should create 10 instances) | |
| 12:34:39 | sean-k-mooney | TENANTS and SERVERS are not paramaters to the function | |
| 12:34:41 | pooja_jadhav | correct? | |
| 12:34:43 | sean-k-mooney | yes | |
| 12:35:07 | pooja_jadhav | but when I see the instanceList, in that I can see 20 instances | |
| 12:35:25 | pooja_jadhav | 1 instance is repeating again (twice) | |
| 12:35:37 | pooja_jadhav | for the existing test cases only | |
| 12:36:04 | sean-k-mooney | can yo point me to a failing test? | |
| 12:36:18 | pooja_jadhav | test is not failing right now | |
| 12:36:36 | pooja_jadhav | actually, I am writting new test for simple tenant usage api for my fix | |
| 12:36:59 | pooja_jadhav | at that time, I was able to see this behaviour | |
| 12:38:07 | pooja_jadhav | ideally, if instance_list is giving 2 instances then for simple tenant usage should give usages for 2 instances only. | |
| 12:38:21 | sean-k-mooney | pooja_jadhav: can you push the fialing test case it would be eaier to debug it with logs | |
| 12:38:49 | pooja_jadhav | test is not failing | |
| 12:39:17 | pooja_jadhav | mriedem ^^ | |
| 12:39:20 | sean-k-mooney | yes but it would be useful if i could see how you were using this fucntion | |
| 12:39:29 | sean-k-mooney | can you at leat past bin it somehwere | |
| 12:39:34 | pooja_jadhav | yes | |
| 12:40:13 | sean-k-mooney | fake_get_active_by_window_joined intally at least looks correct. | |
| 12:40:26 | sean-k-mooney | im wondering are you calling https://github.com/openstack/nova/blob/master/nova/tests/unit/api/openstack/compute/test_simple_tenant_usage.py#L116-L132 somehow? | |
| 12:41:37 | pooja_jadhav | http://paste.openstack.org/show/732423/ | |
| 12:42:44 | pooja_jadhav | I am using fake_get_active_by_window_joined() only not fake_get_active_deleted_flavorless() | |
| 12:49:26 | sean-k-mooney | pooja_jadhav: why are you patching a test function | |
| 12:49:58 | pooja_jadhav | means? | |
| 12:50:16 | pooja_jadhav | I am trying to add new test | |
| 12:50:37 | sean-k-mooney | sorry you are patching get_active_by_window_joined not fake_get_active_by_window_joined | |
| 12:50:52 | pooja_jadhav | hmm | |
| 12:51:54 | pooja_jadhav | I have mock this get_active_by_window_joined() and return_value set list of 2 instances (1 BFV and other is normal instance). | |
| 12:52:51 | pooja_jadhav | In the simple tenant usage API, in the instance_list I can see 4 instances and same in response dict after get API as well | |
| 12:52:53 | sean-k-mooney | yes so this is not using fake_get_active_by_window_joined at all | |
| 12:53:14 | pooja_jadhav | yes | |
| 12:54:02 | sean-k-mooney | what do you mean by the simple tenant api you can see 4 instances. you are working on unit test currectly so how can you see 4 instance when you dont have openstack running | |
| 12:54:52 | pooja_jadhav | in the unit test, I am calling index method | |
| 12:55:03 | pooja_jadhav | res_dict = self.controller.index(req) | |
| 12:58:18 | sean-k-mooney | right but why are you not mocking out _tenant_usages_for_period and the view builder funcitons | |
| 12:59:29 | sean-k-mooney | if you are wringing a unit test for index you should not be calling https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/simple_tenant_usage.py#L313 or https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/simple_tenant_usage.py#L322 within the test | |
| 12:59:34 | pooja_jadhav | but for existing tests as well, they are moacking these above functions | |
| 13:02:18 | sean-k-mooney | that is true that does not mean the exisiting test are well written. | |
| 13:06:30 | sean-k-mooney | pooja_jadhav: can you try using a patch decorator syntax instead of the with condetex manager | |
| 13:07:16 | sean-k-mooney | so precreate the expected dict before the test fuction and use @mock.patch('nova.objects.InstanceList.get_active_by_window_joined', expceted dicts) | |
| 13:08:07 | sean-k-mooney | actully never mind it should be identical | |
| 13:13:02 | sean-k-mooney | pooja_jadhav: am i dont have time to dig into this more right now sorry. | |
| 13:13:17 | pooja_jadhav | np | |
| 13:13:31 | pooja_jadhav | i will dig out :) | |
| 13:13:37 | pooja_jadhav | thanks for ur time | |
| 13:22:19 | mriedem | cfriesen: i have a buttload of questions in the tpm spec https://review.openstack.org/#/c/571111/ | |
| 13:23:00 | mriedem | mmedvede: any idea if emulated tpm is supported on pkvm? https://libvirt.org/formatdomain.html#elementsTpm | |
| 13:23:10 | mriedem | tonyb: ^ | |
| 13:23:24 | sean-k-mooney | moshele: is the ci testing os-vif broken http://13.74.249.42/16/610916/4/check-os-vif/OVS_HW_offload/1c9af37/logs/n-cpu.service.log.gz | |
| 13:24:42 | sean-k-mooney | moshele: the failures seam to be related to looking up the represntors | |
| 13:24:57 | moshele | sean-k-mooney: we has some environment issues with, but it should be working now | |
| 13:25:14 | moshele | lennyb: right ^ | |
| 13:26:36 | sean-k-mooney | i have approved https://review.openstack.org/#/c/611017 which is makeing a minior change to the unitiest code only and i was seeing this error. | |
| 13:27:24 | sean-k-mooney | the test change cannont cause this faiure so i am assuming this is an ci env issue. | |
| 13:29:13 | moshele | sean-k-mooney: I run recheck on the mellanox ci just to make sure it is working, if not I will talk to lennyb to fix it | |
| 13:30:38 | sean-k-mooney | moshele: cool i just wanted to reach out as i do check the result but i also dont block patches if the melonox ci fails and i can determin the failure was unrelated to the current patch as it is in this case | |
| 13:34:32 | openstackgerrit | Stephen Finucane proposed openstack/nova master: api-ref: 'vcpus_total' doesn't reflect overcommit ratio https://review.openstack.org/611604 | |
| 13:36:32 | stephenfin | jaypipes, mriedem: Would it ever make sense to remove the vcpus_used, vcpus_total from the hypervisor APIs, given that placement can do this stuff for us now? They're very misleading ^ | |
| 13:38:52 | openstackgerrit | Merged openstack/nova-specs master: Update pypi url to new url https://review.openstack.org/582092 | |
| 13:40:01 | openstackgerrit | Takashi NATSUME proposed openstack/nova master: Use assertRegex instead of assertRegexpMatches https://review.openstack.org/611608 | |