Thursday, 2016-08-11

*** diogogmt has quit IRC00:16
*** chandanc has joined #openstack-fwaas01:00
*** diogogmt has joined #openstack-fwaas01:03
*** mickeys has quit IRC01:13
*** chandanc has quit IRC01:28
*** chandanc has joined #openstack-fwaas02:40
*** chandanc_ has joined #openstack-fwaas02:42
*** chandanc has quit IRC02:45
*** vishwanathj_ has joined #openstack-fwaas02:54
*** sc68cal has quit IRC03:52
*** sc68cal has joined #openstack-fwaas03:52
*** vishwanathj has quit IRC04:20
*** vishwanathj has joined #openstack-fwaas04:21
*** vishwanathj_ has quit IRC05:36
*** xgerman_ has joined #openstack-fwaas05:45
*** njohnston has quit IRC05:48
*** afranc has quit IRC05:48
*** xgerman has quit IRC05:48
*** mestery has quit IRC05:48
*** njohnston_ has joined #openstack-fwaas05:48
*** afranc has joined #openstack-fwaas05:49
*** xgerman_ is now known as xgerman05:49
*** mestery has joined #openstack-fwaas05:51
*** vishwanathj has quit IRC06:15
*** vishwanathj has joined #openstack-fwaas06:15
*** mickeys has joined #openstack-fwaas06:19
*** mickeys has quit IRC06:26
*** chandanc_ has quit IRC07:05
*** chandanc_ has joined #openstack-fwaas08:14
*** vishwanathj_ has joined #openstack-fwaas08:26
*** chandanc_ has quit IRC11:02
*** vishwanathj_ has quit IRC11:06
*** yamamoto has joined #openstack-fwaas12:02
*** yamamoto has quit IRC12:13
*** yamamoto has joined #openstack-fwaas12:15
*** Trident has joined #openstack-fwaas12:16
*** yamamoto has quit IRC12:19
*** yamamoto has joined #openstack-fwaas12:20
*** yamamoto has quit IRC12:29
*** yamamoto has joined #openstack-fwaas13:09
*** njohnston_ is now known as njohnston13:09
*** chandanc has joined #openstack-fwaas13:10
*** diogogmt has quit IRC13:33
*** diogogmt has joined #openstack-fwaas13:34
*** yamamoto has quit IRC13:41
*** yamamoto has joined #openstack-fwaas13:43
*** yamamoto has quit IRC13:43
*** yamamoto has joined #openstack-fwaas13:44
*** diogogmt has quit IRC13:50
chandancHello Guys13:56
njohnstonHello chandanc13:56
mfranc213hello chandanc and njohnston13:56
chandancHey13:56
chandancjust trying to understand, how to start testing margaret's patch13:57
mfranc213i did unit tests only for the fwaas v2 l3 agent extension. for integration testing i believe that we need to put the following changesets13:57
mfranc213in place:13:57
mfranc213the l3 agent extension PS13:57
chandancok,13:58
mfranc213the v2 fwaas plugin13:58
mfranc213the v2 fwaas driver13:58
chandancthats my changes13:58
chandancright ?13:58
mfranc213yours is the v2 fwaas driver, right?13:58
chandancyes13:58
mfranc213then the db piece13:59
chandancI think we might also need the db13:59
chandancya13:59
mfranc213and it goes on from there13:59
chandancwow, looks like the complete set :)13:59
mfranc213because do we need versioned objects?  i think not, actually, because at the moment we're not doing the new rpc callback mechanism13:59
njohnstonI think these are the ones we were targeting for merge by Friday13:59
chandancok14:00
mfranc213and it would be great to have the cli stuff in order to avoid having to write rest calls14:00
mfranc213i'm really thinking aloud here: maybe i'm getting stuff wrong.14:00
chandanchmm, not sure how far the cli stuff is14:00
mfranc213i'm in the middle of a review.14:00
chandancno i have the same feeling14:00
mfranc213it doesn't look all that far off, as far as i can tell.14:00
chandancok14:01
mfranc213neutron's agent extension generalizaation has already been merged.14:01
chandancok14:01
mfranc213and i wrote the l3 agent extension code as totally analogous to what was in place for the l2 side14:01
chandanclet me start pulling the patches14:01
mfranc213so i'm fairly confident that that one is ready to go14:01
mfranc213chandanc, does it make sense to make sure we're talking about the same changesets (just to be sure)?14:02
chandancany chance you have the links for the patches in one place14:02
mfranc213i mostly do.  1 sec.  let me fetch them.14:02
mfranc213fetch the URLs i should say14:02
chandancyes, it does14:02
chandancgreat, thanks a lot14:03
mfranc2131 sec.  i haven't fallen off a bridge...14:04
chandanc:) its ok, thats a lot of work saved for me14:05
mfranc213CLI: https://review.openstack.org/#/c/35158214:05
mfranc213REST commands: https://review.openstack.org/#/c/26448914:05
mfranc213Plugin: https://review.openstack.org/#/c/26704614:05
mfranc213DB: https://review.openstack.org/#/c/31115914:06
chandancThanks a lot14:06
mfranc213a few more...14:06
mfranc213L3 agent extension manager: https://review.openstack.org/#/c/33924614:07
mfranc213FWaaS agent extension: https://review.openstack.org/#/c/33769914:07
mfranc213And yours is the driver ... blagh can't find that one but i'm sure you know it!14:08
chandancsure, that i will take care of14:09
mfranc213and the devstack fwaas plugin and the agent extension generalization are already merged.14:09
chandancok, cool. Let me get started with14:09
mfranc213i'm not sure all of these are needed for an integration test14:10
mfranc213and i'm not sure which of these are ready for an integration test14:11
mfranc213so those are the two things that you and nate can help with i think14:11
chandancsure, Let me start pulling in the changes and I will let you know the results14:12
mfranc213that is excellent14:12
mfranc213once again you are working very late :(14:13
mfranc213i'm sorry14:13
*** SridarK has joined #openstack-fwaas14:14
chandancno worries :)14:14
mfranc213can you send me the link to the driver changeset?14:18
SridarKmfranc213: chandanc: Hello, so thinks are looking good14:18
mfranc213hello sridar!14:19
chandancThe driver changeset will be part of your patch14:19
SridarKon the driver piece ?14:19
mfranc213the driver isn't its own changeset?14:19
SridarKchandanc: from ur pastebin:14:19
SridarK    0     0 neutron-l3-agent-fwaas-defau  all      qr-c1a76216-8f *       ::/0                 ::/0                     0     0 neutron-l3-agent-fwaas-defau  all      qr-8a37c725-62 *       ::/0                 ::/0                     0     0 neutron-l3-agent-fwaas-defau  all      qr-e2d7c3b0-49 *       ::/0                 ::/014:19
SridarKouch that did not come right14:19
SridarK0 neutron-l3-agent-fwaas-defau  all      qr-c1a76216-8f *       ::/0                 ::/014:20
SridarKand 2 other rows below that14:20
chandancno, I will be merging it with your patch set14:20
mfranc213wait, does that make sense?  the fwaas plugin is its own changeset.14:20
chandancSridarK, maybe the line number will be better14:20
SridarKchandanc: L# 132-13414:21
njohnstonSridarK: I was wondering if you could comment on the questions I asked here, when you get a chance: https://review.openstack.org/#/c/311159/22/neutron_fwaas/db/migration/alembic_migrations/versions/d6a12e637e28_neutron_fwaas_v2_0.py@6014:22
SridarKnjohnston: yes will do, i can address Yushiro's comment also on the next patch i push up14:22
njohnstonThanks.  I am still working on that test failure.14:23
chandancSridarK: yes i got it, you have a question ?14:23
SridarKnjohnston: sorry - i could not look at the UT failure, i ran into an issue testing the policy_update method14:23
njohnstonSridarK: no problem :-)14:23
SridarKso have been debugging that, last night i think i made some headway, i need to clean that up and get that out.14:24
mfranc213chandanc, let me know when you come up for air.  i'd like to talk further about the idea of having both driver and agent extension code in a single changeset14:24
mfranc213i would prefer to have these be separate changesets but maybe i am missing something14:24
mfranc213maybe njohnston and sridark can chime in on this question also14:25
njohnstonWell, we would want the driver to be in use in an L2 context as well as an L3 context, right?14:25
chandancmargaret, sure will ping you14:26
SridarKmfranc213: for the L3, perhaps chandanc can push an update to ur patch14:26
chandancnjohnston: the l3 driver is different from l2 driver14:26
SridarKso we can avoid having another patch in progress14:26
*** vishwanathj_ has joined #openstack-fwaas14:26
njohnstonchandanc: Ah, I see.14:27
chandancl2 driver will be a separate patch14:27
mfranc213i think it will be more difficult to review a single changeset with two pieces of disparate functionality.  i know this topic was discussed earlier; i may be in the minority here.14:28
SridarKmfranc213: since the change is confined to the driver - perhaps it will be easier to avoid another level of dependency for the merge14:29
chandancI would prefer a separate patch, but looks like we are running out of time for reviews and the changes on the driver looked small enough14:30
SridarKwith that in ur patch14:30
SridarKmfranc213: my concern along chandanc: 's comment above14:30
mfranc213chandanc, sridark: okay. sounds good to me.14:31
SridarKmfranc213: thx for ur understanding14:31
chandancby the way, in our previous discussion (me and margaret) we came to the conclusion that to test the driver we need the following set of patches14:32
chandancCLI: https://review.openstack.org/#/c/35158214:32
chandancREST commands: https://review.openstack.org/#/c/26448914:32
chandancPlugin: https://review.openstack.org/#/c/26704614:32
chandancDB: https://review.openstack.org/#/c/31115914:32
chandancL3 agent extension manager: https://review.openstack.org/#/c/33924614:32
chandancFWaaS agent extension: https://review.openstack.org/#/c/33769914:32
chandancnjohnston, SridarK any thought14:33
SridarKchandanc: u are correct14:33
SridarKwe can avoid the CLI14:33
SridarKlet me publish the curl cmds14:33
chandanci was about to ask that :)14:33
SridarKso u can use that to drive the operations14:34
mfranc213chandanc https://review.openstack.org/#/c/337699 has two unit tests missing14:34
mfranc213njohnston has uncovered a scenario in which two developers can overwrite one another's changes if they don't push the PSs up in a particular14:34
mfranc213way.14:34
mfranc213njohnston: could describe this for us?14:34
SridarKchandanc: i dont think u need 339246 & 337699 righ now14:34
mfranc213chandanc: i will work on fixing those 2 unit tests.  i won't push anything until you and i know how to avoid stomping on each other's feet.14:34
chandancSridarK: thanks that will help14:35
SridarKmfranc213: yes we can do some offline coordination to avoid this14:35
mfranc213maybe nate has published this somewhere14:36
chandancSridarK: do you think i should upload my changes and update it later ?14:37
SridarKchandanc: if mfranc213: is at a logical point and not abt to push a patch up, u can make sure that u put ur changes over her latest version14:38
SridarKand push it up14:38
chandancsure14:38
mfranc213i'm at a logical stopping point other than the 2 unit tests--but maybe chandanc can fix those! :)14:39
njohnstonsorry, I stepped away to get myself some breakfast14:39
njohnstoncatching up...14:39
*** yamamoto has quit IRC14:40
njohnstonmfranc213: The way to avoid having two developers overwrite patchsets is just to always make sure you're up to date with `git review -d` before developing, and scrupulously check the change before uploading a patchset to make sure someone else hasn't uploaded one in the mean time.14:40
njohnstonSo yes, I think chandanc should push the driver updates as a new PS for 33769914:42
njohnstonbrb14:43
mfranc213thank you nate14:44
SridarKchandanc: as part of ur testing, will u be setting up a devstack env with the REST, db, plugin, 337699 (+ ur changes)14:44
chandancyes14:44
SridarKi have been testing on devstack with the first 3 patches14:45
SridarKi will add 337699 once i wrap up the issue i am debugging now14:45
*** yamamoto has joined #openstack-fwaas14:47
*** diogogmt has joined #openstack-fwaas14:51
chandancI just posted the patch, will update it once the tests are done14:57
SridarKchandanc: thanks14:58
SridarKchandanc: i have updated #link https://etherpad.openstack.org/p/fwaas-v2-l2-agent with curl cmds for create14:58
chandancsure, plz wait for my update, might take a little time14:59
SridarKchandanc: no worries - i am dealing with db issues now - it will be a while b4 i get to it, but u can use the curl cmds for ur testing14:59
SridarKi have put in some samples, u will need to change the uuid's of the resources accordingly15:00
SridarKstepping away for abt 30 mins or so15:04
chandancany idea how to trigger db migration ?15:18
njohnstonchandanc: I believe this document describes how to do it with `neutron-db-manage`: http://docs.openstack.org/developer/neutron/devref/alembic_migrations.html15:21
*** yamamoto has quit IRC15:28
*** mickeys has joined #openstack-fwaas15:28
*** mickeys has quit IRC15:32
SridarKchandanc: one other alternative is to build out the tables manually15:37
SridarKi have been doing this15:37
SridarKif the migration script runs correctly will be the easiest way to do it15:40
SridarKif the basic set of tables get built out by the migration script - then if u uncover some missing column - u can always add that manually15:42
njohnstonbut if you find a missing column, comment about it on 311159 :-)15:44
*** mickeys has joined #openstack-fwaas15:49
chandancmade it to work with "neutron-db-manage --config-file /etc/neutron/neutron.conf upgrade heads"15:51
chandancbut looks like some tables creation order needs to change15:51
SridarKchandanc: ok tht is good15:52
SridarKif u do a describe <tablename> - for our tables - does it look decent15:53
chandancI will send the diff to nate15:53
SridarKchandanc: i assume this is to do with a foreign key or backref ?15:54
chandancya15:54
chandancthats the issue15:54
chandancmysql> show tables like "%firewall%";15:54
chandanc+---------------------------------------------+15:54
chandanc| Tables_in_neutron (%firewall%)              |15:54
chandanc+---------------------------------------------+15:54
chandanc| cisco_firewall_associations                 |15:54
chandanc| firewall_policies                           |15:54
chandanc| firewall_router_associations                |15:54
chandanc| firewall_rules                              |15:54
chandanc| firewall_v2_address_group_cidr_associations |15:54
chandanc| firewall_v2_address_groups                  |15:54
chandanc| firewall_v2_group_port_associations         |15:54
chandanc| firewall_v2_groups                          |15:54
chandanc| firewall_v2_policies                        |15:54
chandanc| firewall_v2_policy_rule_associations        |15:54
chandanc| firewall_v2_rules                           |15:54
chandanc| firewalls                                   |15:54
chandanc| nsxv_firewall_rule_bindings                 |15:54
chandanc+---------------------------------------------+15:54
SridarKthat looks good15:55
chandancmysql> desc firewall_v2_groups;15:56
chandanc+----------------------------+---------------+------+-----+---------+-------+15:56
chandanc| Field                      | Type          | Null | Key | Default | Extra |15:56
chandanc+----------------------------+---------------+------+-----+---------+-------+15:56
chandanc| id                         | varchar(36)   | NO   | PRI | NULL    |       |15:56
chandanc| name                       | varchar(255)  | YES  |     | NULL    |       |15:56
chandanc| description                | varchar(1024) | YES  |     | NULL    |       |15:56
chandanc| project_id                 | varchar(36)   | YES  |     | NULL    |       |15:56
chandanc| egress_firewall_policy_id  | varchar(36)   | NO   | UNI | NULL    |       |15:56
chandanc| ingress_firewall_policy_id | varchar(36)   | NO   | UNI | NULL    |       |15:56
chandanc+----------------------------+---------------+------+-----+---------+-------+15:56
chandancmysql> desc firewall_v2_policies;15:56
chandanc+-------------+---------------+------+-----+---------+-------+15:56
chandanc| Field       | Type          | Null | Key | Default | Extra |15:56
chandanc+-------------+---------------+------+-----+---------+-------+15:56
chandanc| id          | varchar(36)   | NO   | PRI | NULL    |       |15:56
chandanc| name        | varchar(255)  | YES  |     | NULL    |       |15:56
chandanc| description | varchar(1024) | YES  |     | NULL    |       |15:56
chandanc| project_id  | varchar(36)   | YES  |     | NULL    |       |15:56
chandanc| audited     | tinyint(1)    | YES  |     | NULL    |       |15:56
chandanc| public      | tinyint(1)    | YES  |     | NULL    |       |15:56
chandanc+-------------+---------------+------+-----+---------+-------+15:56
chandanc6 rows in set (0.01 sec)15:56
SridarKHmm the v2 seems to be at the end in the sqlalchemy stuff15:56
SridarKhttps://review.openstack.org/#/c/311159/22/neutron_fwaas/db/firewall/v2/firewall_db_v2.py L#9115:57
njohnstonwe should probably change the sqlalchemy to match the migration15:58
SridarKnjohnston: yes either way - we can track this as a cleanup15:58
SridarKchandanc: a -1 with with this comment pls15:59
SridarKnjohnston: since with our file naming, we seem to be putting v2 at the end16:00
SridarKshall we adopt a similar thing16:01
njohnstonsure, I have no strong opinions on it16:01
SridarKnjohnston: i am defn not religious on this naming thing16:01
SridarKnjohnston: :-)16:01
SridarKchandanc: would it help if quickly push rev up to keep this matching16:02
chandancdone16:02
SridarKie the migration table name and the sqlalc16:02
chandancsorry didnot get your last comment16:02
SridarKchandanc: the naming does not match16:03
SridarKlook at : https://review.openstack.org/#/c/311159/22/neutron_fwaas/db/firewall/v2/firewall_db_v2.py L#9116:03
SridarKthe table name above is firewall_v2_policies16:03
SridarKand in L#9116:04
SridarKit firewall_policies_v216:04
SridarKi am wondering if that is the issue u hit16:04
chandancoh16:04
SridarKi created the tables manually16:04
SridarKso i did not run the script - so i matched up with sqlal16:05
chandancno this means I pulled in wron ps16:05
SridarKhmm not sure16:06
SridarKwhat issue did u see actually16:06
SridarKit is likely that since the sqlal did not match the table name, it would have complained16:07
SridarKnjohnston: are u in the midst of pushing up a PS, if not let me quickly change this16:07
njohnstonI'm working on changing it16:08
SridarKnjohnston: ok perfect16:08
chandancgot it, the migration script needs update16:08
chandancthe model is correct16:08
SridarKok u can pick up the next PS16:08
chandancya16:09
chandancnate, will you also take care of the ordering issue ?16:09
SridarKchandanc: what ordering issue did u hit ?16:09
chandancthe backref and foreign key refs16:10
njohnstonI just updated 311159 for the table names16:11
njohnstonCan you be more specific about the backref/foreign key issue?16:12
njohnstonchandanc: ^^16:12
SridarKnjohnston: ok perfect16:12
SridarKnjohnston: i think in the script, u want the create table ordering as rules, policies, firewall_groups, policy_rule_association16:14
SridarKchandanc: can u pls confirm16:14
chandancbasically create the referenced table before the table referencing them, firewall_groups_v2 to be created before firewall_group_port_associations_v216:15
njohnstongotcha16:15
njohnstoni'll do that now16:15
SridarKnjohnston: also i think we are missing the group port table16:16
chandanccool :), do i get another shot at the migration script with the new patch ? or i have to go manual ?16:16
SridarKlet me point to that in gerrit16:16
SridarKnjohnston: take that back16:17
SridarKit is there16:17
SridarKchandanc: yes pls give it another shot16:17
SridarKafter njohnston 's update16:17
chandancproblem is the db is marked as updated, right ?16:17
SridarKthis makes it so much easier over manually creating tables :-(16:18
chandancI know :)16:18
SridarKi am afraid to mess with my devstack env for this reason16:18
chandanci am assuming mine is already messed, i am not going to commit from this one any more,16:20
chandancubuntu@devstack5:~$ neutron-db-manage current16:20
chandanc  Running current for neutron ...16:20
chandancINFO  [alembic.runtime.migration] Context impl MySQLImpl.16:20
chandancINFO  [alembic.runtime.migration] Will assume non-transactional DDL.16:20
chandanca5648cfeeadf (head)16:20
chandanc7d9d8eeec6ad (head)16:20
chandanc  OK16:20
chandanc  Running current for neutron-fwaas ...16:20
chandancINFO  [alembic.runtime.migration] Context impl MySQLImpl.16:20
chandancINFO  [alembic.runtime.migration] Will assume non-transactional DDL.16:20
chandancd6a12e637e28 (head)16:20
chandancf83a0b2964d0 (head)16:20
chandanc4b47ea298795 (head)16:20
chandanc  OK16:20
chandancd6a12e637e28 is ours16:20
chandancAFK for sometime, its getting late :(16:21
SridarKchandanc: understand - i think the best is to pull njohnston 's latest and restack with OFFLINE=True and RECLONE=no16:23
njohnstonThe table additions are now in correct order in the migration file in https://review.openstack.org/31115916:23
SridarKnjohnston: perfect16:23
SridarKthx16:23
chandancwill try it and let you know16:24
njohnstonthanks!16:25
*** vishwanathj_ has quit IRC16:36
chandancstill need some change16:45
chandancdiff --git a/neutron_fwaas/db/migration/alembic_migrations/versions/d6a12e637e28_neutron_fwaas_v2_0.py b/neutron_fwaas/db/migration/alembic_migrations/versions/d6a12e637e28_neutron_fwaas_v2_0.py16:45
chandancindex 9f46f13..89bab52 10064416:45
chandanc--- a/neutron_fwaas/db/migration/alembic_migrations/versions/d6a12e637e28_neutron_fwaas_v2_0.py16:45
chandanc+++ b/neutron_fwaas/db/migration/alembic_migrations/versions/d6a12e637e28_neutron_fwaas_v2_0.py16:45
chandanc@@ -32,6 +32,22 @@ import sqlalchemy as sa16:45
chandanc def upgrade():16:45
chandanc16:45
chandanc     op.create_table(16:45
chandanc+        'firewall_policies_v2',16:45
chandanc+        sa.Column('id', sa.String(length=36), primary_key=True),16:46
chandanc+        sa.Column('name', sa.String(length=255)),16:46
chandanc+        sa.Column('description', sa.String(length=1024)),16:46
chandanc+        sa.Column('project_id', sa.String(length=36)),16:46
chandanc+        sa.Column('audited', sa.Boolean),16:46
chandanc+        sa.Column('public', sa.Boolean))16:46
chandanc+16:46
chandanc+    op.create_table(16:46
chandanc+        'firewall_address_groups_v2',16:46
chandanc+        sa.Column('id', sa.String(length=36), primary_key=True),16:46
chandanc+        sa.Column('name', sa.String(length=255)),16:46
chandanc+        sa.Column('description', sa.String(length=1024)),16:46
chandanc+        sa.Column('project_id', sa.String(length=36)))16:46
chandanc+16:46
chandanc+    op.create_table(16:46
njohnstonchandanc: probably better to use paste.openstack.org16:46
chandanc         'firewall_rules_v2',16:46
chandanc         sa.Column('id', sa.String(length=36), primary_key=True),16:46
chandanc         sa.Column('name', sa.String(length=255)),16:46
chandanc@@ -54,15 +70,6 @@ def upgrade():16:46
chandanc         sa.Column('enabled', sa.Boolean))16:46
chandanc16:46
chandanc     op.create_table(16:46
chandanc-        'firewall_policies_v2',16:46
chandanc-        sa.Column('id', sa.String(length=36), primary_key=True),16:46
chandanc-        sa.Column('name', sa.String(length=255)),16:46
chandanc-        sa.Column('description', sa.String(length=1024)),16:46
chandanc-        sa.Column('project_id', sa.String(length=36)),16:46
chandanc-        sa.Column('audited', sa.Boolean),16:46
chandanc-        sa.Column('public', sa.Boolean))16:47
chandanc-16:47
chandanc-    op.create_table(16:47
chandanc         'firewall_groups_v2',16:47
chandanc         sa.Column('id', sa.String(length=36), primary_key=True),16:47
njohnstonplease stop16:47
chandanc         sa.Column('name', sa.String(length=255)),16:47
chandanc@@ -76,13 +83,6 @@ def upgrade():16:47
chandanc                   nullable=False, unique=True))16:47
chandanc16:47
chandanc     op.create_table(16:47
chandanc-16:47
chandancsure16:47
chandanchttp://paste.openstack.org/show/554226/16:48
chandancoh sorry dint get you16:48
njohnstonthanks!16:48
njohnstonhttps://review.openstack.org/311159 updated16:50
*** mickeys has quit IRC17:11
*** mickeys has joined #openstack-fwaas17:11
*** mickeys has quit IRC17:16
*** mickeys has joined #openstack-fwaas17:48
*** mickeys_ has joined #openstack-fwaas17:50
*** mickeys has quit IRC17:53
*** SumitNaiksatam has joined #openstack-fwaas17:55
njohnstonSridarK: ping18:19
SridarKnjohnston: yes pls tell me18:19
njohnstonSridarK: I see the issue with the test in 311159.  It is doing "POST /firewall_policies.json HTTP/1.0" but it should be doing "POST /v2.0/fw/firewall_policies.json HTTP/1.0"18:20
njohnstonSridarK: Also, Ihar has a question about the l3 agent extensions patch, and I am not sure of the answer: https://review.openstack.org/#/c/339246/12/neutron/agent/l3/agent.py18:20
SridarKnjohnston: aha - ok so wrong path18:20
njohnstonI'm not sure if the agent extension should be notified of router deletion contemporaneously with it getting enqueued to go to the agent, or if it should wait until the router is deleted18:22
SridarKnjohnston: i guess if someone wants to do something b4 the router gets deleted, we may need to know when the intent is expressed18:24
SridarKthen the ext entities can do something18:24
njohnstonFor the FWaaS use case, though, are we particular about it?18:24
SridarKin our case actually - the deletion is some we dont care abt at all18:24
SridarK*something18:25
SridarKas the rules will get cleaned up18:25
SridarKnjohnston: pls give me a few mins, chandanc would like to debug an issue with his test18:25
njohnstonOK, then I will acquesce to Ihar's suggestion, it seems sane to me18:25
SridarKnjohnston: i was abt to get on a call with him18:26
SridarKnjohnston: ok, i can piggyback there too if u need - i need to understand the workflow on that a bit more18:26
*** mickeys_ has quit IRC18:27
*** mickeys has joined #openstack-fwaas18:27
*** mickeys has quit IRC18:37
*** mickeys has joined #openstack-fwaas18:38
*** chandanc_ has joined #openstack-fwaas18:39
*** mickeys has quit IRC18:42
*** chandanc has quit IRC18:42
SridarKnjohnston: it should be POST /v2.0/fwaas/18:55
njohnstonSridarK: But in the examples at  https://etherpad.openstack.org/p/fwaas-v2-l2-agent it is given as /v2.0/fw not /v2.0/fwaas:  "<your_devstack>:9696/v2.0/fw/firewall_rules"19:03
SridarKnjohnston: yes i have to fix that - it was based on some hacks i had in my workspace19:03
SridarKbut in terms of UT it should show up correctly19:04
njohnstonok19:04
SridarKi have updated the ether pad as well19:06
njohnstonSo I have the extensions patch (264489) patch pulled in to this... yet what I am seeing is:19:13
njohnston    Request is: POST /v2.0/fwaas/firewall_policies.json HTTP/1.019:13
njohnston    Accept: application/json19:13
njohnston    Content-Length: 20219:13
njohnston    {"firewall_policy_v2": {"audited": true, "description": "default description", "shared": true, "firewall_rules": [], "tenant_id": "46f70361-ba71-4bd0-9769-3573fd227c4b", "name": "firewall_policy_v2_1"}}19:13
njohnston    Result is: 404 Not Found19:13
njohnstonIs there any other dependency I need to make a call like that work?19:13
*** mickeys has joined #openstack-fwaas19:15
*** chandanc_ has quit IRC19:22
*** SumitNaiksatam has quit IRC19:41
*** SumitNaiksatam has joined #openstack-fwaas19:46
SridarKnjohnston: in a mtg so give me a few mins19:53
njohnstonnp19:53
SridarKnjohnston: this is via the UT ?19:54
SridarKnjohnston: one thing is that tenant_id is actually project_id with respect to the ext patch19:55
SridarKi had to hack some of this initially19:55
SridarKin the devstack env19:55
SridarKbut with UT it should come out correctly unless there is something missing19:56
njohnstonyes, this is from the UT; I put some print statements in20:07
njohnstonBut tenant_id versus project_id shouldn't give a 404, it's like the /v2.0/fwaas/firewall_policies.json endpoint doesn't exist, or doesn't support the POST method20:09
njohnstonI figure it's something obvious I am missing20:24
njohnstonthese unit tests are more like functional tests if you ask me20:31
njohnstonI was basing them off of the fwaas v1 UTs20:31
*** mickeys has quit IRC20:39
SridarKnjohnston: the other thing possible is that without the plugin - perhaps a mock or something is needed for the new endpoint ?20:45
SridarKmaybe running the UT where the ext / plugin patch is also present - i am a bit puzzled on this as well20:46
njohnstonso I would need to pull in https://review.openstack.org/#/c/267046/ to get this to work, you think?20:47
njohnstonThe alternative would be to separate the unit tests out into a separate patch, that could then be expressed with dependencies on whatever else is needed20:48
njohnstonI don't like that alternative, but it may be what we need to get going and get things merged20:49
SridarKnjohnston: yes i agree too - the multiple patch thing is defn a pain20:55
SridarKi am trying to push out the policy update, keep getting sucked into multiple things , i am going go grab some lunch and let me put a wrap on that and i can also debug this issue20:57
njohnstonthanks!20:57
SridarKif we can avoid another patch that will nice, but i agree  - thats our plan B20:58
*** mickeys has joined #openstack-fwaas20:58
*** SridarK has quit IRC21:02
njohnstonI just committed my PS for 311159 that has print statements, so you can see what I am seeing.21:05
njohnstonI think that if we haven't gotten this sorted by tomorrow morning then we should probably split out the UTs, in order to make the Friday merge deadline.21:12
njohnstonI intend to be in this channel a lot tomorrow, trying to get things merged.  SridarK, do you know if shwetaap could be online to help us expedite getting her change merged?21:14
njohnstonOK, I am going to stop talking to myself and go make dinner.  See you all tomorrow!21:14
*** njohnston is now known as njohnston_21:59
*** njohnston has joined #openstack-fwaas22:01
*** njohnsto_ has joined #openstack-fwaas22:10
*** njohnston has quit IRC22:14
*** mickeys has quit IRC22:22
*** mickeys has joined #openstack-fwaas22:24
*** mickeys has quit IRC22:29
*** SridarK has joined #openstack-fwaas22:31
*** njohnsto_ has quit IRC22:57

Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!