Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Controllers management #93

Open
wants to merge 20 commits into
base: integration-0.8
Choose a base branch
from
Open

Controllers management #93

wants to merge 20 commits into from

Conversation

vicalro
Copy link
Contributor

@vicalro vicalro commented Jul 28, 2015

This PR adds the features of adding, removing and listing controllers to LSIs trough the REST plugin.
Examples on how to use it can be found in the xcli tool.

In order to use the list feature, the API of rofl-common needs to be extended. You may find an extension in vicalro/rofl-common@3d251d6 (branch list_controllers)

vicalro added 17 commits July 2, 2015 14:52
@vicalro
Add controller from rest plugin implemented
9bcb4e1
@vicalro
Basic remove controller functionality for REST interface
f36b843
@vicalro
implementation of list in xcli and in endpoint. It is still not glued…
88efc5d 
… in the REST plugin
@vicalro
Implemented list IDs from LSI
978ca72
@vicalro
Implemented ID assigned when adding a new controller
00f592c
@vicalro
Added warnings on add and remove controller malformed commands
16a2f39
@vicalro
Implementation of REST call show controller
ad421d6
@vicalro
Added examples exec add/remove controller to xcli
d46f6f6
@vicalro
Changed the structure of the information of the controllers for bette…
1ec0b91 
…r parsing
@vicalro
Erased "rpc" from calls to the switch manager. Using only read lock w…
9f334e9 
…hen gathering info of controllers
@vicalro
Small fix in label of controller information
63f7247
@vicalro
Added Equal as a role for controllers and using get_role() in crofbas…
f7a42b0 
…e to get the role
@vicalro
Cleaning old commnents
1902f12
@vicalro
Merge branch 'integration-0.8' into controllers_management
73be2e2
@vicalro
Following previuos order in register of REST calls
d4ee5e7
@vicalro
Changed the way we handle errors in management of controllers with REST
5448b49
@vicalro
Removed unnecessary spaces
302573f
@msune msune added this to the v0.8.0 milestone Jul 28, 2015
@msune
Copy link
Contributor

msune commented Jul 29, 2015

List controllers and show controller info should not need -m option:

xcli(local)>show lsi dp0 controllers
ERROR(401): HTTP/GET 'http://127.0.0.1:5757/info/lsi/dp0/controllers'
Unauthorised; is mgmt support enabled (-m)?

xcli(local)>show lsi dp0 controller 0
ERROR(401): HTTP/GET 'http://127.0.0.1:5757/info/lsi/dp0/controller/0'
Unauthorised; is mgmt support enabled (-m)?

xcli(local)>

@msune
Copy link
Contributor

msune commented Jul 29, 2015 

xcli(local)>show lsi dp0 controller 0
{
    "Controller" : {
        "id" : 0,
        "channel-status" : "not-established",
        "mode" : "equal",
        "connections" : {
            "0" : {
                "protocol-type" : "plain",
                "ip" : "127.0.0.1",
                "port" : 6653
            }
        }
    }
}

xcli(local)>

"Controller" -> "controller"

And:

xcli(local)>exec add controller dp0 tcp:192.168.1.1:6633
{                                                        
    "assigned id" : 1                                    
}

I think for consistency (list controllers) should be:

xcli(local)>exec add controller dp0 tcp:192.168.1.1:6633
{                                                        
    "controller-id" : 1
}

@msune
Copy link
Contributor

msune commented Jul 29, 2015

Exec with bad parameters dos not send an HTTP 500 error, although it fails to add it:

xcli(local)>exec add controller dp0 tcp:192.168.1.1::6633

xcli(local)>show lsi dp0 controllers
{
    "controller-ids" : [
    ]
}

xcli(local)>exec add controller dp0 tcp192.168.1.1::6633

xcli(local)>show lsi dp0 controllers 
{
    "controller-ids" : [
    ]
}

xcli(local)>exec add controller dp0 tcp45654654192.168.1.1::6633

xcli(local)>show lsi dp0 controllers
{
    "controller-ids" : [
    ]
}

@msune
Copy link
Contributor

msune commented Jul 29, 2015

New APIs are not listed in the index (GET to root):

e.g. http://10.0.0.2:5757/

msune
msune reviewed Jul 29, 2015
View reviewed changes
src/xdpd/management/plugins/rest/sw-controllers.cc
json_spirit::Object table;

//Perform security checks
if(!authorised(req,rep)) return;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extra space. Authorisization here is not needed

msune
msune reviewed Jul 29, 2015
View reviewed changes
src/xdpd/management/plugins/rest/sw-controllers.cc
std::string lsi_name = std::string(grps[1]);

//Check if LSI exists;
if(!switch_manager::exists_by_name(lsi_name)){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

space

@vicalro
Polish of REST management of controllers
2f5295d 
- List and show controllers don't require authorized access
- Fixed return message after adding a new controller
- Capturing exception when parsing the parameters of the new controller in xcli
- New APIs listed in the index
@vicalro
Copy link
Contributor Author

vicalro commented Jul 29, 2015

Thanks for the comments. Issues fixed. Check it out!

@msune
Copy link
Contributor

msune commented Jul 29, 2015

ACK. But we need to merge the list_controllers into rofl-common, but first bisdn/rofl-common#17 needs to be fixed, since the commit needs to go on top of integration-0.6 as agreed.

Holding it.

@msune msune mentioned this pull request Aug 17, 2016
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@vicalro vicalro

Labels
cmm feature
Projects
None yet
Milestone
v0.8.0
Development

Successfully merging this pull request may close these issues.
2 participants
@vicalro @msune