----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27006/#review57992 -----------------------------------------------------------
Ship it! Ship It! - John Speidel On Oct. 22, 2014, 9:44 p.m., Robert Nettleton wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27006/ > ----------------------------------------------------------- > > (Updated Oct. 22, 2014, 9:44 p.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 > 10cc016 > > 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 > 260d043 > > 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 > >
