You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ambari.apache.org by nc...@apache.org on 2015/12/01 19:20:21 UTC
[31/50] ambari git commit: AMBARI-14040. Update cluster fails with
NPE if the request contains null configuration property values (Sebastian
Toader via alejandro)
AMBARI-14040. Update cluster fails with NPE if the request contains null configuration property values (Sebastian Toader via alejandro)
Project: http://git-wip-us.apache.org/repos/asf/ambari/repo
Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/a62c4b8a
Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/a62c4b8a
Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/a62c4b8a
Branch: refs/heads/branch-dev-patch-upgrade
Commit: a62c4b8aad56fce846603b9ad45ceead0300ae29
Parents: 0ada28a
Author: Alejandro Fernandez <af...@hortonworks.com>
Authored: Mon Nov 30 16:01:50 2015 -0800
Committer: Alejandro Fernandez <af...@hortonworks.com>
Committed: Mon Nov 30 16:01:50 2015 -0800
----------------------------------------------------------------------
.../AmbariManagementControllerImpl.java | 4 +-
.../internal/StackAdvisorResourceProvider.java | 8 +-
.../AmbariManagementControllerImplTest.java | 90 +++++++++++++++++++-
.../StackAdvisorResourceProviderTest.java | 40 +++++++++
4 files changed, 134 insertions(+), 8 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/ambari/blob/a62c4b8a/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java
index 0e3e7b8..c0dc342 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java
@@ -1432,8 +1432,8 @@ public class AmbariManagementControllerImpl implements AmbariManagementControlle
break;
} else {
for (Entry<String, String> property : requestConfigProperties.entrySet()) {
- if (!StringUtils.equals(property.getValue(),clusterConfigProperties.get(property.getKey()))) {
- isConfigurationCreationNeeded =true;
+ if (!StringUtils.equals(property.getValue(), clusterConfigProperties.get(property.getKey()))) {
+ isConfigurationCreationNeeded = true;
break;
}
}
http://git-wip-us.apache.org/repos/asf/ambari/blob/a62c4b8a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/StackAdvisorResourceProvider.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/StackAdvisorResourceProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/StackAdvisorResourceProvider.java
index fe3d006..0ad9126 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/StackAdvisorResourceProvider.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/StackAdvisorResourceProvider.java
@@ -266,8 +266,12 @@ public abstract class StackAdvisorResourceProvider extends ReadOnlyResourceProvi
siteMap.put(propertiesProperty, propertiesMap);
}
- String value = properties.get(property).toString();
- propertiesMap.put(propertyName, value);
+ Object propVal = properties.get(property);
+ if (propVal != null)
+ propertiesMap.put(propertyName, propVal.toString());
+ else
+ LOG.info(String.format("No value specified for configuration property, name = %s ", property));
+
} catch (Exception e) {
LOG.debug(String.format("Error handling configuration property, name = %s", property), e);
// do nothing
http://git-wip-us.apache.org/repos/asf/ambari/blob/a62c4b8a/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java
index ca3ca36..e2ec5e0 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java
@@ -18,6 +18,8 @@
package org.apache.ambari.server.controller;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Maps;
import com.google.gson.Gson;
import com.google.inject.Binder;
import com.google.inject.Guice;
@@ -52,8 +54,10 @@ import org.apache.ambari.server.security.ldap.LdapBatchDto;
import org.apache.ambari.server.state.Cluster;
import org.apache.ambari.server.state.Clusters;
import org.apache.ambari.server.state.ComponentInfo;
+import org.apache.ambari.server.state.ConfigImpl;
import org.apache.ambari.server.state.Host;
import org.apache.ambari.server.state.MaintenanceState;
+import org.apache.ambari.server.state.PropertyInfo;
import org.apache.ambari.server.state.RepositoryInfo;
import org.apache.ambari.server.state.SecurityType;
import org.apache.ambari.server.state.Service;
@@ -85,7 +89,20 @@ import static org.apache.ambari.server.agent.ExecutionCommand.KeyNames.HOST_SYS_
import static org.apache.ambari.server.agent.ExecutionCommand.KeyNames.JAVA_VERSION;
import static org.apache.ambari.server.agent.ExecutionCommand.KeyNames.STACK_NAME;
import static org.apache.ambari.server.agent.ExecutionCommand.KeyNames.STACK_VERSION;
-import static org.easymock.EasyMock.*;
+import static org.easymock.EasyMock.anyBoolean;
+import static org.easymock.EasyMock.anyObject;
+import static org.easymock.EasyMock.capture;
+import static org.easymock.EasyMock.captureBoolean;
+import static org.easymock.EasyMock.createMock;
+import static org.easymock.EasyMock.createMockBuilder;
+import static org.easymock.EasyMock.createNiceMock;
+import static org.easymock.EasyMock.createStrictMock;
+import static org.easymock.EasyMock.eq;
+import static org.easymock.EasyMock.expect;
+import static org.easymock.EasyMock.expectLastCall;
+import static org.easymock.EasyMock.replay;
+import static org.easymock.EasyMock.reset;
+import static org.easymock.EasyMock.verify;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
@@ -346,7 +363,7 @@ public class AmbariManagementControllerImplTest {
expect(service.getName()).andReturn("service");
expect(service.getServiceComponent("component")).andThrow(
- new ServiceComponentNotFoundException("cluster", "service", "component"));
+ new ServiceComponentNotFoundException("cluster", "service", "component"));
expect(service.getDesiredStackVersion()).andReturn(stackId);
expect(stackId.getStackName()).andReturn("stack");
expect(stackId.getStackVersion()).andReturn("1.0");
@@ -356,7 +373,7 @@ public class AmbariManagementControllerImplTest {
expect(service.getServiceComponents()).andReturn(componentsMap);
expect(component1.getServiceComponentHosts()).andReturn(Collections.EMPTY_MAP);
expect(component2.getServiceComponentHosts()).andReturn(
- Collections.<String, ServiceComponentHost>singletonMap("anyHost", null));
+ Collections.<String, ServiceComponentHost>singletonMap("anyHost", null));
ServiceInfo serviceInfo = createNiceMock(ServiceInfo.class);
ComponentInfo compInfo = createNiceMock(ComponentInfo.class);
@@ -394,7 +411,7 @@ public class AmbariManagementControllerImplTest {
expect(service.getServiceComponents()).andReturn(componentsMap);
expect(component1.getServiceComponentHosts()).andReturn(Collections.EMPTY_MAP);
expect(component2.getServiceComponentHosts()).andReturn(
- Collections.<String, ServiceComponentHost>singletonMap("anyHost", null));
+ Collections.<String, ServiceComponentHost>singletonMap("anyHost", null));
ServiceInfo serviceInfo = createNiceMock(ServiceInfo.class);
expect(serviceInfo.getClientComponent()).andReturn(null);
@@ -566,6 +583,71 @@ public class AmbariManagementControllerImplTest {
}
/**
+ * Ensure that processing update request does not fail on configuration
+ * properties with no value specified (no value = null reference value)
+ */
+ @Test
+ public void testUpdateClustersWithNullConfigPropertyValues() throws Exception {
+ // member state mocks
+ Capture<AmbariManagementController> controllerCapture = new Capture<AmbariManagementController>();
+ Injector injector = createStrictMock(Injector.class);
+ Cluster cluster = createNiceMock(Cluster.class);
+ ActionManager actionManager = createNiceMock(ActionManager.class);
+ ClusterRequest clusterRequest = createNiceMock(ClusterRequest.class);
+
+ // requests
+ Set<ClusterRequest> setRequests = Collections.singleton(clusterRequest);
+
+ KerberosHelper kerberosHelper = createStrictMock(KerberosHelper.class);
+ // expectations
+ injector.injectMembers(capture(controllerCapture));
+ expect(injector.getInstance(Gson.class)).andReturn(null);
+ expect(injector.getInstance(MaintenanceStateHelper.class)).andReturn(null);
+ expect(injector.getInstance(KerberosHelper.class)).andReturn(kerberosHelper);
+ expect(clusterRequest.getClusterName()).andReturn("clusterNew").anyTimes();
+ expect(clusterRequest.getClusterId()).andReturn(1L).anyTimes();
+
+ ConfigurationRequest configReq = new ConfigurationRequest();
+ final Map<String, String> configReqProps = Maps.newHashMap();
+ configReqProps.put("p1", null);
+ configReq.setProperties(configReqProps);
+
+ expect(clusterRequest.getDesiredConfig()).andReturn(ImmutableList.of(configReq)).anyTimes();
+ expect(clusters.getClusterById(1L)).andReturn(cluster).anyTimes();
+ expect(cluster.getClusterName()).andReturn("clusterOld").anyTimes();
+ expect(cluster.getConfigPropertiesTypes(anyObject(String.class))).andReturn(Maps.<PropertyInfo.PropertyType, Set<String>>newHashMap()).anyTimes();
+ expect(cluster.getDesiredConfigByType(anyObject(String.class))).andReturn(new ConfigImpl("config-type") {
+ @Override
+ public Map<String, Map<String, String>> getPropertiesAttributes() {
+ return Maps.newHashMap();
+ }
+
+ @Override
+ public Map<String, String> getProperties() {
+ return configReqProps;
+ }
+
+ }).anyTimes();
+
+ cluster.addSessionAttributes(anyObject(Map.class));
+ expectLastCall().once();
+
+ cluster.setClusterName("clusterNew");
+ expectLastCall();
+
+ // replay mocks
+ replay(actionManager, cluster, clusters, injector, clusterRequest, sessionManager);
+
+ // test
+ AmbariManagementController controller = new AmbariManagementControllerImpl(actionManager, clusters, injector);
+ controller.updateClusters(setRequests, null);
+
+ // assert and verify
+ assertSame(controller, controllerCapture.getValue());
+ verify(actionManager, cluster, clusters, injector, clusterRequest, sessionManager);
+ }
+
+ /**
* Ensure that when the cluster is updated KerberosHandler.toggleKerberos is not invoked unless
* the security type is altered
*/
http://git-wip-us.apache.org/repos/asf/ambari/blob/a62c4b8a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/StackAdvisorResourceProviderTest.java
----------------------------------------------------------------------
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/StackAdvisorResourceProviderTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/StackAdvisorResourceProviderTest.java
index 8c5337b..e3b89b8 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/StackAdvisorResourceProviderTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/StackAdvisorResourceProviderTest.java
@@ -33,6 +33,7 @@ import java.util.Set;
import static org.apache.ambari.server.controller.internal.StackAdvisorResourceProvider.CONFIGURATIONS_PROPERTY_ID;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.mock;
@@ -73,4 +74,43 @@ public class StackAdvisorResourceProviderTest {
assertEquals("string", properties.get("string_prop"));
assertEquals("[array1, array2]", properties.get("array_prop"));
}
+
+ @Test
+ public void testCalculateConfigurationsWithNullPropertyValues() throws Exception {
+
+ Map<Resource.Type, String> keyPropertyIds = Collections.emptyMap();
+ Set<String> propertyIds = Collections.emptySet();
+ AmbariManagementController ambariManagementController = mock(AmbariManagementController.class);
+ RecommendationResourceProvider provider = new RecommendationResourceProvider(propertyIds,
+ keyPropertyIds, ambariManagementController);
+
+ Request request = mock(Request.class);
+ Set<Map<String, Object>> propertiesSet = new HashSet<Map<String, Object>>();
+ Map<String, Object> propertiesMap = new HashMap<String, Object>();
+ propertiesMap.put(CONFIGURATIONS_PROPERTY_ID + "site/properties/string_prop", null); //null value means no value specified for the property
+ List<Object> array = new ArrayList<Object>();
+ array.add("array1");
+ array.add("array2");
+ propertiesMap.put(CONFIGURATIONS_PROPERTY_ID + "site/properties/array_prop", array);
+ propertiesSet.add(propertiesMap);
+
+ doReturn(propertiesSet).when(request).getProperties();
+
+ Map<String, Map<String, Map<String, String>>> calculatedConfigurations = provider.calculateConfigurations(request);
+
+ assertNotNull(calculatedConfigurations);
+ assertEquals(1, calculatedConfigurations.size());
+ Map<String, Map<String, String>> site = calculatedConfigurations.get("site");
+ assertNotNull(site);
+ assertEquals(1, site.size());
+ Map<String, String> properties = site.get("properties");
+ assertNotNull(properties);
+
+ assertEquals("[array1, array2]", properties.get("array_prop"));
+
+
+ // config properties with null values should be ignored
+ assertFalse(properties.containsKey("string_prop"));
+
+ }
}