Return-Path: X-Original-To: apmail-ambari-dev-archive@www.apache.org Delivered-To: apmail-ambari-dev-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id AB14918FAF for ; Fri, 8 Jan 2016 23:50:42 +0000 (UTC) Received: (qmail 29367 invoked by uid 500); 8 Jan 2016 23:50:42 -0000 Delivered-To: apmail-ambari-dev-archive@ambari.apache.org Received: (qmail 29329 invoked by uid 500); 8 Jan 2016 23:50:42 -0000 Mailing-List: contact dev-help@ambari.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@ambari.apache.org Delivered-To: mailing list dev@ambari.apache.org Received: (qmail 29311 invoked by uid 99); 8 Jan 2016 23:50:42 -0000 Received: from reviews-vm.apache.org (HELO reviews.apache.org) (140.211.11.40) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 08 Jan 2016 23:50:42 +0000 Received: from reviews.apache.org (localhost [127.0.0.1]) by reviews.apache.org (Postfix) with ESMTP id 122061DBF7B; Fri, 8 Jan 2016 23:50:42 +0000 (UTC) Content-Type: multipart/alternative; boundary="===============7999300798875823065==" MIME-Version: 1.0 Subject: Re: Review Request 42085: Add INSTALL_ONLY support to Blueprints on a per-component basis From: "Robert Nettleton" To: "Mahadev Konar" , "Sumit Mohanty" , "Sid Wagle" Cc: "Ambari" , "Robert Nettleton" Date: Fri, 08 Jan 2016 23:50:42 -0000 Message-ID: <20160108235042.1692.26388@reviews.apache.org> X-ReviewBoard-URL: https://reviews.apache.org/ Auto-Submitted: auto-generated Sender: "Robert Nettleton" X-ReviewGroup: Ambari X-Auto-Response-Suppress: DR, RN, OOF, AutoReply X-ReviewRequest-URL: https://reviews.apache.org/r/42085/ X-Sender: "Robert Nettleton" References: <20160108233850.1692.75944@reviews.apache.org> In-Reply-To: <20160108233850.1692.75944@reviews.apache.org> Reply-To: "Robert Nettleton" X-ReviewRequest-Repository: ambari --===============7999300798875823065== MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit > On Jan. 8, 2016, 11:38 p.m., Sumit Mohanty wrote: > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ExportBlueprintRequest.java, line 136 > > > > > > This is an independent change I assume? Or is it related to this feature? Thanks for the review! This change is related. I had to update this class in order to translate between the ExportedHostGroup, and the new type I'm introducing in this patch to model the Component instances (including name and provisioning_action). This change was necessary in order to update the API usages througout, due to my changes to the HostGroup interface. > On Jan. 8, 2016, 11:38 p.m., Sumit Mohanty wrote: > > ambari-server/src/main/java/org/apache/ambari/server/orm/entities/HostGroupComponentEntity.java, line 52 > > > > > > How do we manage the default value? The default value is "INSTALL_AND_START", and the handling is done in the BlueprintFactory. The BlueprintFactory interprets a NULL value for this column to mean "INSTALL_AND_START". > On Jan. 8, 2016, 11:38 p.m., Sumit Mohanty wrote: > > ambari-server/src/main/resources/Ambari-DDL-MySQL-CREATE.sql, line 417 > > > > > > Should this be non null? I assume empty value is INSTALL_AND_START. I chose this column to be nullable in order to preserve backwards compatibility. Since most Blueprints won't use the new syntax, and older Blueprints definitely won't, I thought it made sense to keep this column nullable, so that no extra DB handling is required for the most common case. - Robert ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42085/#review113568 ----------------------------------------------------------- On Jan. 8, 2016, 9:35 p.m., Robert Nettleton wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/42085/ > ----------------------------------------------------------- > > (Updated Jan. 8, 2016, 9:35 p.m.) > > > Review request for Ambari, Mahadev Konar, Sumit Mohanty, and Sid Wagle. > > > Bugs: AMBARI-14555 > https://issues.apache.org/jira/browse/AMBARI-14555 > > > Repository: ambari > > > Description > ------- > > This patch implements the new Blueprint support described in AMBARI-14555. > > Blueprints will now support syntax to allow users to mark certain components in a Blueprint host group as "INSTALL_ONLY", meaning that Ambari will install them, but that these components will not be started initially. > > This patch implements the following: > 1. Adds a new column to the "hostgroup_component" table in the Ambari DB, "provision_action", to store the provisioning state desired for a component. > 2. Moves the ProvisionAction enumerated type to a top-level class, in order to share this across the entire Blueprints implementation. > 3. Adds code to the UpgradeCatalog implementation for Ambari 2.2.1, in order to add the new column to older databases during an upgrade, and adds a unit test for this as well. > 4. Refactors the HostGroup interface and implementation, in order to support more configuration data being attached to a given host group component. > 5. Updates various sections of the code that use the HostGroup interface, to accomodate these changes. > 6. Adds handling code to store and read this new Blueprint syntax during the POST/GET calls on the BlueprintResourceProvider. > 7. Modifies the HostComponentResourceProvider's start() method, such that a set of components that are marked as "INSTALL_ONLY" can be passed in. Modifies the start() implementation to use a new set of predicates to filter out any components that should not be started prior to the start() operation being executed on the host. > 8. Updates various unit tests to accomodate interface and implementation changes. > > > Diffs > ----- > > ambari-server/src/main/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRenderer.java 3705ceb > ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessor.java d57c17d > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintResourceProvider.java f3100b5 > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ExportBlueprintRequest.java 8c8b89d > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/HostComponentResourceProvider.java 194d75f > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ProvisionAction.java PRE-CREATION > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ProvisionClusterRequest.java 7b1de26 > ambari-server/src/main/java/org/apache/ambari/server/orm/entities/HostGroupComponentEntity.java 984c549 > ambari-server/src/main/java/org/apache/ambari/server/orm/entities/HostGroupComponentEntityPK.java fb9011b > ambari-server/src/main/java/org/apache/ambari/server/topology/AmbariContext.java 6bfee93 > ambari-server/src/main/java/org/apache/ambari/server/topology/BlueprintFactory.java b8ce749 > ambari-server/src/main/java/org/apache/ambari/server/topology/BlueprintImpl.java 88052b0 > ambari-server/src/main/java/org/apache/ambari/server/topology/BlueprintValidatorImpl.java 1c293ee > ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterTopology.java c3c04db > ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterTopologyImpl.java e78300c > ambari-server/src/main/java/org/apache/ambari/server/topology/Component.java PRE-CREATION > ambari-server/src/main/java/org/apache/ambari/server/topology/HostGroup.java 07e3e88 > ambari-server/src/main/java/org/apache/ambari/server/topology/HostGroupImpl.java b89e7e4 > ambari-server/src/main/java/org/apache/ambari/server/topology/HostRequest.java 440638c > ambari-server/src/main/java/org/apache/ambari/server/topology/LogicalRequest.java bd9f2e0 > ambari-server/src/main/java/org/apache/ambari/server/topology/RequiredPasswordValidator.java e26de3f > ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog221.java 5cde24b > ambari-server/src/main/resources/Ambari-DDL-Derby-CREATE.sql d93a3c2 > ambari-server/src/main/resources/Ambari-DDL-MySQL-CREATE.sql aa8ced1 > ambari-server/src/main/resources/Ambari-DDL-Oracle-CREATE.sql b534344 > ambari-server/src/main/resources/Ambari-DDL-Postgres-CREATE.sql 941fc6e > ambari-server/src/main/resources/Ambari-DDL-Postgres-EMBEDDED-CREATE.sql dd517f8 > ambari-server/src/main/resources/Ambari-DDL-SQLAnywhere-CREATE.sql f837f9e > ambari-server/src/main/resources/Ambari-DDL-SQLServer-CREATE.sql 239d27e > ambari-server/src/test/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRendererTest.java 522d902 > ambari-server/src/test/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessorTest.java 514e6ab > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java 0384b45 > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ExportBlueprintRequestTest.java 94ba90f > ambari-server/src/test/java/org/apache/ambari/server/orm/entities/HostGroupComponentEntityTest.java c0efd0d > ambari-server/src/test/java/org/apache/ambari/server/topology/BlueprintFactoryTest.java 3a3b6dc > ambari-server/src/test/java/org/apache/ambari/server/topology/BlueprintImplTest.java 3addfc4 > ambari-server/src/test/java/org/apache/ambari/server/topology/BlueprintValidatorImplTest.java 304cded > ambari-server/src/test/java/org/apache/ambari/server/topology/ClusterInstallWithoutStartTest.java 1354a72 > ambari-server/src/test/java/org/apache/ambari/server/topology/ClusterTopologyImplTest.java 08aa3d3 > ambari-server/src/test/java/org/apache/ambari/server/topology/RequiredPasswordValidatorTest.java f4ded70 > ambari-server/src/test/java/org/apache/ambari/server/topology/TopologyManagerTest.java 47169f4 > ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog221Test.java 319024b > > Diff: https://reviews.apache.org/r/42085/diff/ > > > Testing > ------- > > 1. Tested a 3-node HDFS HA cluster using a Blueprint with the original syntax (no new additions), and this deployment succeeded. > 2. Tested a 3-node HDFS HA cluster with certain components marked as "INSTALL_ONLY" with the new syntax. Verified that the cluster deployed properly, and that the marked components were installed, but were not started by the Blueprints deployment. > 3. Ran "mvn clean test" in the ambari-server project: > > "Tests in error: > UpgradeCatalog221Test.testExecuteDMLUpdates:158 ยป NullPointer > > Tests run: 3741, Failures: 0, Errors: 1, Skipped: 29" > > The one failure seen also occurs in trunk without any patches applied, so this current patch does not introduce any new failures. > > > Thanks, > > Robert Nettleton > > --===============7999300798875823065==--