> 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? > > Robert Nettleton wrote: > 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.
I agree that the various Stack implementations should at some time be consilidated. This class is basically a wrapper around the currently existing stack api's in AmbariManagementController and AmbariMetaInfo. Those api's are a mess so this was created to abstract the BP code from the details of dealing with these existing api's. As a note, I my currenty work, I have done a lot of cleanup of stack related code and introduced a StackManager implementation which should help in the consolidation effort. - John ----------------------------------------------------------- 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 > >
