You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ambari.apache.org by jl...@apache.org on 2017/09/26 05:19:56 UTC

[05/50] [abbrv] ambari git commit: AMBARI-21999 - Always Take Target Read-Only Properties On Stack Upgrade (jonathanhurley)

AMBARI-21999 -  Always Take Target Read-Only Properties On Stack Upgrade (jonathanhurley)


Project: http://git-wip-us.apache.org/repos/asf/ambari/repo
Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/56c3da78
Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/56c3da78
Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/56c3da78

Branch: refs/heads/branch-feature-AMBARI-14714
Commit: 56c3da7846cf2b30e5860cb5bc32d3c9ecb93424
Parents: 8b9370a
Author: Jonathan Hurley <jh...@hortonworks.com>
Authored: Tue Sep 19 13:36:07 2017 -0400
Committer: Jonathan Hurley <jh...@hortonworks.com>
Committed: Wed Sep 20 10:17:31 2017 -0400

----------------------------------------------------------------------
 .../ambari/server/state/UpgradeHelper.java      |  84 ++++++++++--
 .../StackUpgradeConfigurationMergeTest.java     | 136 +++++++++++++++++++
 2 files changed, 208 insertions(+), 12 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/ambari/blob/56c3da78/ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java b/ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java
index 087adc2..8f9d8e1 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeHelper.java
@@ -926,7 +926,8 @@ public class UpgradeHelper {
    * stack and the target stack. If a value has changed between stacks, then the
    * target stack value should be taken unless the cluster's value differs from
    * the old stack. This can occur if a property has been customized after
-   * installation.</li>
+   * installation. Read-only properties, however, are always taken from the new
+   * stack.</li>
    * <li>Downgrade: Reset the latest configurations from the service's original
    * stack. The new configurations that were created on upgrade must be left
    * intact until all components have been reverted, otherwise heartbeats will
@@ -976,6 +977,11 @@ public class UpgradeHelper {
         continue;
       }
 
+      // the auto-merge must take read-only properties even if they have changed
+      // - if the properties was read-only in the source stack, then we must
+      // take the new stack's value
+      Map<String, Set<String>> readOnlyProperties = getReadOnlyProperties(sourceStackId, serviceName);
+
       // upgrade is a bit harder - we have to merge new stack configurations in
 
       // populate a map of default configurations for the service on the old
@@ -1041,8 +1047,7 @@ public class UpgradeHelper {
         Map<String, String> existingConfigurations = existingServiceConfig.getProperties();
 
         // get the new configurations
-        Map<String, String> newDefaultConfigurations = newServiceDefaultConfigsByType.get(
-            configurationType);
+        Map<String, String> newDefaultConfigurations = newServiceDefaultConfigsByType.get(configurationType);
 
         // if the new stack configurations don't have the type, then simply add
         // all of the existing in
@@ -1061,8 +1066,7 @@ public class UpgradeHelper {
           }
         }
 
-        // process every existing configuration property for this configuration
-        // type
+        // process every existing configuration property for this configuration type
         for (Map.Entry<String, String> existingConfigurationEntry : existingConfigurations.entrySet()) {
           String existingConfigurationKey = existingConfigurationEntry.getKey();
           String existingConfigurationValue = existingConfigurationEntry.getValue();
@@ -1079,17 +1083,22 @@ public class UpgradeHelper {
               // from the original stack
               String oldDefaultValue = oldServiceDefaultConfigs.get(existingConfigurationKey);
 
-              if (!StringUtils.equals(existingConfigurationValue, oldDefaultValue)) {
-                // at this point, we've determined that there is a
-                // difference
-                // between default values between stacks, but the value was
-                // also customized, so keep the customized value
+              // see if this property is a read-only property which means that
+              // we shouldn't care if it was changed - we should take the new
+              // stack's value
+              Set<String> readOnlyPropertiesForType = readOnlyProperties.get(configurationType);
+              boolean readOnly = (null != readOnlyPropertiesForType
+                  && readOnlyPropertiesForType.contains(existingConfigurationKey));
+
+              if (!readOnly && !StringUtils.equals(existingConfigurationValue, oldDefaultValue)) {
+                // at this point, we've determined that there is a difference
+                // between default values between stacks, but the value was also
+                // customized, so keep the customized value
                 newDefaultConfigurations.put(existingConfigurationKey, existingConfigurationValue);
               }
             }
           } else {
-            // there is no entry in the map, so add the existing key/value
-            // pair
+            // there is no entry in the map, so add the existing key/value pair
             newDefaultConfigurations.put(existingConfigurationKey, existingConfigurationValue);
           }
         }
@@ -1143,4 +1152,55 @@ public class UpgradeHelper {
       }
     }
   }
