YARN-6403. Invalid local resource request can raise NPE and make NM exit. Contributed by Tao Yang
This commit is contained in:
parent
34ab8e73d4
commit
e8071aa249
|
@ -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();
|
||||
|
|
|
@ -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"));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -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;
|
||||
|
@ -996,6 +997,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);
|
||||
|
||||
|
|
|
@ -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;
|
||||
|
|
|
@ -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"));
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue