[ 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: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org