You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Chip Childers <ch...@sungard.com> on 2013/10/08 16:23:32 UTC

Contrail plugin

As stated, I've imported the contrail plugin donation into the contrail
branch.

I've taken the time to add the ASF license header to all of the new files
in that branch.  I think we have to complete the following in order to
merge into master.

1) I'd like to see the package structure changed to match
org.apache.cloudstack, instead of the Juniper namespace.  We only have
com.cloud namespaces for legacy reasons, and are trying to consolidate into
the apache ns.

2) Folks with past experience with network plugins need to review the
plugin's code and provide comments or +1s for a merge.  Chiradeep and Hugo,
you've been "randomly" selected to help on this...  ;-)  Pedro, I'll assume
that you will be happy to provide patches via reviewboard against this
branch if changes are requested (including the package structure noted
above).

3) I'd love if we could get some consensus on what additional tests and /
or changes to the test approach are needed.  Prasanna - as with Hugo and
Chiradeep, you've been "randomly" selected to at least provide some input
here.

Anything I'm missing?

-chip

Re: Contrail plugin

Posted by Pedro Roque Marques <pe...@gmail.com>.
On Oct 8, 2013, at 7:36 AM, Chip Childers wrote:

> On Tue, Oct 08, 2013 at 07:59:24PM +0530, Prasanna Santhanam wrote:
>> On Tue, Oct 08, 2013 at 10:23:32AM -0400, Chip Childers wrote:
>>> 3) I'd love if we could get some consensus on what additional tests and /
>>> or changes to the test approach are needed.  Prasanna - as with Hugo and
>>> Chiradeep, you've been "randomly" selected to at least provide some input
>>> here.
>> 
>> I saw the thread earlier about a mysql db generated for performing an
>> integration test. If someone can point me to the spec/docs/readme on
>> how to run these presumably without the contrail device I'm happy to
>> take a look.

The integration tests are being automatically executed by maven.
(mvn -pl :cloud-plugin-network-contrail clean test).

They spawn an instance of mysql on a dynamically allocated port in order to ensure that the database contents are always initialized with the
same content and that these set of tests do not leave content in the database that could influence other tests...

> 
> Pedro, perhaps you can provide some guidance here.
> 
> Prasanna - it appears that most of the stuff is in
> plugins/network-elements/juniper-contrail/test/
> 
correct.

> There are mysql start and stop .sh scripts, and unit tests in there as
> well.
> 

Yes. the shell scripts start and stop a mysql instance. The scripts are invoked from JUnit static initializer (@BeforeClass).

> -chip


Re: Contrail plugin

Posted by Chip Childers <ch...@sungard.com>.
On Tue, Oct 08, 2013 at 07:59:24PM +0530, Prasanna Santhanam wrote:
> On Tue, Oct 08, 2013 at 10:23:32AM -0400, Chip Childers wrote:
> > 3) I'd love if we could get some consensus on what additional tests and /
> > or changes to the test approach are needed.  Prasanna - as with Hugo and
> > Chiradeep, you've been "randomly" selected to at least provide some input
> > here.
> 
> I saw the thread earlier about a mysql db generated for performing an
> integration test. If someone can point me to the spec/docs/readme on
> how to run these presumably without the contrail device I'm happy to
> take a look.

Pedro, perhaps you can provide some guidance here.

Prasanna - it appears that most of the stuff is in
plugins/network-elements/juniper-contrail/test/

There are mysql start and stop .sh scripts, and unit tests in there as
well.

-chip

Re: Contrail plugin

