[
https://issues.apache.org/jira/browse/YARN-9561?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16959191#comment-16959191
]
Jim Brennan commented on YARN-9561:
-----------------------------------
Thanks for the updated patch [~ebadger]!
I had a few comments on patch 005:
runc_write_config()
* add_std_mounts_json() - should we increase shm to 8GB like we did
internally?
main.c
- [BUG] main() - missing {{break}} statement after
{{RUN_AS_USER_SYNC_YARN_SYSFS}} block.
test_string_utils.cc
- TEST_F(TestStringUtils, test_strbuf_detach)
this test would be a little better if it moved the buf contents assert to after
the third append format and/or included a check that sb.buffer != buf (after
that last append).
- TEST_F(TestStringUtils, test_strbuf_realloc)
looks like this has some left over debug std::cout lines?
test_runc_util.cc
- build_process_struct()
returns true when is fails and false when it succeeds?
- build_mounts_json()
is there a reason for three options string arrays? The first two are
identical. Can’t you just have options_ro and options_rw?
- (nit) unindented line: {{remove(pid_file);}}
- Would be nice to move these lines into a function - they are repeated a lot:
{noformat}
std::string container_executor_cfg_contents = "[runc]\n "
"runc.allowed.rw-mounts=/opt,/var,/usr/bin/cut,/usr/bin/awk\n "
"runc.allowed.ro-mounts=/etc/passwd";
ret = setup_container_executor_cfg(container_executor_cfg_contents);
ASSERT_EQ(ret, 0) << "Container executor cfg setup failed\n"; {noformat}
> Add C changes for the new RuncContainerRuntime
> ----------------------------------------------
>
> Key: YARN-9561
> URL: https://issues.apache.org/jira/browse/YARN-9561
> Project: Hadoop YARN
> Issue Type: Sub-task
> Reporter: Eric Badger
> Assignee: Eric Badger
> Priority: Major
> Attachments: YARN-9561.001.patch, YARN-9561.002.patch,
> YARN-9561.003.patch, YARN-9561.004.patch, YARN-9561.005.patch
>
>
> This JIRA will be used to add the C changes to the container-executor native
> binary that are necessary for the new RuncContainerRuntime. There should be
> no changes to existing code paths.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
