YARN-6403. Invalid local resource request can raise NPE and make NM exit. Contributed by Tao Yang

(cherry picked from commit e8071aa249)
This commit is contained in:
Jason Lowe 2017-04-05 12:30:45 -05:00
parent 4cdda4ba87
commit 903278559a
5 changed files with 108 additions and 0 deletions

View File

@ -208,11 +208,24 @@ extends ContainerLaunchContext {
final Map<String, LocalResource> localResources) {
if (localResources == null)
return;
checkLocalResources(localResources);
initLocalResources();
this.localResources.clear();
this.localResources.putAll(localResources);
}
private void checkLocalResources(Map<String, LocalResource> localResources) {
for (Map.Entry<String, LocalResource> rsrcEntry : localResources
.entrySet()) {
if (rsrcEntry.getValue() == null
|| rsrcEntry.getValue().getResource() == null) {
throw new NullPointerException(
"Null resource URL for local resource " + rsrcEntry.getKey() + " : "
+ rsrcEntry.getValue());
}
}
}
private void addLocalResourcesToProto() {
maybeInitBuilder();
builder.clearLocalResources();

View File

@ -30,6 +30,10 @@ import org.apache.hadoop.security.Credentials;
import org.apache.hadoop.yarn.api.records.ApplicationAccessType;
import org.apache.hadoop.yarn.api.records.ContainerLaunchContext;
import org.apache.hadoop.yarn.api.records.LocalResource;
import org.apache.hadoop.yarn.api.records.LocalResourceType;
import org.apache.hadoop.yarn.api.records.LocalResourceVisibility;
import org.apache.hadoop.yarn.factories.RecordFactory;
import org.apache.hadoop.yarn.factory.providers.RecordFactoryProvider;
import org.junit.Assert;
import org.junit.Test;
@ -66,4 +70,29 @@ public class TestApplicationClientProtocolRecords {
clcProto.getEnvironment().get("testCLCPBImplNullEnv"));
}
/*
* This test validates the scenario in which the client sets a null value for
* local resource URL.
*/
@Test
public void testCLCPBImplNullResourceURL() throws IOException {
RecordFactory recordFactory = RecordFactoryProvider.getRecordFactory(null);
try {
LocalResource rsrc_alpha = recordFactory.newRecordInstance(LocalResource.class);
rsrc_alpha.setResource(null);
rsrc_alpha.setSize(-1);
rsrc_alpha.setVisibility(LocalResourceVisibility.APPLICATION);
rsrc_alpha.setType(LocalResourceType.FILE);
rsrc_alpha.setTimestamp(System.currentTimeMillis());
Map<String, LocalResource> localResources =
new HashMap<String, LocalResource>();
localResources.put("null_url_resource", rsrc_alpha);
ContainerLaunchContext containerLaunchContext = recordFactory.newRecordInstance(ContainerLaunchContext.class);
containerLaunchContext.setLocalResources(localResources);
Assert.fail("Setting an invalid local resource should be an error!");
} catch (NullPointerException e) {
Assert.assertTrue(e.getMessage().contains("Null resource URL for local resource"));
}
}
}

View File

@ -64,6 +64,7 @@ import org.apache.hadoop.yarn.api.records.ContainerId;
import org.apache.hadoop.yarn.api.records.ContainerLaunchContext;
import org.apache.hadoop.yarn.api.records.ContainerState;
import org.apache.hadoop.yarn.api.records.ContainerStatus;
import org.apache.hadoop.yarn.api.records.LocalResource;
import org.apache.hadoop.yarn.api.records.LocalResourceVisibility;
import org.apache.hadoop.yarn.api.records.LogAggregationContext;
import org.apache.hadoop.yarn.api.records.NodeId;
@ -963,6 +964,15 @@ public class ContainerManagerImpl extends CompositeService implements
ContainerLaunchContext launchContext = request.getContainerLaunchContext();
// Sanity check for local resources
for (Map.Entry<String, LocalResource> rsrc : launchContext
.getLocalResources().entrySet()) {
if (rsrc.getValue() == null || rsrc.getValue().getResource() == null) {
throw new YarnException(
"Null resource URL for local resource " + rsrc.getKey() + " : " + rsrc.getValue());
}
}
Credentials credentials =
YarnServerSecurityUtils.parseCredentials(launchContext);

View File

@ -364,6 +364,17 @@ public class TestContainerManagerWithLCE extends TestContainerManager {
super.testContainerRestart();
}
@Override
public void testStartContainerFailureWithInvalidLocalResource() throws Exception {
// Don't run the test if the binary is not available.
if (!shouldRunTest()) {
LOG.info("LCE binary path is not passed. Not running the test");
return;
}
LOG.info("Running testStartContainerFailureWithInvalidLocalResource");
super.testStartContainerFailureWithInvalidLocalResource();
}
private boolean shouldRunTest() {
return System
.getProperty(YarnConfiguration.NM_LINUX_CONTAINER_EXECUTOR_PATH) != null;

View File

@ -1859,4 +1859,49 @@ public class TestContainerManager extends BaseContainerManagerTest {
Assert.assertEquals(signal, signalContext.getSignal());
}
}
@Test
public void testStartContainerFailureWithInvalidLocalResource()
throws Exception {
containerManager.start();
LocalResource rsrc_alpha =
recordFactory.newRecordInstance(LocalResource.class);
rsrc_alpha.setResource(null);
rsrc_alpha.setSize(-1);
rsrc_alpha.setVisibility(LocalResourceVisibility.APPLICATION);
rsrc_alpha.setType(LocalResourceType.FILE);
rsrc_alpha.setTimestamp(System.currentTimeMillis());
Map<String, LocalResource> localResources =
new HashMap<String, LocalResource>();
localResources.put("invalid_resource", rsrc_alpha);
ContainerLaunchContext containerLaunchContext =
recordFactory.newRecordInstance(ContainerLaunchContext.class);
ContainerLaunchContext spyContainerLaunchContext =
Mockito.spy(containerLaunchContext);
Mockito.when(spyContainerLaunchContext.getLocalResources())
.thenReturn(localResources);
ContainerId cId = createContainerId(0);
String user = "start_container_fail";
Token containerToken =
createContainerToken(cId, DUMMY_RM_IDENTIFIER, context.getNodeId(),
user, context.getContainerTokenSecretManager());
StartContainerRequest request = StartContainerRequest
.newInstance(spyContainerLaunchContext, containerToken);
// start containers
List<StartContainerRequest> startRequest =
new ArrayList<StartContainerRequest>();
startRequest.add(request);
StartContainersRequest requestList =
StartContainersRequest.newInstance(startRequest);
StartContainersResponse response =
containerManager.startContainers(requestList);
Assert.assertTrue(response.getFailedRequests().size() == 1);
Assert.assertTrue(response.getSuccessfullyStartedContainers().size() == 0);
Assert.assertTrue(response.getFailedRequests().containsKey(cId));
Assert.assertTrue(response.getFailedRequests().get(cId).getMessage()
.contains("Null resource URL for local resource"));
}
}