| *** padkrish has joined #openstack-fwaas | 00:38 | |
| *** padkrish_ has joined #openstack-fwaas | 00:39 | |
| *** padkrish has quit IRC | 00:42 | |
| *** padkrish_ has quit IRC | 00:56 | |
| *** padkrish has joined #openstack-fwaas | 01:25 | |
| *** padkrish has quit IRC | 01:32 | |
| *** padkrish has joined #openstack-fwaas | 01:43 | |
| SridarK | njohnston: whenever u get a chance to read this: fixed 4 more | 02:02 |
|---|---|---|
| SridarK | last few: http://paste.openstack.org/show/560707/ | 02:03 |
| SridarK | will need to step out for some time | 02:03 |
| *** diogogmt has joined #openstack-fwaas | 02:17 | |
| *** diogogmt has quit IRC | 02:18 | |
| *** vishwanathj has quit IRC | 02:39 | |
| *** padkrish has quit IRC | 03:05 | |
| *** padkrish has joined #openstack-fwaas | 03:05 | |
| *** padkrish has quit IRC | 03:12 | |
| *** padkrish has joined #openstack-fwaas | 03:17 | |
| *** padkrish has quit IRC | 03:28 | |
| yushiro | Hi SridarK , I just wrote UTs for https://review.openstack.org/#/c/356306/ . Can I update it? | 03:36 |
| yushiro | If there is no problem, I'll put the patch and add njohnston as a reviewer. | 03:37 |
| *** mickeys has joined #openstack-fwaas | 03:42 | |
| *** mickeys has quit IRC | 03:51 | |
| *** vishwanathj has joined #openstack-fwaas | 03:56 | |
| *** padkrish has joined #openstack-fwaas | 04:11 | |
| yushiro | I've added UTs for https://review.openstack.org/#/c/356306/ and logic has no difference. | 04:15 |
| *** vishwanathj has quit IRC | 04:17 | |
| *** chandanc_ has joined #openstack-fwaas | 04:21 | |
| *** vishwanathj has joined #openstack-fwaas | 04:38 | |
| *** padkrish has quit IRC | 04:44 | |
| *** mickeys has joined #openstack-fwaas | 04:52 | |
| *** mickeys has quit IRC | 04:56 | |
| SridarK | yushiro: thx for the additional UT on the patch | 05:00 |
| yushiro | SridarK, No problems. | 05:01 |
| *** vishwanathj is now known as vishwanathj_zzz | 05:08 | |
| *** padkrish has joined #openstack-fwaas | 05:28 | |
| *** padkrish has quit IRC | 05:45 | |
| *** SarathMekala has joined #openstack-fwaas | 05:47 | |
| *** padkrish has joined #openstack-fwaas | 05:53 | |
| *** padkrish has quit IRC | 06:09 | |
| *** yamamoto has quit IRC | 06:21 | |
| yushiro | Hi SarathMekala, thanks for your review. | 06:26 |
| SarathMekala | you are welcome :) | 06:27 |
| *** mickeys has joined #openstack-fwaas | 06:41 | |
| *** mickeys has quit IRC | 06:47 | |
| *** yamamoto has joined #openstack-fwaas | 07:00 | |
| *** yamamoto has quit IRC | 07:38 | |
| yushiro | Hi, fwaas team. Are you there? | 08:01 |
| SarathMekala | I am around :) | 08:02 |
| yushiro | SarathMekala: hi. | 08:02 |
| SarathMekala | hi | 08:03 |
| yushiro | SarathMekala: I'd like to confirm the attributes of firewall_rule | 08:03 |
| SarathMekala | sure | 08:03 |
| yushiro | SarathMekala: In v2 SPEC(http://specs.openstack.org/openstack/neutron-specs/specs/newton/fwaas-api-2.0.html#firewall-rules) | 08:03 |
| SarathMekala | yes..tell me | 08:04 |
| yushiro | SarathMekala: we've skip service_group_id, source_address_group_id and destination_address_group_id. | 08:04 |
| SarathMekala | Ok. Is it for the current cycle or for the release? | 08:05 |
| yushiro | SarathMekala: I think current cycle is. We should support ASAP. | 08:06 |
| yushiro | SarathMekala: current extension dictionary(https://review.openstack.org/#/c/264489/15/neutron_fwaas/extensions/firewall_v2.py) | 08:06 |
| SarathMekala | yeah.. i see that the parameters are missing | 08:07 |
| SarathMekala | you want to ping Shweta reg this? | 08:08 |
| SarathMekala | we need this info for the driver implementation as well | 08:08 |
| yushiro | SarathMekala: 'action' is required. | 08:09 |
| SarathMekala | yes | 08:09 |
| SarathMekala | it one of the main attributes | 08:09 |
| yushiro | However, the SPEC is 'Action to be performed on the traffic matching the rule (ALLOW, DENY, REJECT). Default: DENY.' | 08:09 |
| yushiro | Hmm, I think I should align the SPEC. How do you think? | 08:10 |
| SarathMekala | yeah.. right | 08:10 |
| SarathMekala | this has to be a required attribute | 08:10 |
| SarathMekala | One more thing.. in V1 a rule can be shared between policies | 08:11 |
| yushiro | yes | 08:11 |
| SarathMekala | i dont see the 'shared' attribute in the spec | 08:12 |
| yushiro | Ah, 'shared' is changed into 'public'. | 08:12 |
| yushiro | We can use 'public' same as 'shared' | 08:12 |
| SarathMekala | oh.. ok | 08:12 |
| yushiro | SarathMekala: Oh, sorry. I forgot. 'action' has been set a default value on L.210 | 08:18 |
| SarathMekala | how about the CLI? | 08:19 |
| yushiro | in V1 CLI? | 08:20 |
| SarathMekala | V2 CLI... we will need to handle it | 08:20 |
| yushiro | V2 CLI is what I'm implementing now. | 08:20 |
| yushiro | That's why I'd like to confirm about that :) | 08:21 |
| SarathMekala | :) ok.. I think in UI its a drop down field set to deny by default.. so no problem | 08:21 |
| yushiro | Yes, default is DENY. I think so too. | 08:22 |
| yushiro | Thank you, SarathMekala. | 08:22 |
| SarathMekala | you are welcome | 08:22 |
| *** yamamoto has joined #openstack-fwaas | 08:39 | |
| *** mickeys has joined #openstack-fwaas | 08:49 | |
| *** yamamoto has quit IRC | 08:52 | |
| *** mickeys has quit IRC | 08:54 | |
| *** yamamoto has joined #openstack-fwaas | 08:56 | |
| *** yamamoto has quit IRC | 08:56 | |
| *** yamamoto has joined #openstack-fwaas | 08:56 | |
| *** yamamoto has quit IRC | 09:10 | |
| *** chandanc_ has quit IRC | 09:12 | |
| *** yamamoto has joined #openstack-fwaas | 09:12 | |
| *** yamamoto has quit IRC | 09:12 | |
| mfranc213 | Hi yushiro and everyone. I'm here. | 09:30 |
| *** yamamoto has joined #openstack-fwaas | 09:30 | |
| *** yamamoto has quit IRC | 09:30 | |
| *** yamamoto has joined #openstack-fwaas | 09:30 | |
| yushiro | mfranc213: Hi. Long time no see. | 09:30 |
| *** yamamoto has quit IRC | 09:30 | |
| mfranc213 | yushiro i am looking at your CLI patch now (https://review.openstack.org/#/c/351582) | 09:31 |
| *** yamamoto has joined #openstack-fwaas | 09:31 | |
| mfranc213 | i hope to post my comments in an hour (a guess) | 09:31 |
| yushiro | mfranc213, Thank you so much. I'm fixing UT now. | 09:33 |
| mfranc213 | yushiro: excellent | 09:33 |
| mfranc213 | once i'm done with the comments on the CLI i'll move to the UTs for the oslo version object changeset (https://review.openstack.org/#/c/342476) | 09:34 |
| *** yamamoto has quit IRC | 09:36 | |
| yushiro | mfranc213: OK. ah, please let me confirm. | 09:41 |
| mfranc213 | yushiro: confirm? | 09:41 |
| yushiro | mfranc213: are you going to move CLI UTs into versioned object patch? | 09:41 |
| mfranc213 | yushiro: nope! | 09:42 |
| mfranc213 | yushiro: the ovo patch will have its own. | 09:42 |
| yushiro | mfranc213: OK. | 09:42 |
| *** yamamoto has joined #openstack-fwaas | 10:01 | |
| *** yamamoto has quit IRC | 10:01 | |
| *** yamamoto has joined #openstack-fwaas | 10:32 | |
| *** yamamoto has quit IRC | 10:37 | |
| *** yamamoto has joined #openstack-fwaas | 10:41 | |
| *** SarathMekala has quit IRC | 10:57 | |
| *** padkrish has joined #openstack-fwaas | 10:58 | |
| *** padkrish has quit IRC | 11:22 | |
| mfranc213 | yushiro: i just realized that i have more feedback you on the CLI patch. would it be better if i gave it now or waited? | 11:27 |
| yushiro | mfranc213: OK. Please review it now :) | 11:28 |
| mfranc213 | ok, doing that now. thank you! | 11:30 |
| mfranc213 | yushiro: done for real this time. now there are comments on PS3 along with the ones i gave on PS2. i hope they are helpful. | 11:40 |
| yushiro | mfranc213: Thank you so much. I'll confirm it. | 11:41 |
| yushiro | Especially, English grammar review is very helpful :) | 11:42 |
| SridarK | hi all | 12:04 |
| SridarK | sorry could not stay up last night | 12:04 |
| SridarK | yushiro: i think u had some q | 12:04 |
| yushiro | SridarK, Hi. | 12:05 |
| njohnston | hi all | 12:05 |
| SridarK | yushiro: service_group_id, source_address_group_id and destination_address_group_id - we are not supporting that | 12:05 |
| njohnston | SridarK: Sorry I didn't get to work on the DB patch more last night, family issues. | 12:05 |
| yushiro | SridarK, yes. | 12:05 |
| yushiro | njohnston, hi :) | 12:06 |
| SridarK | njohnston: ok no worries - i have one more UT done | 12:06 |
| SridarK | njohnston: i too had to be out and could not stay up very late | 12:06 |
| SridarK | let me push that out too | 12:06 |
| yushiro | SridarK, currently, I'm OK now. Thank you. | 12:09 |
| SridarK | i think we are now down to 7 | 12:09 |
| SridarK | yushiro: ok cool | 12:09 |
| njohnston | SridarK: But let me run something by you to be sure. The question came up, what is the difference between 'public' and 'shared'. | 12:09 |
| SridarK | njohnston: no difference | 12:09 |
| njohnston | My reply was that 'shared' is a FWaaS v1 term, and it was replaced by 'public' in v2. I just wanted to be sure I am correct. | 12:09 |
| SridarK | njohnston: yes exactly | 12:10 |
| njohnston | Why the change in vernacular? | 12:10 |
| yushiro | njohnston, SridarK I'd like to know it too. | 12:10 |
| SridarK | njohnston: actually i dont know either - i cant recall but someone pushed that in the spec | 12:10 |
| mfranc213 | i posed that question to yushiro earlier because i wasn't sure. | 12:10 |
| mfranc213 | and i'm afraid that the change in vernacular will be confusing, prompting in others the same question that i had. | 12:10 |
| SridarK | njohnston: actually i kind of found tht annoying | 12:11 |
| njohnston | probably only in those migrating from v1 to v2 | 12:11 |
| SridarK | mfranc213: i am with u | 12:11 |
| mfranc213 | yushiro and njohston had the same response as one another, by the way! | 12:11 |
| mfranc213 | is it possible to move to "public" at this time? | 12:11 |
| mfranc213 | s/public/shared | 12:11 |
| SridarK | it also causes kind of busy work | 12:11 |
| mfranc213 | doh!!! | 12:11 |
| mfranc213 | see? | 12:11 |
| mfranc213 | SridarK: busy work? | 12:12 |
| SridarK | no making sure we dont have some stale | 12:12 |
| njohnston | mfranc213: In order to re-use the UTs from v1 there are a l;ot of 'shared' to 'public' changes to be made | 12:12 |
| njohnston | for example | 12:12 |
| mfranc213 | njohnston: i'm afraid there would need to be changes in a LOT of modules | 12:12 |
| SridarK | i think we have kind of taken care of s/shared/public | 12:12 |
| mfranc213 | this stinks | 12:12 |
| SridarK | mfranc213: sigh yes - | 12:13 |
| njohnston | I am afraid it's a done deal now | 12:13 |
| mfranc213 | is is possible just to change the CLI for now? | 12:13 |
| SridarK | mfranc213: i think we have already changed it | 12:13 |
| njohnston | We could make 'shared' a synonym to 'public' in the CLI perhaps | 12:13 |
| mfranc213 | i don't like introducing 'shared' to the users | 12:13 |
| mfranc213 | so CLI and REST | 12:14 |
| njohnston | mfranc213: you mean 'public' ;-) | 12:14 |
| mfranc213 | see? | 12:14 |
| mfranc213 | i can't stop doing this myself. sorry. | 12:14 |
| yushiro | (nova or glance uses 'public', neutron uses 'shared' and these are almost same usage) | 12:15 |
| njohnston | Well we're going to have enough trouble delivering core functionality as it is, so I'd say we need to suck this one up and call it a user documentation issue | 12:15 |
| mfranc213 | okay njohnston. | 12:15 |
| SridarK | njohnston: +1 | 12:15 |
| mfranc213 | yushiro: thanks for checking out the other projects | 12:15 |
| mfranc213 | i like njohnston's idea to make 'shared' a synonym for 'public' in the CLI, and including information about this in the help. | 12:16 |
| mfranc213 | what do you think, yushiro? ^^ | 12:16 |
| yushiro | mfranc213: that make sense. | 12:17 |
| yushiro | excuse me, please let me confirm. | 12:19 |
| yushiro | 'sysnonym' means | 12:19 |
| yushiro | CLI can specify not only 'public' but 'shared' ? | 12:20 |
| mfranc213 | yushiro: 'synonyms' are two different words that mean the same thing. | 12:21 |
| mfranc213 | yushiro: i think that we could change the CLI so that it accepts either 'public' or 'shared' as arguments. or, to make things much simpler for now, just change the help so that it says something about 'public' meaning what 'shared' means elsewhere. | 12:23 |
| njohnston | SridarK: Did you have changes to 311159 you needed to push before I get working? | 12:23 |
| yushiro | mfranc213: OK. Thanks for your help. I understand. | 12:25 |
| *** padkrish has joined #openstack-fwaas | 12:26 | |
| SridarK | njohnston: yes just getting it out | 12:28 |
| mfranc213 | i have an appointment to go to. i will be back in 2-3 hours. when i get back i will work on the UTs for the oslo versioned objects, unless someone wants me to do something else. | 12:28 |
| yushiro | SridarK, njohnston Thanks for your quick review!! | 12:29 |
| SridarK | njohnston: http://paste.openstack.org/show/560792/ | 12:31 |
| SridarK | if u can start with looking into the one i have '*' at the end | 12:32 |
| SridarK | actually 2 of them | 12:32 |
| njohnston | Yep, I'm digging in now | 12:32 |
| SridarK | it seems like a the call is not formed correctly - it does not show up on the db | 12:32 |
| SridarK | i think the top 3 can be marked for a next rev | 12:33 |
| njohnston | I don't see a test named 'test_update_firewall_with_fwp' | 12:35 |
| njohnston | searching for that gives no results in https://review.openstack.org/#/c/311159/45/neutron_fwaas/tests/unit/db/firewall/v2/test_firewall_db_v2.py | 12:36 |
| njohnston | so I will work on test_update_firewall | 12:36 |
| SridarK | sorry back - the dog calls | 12:39 |
| SridarK | hmm ok | 12:39 |
| SridarK | oh that became 'test_update_firewall_group_with_fwp' | 12:40 |
| SridarK | i was debugging that | 12:40 |
| SridarK | but yes the other is fine | 12:40 |
| SridarK | the top 3 with the rule show, we can defer - another contributor has picked up insert_rule and remove_rule - i deferred them as they need more careful testing | 12:42 |
| SridarK | he will need to set things with the rule position - i will req him to pick up the top 3 | 12:42 |
| SridarK | yushiro: what are things u would like me to look into before u sign off for the night | 12:44 |
| SridarK | yushiro: i have been looking at the emails - just did not want to distract from the db patch uts | 12:45 |
| yushiro | SridarK: let me see... my e-mail about 'status' for firewall_group is still under discussion. | 12:47 |
| *** chandanc_ has joined #openstack-fwaas | 12:48 | |
| yushiro | SridarK: OK, nothing from me. Please let me inform to you my progress. | 12:48 |
| yushiro | ahhahaa, sorry. s/inform/report | 12:49 |
| yushiro | #link https://review.openstack.org/#/c/323971/ | 12:49 |
| yushiro | I've updated l2-agent patch. But unfortunately, I've changed topic branch name with mistake so I'll revert the branch name. | 12:50 |
| yushiro | CLI patch https://review.openstack.org/#/c/351582 | 12:51 |
| njohnston | yushiro: You can change the topic name from within the gerrit web UI | 12:52 |
| yushiro | Yesterday night, I updated. Today, I'm updating mfranc213's comment. | 12:52 |
| yushiro | njohnston, Yes, thank you :) | 12:52 |
| yushiro | Tomorrow, I'll update CLI patch about today's discussion('shared' and 'public') | 12:54 |
| SridarK | yushiro: ok thx - i responded to the email | 12:55 |
| SridarK | yushiro: also i am hoping that as u start ur day - u will be able to do a scan of the db patch - i am hoping the UT will all be done | 12:55 |
| SridarK | yushiro: i know u have been reviewing it constantly | 12:56 |
| yushiro | SridarK, Of course. In fact, I'm watching/writing/confusing/reviewing.... community contribution is very fun :) | 12:57 |
| njohnston | yushiro: I love the way you put that! :-) | 12:58 |
| njohnston | very well said | 12:58 |
| yushiro | njohnston, :-) | 12:58 |
| SridarK | yushiro: i had to revert back one of the changes i did for ur comment in https://review.openstack.org/#/c/311159/46/neutron_fwaas/db/firewall/v2/firewall_db_v2.py L#538 | 12:58 |
| *** padkrish has quit IRC | 12:58 | |
| SridarK | this is because when the method is called for an UPD - then it is not guaranteed that the ingress_firewall_policy and egress_firewall_policy keys will be present | 12:59 |
| SridarK | so i revert back to checking if the key exists | 13:00 |
| SridarK | yushiro: :-) on ur comment above - it has been so helpful | 13:00 |
| yushiro | SridarK, ooooooh, you're right. Please revert it. | 13:00 |
| njohnston | SridarK: fixed the two starred UTs, moving on to the next | 13:03 |
| yushiro | SridarK, ah, after reverted the code, I think some fix is necessary. | 13:03 |
| SridarK | njohnston: ok cool | 13:04 |
| SridarK | yushiro: ok pls comment on the patch when u get a chance | 13:04 |
| yushiro | After key check, second check fwg['ingress_firewall_policy_id'] is None or not. | 13:04 |
| yushiro | SridarK, yes. | 13:04 |
| yushiro | BTW, is anyone working for API-reference? I think priority is not so high but necessary for user. | 13:06 |
| SridarK | yushiro: ok sorry slow response in another discussion | 13:07 |
| yushiro | SridarK, no problems. | 13:07 |
| SridarK | yushiro: ok got it - to ensure for CR | 13:30 |
| SridarK | when u have time pls put a comment on gerrit | 13:30 |
| SridarK | and i will pick it up | 13:30 |
| yushiro | SridarK, Sure. | 13:30 |
| SridarK | yushiro: thx | 13:30 |
| SridarK | njohnston: u can comment out the the top 3 tests in the paste (with a TODO) | 13:31 |
| SridarK | old model - rule cannot be shared, and the rule had the policy and position in that policy | 13:32 |
| SridarK | i have refactored the code in that area so rules can be shared across policy - some clean up is needed on the show rule - which will fix those UT | 13:32 |
| SridarK | njohnston: if u are good - i will get back to the plugin patch | 13:33 |
| njohnston | back, call of the dog | 13:33 |
| njohnston | SridarK: Will do. Yep, I think I am good. I'll report when I have all of the UTs except for the top 3 fixed, or if I hit a block. | 13:33 |
| SridarK | njohnston: sounds good | 13:34 |
| SridarK | njohnston: aha i think now my dog heard ur dog ;-) | 13:34 |
| njohnston | heh | 13:34 |
| *** padkrish has joined #openstack-fwaas | 14:01 | |
| *** chandanc_ is now known as chandanc_afk | 14:03 | |
| *** SridarK has quit IRC | 14:08 | |
| njohnston | SridarK: When you get back... I am trying to locate precisely the place where the logic is located that decides that a public=True firewall policy from another tenant can be assigned to a firewall group. I think it's in _check_rules_for_policy_is_valid. | 14:13 |
| njohnston | But the problem is that I am working on the unit test test_update_firewall_group_with_public_fwp, and when contact is tenant1 non-admin, and I try to assign a firewall policy that is public=True from tenant2, I get FirewallPolicyNotFound. | 14:14 |
| njohnston | What vexes me about this is that FirewallPolicyNotFound looks like it is only generated from within _get_firewall_policy when a NoResultFound occurs when pulling from the DB model... but I don't see anything in the DB model that enforces this. | 14:16 |
| njohnston | If it was really _check_rules_for_policy_is_valid then I would expect a FirewallRuleSharingConflict exception, or a FirewallPolicyConflict exception if the logic involved comes from _validate_fwg_parameters | 14:18 |
| njohnston | I posted my changes to 311159 so far | 14:30 |
| njohnston | moving on to test_update_firewall_rule_associated_with_other_tenant_policy | 14:36 |
| *** padkrish has quit IRC | 14:41 | |
| *** yamamoto has quit IRC | 15:01 | |
| *** yamamoto has joined #openstack-fwaas | 15:04 | |
| *** yamamoto has quit IRC | 15:09 | |
| *** yushiro has quit IRC | 15:12 | |
| *** mickeys has joined #openstack-fwaas | 15:59 | |
| *** yamamoto has joined #openstack-fwaas | 16:06 | |
| *** padkrish has joined #openstack-fwaas | 16:08 | |
| *** yamamoto has quit IRC | 16:11 | |
| *** chandanc_afk has quit IRC | 16:14 | |
| *** padkrish has quit IRC | 16:25 | |
| *** xdcc has joined #openstack-fwaas | 16:25 | |
| *** padkrish has joined #openstack-fwaas | 16:41 | |
| njohnston | padkrish: How's it going? | 16:51 |
| padkrish | njohnston# hi, not too bad :) | 16:51 |
| padkrish | #njohnston# hoping to get some time at night for writting some mock cases for VO....need to sync up with mfranc213..how about you.... | 16:53 |
| mfranc213 | hello | 16:54 |
| njohnston | padkrish: I was just curious, who do you work for? Stackalytics shows you as 'independent'. | 16:54 |
| njohnston | padkrish: Oh, grinding on some truculent unit tests for the DB patch. | 16:54 |
| padkrish | njohnston# i work for Cisco :)....my user ID is my personal one i created a while ago...when i tried to change it to my company's in git/gerrit, that gave me lot of grief, so sticking with my personal id | 16:55 |
| padkrish | but didn't check Stackalytic | 16:56 |
| mfranc213 | padkrish how are you? i can continue to look at the test cases for VO (i've done only a very little bit) or do something else if you would like. | 16:56 |
| padkrish | mfranc213# i was just looking at the QoS examples again for the VO test cases | 16:57 |
| mfranc213 | excellent. so i'll pass the baton for VO back to you then and will get started on something else. | 16:57 |
| padkrish | mfranc213# doing allrite...i was planning to start on it tonite... | 16:58 |
| padkrish | mfranc213# ok sure, thanks for addressing the comments, going little crazy with my regular job :) | 16:58 |
| padkrish | that really helped | 16:58 |
| mfranc213 | very happy to help. let me know if you would like me to do anything further with the VO changeset, for the tests or anything else. | 16:59 |
| padkrish | mfranc213# sure, i had some questions on Ihar's comments for the address_group VO, not super urgent since that got removed for now :) | 17:00 |
| padkrish | i will ping you sometime later, we can discuss about that | 17:00 |
| mfranc213 | sounds perfect. and if it turns out you can't get to the UTs, holler and hopefully i can help out with that, too. | 17:01 |
| padkrish | sure, will do...thanks again.. | 17:01 |
| *** vishwanathj_zzz is now known as vishwanathj | 17:04 | |
| njohnston | Hmm, I seem to have introduced an error to the user tests for the db patch, causing the issue: {"NeutronError": {"type": "FirewallPolicyNotFound", "detail": "", "message": "Firewall Policy None could not be found."}} | 17:24 |
| *** SumitNaiksatam has joined #openstack-fwaas | 17:26 | |
| njohnston | mfranc213: When you have a moment, could I ask for your help on that? | 17:29 |
| mfranc213 | yep | 17:29 |
| mfranc213 | thinking | 17:29 |
| mfranc213 | do you want to push the PS with a workflow -1 so i can grab it? | 17:30 |
| njohnston | good example of that... grab 311159 and tox -e py27 neutron_fwaas.tests.unit.db.firewall.v2.test_firewall_db_v2.TestFirewallDBPluginV2.test_list_firewall_groups | 17:30 |
| njohnston | you should be able to grab it now | 17:30 |
| mfranc213 | okay, getting it now. i'll ping you once i have something, or know that i don't, etc. | 17:30 |
| njohnston | there is no magic to the workflow bit that would prevent you from grabbing it | 17:30 |
| njohnston | thanks much! | 17:30 |
| njohnston | I know it wasn't doing this a couple of days ago | 17:31 |
| mfranc213 | no, i know. didn't know though if your stuff was available for me to grab. | 17:31 |
| mfranc213 | oh, really? this is a new thing? | 17:31 |
| njohnston | yep, you can do `git review -d` pretty much anything | 17:31 |
| mfranc213 | yes, just wasn't sure if what you had was up in gerrit, is all. | 17:32 |
| njohnston | yes, I believe it is new, but please don't let me prejudice your investigation unduly | 17:32 |
| mfranc213 | :) | 17:32 |
| mfranc213 | just to be sure, it's this one, right: 311159 ? | 17:32 |
| njohnston | yes | 17:32 |
| mfranc213 | ty | 17:33 |
| njohnston | no, thank you! | 17:33 |
| mfranc213 | @padkrish: i have some changes to the VO patchset to push. i forgot that i had dealt with some of the nullability issues that ihar raised. | 17:34 |
| mfranc213 | i will push those up now. | 17:34 |
| padkrish | mfranc213# sure, sounds good | 17:34 |
| njohnston | brb | 17:37 |
| mfranc213 | @njohnston, could you rebase against master and then push a new PS? | 17:37 |
| mfranc213 | well, i guess i don't really need you to do that. i'll fix up that part of things locally. | 17:40 |
| njohnston | Hmm, I get 'up to date' when I rebase against master. | 17:43 |
| mfranc213 | neutron changed. | 17:47 |
| mfranc213 | okay, i've replicated the error. | 17:48 |
| *** SridarK has joined #openstack-fwaas | 17:50 | |
| SridarK | njohnston: give me few mins - looking at ur question on email | 17:52 |
| njohnston | SridarK: Thanks! | 17:53 |
| mfranc213 | njohnston: fwg1 has no egress_firewall_policy_id. i know that a fw allows either (perhaps both) the egress_policy_id and the ingress_policy_id to be null | 17:53 |
| mfranc213 | s/fw/fwg above | 17:54 |
| njohnston | if that is true, should the migration say 'nullable=False' for both ingress_firewall_policy_id and egress_firewall_policy_id? | 17:55 |
| mfranc213 | thinking | 17:55 |
| mfranc213 | i believe nullable=True actually. but that is a recent change on the db side, i think, right? | 17:55 |
| njohnston | It's a foreign key relation, but I think it may be an error to say nullable=False, as well as to have ondelete cascade | 17:55 |
| mfranc213 | not sure | 17:56 |
| mfranc213 | well, wait. | 17:56 |
| mfranc213 | the current state of the code is that nullable=True? | 17:56 |
| mfranc213 | (i could look, but you're here :) ) | 17:56 |
| mfranc213 | okay, i believe cascading delete is wrong. | 17:57 |
| mfranc213 | at least the db schema that i have indicates this. that that is a 1:m relationship between a FP and a FWG. so a FP can survive the deletion of its FWG. | 17:57 |
| -njohnston- . | 17:58 | |
| njohnston | sa.Column('egress_firewall_policy_id', sa.String(length=36), | 17:58 |
| njohnston | sa.ForeignKey('firewall_policies_v2.id', ondelete='CASCADE'), | 17:58 |
| njohnston | nullable=False, unique=True), | 17:58 |
| njohnston | sa.Column('ingress_firewall_policy_id', sa.String(length=36), | 17:58 |
| njohnston | sa.ForeignKey('firewall_policies_v2.id', ondelete='CASCADE'), | 17:58 |
| njohnston | nullable=False, unique=True)) | 17:58 |
| mfranc213 | so stepping back. | 17:58 |
| njohnston | that is from the definiton of firewall_groups_v2 | 17:58 |
| mfranc213 | you can have a fwg with just an egress policy, for instance, right? | 17:58 |
| njohnston | yes, I believe that is correct | 17:59 |
| mfranc213 | okay, and the cascading delete thing is an orthogonal issue. | 17:59 |
| mfranc213 | so if it is the case that a fwg can exist without one or the other (or both?) of ingress policy and egress policy, then i think nullable=True is correct | 18:00 |
| mfranc213 | and if a FP can be attached to multiple FWG, then cascading delete should not be specified | 18:00 |
| SridarK | mfranc213: if it is associated with multiple groups - delete is not allowed | 18:00 |
| mfranc213 | but there is ondelete='CASCADE' | 18:01 |
| mfranc213 | i actually think that these particular issues are separate from the bug we're seeing. let me check. | 18:01 |
| SridarK | L#515 in https://review.openstack.org/#/c/311159/47/neutron_fwaas/db/firewall/v2/firewall_db_v2.py | 18:02 |
| mfranc213 | so the info you sent me njohnston is in alembic which means that it shouldn't affect the unit test results, right? | 18:04 |
| njohnston | SridarK: That's the call to _get_firewall_policy, yes? | 18:04 |
| SridarK | On the FirewallPolicyRuleAssociation we should do a cascade delete if the policy we have a cascade delete but really there too if a rule is associated with a pol del is not allowed | 18:04 |
| SridarK | njohnston: that would be in the delete path | 18:05 |
| mfranc213 | SridarK i believe the issue here is the relationship between a firewall group and its policies. | 18:05 |
| SridarK | mfranc213: pls go ahead | 18:05 |
| SridarK | njohnston: mfranc213: may be a quick call with screen share ? | 18:06 |
| mfranc213 | sure | 18:06 |
| mfranc213 | do you want to set it up? | 18:06 |
| njohnston | mfranc213 and SridarK: Just to make sure evryone is clear, I emailed SridarK with a question (copied you mfranc213) but then I asked mfranc213 to look at something else. Just didn't want people to think they were talking about the same thing when they might not be. | 18:06 |
| mfranc213 | since i'm hopeless at that | 18:06 |
| mfranc213 | ahhh | 18:06 |
| mfranc213 | so sorry | 18:06 |
| SridarK | let me go find a room | 18:06 |
| SridarK | :-) | 18:06 |
| SridarK | no worries - i will set it up | 18:06 |
| mfranc213 | so i won't do a screenshare. | 18:06 |
| SridarK | njohnston: ok cool | 18:06 |
| mfranc213 | the screenshare | 18:06 |
| mfranc213 | i'll just keep looking at the issue nate asked me about | 18:07 |
| SridarK | mfranc213: no it will be good to nail this down too on the relationship btwn tables | 18:07 |
| mfranc213 | okay, sounds good. | 18:08 |
| *** xdcc has quit IRC | 18:31 | |
| *** xdcc has joined #openstack-fwaas | 18:31 | |
| *** mickeys has quit IRC | 18:33 | |
| *** mickeys has joined #openstack-fwaas | 18:34 | |
| *** mickeys has quit IRC | 18:39 | |
| *** xdcc has quit IRC | 18:43 | |
| *** xdcc has joined #openstack-fwaas | 18:43 | |
| *** padkrish has quit IRC | 18:51 | |
| *** padkrish has joined #openstack-fwaas | 19:17 | |
| *** xdcc has quit IRC | 19:34 | |
| njohnston | SridarK: I filed https://bugs.launchpad.net/neutron/+bug/1614673 for those methods that need to be looked at as far as positioning | 19:40 |
| openstack | Launchpad bug 1614673 in neutron "[FWaaS] Rule position testing is insufficient" [Wishlist,Confirmed] | 19:40 |
| SridarK | njohnston: many thx - will keep track of this | 19:44 |
| njohnston | SridarK: New version of https://review.openstack.org/311159 uploaded. | 20:06 |
| njohnston | I added a second bug, covering 2 tests, that also needs follow-up: https://bugs.launchpad.net/neutron/+bug/1614680 | 20:06 |
| openstack | Launchpad bug 1614680 in neutron "In FWaaS v2 cross-tenant assignment of policies is inconsistent" [Undecided,New] | 20:06 |
| njohnston | so those 2 are skipped, as well as the original 3 | 20:06 |
| njohnston | and now UTs pass. I also got it in shape for pep8 and removed some of the debugging hacks. | 20:07 |
| njohnston | So please take a look, and if it looks good to you then we can let xgerman and yushiro know | 20:07 |
| SridarK | njohnston: aah great | 20:08 |
| SridarK | i will run thru the latest PS in a bit | 20:08 |
| SridarK | yushiro will be in bed - but we can try to coordinate with xgerman & yushiro as the day starts in Japan | 20:09 |
| xgerman | sounds good | 20:10 |
| *** padkrish has quit IRC | 20:13 | |
| *** padkrish has joined #openstack-fwaas | 20:17 | |
| *** diogogmt has joined #openstack-fwaas | 20:27 | |
| *** SumitNaiksatam has quit IRC | 20:55 | |
| *** yamamoto has joined #openstack-fwaas | 21:09 | |
| *** yamamoto has quit IRC | 21:09 | |
| njohnston | SridarK: Does the fwaas l3 agent need access to an instance of RouterInfo, or just to the RouterInfo class? Based on the calls, I think the latter, but I want to be sure. | 21:12 |
| njohnston | Also, you mentioned a step before RouterInfo was consulted, which is a method whereby, using the port, one could look up the router attached to that port. What datastructure or functionality is consulted to make that lookup? | 21:14 |
| SridarK | njohnston: if u are in the context of L3Agent, u should have access to the router info as self.ri | 21:35 |
| njohnston | Hmm, I'll test that | 21:44 |
| njohnston | Once we undo armax's change so the fwaas l3 extension no longer inherits from L3Agent, then we won't have access to router info anymore I believe, because we will be outside the L3Agent context | 21:46 |
| njohnston | This was the point where I got confused before when making the l3 agent extension code, and didn't include the agent extension api. | 21:47 |
| SridarK | njohnston: with L3 Agent ext piece - i thought this will give us access to the router info | 21:51 |
| SridarK | do the callbacks for the router create etc give u this ? | 21:52 |
| njohnston | That was what I thought initially. But after looking at it, I don't think the router namespace is in the callback, since I believe that info never goes over the wire. | 21:53 |
| SridarK | njohnston: that could be an issue | 22:04 |
| njohnston | I think that the new patch will give us access to what we need; I just need to know what we use to take the port and look up the id of the router that is attached, to make sure that functionality is also included. | 22:05 |
| SridarK | from the port_id - we can find the owner and the router_id should be an index into the ri | 22:06 |
| SridarK | we can also send in the router_id from the plugin | 22:07 |
| SridarK | for the ports | 22:07 |
| njohnston | ok, so then all we would need access to is the router info | 22:17 |
| SridarK | yes | 22:17 |
| *** padkrish has quit IRC | 22:17 | |
| njohnston | good. Let me see if I can short-cycle this patch; my brain is a bit fried, so tomorrow i will look at setting up tests for it that will ensure that the fwaas agent extension will be able to get the things it needs. | 22:18 |
| SridarK | ok cool, i am adding UT and if i get the plugin set - i will get freed up too | 22:18 |
| SridarK | i will follow up with yushiro tonight | 22:19 |
| njohnston | Excellent. | 22:20 |
| njohnston | Thanks! | 22:20 |
| SridarK | no worries | 22:20 |
| SridarK | fingers crossed :-) | 22:20 |
| *** padkrish has joined #openstack-fwaas | 22:21 | |
| njohnston | all right, now I'm off - Nate needs to step away from the keyboard... | 22:31 |
| *** padkrish has quit IRC | 22:35 | |
| *** yamamoto has joined #openstack-fwaas | 22:42 | |
| *** padkrish has joined #openstack-fwaas | 22:46 | |
| *** mickeys has joined #openstack-fwaas | 22:48 | |
| *** mickeys has quit IRC | 23:04 | |
| *** vishwanathj has quit IRC | 23:29 | |
Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!