> On Oct. 22, 2014, 12:46 p.m., Nate Cole wrote: > >
Thanks for the review! > On Oct. 22, 2014, 12:46 p.m., Nate Cole wrote: > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/Cardinality.java, > > line 40 > > <https://reviews.apache.org/r/27006/diff/1/?file=728468#file728468line40> > > > > Never understood why * was never used, but ok. I'm not exactly sure, since I've basically moved this existing code to a top-level class, and have left the impl unchanged. Since this cardinality value is used in the stacks, it might be difficult to change without breaking things. > On Oct. 22, 2014, 12:46 p.m., Nate Cole wrote: > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/Stack.java, > > lines 40-47 > > <https://reviews.apache.org/r/27006/diff/1/?file=728470#file728470line40> > > > > We have a lot of Stack representations already. Can one of them be > > augmented? It's possible, but this was the only Stack parsing class I could find that included support for Cardinality. In the future, I'm definitely willing to use a more general class if it becomes available, but would rather stick with this one for the 1.7.0 timeframe. This particular class has the advantage of handling all the details of the Stack query API, so I find it a bit simpler to use. - Robert ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27006/#review57776 ----------------------------------------------------------- On Oct. 22, 2014, 12:57 a.m., Robert Nettleton wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27006/ > ----------------------------------------------------------- > > (Updated Oct. 22, 2014, 12:57 a.m.) > > > Review request for Ambari, John Speidel and Nate Cole. > > > Repository: ambari > > > Description > ------- > > This patch implements a fix for bug: AMBARI-7738. > > The Blueprints configuration processor was not properly handling > the case of an optional service in an HDP 2.1 stack deployment. > > The problem occurs when the Yarn "APP_TIMELINE_SERVER" is not > included in a Blueprint that references HDP 2.1. Posting the > Blueprint will work properly, but launching a cluster based > on this Blueprint will fail, with an error mentioning that > the APP_TIMELINE_SERVER is not included. > > The APP_TIMELINE_SERVER has a cardinality of "0-1" for > HDP 2.1, meaning that 0 or 1 instances of this service > can be deployed. This cardinality was selected for HDP > 2.1 because this service was considered a technical > preview for the HDP 2.1 stack. > > While this particular Yarn component demonstrates the problem, > the issue is actually a bit more generic. The Blueprint > config processor is not currently aware of the cardinality > of each service being deployed, so the failure occurs when the > processor attempts to update configuration for a serivce that > doesn't exist in the proposed cluster. > > This patch addresses the problem with the following changes: > > - Moves two inner classes (Stack, Cardinality) to the > top-level, as package-level classes. These classes > offer useful processing features for the Blueprint > config processor, and are more useful as standalone > classes. > > - Modifies the signatures of the PropertyUpdater interface, > any implementations of this interface, and the call stack > from the ClusterResourceProvider to the BlueprintConfiguration > processor, such that the parsed Stack object is available > to the PropertyUpdater instances. > > - Modifies the implementation for the SingleHostTopologyUpdater > to query the Stack definition in the event that no hosts > are found when trying to update a property. If a cardinality > of 0 is found to be valid for that service, then the original > value for the property is returned. If a cardinality of 0 > is not valid for this service, then an IllegalArgumentException > is thrown back to the caller, indicating that an error has > occurred (this was the previous behavior in both cases). > > - Updates various existing unit tests to reflect this change. > > - Adds new unit tests to verify the fix. > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BaseBlueprintProcessor.java > abd22a4 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java > 0b58c8d > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/Cardinality.java > PRE-CREATION > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterResourceProvider.java > 090eb92 > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/Stack.java > PRE-CREATION > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BaseBlueprintProcessorTest.java > be5aea8 > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java > a57bacb > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterResourceProviderTest.java > c1185ae > > Diff: https://reviews.apache.org/r/27006/diff/ > > > Testing > ------- > > 1. Ran the ambari-server unit test suite (both trunk and 1.7.0), and all unit > tests are passing with this patch applied. > 2. Manually verified the fix against the trunk code. > 3. Manually verified the fix against the 1.7.0 code. > > > Thanks, > > Robert Nettleton > >
