| *** spatel has joined #senlin | 03:04 | |
| eandersson | spatel looks like this test needs to be updated TestNovaServerUpdate.test_update_network | 03:53 |
|---|---|---|
| spatel | eandersson: i was about to ask you how to deal with those failure, this is my first commit and no idea how to handle this ZUUL pipeline | 03:54 |
| eandersson | > "vnic_type: 'None' is not supported.(supported types are: normal, direct, macvtap) | 03:54 |
| eandersson | Are you able to run the tests locally? | 03:54 |
| eandersson | https://zuul.opendev.org/t/openstack/build/c381d1fd130f4c42807c78dc179320a0 | 03:56 |
| eandersson | If you click on one of the tests it will give you more details about the failure. | 03:56 |
| eandersson | It looks like two things are issues at the moment. | 03:56 |
| spatel | Yes i am testing that part on my lab | 03:56 |
| eandersson | The second one would indicate that there is no default vnic type set | 03:56 |
| spatel | default is normal so i thought we don't need to specify. | 03:57 |
| spatel | if vnic_type is not set in profile.yml then it use None value which is default normal | 03:58 |
| eandersson | > vnic_type = net_spec.get(self.VNIC_TYPE, None) | 03:58 |
| eandersson | Hmm yea | 03:58 |
| eandersson | You are right | 03:58 |
| spatel | my code is working fine in lab, i am able to create both cluster normal and sriov | 03:59 |
| eandersson | Ah | 03:59 |
| eandersson | Looks like the spec itself does not allow a None value | 03:59 |
| spatel | Yes | 03:59 |
| eandersson | VNIC_TYPE: schema.String( | 03:59 |
| eandersson | The problem here is that if you don't provide one it will fail | 03:59 |
| spatel | but its working for me. | 04:00 |
| spatel | I have created two profile. 1. without vnic_type 2. using vnic_type: direct | 04:01 |
| eandersson | Yea that is odd | 04:01 |
| spatel | 1. spun up vms with normal nic | 04:01 |
| spatel | 2. spun up with SRIOV direct nic | 04:01 |
| eandersson | https://zuul.opendev.org/t/openstack/build/d21154c7849e45f0aa8d90783daabd4d | 04:02 |
| spatel | If you specify vnic_type then it use default value "None" | 04:02 |
| spatel | eandersson: I am running ussuri in my lab | 04:03 |
| spatel | do you think that could be the issue, its working for ussuri but not victoria? | 04:04 |
| eandersson | ah I see | 04:05 |
| eandersson | if vnic_type is None: | 04:05 |
| eandersson | On line 651 | 04:05 |
| eandersson | You don't allow for a None value | 04:05 |
| eandersson | and None would be the default value right? | 04:05 |
| spatel | Yes | 04:06 |
| eandersson | Really None is a valid value | 04:06 |
| spatel | This is by logic if you don't pass any value it use "None" | 04:06 |
| eandersson | in fact | 04:06 |
| eandersson | that should be is not None right? | 04:06 |
| spatel | so i put login around that if vnic_type is None: then check what is the human input? | 04:07 |
| spatel | logic* | 04:07 |
| spatel | sorry if vnic_type not None in that logic then it will check array for correct input value | 04:08 |
| eandersson | Yea | 04:08 |
| spatel | I have tested my logic from all kind of scenario and it passed | 04:09 |
| spatel | question is why this build is failing.. | 04:09 |
| spatel | I may setup victoria lab and give it a try with latest code. | 04:09 |
| eandersson | So the code cannot work | 04:09 |
| eandersson | because the if statement is inversed | 04:09 |
| eandersson | and another test is failing here https://github.com/openstack/senlin/blob/6dff71e13f4752c02b5c119d25f3e31a93ade1bf/senlin/tests/unit/profiles/test_nova_server_update.py#L120 | 04:11 |
| eandersson | because vnic_part: None isn't in this test | 04:12 |
| spatel | This is the logic - http://paste.openstack.org/show/800762/ | 04:13 |
| eandersson | Yep | 04:13 |
| eandersson | That if statement basically means that you wil lonly check the vnic type of NONE is specified | 04:14 |
| eandersson | But why would you ever check vnic type if it is not specified? | 04:14 |
| eandersson | You already know that vnic_type is None (and not e.g. normal, direct or macvtap) | 04:14 |
| spatel | yes, that is correct, if you don't specify vnic_type then do nothing | 04:15 |
| eandersson | Yep | 04:15 |
| eandersson | > if vnic_type is None: | 04:15 |
| eandersson | Should really be | 04:15 |
| eandersson | > if vnic_type is not None: | 04:15 |
| eandersson | Because we shouldn't validate a vnic_type set to None | 04:15 |
| eandersson | Because otherwise you might as well just change this line | 04:16 |
| eandersson | > supported_vnic_types.index(vnic_type) | 04:16 |
| eandersson | to | 04:16 |
| eandersson | > supported_vnic_types.index(None) | 04:16 |
| spatel | I can try that and see how that logic work. | 04:16 |
| spatel | let me do some experiment there. | 04:16 |
| eandersson | You also need to update this https://github.com/openstack/senlin/blob/6dff71e13f4752c02b5c119d25f3e31a93ade1bf/senlin/tests/unit/profiles/test_nova_server_update.py#L125 | 04:16 |
| eandersson | to include | 04:16 |
| eandersson | 'vnic_type': 'normal', | 04:17 |
| eandersson | I think | 04:17 |
| eandersson | and ideally add a test as well in there | 04:17 |
| spatel | inside network section? https://github.com/openstack/senlin/blob/6dff71e13f4752c02b5c119d25f3e31a93ade1bf/senlin/tests/unit/profiles/test_nova_server_update.py#L143 | 04:17 |
| eandersson | Good question :D | 04:18 |
| eandersson | I don't know hehe | 04:18 |
| eandersson | Are you able to run | 04:18 |
| eandersson | tox -e py36 | 04:18 |
| eandersson | on your dev machine inside the senlin repo | 04:18 |
| spatel | i will try, i never used tox before :) | 04:19 |
| eandersson | you may need to install tox | 04:19 |
| eandersson | pip3 install tox | 04:19 |
| spatel | i will find some help on internet about it and test it out on my lab | 04:19 |
| spatel | what this Test does by the way? - https://github.com/openstack/senlin/blob/6dff71e13f4752c02b5c119d25f3e31a93ade1bf/senlin/tests/unit/profiles/test_nova_server_update.py#L120 | 04:21 |
| eandersson | I think it just tests the input of a profile update | 04:25 |
| eandersson | *server update | 04:25 |
| eandersson | but dtruong probably knows more about that | 04:25 |
| spatel | eandersson: look like it works! > if vnic_type is not None: | 04:29 |
| eandersson | Awesome | 04:30 |
| spatel | let me show you what i did | 04:32 |
| spatel | eandersson: here is the full POC - http://paste.openstack.org/show/800763/ | 04:35 |
| spatel | so everything looks good so far. | 04:36 |
| eandersson | Nice | 04:44 |
| spatel | I am also going to remove > def _check_vnic_type(self, nc, net_spec, result): | 04:44 |
| spatel | we don't need that block as per dtruong code block | 04:45 |
| *** openstackgerrit has joined #senlin | 04:52 | |
| *** ChanServ sets mode: +v openstackgerrit | 04:52 | |
| openstackgerrit | Satish Patel proposed openstack/senlin master: Add support of vnic_type to Profile https://review.opendev.org/c/openstack/senlin/+/765215 | 04:52 |
| spatel | Hmm! This Test code are also reference in other files also - https://github.com/openstack/senlin/blob/6dff71e13f4752c02b5c119d25f3e31a93ade1bf/senlin/tests/unit/objects/requests/test_profiles.py#L21 | 05:00 |
| eandersson | I don't think you need to update that one. | 05:02 |
| eandersson | Were you able to get tox to run locally? | 05:02 |
| spatel | eandersson: installing it now.. | 05:12 |
| spatel | running tox -e py36 | 05:17 |
| spatel | what that command doing ? | 05:19 |
| spatel | its hanging here | 05:23 |
| -spatel- py36 run-test: commands[0] | find . -type f -name '*.py[c|o]' -delete | 05:23 | |
| -spatel- py36 run-test: commands[1] | stestr run | 05:23 | |
| openstackgerrit | Satish Patel proposed openstack/senlin master: Add support of vnic_type to Profile https://review.opendev.org/c/openstack/senlin/+/765215 | 05:28 |
| *** sapd1 has joined #senlin | 05:28 | |
| spatel | eandersson: its stuck here last 15 minutes - http://paste.openstack.org/show/800765/ | 05:33 |
| *** sapd1 has quit IRC | 05:36 | |
| *** sapd1 has joined #senlin | 05:38 | |
| spatel | feeling sleepy so lets talk about this monday also i have updated patch so you can review again | 05:46 |
| eandersson | Sounds good. Have a good weekend. | 05:53 |
| eandersson | Yea - that should at most take 3-5 minutes | 05:53 |
| *** spatel has quit IRC | 06:15 | |
| *** sapd1 has quit IRC | 08:33 | |
| *** sapd1 has joined #senlin | 08:57 | |
| *** sapd1 has quit IRC | 11:22 | |
| *** spatel has joined #senlin | 12:37 | |
| *** spatel has quit IRC | 12:42 | |
| *** jmlowe has quit IRC | 15:30 | |
| *** jmlowe has joined #senlin | 15:51 | |
| *** jmlowe has quit IRC | 16:32 | |
| *** jmlowe has joined #senlin | 16:35 | |
| *** jmlowe has quit IRC | 18:16 | |
| *** spatel has joined #senlin | 18:16 | |
| *** spatel has quit IRC | 18:21 | |
| *** spatel has joined #senlin | 18:21 | |
| *** jmlowe has joined #senlin | 19:06 | |
| *** spatel has quit IRC | 20:10 | |
Generated by irclog2html.py 2.17.2 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!