Repository: ambari Updated Branches: refs/heads/branch-2.6 629f3ee6d -> 070f69d40
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/070f69d4 Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/070f69d4 Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/070f69d4 Branch: refs/heads/branch-2.6 Commit: 070f69d4023ce8754dcbb6105fa57ef9f987dd89 Parents: 629f3ee Author: Jonathan Hurley <jhur...@hortonworks.com> Authored: Tue Sep 19 13:36:07 2017 -0400 Committer: Jonathan Hurley <jhur...@hortonworks.com> Committed: Wed Sep 20 10:10:47 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/070f69d4/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 9aa9e31..77e2d47 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 @@ -984,6 +985,11 @@ public class UpgradeHelper { ConfigHelper configHelper = m_configHelperProvider.get(); + // 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); + // populate a map of default configurations for the service on the old // stack (this is used when determining if a property has been // customized and should be overriden with the new stack value) @@ -1047,8 +1053,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 @@ -1067,8 +1072,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(); @@ -1085,17 +1089,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); } } @@ -1149,4 +1158,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/070f69d4/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 474eb50..778b21e 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,6 +57,7 @@ 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; @@ -66,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; @@ -96,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 @@ -287,6 +293,135 @@ 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 { @@ -326,6 +461,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); }
