Jason Lowe commented on YARN-7677:

Thanks for updating the patch!  Looks good overall, just a few nits:

It might be useful to reduce the varname duplication in sanitizeEnv and help 
avoid future copy-n-paste errors by creating a small helper function that takes 
the NM var set, the env to update, the variable name, and the value and updates 
both the env and the nm var set.

I don't think writeLaunchEnv should expect nmVars to be null.  Then NM will 
always have at least one variable to set for each container (e.g.: 
CONTAINER_ID), so in practice this will never be null.  It can only be null for 
tests, and I would argue the test code is responsible for passing something 
sane (e.g: Collections.emptySet());

sanitizeEnv and sanitizeWindowsEnv should take a Set rather than a 
LinkedHashSet.  Those method implementations do not require the incoming set to 
be a LinkedHashSet for them to do what they do (even though in practice that is 
what it will be).

> Docker image cannot set HADOOP_CONF_DIR
> ---------------------------------------
>                 Key: YARN-7677
>                 URL: https://issues.apache.org/jira/browse/YARN-7677
>             Project: Hadoop YARN
>          Issue Type: Bug
>    Affects Versions: 3.0.0
>            Reporter: Eric Badger
>            Assignee: Jim Brennan
>            Priority: Major
>         Attachments: YARN-7677.001.patch, YARN-7677.002.patch, 
> YARN-7677.003.patch, YARN-7677.004.patch
> Currently, {{HADOOP_CONF_DIR}} is being put into the task environment whether 
> it's set by the user or not. It completely bypasses the whitelist and so 
> there is no way for a task to not have {{HADOOP_CONF_DIR}} set. This causes 
> problems in the Docker use case where Docker containers will set up their own 
> environment and have their own {{HADOOP_CONF_DIR}} preset in the image 
> itself. 

This message was sent by Atlassian JIRA

To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org

Reply via email to