Posted by Prasanna Santhanam <ts...@apache.org>.
On Tue, Oct 08, 2013 at 10:23:32AM -0400, Chip Childers wrote:
> As stated, I've imported the contrail plugin donation into the contrail
> branch.
> 
> I've taken the time to add the ASF license header to all of the new files
> in that branch.  I think we have to complete the following in order to
> merge into master.
> 
> 1) I'd like to see the package structure changed to match
> org.apache.cloudstack, instead of the Juniper namespace.  We only have
> com.cloud namespaces for legacy reasons, and are trying to consolidate into
> the apache ns.
> 
> 2) Folks with past experience with network plugins need to review the
> plugin's code and provide comments or +1s for a merge.  Chiradeep and Hugo,
> you've been "randomly" selected to help on this...  ;-)  Pedro, I'll assume
> that you will be happy to provide patches via reviewboard against this
> branch if changes are requested (including the package structure noted
> above).
> 
> 3) I'd love if we could get some consensus on what additional tests and /
> or changes to the test approach are needed.  Prasanna - as with Hugo and
> Chiradeep, you've been "randomly" selected to at least provide some input
> here.

I saw the thread earlier about a mysql db generated for performing an
integration test. If someone can point me to the spec/docs/readme on
how to run these presumably without the contrail device I'm happy to
take a look.

> 
> Anything I'm missing?
> 
> -chip

-- 
Prasanna.,

------------------------
Powered by BigRock.com


Re: Contrail plugin

Posted by Pedro Roque Marques <pe...@gmail.com>.
Darren,

On Oct 9, 2013, at 8:35 AM, Darren Shepherd wrote:

> Pedro,
> 
> I completely understand what you are saying as I think this is a gap
> too I would like to address.  I'm thinking of something a little
> bigger and grander than what you would need right now so that's not
> helpful, as I won't get around to doing anything for a couple months.
> I do not think EventBus is the correct approach.  That is complete
> overkill for what you need.  I hate to tell people that in order to
> run contrail they need to setup a HA instance of rabbitmq.  That is a
> lot of headache.
> 
> I was thinking maybe a small listener framework on API commands would
> suffice, but that leads me to my next concern.  I'm a stickler on
> reliability.  So currently with ACS AOP approach, or some listener
> framework, you can't achieve complete reliability.  99% of the time it
> will work, but since there is no way to tie your code to the
> transaction of the API code, there is a small window if the JVM dies,
> you won't be called.  I know this sounds nit picky, but I hate when
> there is a situation that could happen that there is no recovery from.

The contrail plugin can resynchronize itself on failure. It assumes that the API connection between the management server and the contrail-api server can have transient failures... when that API connection comes up the code synchronizes the databases. 2/3 of the code in the plugin is actually to perform this task: being able to deal with transient failures.

> 
> Is it possible for the contrail plugin to on-demand register the
> account/projects?

Currently we do that via the ActionEvent mechanism but also assume the possibility of transient failure / timing issues.

>  So only when the element/guru/whatever is called,
> you try to sync up the two systems?

The plugin re-sync when the API session is established and it also runs a periodic check between the databases in order to detect  any synchronization failure... which typically implies a bug on the plugin side.
The model is one where the CloudStack DB is consider the master and the Contrail API is updated with the contents that are present in the CloudStack DB.

> 
> Darren


Re: Contrail plugin

Posted by Darren Shepherd <da...@gmail.com>.
Pedro,

I completely understand what you are saying as I think this is a gap
too I would like to address.  I'm thinking of something a little
bigger and grander than what you would need right now so that's not
helpful, as I won't get around to doing anything for a couple months.
I do not think EventBus is the correct approach.  That is complete
overkill for what you need.  I hate to tell people that in order to
run contrail they need to setup a HA instance of rabbitmq.  That is a
lot of headache.

I was thinking maybe a small listener framework on API commands would
suffice, but that leads me to my next concern.  I'm a stickler on
reliability.  So currently with ACS AOP approach, or some listener
framework, you can't achieve complete reliability.  99% of the time it
will work, but since there is no way to tie your code to the
transaction of the API code, there is a small window if the JVM dies,
you won't be called.  I know this sounds nit picky, but I hate when
there is a situation that could happen that there is no recovery from.

Is it possible for the contrail plugin to on-demand register the
account/projects?  So only when the element/guru/whatever is called,
you try to sync up the two systems?

Darren

Re: Contrail plugin

