[
https://issues.apache.org/jira/browse/YARN-7626?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16363078#comment-16363078
]
Miklos Szegedi commented on YARN-7626:
--------------------------------------
Thank you for the patch [~Zian Chen].
{code:java}
return !(len > 2 && str[0] == '^' && str[len-1] == '$');{code}
Optional: I think this is still misleading. is_regex should return one on
success and 0 on failure.
{code:java}
// Iterate each permitted values.{code}
Optional: I think it would be better to write 'Iterate through each permitted
value'
{code:java}
'/dev/nvidia1:/dev/nvidia1'
if (prefix == 0) {
ret = strcmp(values[i], permitted_values[j]);
} else {
// If permitted-Values[j] is a REGEX, use REGEX to compare
if (is_regex(permitted_values[j]) == 0) {
ret = validate_volume_name_with_argument(values[i],
permitted_values[j]);
} else {
ret = strncmp(values[i], permitted_values[j], tmp_ptr -
values[i]);
}
}
{code}
Technically the code, where prefix is not null including the regex match,
should check only the characters before the prefix :. It is checking now the
whole value[i], you should apply the regex only to [values[i] ... tmp_ptr].
{code:java}
/**
* Helper function to help normalize mounts for checking if mounts are
* permitted. The function does the following -
* 1. Find the canonical path for mount using realpath
* 2. If the path is a directory, add a '/' at the end (if not present)
* 3. Return a copy of the canonicalised path(to be freed by the caller)
* @param mount path to be canonicalised
* @return pointer to canonicalised path, NULL on error
*/
static char* normalize_mount(const char* mount, int isUserMount) {
{code}
There is no @param documentation for isUserMount, in fact I would name it
isRegexAllowed to avoid confusion.
{code:java}
const char *container_executor_cfg_path = normalize_mount(get_config_path(""),
1);{code}
I do not understand why the config path could be a regex.
{code:java}
tmp_path_buffer[0] = normalize_mount(mount_src, 1);{code}
Should not this be 0, too?
I have a few conceptual issues with the latest patch.
# First of all, normalize_mounts, walks through the permitted mounts and it
resolves symlinks but it does not resolve symlinks, if isUserMount (isRegex) is
1. What if the regex resolves to a symlink? I think it would probably be more
future proof, if normalize_mounts applied the regex to the directory tree and
then called the original normalize_mount on the resulting file names, that
returns the real path for each. This would eliminate the need for passing
isUserMount all the way through the call structure. It would also help to avoid
issues that appear with invalid regexes, etc.
# Technically a regex without the ^$ pair is a valid regex. It would be more
precise and future proof to mark regexes with a prefix like
{{regex:/dev/device[0-9]+}}. In this case we would not need to use just a
subset for matching.
> Allow regular expression matching in container-executor.cfg for devices and
> named docker volumes mount
> ------------------------------------------------------------------------------------------------------
>
> Key: YARN-7626
> URL: https://issues.apache.org/jira/browse/YARN-7626
> Project: Hadoop YARN
> Issue Type: New Feature
> Reporter: Zian Chen
> Assignee: Zian Chen
> Priority: Major
> Attachments: YARN-7626.001.patch, YARN-7626.002.patch,
> YARN-7626.003.patch, YARN-7626.004.patch, YARN-7626.005.patch,
> YARN-7626.006.patch, YARN-7626.007.patch, YARN-7626.008.patch
>
>
> Currently when we config some of the GPU devices related fields (like ) in
> container-executor.cfg, these fields are generated based on different driver
> versions or GPU device names. We want to enable regular expression matching
> so that user don't need to manually set up these fields when config
> container-executor.cfg,
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
