AMBARI-15858. Add new recommendation strategy option to Blueprint: ALWAYS_APPLY_DONT_OVERRIDE_CUSTOM_VALUES (magyari_sandor)

ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessor.java

@@ -69,12 +69,13 @@ public class StackAdvisorBlueprintProcessor {
   /**
    * Recommend configurations by the stack advisor, then store the results in cluster topology.
    * @param clusterTopology cluster topology instance
+   * @param existingConfigurations Existing configurations of cluster
    */
-  public void adviseConfiguration(ClusterTopology clusterTopology) throws ConfigurationTopologyException {
+  public void adviseConfiguration(ClusterTopology clusterTopology, Map<String, Map<String, String>> existingConfigurations) throws ConfigurationTopologyException {
     StackAdvisorRequest request = createStackAdvisorRequest(clusterTopology, StackAdvisorRequestType.CONFIGURATIONS);
     try {
       RecommendationResponse response = stackAdvisorHelper.recommend(request);
-      addAdvisedConfigurationsToTopology(response, clusterTopology);
+      addAdvisedConfigurationsToTopology(response, clusterTopology, existingConfigurations);
     } catch (StackAdvisorException e) {
       throw new ConfigurationTopologyException(RECOMMENDATION_FAILED, e);
     } catch (IllegalArgumentException e) {
@@ -159,7 +160,7 @@ public class StackAdvisorBlueprintProcessor {
   }
 
   private void addAdvisedConfigurationsToTopology(RecommendationResponse response,
-                                                  ClusterTopology topology) {
+                                                  ClusterTopology topology, Map<String, Map<String, String>> existingConfigurations) {
     Preconditions.checkArgument(response.getRecommendations() != null,
       "Recommendation response is empty.");
     Preconditions.checkArgument(response.getRecommendations().getBlueprint() != null,
@@ -167,7 +168,7 @@ public class StackAdvisorBlueprintProcessor {
     Preconditions.checkArgument(response.getRecommendations().getBlueprint().getConfigurations() != null,
       "Configurations are missing from the recommendation blueprint response.");
 
-    Map<String, Map<String, String>> userProvidedProperties = getUserProvidedProperties(topology);
+    Map<String, Map<String, String>> userProvidedProperties = getUserProvidedProperties(topology, existingConfigurations);
     Map<String, BlueprintConfigurations> recommendedConfigurations =
       response.getRecommendations().getBlueprint().getConfigurations();
     for (Map.Entry<String, BlueprintConfigurations> configEntry : recommendedConfigurations.entrySet()) {
@@ -182,15 +183,13 @@ public class StackAdvisorBlueprintProcessor {
   /**
    * Gather user defined properties. (keep that only which is not included in the stack defaults or it overrides the stack default value)
    */
-  private Map<String, Map<String, String>> getUserProvidedProperties(ClusterTopology topology) {
+  private Map<String, Map<String, String>> getUserProvidedProperties(ClusterTopology topology, Map<String, Map<String, String>> existingConfigurations) {
     Map<String, Map<String, String>> userProvidedProperties = Maps.newHashMap();
     Blueprint blueprint = topology.getBlueprint();
     Configuration stackDefaults = blueprint.getStack().getConfiguration(blueprint.getServices());
-
     Map<String, Map<String, String>> stackDefaultProps = stackDefaults.getProperties();
-    Map<String, Map<String, String>> fullConfig = topology.getConfiguration().getFullProperties();
 
-    for (Map.Entry<String, Map<String, String>> configGroup : fullConfig.entrySet()) {
+    for (Map.Entry<String, Map<String, String>> configGroup : existingConfigurations.entrySet()) {
       String configType = configGroup.getKey();
       Map<String, String> configsToAdd = Maps.newHashMap();
       for (Map.Entry<String, String> configProp : configGroup.getValue().entrySet()) {
@@ -212,12 +211,15 @@ public class StackAdvisorBlueprintProcessor {
   }
 
   /**
-   * Remove user defined properties from stack advisor output in case of it applies only on the stack defaults
+   * Remove user defined properties from stack advisor output in case of ONLY_STACK_DEFAULTS_APPLY or
+   * ALWAYS_APPLY_DONT_OVERRIDE_CUSTOM_VALUES.
    */
   private BlueprintConfigurations filterBlueprintConfig(String configType, BlueprintConfigurations config,
                                                         Map<String, Map<String, String>> userProvidedProperties,
                                                         ClusterTopology topology) {
-    if (topology.getConfigRecommendationStrategy() == ConfigRecommendationStrategy.ONLY_STACK_DEFAULTS_APPLY) {
+    if (topology.getConfigRecommendationStrategy() == ConfigRecommendationStrategy.ONLY_STACK_DEFAULTS_APPLY ||
+      topology.getConfigRecommendationStrategy() == ConfigRecommendationStrategy
+        .ALWAYS_APPLY_DONT_OVERRIDE_CUSTOM_VALUES) {
       if (userProvidedProperties.containsKey(configType)) {
         BlueprintConfigurations newConfig = new BlueprintConfigurations();
         Map<String, String> filteredProps = Maps.filterKeys(config.getProperties(),

ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/CreateKeytabFilesServerAction.java

@@ -153,8 +153,9 @@ public class CreateKeytabFilesServerAction extends KerberosServerAction {
 
     CreateKeyTabKerberosAuditEvent.CreateKeyTabKerberosAuditEventBuilder auditEventBuilder = CreateKeyTabKerberosAuditEvent.builder();
     auditEventBuilder.withTimestamp(System.currentTimeMillis());
-    auditEventBuilder.withRequestId(getHostRoleCommand().getRequestId());
-    auditEventBuilder.withTaskId(getHostRoleCommand().getTaskId());
+    // in case this is called directly from TopologyManager there's no HostRoleCommand
+    auditEventBuilder.withRequestId(getHostRoleCommand() != null ? getHostRoleCommand().getRequestId() : -1);
+    auditEventBuilder.withTaskId(getHostRoleCommand() != null ? getHostRoleCommand().getTaskId() : -1);
 
     CommandReport commandReport = null;
     String message = null;

ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/CreatePrincipalsServerAction.java

@@ -18,6 +18,7 @@
 
 package org.apache.ambari.server.serveraction.kerberos;
 
+import com.google.common.base.Optional;
 import com.google.inject.Inject;
 import org.apache.ambari.server.AmbariException;
 import org.apache.ambari.server.actionmanager.HostRoleStatus;
@@ -174,10 +175,12 @@ public class CreatePrincipalsServerAction extends KerberosServerAction {
                                                Map<String, String> kerberosConfiguration,
                                                KerberosOperationHandler kerberosOperationHandler,
                                                ActionLog actionLog) {
+
+    // in case this is called directly from TopologyManager there's no HostRoleCommand
     CreatePrincipalKerberosAuditEvent.CreatePrincipalKerberosAuditEventBuilder auditEventBuilder = CreatePrincipalKerberosAuditEvent.builder()
       .withTimestamp(System.currentTimeMillis())
-      .withRequestId(getHostRoleCommand().getRequestId())
-      .withTaskId(getHostRoleCommand().getTaskId())
+      .withRequestId(getHostRoleCommand() != null ? getHostRoleCommand().getRequestId() : -1)
+      .withTaskId(getHostRoleCommand() != null ? getHostRoleCommand().getTaskId() : -1)
       .withPrincipal(principal);
     CreatePrincipalResult result = null;
     String message = null;

ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterConfigurationRequest.java

@@ -137,14 +137,17 @@ public class ClusterConfigurationRequest {
     // this will update the topo cluster config and all host group configs in the cluster topology
     Set<String> updatedConfigTypes = new HashSet<>();
 
+    Configuration clusterConfiguration = clusterTopology.getConfiguration();
+    Map<String, Map<String, String>> existingConfigurations = clusterConfiguration.getFullProperties();
+
     try {
       if (configureSecurity) {
-        updatedConfigTypes.addAll(configureKerberos());
+        updatedConfigTypes.addAll(configureKerberos(clusterConfiguration, existingConfigurations));
       }
 
       // obtain recommended configurations before config updates
       if (!ConfigRecommendationStrategy.NEVER_APPLY.equals(this.clusterTopology.getConfigRecommendationStrategy())) {
-        stackAdvisorBlueprintProcessor.adviseConfiguration(this.clusterTopology);
+        stackAdvisorBlueprintProcessor.adviseConfiguration(this.clusterTopology, existingConfigurations);
       }
 
       updatedConfigTypes.addAll(configurationProcessor.doUpdateForClusterCreate());
@@ -156,7 +159,7 @@ public class ClusterConfigurationRequest {
     setConfigurationsOnCluster(clusterTopology, TopologyManager.TOPOLOGY_RESOLVED_TAG, updatedConfigTypes);
   }
 
-  private Set<String> configureKerberos() throws AmbariException {
+  private Set<String> configureKerberos(Configuration clusterConfiguration, Map<String, Map<String, String>> existingConfigurations) throws AmbariException {
     Set<String> updatedConfigTypes = new HashSet<>();
 
     Cluster cluster = getCluster();
@@ -164,8 +167,7 @@ public class ClusterConfigurationRequest {
 
     Configuration stackDefaults = blueprint.getStack().getConfiguration(blueprint.getServices());
     Map<String, Map<String, String>> stackDefaultProps = stackDefaults.getProperties();
-    Configuration clusterConfiguration = clusterTopology.getConfiguration();
-    Map<String, Map<String, String>> existingConfigurations = clusterConfiguration.getFullProperties();
+
     // add clusterHostInfo containing components to hosts map, based on Topology, to use this one instead of
     // StageUtils.getClusterInfo()
     Map<String, String> componentHostsMap = createComponentHostMap(blueprint);

ambari-server/src/main/java/org/apache/ambari/server/topology/ConfigRecommendationStrategy.java

@@ -33,5 +33,10 @@ public enum ConfigRecommendationStrategy {
    *  Configuration recommendations are always applied for properties listed as stack defaults,
    *  but not for configurations defined by the user in the Blueprint and/or Cluster Creation Template.
    */
-  ONLY_STACK_DEFAULTS_APPLY;
+  ONLY_STACK_DEFAULTS_APPLY,
+  /**
+   *  Configuration recommendations are always applied, overriding stack defaults but they don't
+   *  override configuration defined by the user in the Blueprint and/or Cluster Creation Template.
+   */
+  ALWAYS_APPLY_DONT_OVERRIDE_CUSTOM_VALUES;
 }

ambari-server/src/test/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorBlueprintProcessorTest.java

@@ -72,13 +72,14 @@ public class StackAdvisorBlueprintProcessorTest {
   @Test
   public void testAdviseConfiguration() throws StackAdvisorException, ConfigurationTopologyException {
     // GIVEN
+    Map<String, Map<String, String>> props = createProps();
     Map<String, AdvisedConfiguration> advisedConfigurations = new HashMap<String, AdvisedConfiguration>();
     expect(clusterTopology.getBlueprint()).andReturn(blueprint).anyTimes();
     expect(clusterTopology.getHostGroupInfo()).andReturn(createHostGroupInfo()).anyTimes();
     expect(clusterTopology.getAdvisedConfigurations()).andReturn(advisedConfigurations).anyTimes();
     expect(clusterTopology.getConfiguration()).andReturn(configuration).anyTimes();
     expect(clusterTopology.isClusterKerberosEnabled()).andReturn(false).anyTimes();
-    expect(clusterTopology.getConfigRecommendationStrategy()).andReturn(ConfigRecommendationStrategy.ALWAYS_APPLY);
+    expect(clusterTopology.getConfigRecommendationStrategy()).andReturn(ConfigRecommendationStrategy.ALWAYS_APPLY).anyTimes();
     expect(blueprint.getStack()).andReturn(stack).anyTimes();
     expect(stack.getVersion()).andReturn("2.3").anyTimes();
     expect(stack.getName()).andReturn("HDP").anyTimes();
@@ -87,11 +88,11 @@ public class StackAdvisorBlueprintProcessorTest {
     expect(blueprint.getHostGroups()).andReturn(createHostGroupMap()).anyTimes();
     expect(hostGroup.getComponentNames()).andReturn(Arrays.asList("comp1", "comp2")).anyTimes();
     expect(stackAdvisorHelper.recommend(anyObject(StackAdvisorRequest.class))).andReturn(createRecommendationResponse());
-    expect(configuration.getFullProperties()).andReturn(createProps()).anyTimes();
+    expect(configuration.getFullProperties()).andReturn(props).anyTimes();
 
     replay(clusterTopology, blueprint, stack, hostGroup, configuration, stackAdvisorHelper);
     // WHEN
-    underTest.adviseConfiguration(clusterTopology);
+    underTest.adviseConfiguration(clusterTopology, props);
     // THEN
     assertTrue(advisedConfigurations.get("core-site").getProperties().containsKey("dummyKey1"));
     assertTrue(advisedConfigurations.get("core-site").getProperties().containsKey("dummyKey3"));
@@ -105,6 +106,7 @@ public class StackAdvisorBlueprintProcessorTest {
   @Test
   public void testAdviseConfigurationWithOnlyStackDefaultsApply() throws StackAdvisorException, ConfigurationTopologyException {
     // GIVEN
+    Map<String, Map<String, String>> props = createProps();
     Map<String, AdvisedConfiguration> advisedConfigurations = new HashMap<String, AdvisedConfiguration>();
     expect(clusterTopology.getBlueprint()).andReturn(blueprint).anyTimes();
     expect(clusterTopology.getHostGroupInfo()).andReturn(createHostGroupInfo()).anyTimes();
@@ -120,11 +122,11 @@ public class StackAdvisorBlueprintProcessorTest {
     expect(blueprint.getHostGroups()).andReturn(createHostGroupMap()).anyTimes();
     expect(hostGroup.getComponentNames()).andReturn(Arrays.asList("comp1", "comp2")).anyTimes();
     expect(stackAdvisorHelper.recommend(anyObject(StackAdvisorRequest.class))).andReturn(createRecommendationResponse());
-    expect(configuration.getFullProperties()).andReturn(createProps()).anyTimes();
+    expect(configuration.getFullProperties()).andReturn(props).anyTimes();
 
     replay(clusterTopology, blueprint, stack, hostGroup, configuration, stackAdvisorHelper);
     // WHEN
-    underTest.adviseConfiguration(clusterTopology);
+    underTest.adviseConfiguration(clusterTopology, props);
     // THEN
     assertTrue(advisedConfigurations.get("core-site").getProperties().containsKey("dummyKey1"));
     assertFalse(advisedConfigurations.get("core-site").getProperties().containsKey("dummyKey3"));
@@ -159,7 +161,7 @@ public class StackAdvisorBlueprintProcessorTest {
 
     replay(clusterTopology, blueprint, stack, hostGroup, configuration, stackAdvisorHelper);
     // WHEN
-    underTest.adviseConfiguration(clusterTopology);
+    underTest.adviseConfiguration(clusterTopology, props);
     // THEN
     assertTrue(advisedConfigurations.get("core-site").getProperties().containsKey("dummyKey1"));
     assertTrue(advisedConfigurations.get("core-site").getProperties().containsKey("dummyKey3"));
@@ -170,9 +172,43 @@ public class StackAdvisorBlueprintProcessorTest {
       .getPropertyValueAttributes().get("dummyKey2").getDelete());
   }
 
+  @Test
+  public void testAdviseConfigurationWith_ALWAYS_APPLY_DONT_OVERRIDE_CUSTOM_VALUES() throws StackAdvisorException,
+    ConfigurationTopologyException {
+    // GIVEN
+    Map<String, Map<String, String>> props = createProps();
+    Map<String, AdvisedConfiguration> advisedConfigurations = new HashMap<String, AdvisedConfiguration>();
+    expect(clusterTopology.getBlueprint()).andReturn(blueprint).anyTimes();
+    expect(clusterTopology.getHostGroupInfo()).andReturn(createHostGroupInfo()).anyTimes();
+    expect(clusterTopology.getAdvisedConfigurations()).andReturn(advisedConfigurations).anyTimes();
+    expect(clusterTopology.getConfiguration()).andReturn(configuration).anyTimes();
+    expect(clusterTopology.isClusterKerberosEnabled()).andReturn(false).anyTimes();
+    expect(clusterTopology.getConfigRecommendationStrategy()).andReturn(ConfigRecommendationStrategy.ALWAYS_APPLY_DONT_OVERRIDE_CUSTOM_VALUES).anyTimes();
+    expect(blueprint.getStack()).andReturn(stack).anyTimes();
+    expect(stack.getVersion()).andReturn("2.3").anyTimes();
+    expect(stack.getName()).andReturn("HDP").anyTimes();
+    expect(stack.getConfiguration(Arrays.asList("HDFS", "YARN", "HIVE"))).andReturn(createStackDefaults()).anyTimes();
+    expect(blueprint.getServices()).andReturn(Arrays.asList("HDFS", "YARN", "HIVE")).anyTimes();
+    expect(blueprint.getHostGroups()).andReturn(createHostGroupMap()).anyTimes();
+    expect(hostGroup.getComponentNames()).andReturn(Arrays.asList("comp1", "comp2")).anyTimes();
+    expect(stackAdvisorHelper.recommend(anyObject(StackAdvisorRequest.class))).andReturn(createRecommendationResponse());
+    expect(configuration.getFullProperties()).andReturn(props).anyTimes();
+
+    replay(clusterTopology, blueprint, stack, hostGroup, configuration, stackAdvisorHelper);
+    // WHEN
+    underTest.adviseConfiguration(clusterTopology, props);
+    // THEN
+    assertTrue(advisedConfigurations.get("core-site").getProperties().containsKey("dummyKey1"));
+    assertTrue(advisedConfigurations.get("core-site").getPropertyValueAttributes().containsKey("dummyKey2"));
+    assertEquals("dummyValue", advisedConfigurations.get("core-site").getProperties().get("dummyKey1"));
+    assertEquals(Boolean.toString(true), advisedConfigurations.get("core-site")
+      .getPropertyValueAttributes().get("dummyKey2").getDelete());
+  }
+
   @Test
   public void testAdviseConfigurationWhenConfigurationRecommendFails() throws StackAdvisorException, ConfigurationTopologyException {
     // GIVEN
+    Map<String, Map<String, String>> props = createProps();
     Map<String, AdvisedConfiguration> advisedConfigurations = new HashMap<String, AdvisedConfiguration>();
     expect(clusterTopology.getBlueprint()).andReturn(blueprint).anyTimes();
     expect(clusterTopology.getHostGroupInfo()).andReturn(createHostGroupInfo()).anyTimes();
@@ -186,12 +222,12 @@ public class StackAdvisorBlueprintProcessorTest {
     expect(hostGroup.getComponentNames()).andReturn(Arrays.asList("comp1", "comp2")).anyTimes();
     expect(blueprint.getServices()).andReturn(Arrays.asList("HDFS", "YARN", "HIVE")).anyTimes();
     expect(stackAdvisorHelper.recommend(anyObject(StackAdvisorRequest.class))).andThrow(new StackAdvisorException("ex"));
-    expect(configuration.getFullProperties()).andReturn(createProps());
+    expect(configuration.getFullProperties()).andReturn(props);
 
     replay(clusterTopology, blueprint, stack, hostGroup, configuration, stackAdvisorHelper);
     // WHEN
     try {
-      underTest.adviseConfiguration(clusterTopology);
+      underTest.adviseConfiguration(clusterTopology, props);
       fail("Invalid state");
     } catch (ConfigurationTopologyException e) {
       assertEquals(StackAdvisorBlueprintProcessor.RECOMMENDATION_FAILED, e.getMessage());
@@ -201,6 +237,7 @@ public class StackAdvisorBlueprintProcessorTest {
   @Test
   public void testAdviseConfigurationWhenConfigurationRecommendHasInvalidResponse() throws StackAdvisorException, ConfigurationTopologyException {
     // GIVEN
+    Map<String, Map<String, String>> props = createProps();
     Map<String, AdvisedConfiguration> advisedConfigurations = new HashMap<String, AdvisedConfiguration>();
     expect(clusterTopology.getBlueprint()).andReturn(blueprint).anyTimes();
     expect(clusterTopology.getHostGroupInfo()).andReturn(createHostGroupInfo()).anyTimes();
@@ -214,12 +251,12 @@ public class StackAdvisorBlueprintProcessorTest {
     expect(blueprint.getHostGroups()).andReturn(createHostGroupMap()).anyTimes();
     expect(hostGroup.getComponentNames()).andReturn(Arrays.asList("comp1", "comp2")).anyTimes();
     expect(stackAdvisorHelper.recommend(anyObject(StackAdvisorRequest.class))).andReturn(new RecommendationResponse());
-    expect(configuration.getFullProperties()).andReturn(createProps());
+    expect(configuration.getFullProperties()).andReturn(props);
 
     replay(clusterTopology, blueprint, stack, hostGroup, configuration, stackAdvisorHelper);
     // WHEN
     try {
-      underTest.adviseConfiguration(clusterTopology);
+      underTest.adviseConfiguration(clusterTopology, props);
       fail("Invalid state");
     } catch (ConfigurationTopologyException e) {
       assertEquals(StackAdvisorBlueprintProcessor.INVALID_RESPONSE, e.getMessage());