| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-sdks - 2017-03-01 | |||
| 17:14:16 | dtroyer | rather, I wanted it to protect against that assumption changing | |
| 17:14:53 | sshank | dtroyer, yeah right now it seems like only dhcp can be added to network. | |
| 17:14:58 | dtroyer | if that is a safe assumption, let's just remove —agent-type. But we will forever have that assumption of the default type if there ever is a nother one possible in this command | |
| 17:15:20 | dtroyer | can we live with thqat? | |
| 17:15:22 | dtroyer | that? | |
| 17:15:37 | sshank | ankur-gupta-f1, ^^? | |
| 17:16:08 | sshank | dtroyer, he is not at his desk. Lemme ping him | |
| 17:16:45 | dtroyer | I'll leaving a comment in the review to doc this… | |
| 17:16:47 | ankur-gupta-f1 | yea i am fine with removing --agent-type | |
| 17:17:07 | dtroyer | and living with the assumption forever? :) | |
| 17:17:55 | ankur-gupta-f1 | ah theres the catch | |
| 17:18:20 | dtroyer | that's why I have been deliberate on this… | |
| 17:18:21 | ankur-gupta-f1 | I still stick by my original idea of putting the --<agent-binary> flag. | |
| 17:18:42 | dtroyer | what is that again? | |
| 17:19:29 | ankur-gupta-f1 | so in case an agent is created down the road where it can add an agent there must be some way of indicating which of the two types of agents. | |
| 17:19:30 | dtroyer | also, there is the slightly larger pattern for all "network agent add/remove XXXX" commands. If others may need to declare an agent type | |
| 17:19:42 | dtroyer | right, that is what —agent-type is for | |
| 17:19:50 | dtroyer | but unneeded at the moment | |
| 17:20:26 | ankur-gupta-f1 | What I am proposing is instead of --agent-type <agent-name>, just have a single flag --<agent-name> | |
| 17:20:53 | ankur-gupta-f1 | network agent add network --dhcp <agent-id> <network> | |
| 17:21:16 | dtroyer | ok, yeah, that is actaully closer to what we do in a lot of other places. | |
| 17:21:31 | ankur-gupta-f1 | parser then will only have to check which flag is set and then call the appropriate object function | |
| 17:21:52 | dtroyer | sometimes we pick the enumerated options over explicit options when there are either a lot of them, or the list is semi-fluid | |
| 17:22:21 | dtroyer | in the general "network agent add/remove XXX" command case, is this still a safe approach? | |
| 17:22:44 | reedip | hi ankur-gupta-f1 | |
| 17:22:52 | reedip | cant we add lbaas as an agent ? | |
| 17:23:03 | ankur-gupta-f1 | yes. many-to-many relationship I think should be the guideline whether to use add/remove and set | |
| 17:23:58 | dtroyer | specifically though WRT —dhcp style options rather than a single generic --agent-type | |
| 17:25:21 | ankur-gupta-f1 | yes | |
| 17:25:29 | reedip | I agree with "network agent add/remove ABC" optio | |
| 17:25:44 | ankur-gupta-f1 | reedip: currently the only lbaasv2 command for the lbaas-agent checks lbaas-agent vs other loadbalancing resources | |
| 17:26:15 | reedip | ankur-gupta-f1 : what I meant was except dhcp , we have other options as wel | |
| 17:27:03 | ankur-gupta-f1 | reedip: explain? | |
| 17:27:18 | ankur-gupta-f1 | there will be a flag for --l3 as well | |
| 17:27:56 | dtroyer | so to restate for clarity: for any "network agent" commands that may need to handle multiple agent types, we will use agent-type-specific options (such as —dhcp and —l3) to identify the agent type rather than a single —agent-type <type> option; also where only a single type can be assumed we will use that assumption as the default. | |
| 17:28:17 | ankur-gupta-f1 | correct | |
| 17:28:22 | dtroyer | \o/ | |
| 17:28:35 | ankur-gupta-f1 | adding to both those reviews for reference | |
| 17:28:49 | sshank | so its 'openstack network agent add network --dhcp <agent-id> <network>' correct? | |
| 17:29:05 | ankur-gupta-f1 | yes | |
| 17:29:18 | dtroyer | —dhscp is not required in that command as it is the only agent type to need "add network" is my understanding | |
| 17:29:49 | dtroyer | if we want to allow —dhcp that is fine with me, and is essentially what the current —agent-type option is doing | |
| 17:29:51 | reedip | dtroyer: yes, thats correct | |
| 17:29:58 | sshank | So its 'openstack network agent add network <agent-id> <network>' since dhcp is default? | |
| 17:30:05 | dtroyer | yes | |
| 17:30:12 | sshank | Ok | |
| 17:30:16 | reedip | but cant we have any oher network agent in the future ? | |
| 17:30:29 | dtroyer | we can, but DHCP will be the default forever | |
| 17:30:30 | ankur-gupta-f1 | wait no we just went backwards | |
| 17:30:48 | dtroyer | ?? | |
| 17:31:25 | reedip | dtroyer : so you want --dhcp to be the default optional ? | |
| 17:32:18 | reedip | I am not sure about that .... User should know which agent he is working upon. I would suggest an agent type to be specified. It may not be a flag , but an agent name ( like dhcp / l3 ) which can be checked | |
| 17:33:17 | dtroyer | so then the need is to make the rule for how to specify network agent type for all network agent commands | |
| 17:33:57 | sshank | I personally think 'openstack network agent add network --dhcp <agent-id> <network>' would be better. It is explicit and causes no confusion. | |
| 17:34:15 | reedip | sshank : --dhcp => dhcp | |
| 17:34:37 | reedip | does that seem better or equally intriguing to u? | |
| 17:34:39 | dtroyer | no, if it is there it is an option | |
| 17:34:43 | dtroyer | —dhcp | |
| 17:34:59 | reedip | dtroyer ; thats my question, does it have to be an option? | |
| 17:35:07 | dtroyer | otherwise w echange the resource name from "network agent" to "dhcp agent" or omething like that, then deal with the explosion of commands | |
| 17:36:08 | reedip | dtroyer : oh ok, got it. The agent-id will be used with the Optional flag | |
| 17:36:19 | reedip | and that would make our complete agent | |
| 17:37:01 | dtroyer | no, the option is stand-aline | |
| 17:37:15 | dtroyer | agent id is a positional argument | |
| 17:38:16 | ankur-gupta-f1 | http://paste.openstack.org/show/600983/ | |
| 17:38:21 | reedip | dtroyer ; U didnt get what I meant ( I was not clear :) ) . I understand that agent-id is the positional argument. Just that from the neutron's POV the agent-id and the agent-type would define the complete agent info | |
| 17:38:54 | dtroyer | ok, fair enough, I've only been thinking about the command structure, not implementation. | |
| 17:39:04 | dtroyer | that said, agent-id isn't uniqueue enough to identify it? | |
| 17:39:30 | reedip | dtroyer: agent id is unique enough ( being the UUID value ) | |
| 17:40:00 | dtroyer | good. I've been surorised about that sort of thing before, OpenStack is always finding new and unique ways to surprise me :) | |
| 17:40:10 | dtroyer | *surprised | |
| 17:40:31 | reedip | we have to fetch its type and verify if the passed option ( like --dhcp ) matches with the agent passed ( or else leave the handling to neutron server and wait for them[ in this case, maybe Us] to pop up :P ) | |
| 17:41:11 | dtroyer | without knowing much else, such as the specifics of the error that is returned, I would let Neutron validate the agent id | |
| 17:41:32 | reedip | ankur-gupta-f1 : so your proposal is to have "network agent add router" and "neutron agent add network " as 2 different CLIs ? | |
| 17:41:43 | dtroyer | unless that is something we can easily validate because we already have the information without another trip to the server | |
| 17:41:46 | reedip | dtroyer : yeah, neutron can do that | |
| 17:42:04 | ankur-gupta-f4 | reedip: yes | |
| 17:42:17 | reedip | dtoryer; No , we need to fetch the agent type from its ID from the server, so its better to let Neutron handle it, if possible | |
| 17:43:04 | reedip | ankur-gupta-f4 ( I am confused which ID is on ) : but using "network agent add --XXX" makes handling simpler , isnt it? | |
| 17:44:59 | ankur-gupta-f4 | reedip: how would you implement that. | |
| 17:45:37 | dtroyer | reedip: recall the structure of add/remove commands, with 2 resources | |
| 17:45:57 | reedip | dtroyer: yeah that thought crossed my mind | |
| 17:47:14 | reedip | ankur-gupta-f4 : do we need to keep separate resources for router and network ? | |
| 17:47:35 | reedip | my implementation may be wrong | |
| 17:48:05 | ankur-gupta-f4 | Yea. There should be 2 args when adding | |
| 17:49:34 | ankur-gupta-f4 | No need to overcomplicate | |
| 17:50:24 | dtroyer | network agent add XXX [[—<agent-type-option>] <agent-id> <XXX-name-or-id> | |
| 17:51:15 | sshank | dtroyer: +1 | |
| 17:52:10 | ankur-gupta-f4 | Yes. +1 | |
| 17:54:55 | reedip | yeah, kinda what I would like.. handling things in one go +1 dtroyer | |
| 17:57:48 | ankur-gupta-f4 | 30 min later we finalize the solution we came up with at the beginning 😞 | |
| 17:58:02 | ankur-gupta-f4 | Thanks for the feedback everyone | |
| 17:58:12 | ankur-gupta-f4 | sshank: we got work to do | |
| 17:58:46 | sshank | Thanks all. Finally a conclusion. :D \o/ | |
| 18:02:08 | ankur-gupta-f4 | sshank: u say that now | |
| 18:02:12 | reedip | ankur-gupta-f4 : HA HA HAAA | |
| 18:02:22 | reedip | :P | |
| 18:03:16 | ankur-gupta-f4 | sshank: expect heavy doses of -1s... mostly from me | |
| 18:03:20 | openstackgerrit | Merged openstack/python-openstackclient master: Auto allocated topology for OSC https://review.openstack.org/391331 | |
| 18:04:24 | reedip | ankur-gupta-f4 : can you review the floating ip set/unset | |