+
+  /**
+   * Gets all of the read-only properties for the given service. This will also
+   * include any stack properties as well which are read-only.
+   *
+   * @param stackId
+   *          the stack to get read-only properties for (not {@code null}).
+   * @param serviceName
+   *          the namee of the service (not {@code null}).
+   * @return a map of configuration type to set of property names which are
+   *         read-only
+   * @throws AmbariException
+   */
+  private Map<String, Set<String>> getReadOnlyProperties(StackId stackId, String serviceName)
+      throws AmbariException {
+    Map<String, Set<String>> readOnlyProperties = new HashMap<>();
+
+    Set<PropertyInfo> properties = new HashSet<>();
+
+    Set<PropertyInfo> stackProperties = m_ambariMetaInfoProvider.get().getStackProperties(
+        stackId.getStackName(), stackId.getStackVersion());
+
+    Set<PropertyInfo> serviceProperties = m_ambariMetaInfoProvider.get().getServiceProperties(
+        stackId.getStackName(), stackId.getStackVersion(), serviceName);
+
+    if (CollectionUtils.isNotEmpty(stackProperties)) {
+      properties.addAll(stackProperties);
+    }
+
+    if (CollectionUtils.isNotEmpty(serviceProperties)) {
+      properties.addAll(serviceProperties);
+    }
+
+    for (PropertyInfo property : properties) {
+      ValueAttributesInfo valueAttributes = property.getPropertyValueAttributes();
+      if (null != valueAttributes && valueAttributes.getReadOnly() == Boolean.TRUE) {
+        String type = ConfigHelper.fileNameToConfigType(property.getFilename());
+
+        // get the set of properties for this type, initializing it if needed
+        Set<String> readOnlyPropertiesForType = readOnlyProperties.get(type);
+        if (null == readOnlyPropertiesForType) {
+          readOnlyPropertiesForType = new HashSet<>();
+          readOnlyProperties.put(type, readOnlyPropertiesForType);
+        }
+
+        readOnlyPropertiesForType.add(property.getName());
+      }
+    }
+
+    return readOnlyProperties;
+  }
 }

http://git-wip-us.apache.org/repos/asf/ambari/blob/56c3da78/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/StackUpgradeConfigurationMergeTest.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/StackUpgradeConfigurationMergeTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/StackUpgradeConfigurationMergeTest.java
index 734fa96..39ab1d7 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/StackUpgradeConfigurationMergeTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/StackUpgradeConfigurationMergeTest.java
@@ -33,6 +33,7 @@ import org.apache.ambari.server.actionmanager.HostRoleCommandFactory;
 import org.apache.ambari.server.actionmanager.HostRoleCommandFactoryImpl;
 import org.apache.ambari.server.actionmanager.RequestFactory;
 import org.apache.ambari.server.actionmanager.StageFactory;
+import org.apache.ambari.server.api.services.AmbariMetaInfo;
 import org.apache.ambari.server.controller.AbstractRootServiceResponseFactory;
 import org.apache.ambari.server.controller.AmbariManagementController;
 import org.apache.ambari.server.controller.KerberosHelper;
@@ -56,8 +57,10 @@ import org.apache.ambari.server.state.Config;
 import org.apache.ambari.server.state.ConfigFactory;
 import org.apache.ambari.server.state.ConfigHelper;
 import org.apache.ambari.server.state.DesiredConfig;
+import org.apache.ambari.server.state.PropertyInfo;
 import org.apache.ambari.server.state.RepositoryType;
 import org.apache.ambari.server.state.Service;
+import org.apache.ambari.server.state.ServiceComponent;
 import org.apache.ambari.server.state.ServiceComponentFactory;
 import org.apache.ambari.server.state.ServiceComponentHostFactory;
 import org.apache.ambari.server.state.ServiceFactory;
@@ -65,6 +68,7 @@ import org.apache.ambari.server.state.StackId;
 import org.apache.ambari.server.state.UpgradeContext;
 import org.apache.ambari.server.state.UpgradeContextFactory;
 import org.apache.ambari.server.state.UpgradeHelper;
+import org.apache.ambari.server.state.ValueAttributesInfo;
 import org.apache.ambari.server.state.configgroup.ConfigGroupFactory;
 import org.apache.ambari.server.state.scheduler.RequestExecutionFactory;
 import org.apache.ambari.server.state.stack.OsFamily;
