You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Sheng Yang <sh...@yasker.org> on 2013/06/19 20:00:33 UTC
[Review Request] Re-enabling baremetal on master
Hi,
I've created the https://reviews.apache.org/r/11977/ for review. The
branch re-enabled the baremetal for master. And all major bugs are cleaned.
https://issues.apache.org/jira/browse/CLOUDSTACK-1610
https://issues.apache.org/jira/browse/CLOUDSTACK-1618
https://issues.apache.org/jira/browse/CLOUDSTACK-1614
https://issues.apache.org/jira/browse/CLOUDSTACK-1440
In fact it's not a feature merge, because the code is already in MASTER
ready. We just disable it due to stability problem of 4.1 release. Now I've
tried to enable it, and the changeset is very small, mostly just revert the
old disabling baremetal codes, and fix some issues with introducing other
new features. Here is the summary:
client/pom.xml
| 5 +++++
client/tomcatconf/applicationContext.xml.in
| 4 ----
client/tomcatconf/componentContext.xml.in
| 10 ----------
client/tomcatconf/nonossComponentContext.xml.in
| 8 --------
plugins/hypervisors/baremetal/src/com/cloud/baremetal/manager/BareMetalDiscoverer.java
| 3 ++-
plugins/hypervisors/baremetal/src/com/cloud/baremetal/manager/BareMetalTemplateAdapter.java
| 6 ------
plugins/hypervisors/baremetal/src/com/cloud/baremetal/networkservice/BaremetalDhcpElement.java
| 6 ++++--
plugins/network-elements/dns-notifier/resources/components-example.xml
| 8 --------
server/src/com/cloud/configuration/Config.java
| 2 +-
server/src/com/cloud/network/NetworkServiceImpl.java
| 9 +++------
setup/db/db/schema-410to420.sql
| 16 ++++++++++++++++
11 files changed, 31 insertions(+), 46 deletions(-)
Alena has pointed out db upgrade issue in my previous post and I've fixed
it.
Thanks!
--Sheng
Re: [Review Request] Re-enabling baremetal on master
Posted by Sheng Yang <sh...@yasker.org>.
Thank you all!
I would commit the change to MASTER soon.
--Sheng
On Thu, Jun 20, 2013 at 7:22 PM, David Nalley <da...@gnsa.us> wrote:
> Yes
> Happy to +1. Sheng, thanks for stepping up and getting this done.
>
> --David
> On Jun 20, 2013 7:19 PM, "Chip Childers" <ch...@sungard.com>
> wrote:
>
> > Nice! I'm glad the feature has the benefit of tests now. Thanks for
> > doing this Sheng!
> >
> > David - are you comfortable with this, and will you now +1 the feature?
> >
> > On Thu, Jun 20, 2013 at 9:55 PM, Sheng Yang <sh...@yasker.org> wrote:
> > > Hi,
> > >
> > > I've updated baremetal-4.2 branch, added integration test for some of
> > > baremetal related APIs, also fixed a bunch of baremetal API issues
> > exposed
> > > by the testing.
> > >
> > > Thanks!
> > >
> > > --Sheng
> > >
> > >
> > >
> > >
> > > On Wed, Jun 19, 2013 at 11:41 AM, Chip Childers
> > > <ch...@sungard.com>wrote:
> > >
> > >> On Wed, Jun 19, 2013 at 11:36:11AM -0700, Sheng Yang wrote:
> > >> > On Wed, Jun 19, 2013 at 11:24 AM, Chip Childers
> > >> > <ch...@sungard.com>wrote:
> > >> >
> > >> > > On Wed, Jun 19, 2013 at 11:00:33AM -0700, Sheng Yang wrote:
> > >> > > > Hi,
> > >> > > >
> > >> > > > I've created the https://reviews.apache.org/r/11977/ for
> review.
> > >> The
> > >> > > > branch re-enabled the baremetal for master. And all major bugs
> are
> > >> > > cleaned.
> > >> > > >
> > >> > > > https://issues.apache.org/jira/browse/CLOUDSTACK-1610
> > >> > > > https://issues.apache.org/jira/browse/CLOUDSTACK-1618
> > >> > > > https://issues.apache.org/jira/browse/CLOUDSTACK-1614
> > >> > > > https://issues.apache.org/jira/browse/CLOUDSTACK-1440
> > >> > > >
> > >> > > > In fact it's not a feature merge, because the code is already in
> > >> MASTER
> > >> > > > ready. We just disable it due to stability problem of 4.1
> release.
> > >> Now
> > >> > > I've
> > >> > > > tried to enable it, and the changeset is very small, mostly just
> > >> revert
> > >> > > the
> > >> > > > old disabling baremetal codes, and fix some issues with
> > introducing
> > >> other
> > >> > > > new features. Here is the summary:
> > >> > >
> > >> > > [snip]
> > >> > >
> > >> > > So David's standing veto was because of this comment (from him):
> > >> > >
> > >> > > "Baremetal seems to be suffering from a significant lack of unit
> > tests
> > >> > > and integration tests for marvin to consume. Let's get those in
> > place
> > >> > > before we consider re-enabling this."
> > >> > >
> > >> > > If I remember correctly, the reason that master has the code in
> it,
> > is
> > >> > > specifically because we decided that disabling the feature was
> > easier
> > >> to
> > >> > > honor the veto than reverting all of the changes.
> > >> > >
> > >> > > That being said, have we addressed the original veto's concerns?
> > >> > >
> > >> >
> > >> > Not yet. I didn't realize it's vetoed due to this. Let me see what
> > can I
> > >> do
> > >> > about it.
> > >>
> > >> Awesome. Thanks Sheng!
> > >>
> > >> >
> > >> > In fact the above bugs cannot be detected for unit test or marvin
> > test(I
> > >> > even not sure if they're valid bugs or not, but at that time Frank
> is
> > on
> > >> > vacation and nobody took a look at these then decided disable the
> > >> feature,
> > >> > and after I re-enabled them, everything works fine for me).
> > >>
> > >> Yeah, I think that the bugs were just in need of triage. The bugs
> > >> themselves weren't the major issue (although they were concerning), as
> > >> much as test coverage at either (or both) unit or integration levels.
> > >>
> > >> -chip
> > >>
> >
>
Re: [Review Request] Re-enabling baremetal on master
Posted by David Nalley <da...@gnsa.us>.
Yes
Happy to +1. Sheng, thanks for stepping up and getting this done.
--David
On Jun 20, 2013 7:19 PM, "Chip Childers" <ch...@sungard.com> wrote:
> Nice! I'm glad the feature has the benefit of tests now. Thanks for
> doing this Sheng!
>
> David - are you comfortable with this, and will you now +1 the feature?
>
> On Thu, Jun 20, 2013 at 9:55 PM, Sheng Yang <sh...@yasker.org> wrote:
> > Hi,
> >
> > I've updated baremetal-4.2 branch, added integration test for some of
> > baremetal related APIs, also fixed a bunch of baremetal API issues
> exposed
> > by the testing.
> >
> > Thanks!
> >
> > --Sheng
> >
> >
> >
> >
> > On Wed, Jun 19, 2013 at 11:41 AM, Chip Childers
> > <ch...@sungard.com>wrote:
> >
> >> On Wed, Jun 19, 2013 at 11:36:11AM -0700, Sheng Yang wrote:
> >> > On Wed, Jun 19, 2013 at 11:24 AM, Chip Childers
> >> > <ch...@sungard.com>wrote:
> >> >
> >> > > On Wed, Jun 19, 2013 at 11:00:33AM -0700, Sheng Yang wrote:
> >> > > > Hi,
> >> > > >
> >> > > > I've created the https://reviews.apache.org/r/11977/ for review.
> >> The
> >> > > > branch re-enabled the baremetal for master. And all major bugs are
> >> > > cleaned.
> >> > > >
> >> > > > https://issues.apache.org/jira/browse/CLOUDSTACK-1610
> >> > > > https://issues.apache.org/jira/browse/CLOUDSTACK-1618
> >> > > > https://issues.apache.org/jira/browse/CLOUDSTACK-1614
> >> > > > https://issues.apache.org/jira/browse/CLOUDSTACK-1440
> >> > > >
> >> > > > In fact it's not a feature merge, because the code is already in
> >> MASTER
> >> > > > ready. We just disable it due to stability problem of 4.1 release.
> >> Now
> >> > > I've
> >> > > > tried to enable it, and the changeset is very small, mostly just
> >> revert
> >> > > the
> >> > > > old disabling baremetal codes, and fix some issues with
> introducing
> >> other
> >> > > > new features. Here is the summary:
> >> > >
> >> > > [snip]
> >> > >
> >> > > So David's standing veto was because of this comment (from him):
> >> > >
> >> > > "Baremetal seems to be suffering from a significant lack of unit
> tests
> >> > > and integration tests for marvin to consume. Let's get those in
> place
> >> > > before we consider re-enabling this."
> >> > >
> >> > > If I remember correctly, the reason that master has the code in it,
> is
> >> > > specifically because we decided that disabling the feature was
> easier
> >> to
> >> > > honor the veto than reverting all of the changes.
> >> > >
> >> > > That being said, have we addressed the original veto's concerns?
> >> > >
> >> >
> >> > Not yet. I didn't realize it's vetoed due to this. Let me see what
> can I
> >> do
> >> > about it.
> >>
> >> Awesome. Thanks Sheng!
> >>
> >> >
> >> > In fact the above bugs cannot be detected for unit test or marvin
> test(I
> >> > even not sure if they're valid bugs or not, but at that time Frank is
> on
> >> > vacation and nobody took a look at these then decided disable the
> >> feature,
> >> > and after I re-enabled them, everything works fine for me).
> >>
> >> Yeah, I think that the bugs were just in need of triage. The bugs
> >> themselves weren't the major issue (although they were concerning), as
> >> much as test coverage at either (or both) unit or integration levels.
> >>
> >> -chip
> >>
>
Re: [Review Request] Re-enabling baremetal on master
Posted by Chip Childers <ch...@sungard.com>.
Nice! I'm glad the feature has the benefit of tests now. Thanks for
doing this Sheng!
David - are you comfortable with this, and will you now +1 the feature?
On Thu, Jun 20, 2013 at 9:55 PM, Sheng Yang <sh...@yasker.org> wrote:
> Hi,
>
> I've updated baremetal-4.2 branch, added integration test for some of
> baremetal related APIs, also fixed a bunch of baremetal API issues exposed
> by the testing.
>
> Thanks!
>
> --Sheng
>
>
>
>
> On Wed, Jun 19, 2013 at 11:41 AM, Chip Childers
> <ch...@sungard.com>wrote:
>
>> On Wed, Jun 19, 2013 at 11:36:11AM -0700, Sheng Yang wrote:
>> > On Wed, Jun 19, 2013 at 11:24 AM, Chip Childers
>> > <ch...@sungard.com>wrote:
>> >
>> > > On Wed, Jun 19, 2013 at 11:00:33AM -0700, Sheng Yang wrote:
>> > > > Hi,
>> > > >
>> > > > I've created the https://reviews.apache.org/r/11977/ for review.
>> The
>> > > > branch re-enabled the baremetal for master. And all major bugs are
>> > > cleaned.
>> > > >
>> > > > https://issues.apache.org/jira/browse/CLOUDSTACK-1610
>> > > > https://issues.apache.org/jira/browse/CLOUDSTACK-1618
>> > > > https://issues.apache.org/jira/browse/CLOUDSTACK-1614
>> > > > https://issues.apache.org/jira/browse/CLOUDSTACK-1440
>> > > >
>> > > > In fact it's not a feature merge, because the code is already in
>> MASTER
>> > > > ready. We just disable it due to stability problem of 4.1 release.
>> Now
>> > > I've
>> > > > tried to enable it, and the changeset is very small, mostly just
>> revert
>> > > the
>> > > > old disabling baremetal codes, and fix some issues with introducing
>> other
>> > > > new features. Here is the summary:
>> > >
>> > > [snip]
>> > >
>> > > So David's standing veto was because of this comment (from him):
>> > >
>> > > "Baremetal seems to be suffering from a significant lack of unit tests
>> > > and integration tests for marvin to consume. Let's get those in place
>> > > before we consider re-enabling this."
>> > >
>> > > If I remember correctly, the reason that master has the code in it, is
>> > > specifically because we decided that disabling the feature was easier
>> to
>> > > honor the veto than reverting all of the changes.
>> > >
>> > > That being said, have we addressed the original veto's concerns?
>> > >
>> >
>> > Not yet. I didn't realize it's vetoed due to this. Let me see what can I
>> do
>> > about it.
>>
>> Awesome. Thanks Sheng!
>>
>> >
>> > In fact the above bugs cannot be detected for unit test or marvin test(I
>> > even not sure if they're valid bugs or not, but at that time Frank is on
>> > vacation and nobody took a look at these then decided disable the
>> feature,
>> > and after I re-enabled them, everything works fine for me).
>>
>> Yeah, I think that the bugs were just in need of triage. The bugs
>> themselves weren't the major issue (although they were concerning), as
>> much as test coverage at either (or both) unit or integration levels.
>>
>> -chip
>>
Re: [Review Request] Re-enabling baremetal on master
Posted by Sheng Yang <sh...@yasker.org>.
Hi,
I've updated baremetal-4.2 branch, added integration test for some of
baremetal related APIs, also fixed a bunch of baremetal API issues exposed
by the testing.
Thanks!
--Sheng
On Wed, Jun 19, 2013 at 11:41 AM, Chip Childers
<ch...@sungard.com>wrote:
> On Wed, Jun 19, 2013 at 11:36:11AM -0700, Sheng Yang wrote:
> > On Wed, Jun 19, 2013 at 11:24 AM, Chip Childers
> > <ch...@sungard.com>wrote:
> >
> > > On Wed, Jun 19, 2013 at 11:00:33AM -0700, Sheng Yang wrote:
> > > > Hi,
> > > >
> > > > I've created the https://reviews.apache.org/r/11977/ for review.
> The
> > > > branch re-enabled the baremetal for master. And all major bugs are
> > > cleaned.
> > > >
> > > > https://issues.apache.org/jira/browse/CLOUDSTACK-1610
> > > > https://issues.apache.org/jira/browse/CLOUDSTACK-1618
> > > > https://issues.apache.org/jira/browse/CLOUDSTACK-1614
> > > > https://issues.apache.org/jira/browse/CLOUDSTACK-1440
> > > >
> > > > In fact it's not a feature merge, because the code is already in
> MASTER
> > > > ready. We just disable it due to stability problem of 4.1 release.
> Now
> > > I've
> > > > tried to enable it, and the changeset is very small, mostly just
> revert
> > > the
> > > > old disabling baremetal codes, and fix some issues with introducing
> other
> > > > new features. Here is the summary:
> > >
> > > [snip]
> > >
> > > So David's standing veto was because of this comment (from him):
> > >
> > > "Baremetal seems to be suffering from a significant lack of unit tests
> > > and integration tests for marvin to consume. Let's get those in place
> > > before we consider re-enabling this."
> > >
> > > If I remember correctly, the reason that master has the code in it, is
> > > specifically because we decided that disabling the feature was easier
> to
> > > honor the veto than reverting all of the changes.
> > >
> > > That being said, have we addressed the original veto's concerns?
> > >
> >
> > Not yet. I didn't realize it's vetoed due to this. Let me see what can I
> do
> > about it.
>
> Awesome. Thanks Sheng!
>
> >
> > In fact the above bugs cannot be detected for unit test or marvin test(I
> > even not sure if they're valid bugs or not, but at that time Frank is on
> > vacation and nobody took a look at these then decided disable the
> feature,
> > and after I re-enabled them, everything works fine for me).
>
> Yeah, I think that the bugs were just in need of triage. The bugs
> themselves weren't the major issue (although they were concerning), as
> much as test coverage at either (or both) unit or integration levels.
>
> -chip
>
Re: [Review Request] Re-enabling baremetal on master
Posted by Chip Childers <ch...@sungard.com>.
On Wed, Jun 19, 2013 at 11:36:11AM -0700, Sheng Yang wrote:
> On Wed, Jun 19, 2013 at 11:24 AM, Chip Childers
> <ch...@sungard.com>wrote:
>
> > On Wed, Jun 19, 2013 at 11:00:33AM -0700, Sheng Yang wrote:
> > > Hi,
> > >
> > > I've created the https://reviews.apache.org/r/11977/ for review. The
> > > branch re-enabled the baremetal for master. And all major bugs are
> > cleaned.
> > >
> > > https://issues.apache.org/jira/browse/CLOUDSTACK-1610
> > > https://issues.apache.org/jira/browse/CLOUDSTACK-1618
> > > https://issues.apache.org/jira/browse/CLOUDSTACK-1614
> > > https://issues.apache.org/jira/browse/CLOUDSTACK-1440
> > >
> > > In fact it's not a feature merge, because the code is already in MASTER
> > > ready. We just disable it due to stability problem of 4.1 release. Now
> > I've
> > > tried to enable it, and the changeset is very small, mostly just revert
> > the
> > > old disabling baremetal codes, and fix some issues with introducing other
> > > new features. Here is the summary:
> >
> > [snip]
> >
> > So David's standing veto was because of this comment (from him):
> >
> > "Baremetal seems to be suffering from a significant lack of unit tests
> > and integration tests for marvin to consume. Let's get those in place
> > before we consider re-enabling this."
> >
> > If I remember correctly, the reason that master has the code in it, is
> > specifically because we decided that disabling the feature was easier to
> > honor the veto than reverting all of the changes.
> >
> > That being said, have we addressed the original veto's concerns?
> >
>
> Not yet. I didn't realize it's vetoed due to this. Let me see what can I do
> about it.
Awesome. Thanks Sheng!
>
> In fact the above bugs cannot be detected for unit test or marvin test(I
> even not sure if they're valid bugs or not, but at that time Frank is on
> vacation and nobody took a look at these then decided disable the feature,
> and after I re-enabled them, everything works fine for me).
Yeah, I think that the bugs were just in need of triage. The bugs
themselves weren't the major issue (although they were concerning), as
much as test coverage at either (or both) unit or integration levels.
-chip
Re: [Review Request] Re-enabling baremetal on master
Posted by Sheng Yang <sh...@yasker.org>.
On Wed, Jun 19, 2013 at 11:24 AM, Chip Childers
<ch...@sungard.com>wrote:
> On Wed, Jun 19, 2013 at 11:00:33AM -0700, Sheng Yang wrote:
> > Hi,
> >
> > I've created the https://reviews.apache.org/r/11977/ for review. The
> > branch re-enabled the baremetal for master. And all major bugs are
> cleaned.
> >
> > https://issues.apache.org/jira/browse/CLOUDSTACK-1610
> > https://issues.apache.org/jira/browse/CLOUDSTACK-1618
> > https://issues.apache.org/jira/browse/CLOUDSTACK-1614
> > https://issues.apache.org/jira/browse/CLOUDSTACK-1440
> >
> > In fact it's not a feature merge, because the code is already in MASTER
> > ready. We just disable it due to stability problem of 4.1 release. Now
> I've
> > tried to enable it, and the changeset is very small, mostly just revert
> the
> > old disabling baremetal codes, and fix some issues with introducing other
> > new features. Here is the summary:
>
> [snip]
>
> So David's standing veto was because of this comment (from him):
>
> "Baremetal seems to be suffering from a significant lack of unit tests
> and integration tests for marvin to consume. Let's get those in place
> before we consider re-enabling this."
>
> If I remember correctly, the reason that master has the code in it, is
> specifically because we decided that disabling the feature was easier to
> honor the veto than reverting all of the changes.
>
> That being said, have we addressed the original veto's concerns?
>
Not yet. I didn't realize it's vetoed due to this. Let me see what can I do
about it.
In fact the above bugs cannot be detected for unit test or marvin test(I
even not sure if they're valid bugs or not, but at that time Frank is on
vacation and nobody took a look at these then decided disable the feature,
and after I re-enabled them, everything works fine for me).
--Sheng
>
> -chip
>
Re: [Review Request] Re-enabling baremetal on master
Posted by Chip Childers <ch...@sungard.com>.
On Wed, Jun 19, 2013 at 11:00:33AM -0700, Sheng Yang wrote:
> Hi,
>
> I've created the https://reviews.apache.org/r/11977/ for review. The
> branch re-enabled the baremetal for master. And all major bugs are cleaned.
>
> https://issues.apache.org/jira/browse/CLOUDSTACK-1610
> https://issues.apache.org/jira/browse/CLOUDSTACK-1618
> https://issues.apache.org/jira/browse/CLOUDSTACK-1614
> https://issues.apache.org/jira/browse/CLOUDSTACK-1440
>
> In fact it's not a feature merge, because the code is already in MASTER
> ready. We just disable it due to stability problem of 4.1 release. Now I've
> tried to enable it, and the changeset is very small, mostly just revert the
> old disabling baremetal codes, and fix some issues with introducing other
> new features. Here is the summary:
[snip]
So David's standing veto was because of this comment (from him):
"Baremetal seems to be suffering from a significant lack of unit tests
and integration tests for marvin to consume. Let's get those in place
before we consider re-enabling this."
If I remember correctly, the reason that master has the code in it, is
specifically because we decided that disabling the feature was easier to
honor the veto than reverting all of the changes.
That being said, have we addressed the original veto's concerns?
-chip