Posted by Murali Reddy <Mu...@citrix.com>.
On 09/10/13 6:10 AM, "Pedro Roque Marques" <pe...@gmail.com>
wrote:

>Darren,
>Using ActionEvents is not desirable for the plugin either... today
>CloudStack lacks the ability for a component/plugin to associate itself
>to the life-cycle of an object. It would be ideal if there was a generic
>way to accomplish that...
>The contrail plugin wants to know about project creation and deletion.
>Projects need to be reflected in the contrail-api server; the project
>delete notification is necessary to understand that the project is not
>longer used.
>
>When it comes to network objects it would also be nice if there could a
>for a component/plugin to associate itself with network creation /
>implementation... Currently there are calls from the NetworkManager to
>components such as firewall/LB/etc which should be optional.
>
>Ideally, i would like to have a common notification mechanism for a
>cloudstack object (e.g. project, network, nic). This could optionally
>allow a plugin to "veto" an operation and/or just get notified that it
>occurred.

For 'veto' to operations, yes you are right that CloudStack core does not
give hooks into workflow for plug-ins to veto. But there are defined
abstraction (Guru, Planner, Investigator etc.) through which plug-ins can
hook into orchestration. Also there is a publisher/subscriber modelled
EventBus that was added in 4.1 by which CloudStack components can get the
entity life cycle and state changes. I see that contrail plug-in
implements an Interceptor for achieving this, but I will leave a comment
in the review if 'EventBus' can leveraged.

Thanks,
Murali

>
>thanks,
>  Pedro.
>
>On Oct 8, 2013, at 10:25 AM, Darren Shepherd wrote:
>
>> I'll take some time and review this code too.  I already know there's
>> going to be a conflict with the stuff I did in the spring
>> modularization branch.  Moving to full spring we have gotten rid of
>> the custom ACS AOP for the mgmt server.  This code relies on that
>> framework so it will have to move to being a standard
>> org.aopalliance.intercept.MethodInteceptor.  I don't particularly care
>> for the fact that functionally it being keyed off of ActionEvents (or
>> AOP in general).  I'll need to review the code further to provide more
>> useful feedback, but just giving the heads up that the AOP stuff will
>> have to change a bit.
>> 
>> Darren
>
>



Re: Contrail plugin

Posted by Pedro Roque Marques <pe...@gmail.com>.
Darren,
Using ActionEvents is not desirable for the plugin either... today CloudStack lacks the ability for a component/plugin to associate itself to the life-cycle of an object. It would be ideal if there was a generic way to accomplish that...
The contrail plugin wants to know about project creation and deletion. Projects need to be reflected in the contrail-api server; the project delete notification is necessary to understand that the project is not longer used.

When it comes to network objects it would also be nice if there could a for a component/plugin to associate itself with network creation / implementation... Currently there are calls from the NetworkManager to components such as firewall/LB/etc which should be optional.

Ideally, i would like to have a common notification mechanism for a cloudstack object (e.g. project, network, nic). This could optionally allow a plugin to "veto" an operation and/or just get notified that it occurred.

thanks,
  Pedro.

On Oct 8, 2013, at 10:25 AM, Darren Shepherd wrote:

> I'll take some time and review this code too.  I already know there's
> going to be a conflict with the stuff I did in the spring
> modularization branch.  Moving to full spring we have gotten rid of
> the custom ACS AOP for the mgmt server.  This code relies on that
> framework so it will have to move to being a standard
> org.aopalliance.intercept.MethodInteceptor.  I don't particularly care
> for the fact that functionally it being keyed off of ActionEvents (or
> AOP in general).  I'll need to review the code further to provide more
> useful feedback, but just giving the heads up that the AOP stuff will
> have to change a bit.
> 
> Darren


Re: Contrail plugin