@@ -95,12 +99,15 @@ import com.google.inject.assistedinject.FactoryModuleBuilder;
 public class StackUpgradeConfigurationMergeTest extends EasyMockSupport {
 
   private Injector m_injector;
+  private AmbariMetaInfo m_metainfo;
 
   /**
    * @throws Exception
    */
   @Before
   public void before() throws Exception {
+    m_metainfo = createNiceMock(AmbariMetaInfo.class);
+
     MockModule mockModule = new MockModule();
 
     // create an injector which will inject the mocks
@@ -286,6 +293,134 @@ public class StackUpgradeConfigurationMergeTest extends EasyMockSupport {
     assertEquals("stack-220-original", expectedBarType.get("bar-property-2"));
   }
 
+  /**
+   * Tests that any read-only properties are not taken from the existing
+   * configs, but from the new stack value.
+   *
+   * @throws Exception
+   */
+  @Test
+  public void testReadOnlyPropertyIsTakenFromTargetStack() throws Exception {
+    RepositoryVersionEntity repoVersion211 = createNiceMock(RepositoryVersionEntity.class);
+    RepositoryVersionEntity repoVersion220 = createNiceMock(RepositoryVersionEntity.class);
+
+    StackId stack211 = new StackId("HDP-2.1.1");
+    StackId stack220 = new StackId("HDP-2.2.0");
+
+    String version211 = "2.1.1.0-1234";
+    String version220 = "2.2.0.0-1234";
+
+    expect(repoVersion211.getStackId()).andReturn(stack211).atLeastOnce();
+    expect(repoVersion211.getVersion()).andReturn(version211).atLeastOnce();
+
+    expect(repoVersion220.getStackId()).andReturn(stack220).atLeastOnce();
+    expect(repoVersion220.getVersion()).andReturn(version220).atLeastOnce();
+
+    String fooSite = "foo-site";
+    String fooPropertyName = "foo-property-1";
+    String serviceName = "ZOOKEEPER";
+
+    Map<String, Map<String, String>> stack211Configs = new HashMap<>();
+    Map<String, String> stack211FooType = new HashMap<>();
+    stack211Configs.put(fooSite, stack211FooType);
+    stack211FooType.put(fooPropertyName, "stack-211-original");
+
+    Map<String, Map<String, String>> stack220Configs = new HashMap<>();
+    Map<String, String> stack220FooType = new HashMap<>();
+    stack220Configs.put(fooSite, stack220FooType);
+    stack220FooType.put(fooPropertyName, "stack-220-original");
+
+    PropertyInfo readOnlyProperty = new PropertyInfo();
+    ValueAttributesInfo valueAttributesInfo = new ValueAttributesInfo();
+    valueAttributesInfo.setReadOnly(true);
+    readOnlyProperty.setName(fooPropertyName);
+    readOnlyProperty.setFilename(fooSite + ".xml");
+    readOnlyProperty.setPropertyValueAttributes(null);
+    readOnlyProperty.setPropertyValueAttributes(valueAttributesInfo);
+
+    expect(m_metainfo.getServiceProperties(stack211.getStackName(), stack211.getStackVersion(),
+        serviceName)).andReturn(Sets.newHashSet(readOnlyProperty)).atLeastOnce();
+
+    Map<String, String> existingFooType = new HashMap<>();
+
+    ClusterConfigEntity fooConfigEntity = createNiceMock(ClusterConfigEntity.class);
+
+    expect(fooConfigEntity.getType()).andReturn(fooSite);
+
+    Config fooConfig = createNiceMock(Config.class);
+
+    existingFooType.put(fooPropertyName, "my-foo-property-1");
+
+    expect(fooConfig.getType()).andReturn(fooSite).atLeastOnce();
+    expect(fooConfig.getProperties()).andReturn(existingFooType);
+
+    Map<String, DesiredConfig> desiredConfigurations = new HashMap<>();
+    desiredConfigurations.put(fooSite, null);
+
+    Service zookeeper = createNiceMock(Service.class);
+    expect(zookeeper.getName()).andReturn(serviceName).atLeastOnce();
+    expect(zookeeper.getServiceComponents()).andReturn(new HashMap<String, ServiceComponent>()).once();
+    zookeeper.setDesiredRepositoryVersion(repoVersion220);
+    expectLastCall().once();
+
+    Cluster cluster = createNiceMock(Cluster.class);
+    expect(cluster.getCurrentStackVersion()).andReturn(stack211).atLeastOnce();
+    expect(cluster.getDesiredStackVersion()).andReturn(stack220);
+    expect(cluster.getDesiredConfigs()).andReturn(desiredConfigurations);
+    expect(cluster.getDesiredConfigByType(fooSite)).andReturn(fooConfig);
+    expect(cluster.getService(serviceName)).andReturn(zookeeper);
+
+    ConfigHelper configHelper = m_injector.getInstance(ConfigHelper.class);
+
+    expect(configHelper.getDefaultProperties(stack211, serviceName)).andReturn(stack211Configs).anyTimes();
+    expect(configHelper.getDefaultProperties(stack220, serviceName)).andReturn(stack220Configs).anyTimes();
+
+    Capture<Map<String, Map<String, String>>> expectedConfigurationsCapture = EasyMock.newCapture();
+
+    configHelper.createConfigTypes(EasyMock.anyObject(Cluster.class),
+        EasyMock.anyObject(StackId.class), EasyMock.anyObject(AmbariManagementController.class),
+        EasyMock.capture(expectedConfigurationsCapture), EasyMock.anyObject(String.class),
+        EasyMock.anyObject(String.class));
+
+    expectLastCall().once();
+
+    // mock the service config DAO and replay it
+    ServiceConfigEntity zookeeperServiceConfig = createNiceMock(ServiceConfigEntity.class);
+    expect(zookeeperServiceConfig.getClusterConfigEntities()).andReturn(
+        Lists.newArrayList(fooConfigEntity));
+
+    ServiceConfigDAO serviceConfigDAOMock = m_injector.getInstance(ServiceConfigDAO.class);
+    List<ServiceConfigEntity> latestServiceConfigs = Lists.newArrayList(zookeeperServiceConfig);
+    expect(serviceConfigDAOMock.getLastServiceConfigsForService(EasyMock.anyLong(),
+        eq(serviceName))).andReturn(latestServiceConfigs).once();
+
+    UpgradeContext context = createNiceMock(UpgradeContext.class);
+    expect(context.getCluster()).andReturn(cluster).atLeastOnce();
+    expect(context.getType()).andReturn(UpgradeType.ROLLING).atLeastOnce();
+    expect(context.getDirection()).andReturn(Direction.UPGRADE).atLeastOnce();
+    expect(context.getRepositoryVersion()).andReturn(repoVersion220).anyTimes();
+    expect(context.getSupportedServices()).andReturn(Sets.newHashSet(serviceName)).atLeastOnce();
+    expect(context.getSourceRepositoryVersion(EasyMock.anyString())).andReturn(repoVersion211).atLeastOnce();
+    expect(context.getTargetRepositoryVersion(EasyMock.anyString())).andReturn(repoVersion220).atLeastOnce();
+    expect(context.getOrchestrationType()).andReturn(RepositoryType.STANDARD).anyTimes();
+    expect(context.getHostRoleCommandFactory()).andStubReturn(m_injector.getInstance(HostRoleCommandFactory.class));
+    expect(context.getRoleGraphFactory()).andStubReturn(m_injector.getInstance(RoleGraphFactory.class));
+
+    replayAll();
+
+    UpgradeHelper upgradeHelper = m_injector.getInstance(UpgradeHelper.class);
+    upgradeHelper.updateDesiredRepositoriesAndConfigs(context);
+
+    Map<String, Map<String, String>> expectedConfigurations = expectedConfigurationsCapture.getValue();
+    Map<String, String> expectedFooType = expectedConfigurations.get(fooSite);
+
+    // As the upgrade pack did not have any Flume updates, its configs should
+    // not be updated.
+    assertEquals(1, expectedConfigurations.size());
+    assertEquals(1, expectedFooType.size());
+
+    assertEquals("stack-220-original", expectedFooType.get(fooPropertyName));
+  }
 
   private class MockModule implements Module {
 
@@ -325,6 +460,7 @@ public class StackUpgradeConfigurationMergeTest extends EasyMockSupport {
       binder.bind(ServiceConfigDAO.class).toInstance(createNiceMock(ServiceConfigDAO.class));
       binder.install(new FactoryModuleBuilder().build(UpgradeContextFactory.class));
       binder.bind(HostRoleCommandFactory.class).to(HostRoleCommandFactoryImpl.class);
+      binder.bind(AmbariMetaInfo.class).toInstance(m_metainfo);
 
       binder.requestStaticInjection(UpgradeResourceProvider.class);
     }