----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38183/#review98052 -----------------------------------------------------------
This patch looks fine to me. Overall, the general approach taken here looks correct. The way that "excluded-config" has been interpreted by the UI and by Blueprints has historically been slightly different. Because of this, I think it may make sense to do a little more functional testing. I'd request that the author of this patch run some additional cluster deployments to test this change out, and please use the other services (in addition to Falcon) as part of these tests. It looks like the following components use this excluded-config construct: AMBARI METRICS, FALCON, MAPREDUCE, YARN, HBASE. I think this extra testing would be worthwhile, given that this modifies how the Blueprints processor treats excluded config. - Robert Nettleton On Sept. 8, 2015, 4:05 p.m., Sandor Magyari wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38183/ > ----------------------------------------------------------- > > (Updated Sept. 8, 2015, 4:05 p.m.) > > > Review request for Ambari and Robert Nettleton. > > > Bugs: AMBARI-13017 > https://issues.apache.org/jira/browse/AMBARI-13017 > > > Repository: ambari > > > Description > ------- > > PROBLEM > > Installing falcon with blueprint, oozie extensions are missing, hence causing > misconfigured installations that are using blueprint. > > SOLUTION > > This issue seems to be related not only for case of falcon but for every > service which has a configuration file for another service. > In case of falcon there is an oozie-site.xml (this contains oozie extensions) > in configuration of falcon service, which is marked as exclued-config-type in > service metainfo.xml. The same is true for AMBARI_METRICS which includes the > storm-site.xml. In case of a bluprint install these are excluded from > properties, which is fine because you can not add a config-type twice. The > solution would to add these properties from excluded-config-type at update > phase of properties in > BluprintConfigurationProcessor.doUpdateForClusterCreate. > We can apply this only specifically for FALCON but I dont see yet any > drawbacks of applying it for each service with excluded property. > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java > 892cf32 > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java > a881472 > > Diff: https://reviews.apache.org/r/38183/diff/ > > > Testing > ------- > > Manually tested, creating new cluster with blueprint, unitests passed, > created a new testcase. > > > Thanks, > > Sandor Magyari > >