Posted by Darren Shepherd <da...@gmail.com>.
I'll take some time and review this code too.  I already know there's
going to be a conflict with the stuff I did in the spring
modularization branch.  Moving to full spring we have gotten rid of
the custom ACS AOP for the mgmt server.  This code relies on that
framework so it will have to move to being a standard
org.aopalliance.intercept.MethodInteceptor.  I don't particularly care
for the fact that functionally it being keyed off of ActionEvents (or
AOP in general).  I'll need to review the code further to provide more
useful feedback, but just giving the heads up that the AOP stuff will
have to change a bit.

Darren

Re: Contrail plugin

Posted by Chip Childers <ch...@sungard.com>.
On Tue, Oct 08, 2013 at 09:43:39AM -0700, Pedro Roque Marques wrote:
> Chip,
> 
> On Oct 8, 2013, at 7:23 AM, Chip Childers wrote:
> 
> > As stated, I've imported the contrail plugin donation into the contrail branch.
> > 
> > I've taken the time to add the ASF license header to all of the new files in that branch.  I think we have to complete the following in order to merge into master.
> > 
> > 1) I'd like to see the package structure changed to match org.apache.cloudstack, instead of the Juniper namespace.  We only have com.cloud namespaces for legacy reasons, and are trying to consolidate into the apache ns.
> 
> Will do.

Fantastic, thanks!

> 
> > 
> > 2) Folks with past experience with network plugins need to review the plugin's code and provide comments or +1s for a merge.  Chiradeep and Hugo, you've been "randomly" selected to help on this...  ;-)  Pedro, I'll assume that you will be happy to provide patches via reviewboard against this branch if changes are requested (including the package structure noted above).
> 
> Yes. There is a team of us at Juniper that will be working on the contrail plugin. We will be more than happy to follow the structure that you recommend.

Hopefully we can get other's to provide feedback on the plugin code
itself...  I think that would be useful for all involved.

> 
> > 
> > 3) I'd love if we could get some consensus on what additional tests and / or changes to the test approach are needed.  Prasanna - as with Hugo and Chiradeep, you've been "randomly" selected to at least provide some input here.
> 
> Our plan at the moment is to:
>  - Add unit tests to cover all the <Object>Model classes.
>  - Create additional integration tests to cover the plugin integration with the CloudStack NetworkManager. At the moment we are struggling a bit with the changes from 4.1 (where we did most of the development) to 4.2 to 4.3...
> 

That sounds like a great plan.  One request - please be sure that the
individuals doing the work are submitting their patches to the project
as individuals.  That'll eliminate any issues with going through the
clearance process, and give them the rightful credit for their
contributions.

> 
>   Pedro.
> 
> 

Thanks Pedro!

-chip

Re: Contrail plugin

Posted by Pedro Roque Marques <pe...@gmail.com>.
Chip,

On Oct 8, 2013, at 7:23 AM, Chip Childers wrote:

> As stated, I've imported the contrail plugin donation into the contrail branch.
> 
> I've taken the time to add the ASF license header to all of the new files in that branch.  I think we have to complete the following in order to merge into master.
> 
> 1) I'd like to see the package structure changed to match org.apache.cloudstack, instead of the Juniper namespace.  We only have com.cloud namespaces for legacy reasons, and are trying to consolidate into the apache ns.

Will do.

> 
> 2) Folks with past experience with network plugins need to review the plugin's code and provide comments or +1s for a merge.  Chiradeep and Hugo, you've been "randomly" selected to help on this...  ;-)  Pedro, I'll assume that you will be happy to provide patches via reviewboard against this branch if changes are requested (including the package structure noted above).

Yes. There is a team of us at Juniper that will be working on the contrail plugin. We will be more than happy to follow the structure that you recommend.

> 
> 3) I'd love if we could get some consensus on what additional tests and / or changes to the test approach are needed.  Prasanna - as with Hugo and Chiradeep, you've been "randomly" selected to at least provide some input here.

Our plan at the moment is to:
 - Add unit tests to cover all the <Object>Model classes.
 - Create additional integration tests to cover the plugin integration with the CloudStack NetworkManager. At the moment we are struggling a bit with the changes from 4.1 (where we did most of the development) to 4.2 to 4.3...


  Pedro.