From 3b9752f6176ef82e470c39da1d2b058a0c77712a Mon Sep 17 00:00:00 2001 From: Felipe Padula Sanches Date: Wed, 9 Oct 2024 12:13:02 +1300 Subject: [PATCH 01/11] Create extension file --- src/rocker/ulimit_extension.py | 40 ++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 src/rocker/ulimit_extension.py diff --git a/src/rocker/ulimit_extension.py b/src/rocker/ulimit_extension.py new file mode 100644 index 0000000..d791e26 --- /dev/null +++ b/src/rocker/ulimit_extension.py @@ -0,0 +1,40 @@ +# Copyright 2019 Open Source Robotics Foundation + +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from argparse import ArgumentTypeError +import os +from rocker.extensions import RockerExtension + + +class Ulimit(RockerExtension): + + ARG_DOCKER_ULIMIT = "--ulimit" + ARG_ROCKER_VOLUME = "--ulimit" + name = 'ulimit' + + @classmethod + def get_name(cls): + return cls.name + + def get_docker_args(self, cliargs): + args = [''] + return ' '.join(args) + + @staticmethod + def register_arguments(parser, defaults): + parser.add_argument(Ulimit.ARG_ROCKER_VOLUME, + type=str, + nargs='+', + action='append', + help='ulimit options to add into the container.') From ecf1f6ef05bf959cc5e51f087bb7b84f6f9288da Mon Sep 17 00:00:00 2001 From: Felipe Padula Sanches Date: Wed, 9 Oct 2024 12:25:10 +1300 Subject: [PATCH 02/11] Barebones extension --- src/rocker/ulimit_extension.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/rocker/ulimit_extension.py b/src/rocker/ulimit_extension.py index d791e26..055e422 100644 --- a/src/rocker/ulimit_extension.py +++ b/src/rocker/ulimit_extension.py @@ -21,15 +21,13 @@ class Ulimit(RockerExtension): ARG_DOCKER_ULIMIT = "--ulimit" ARG_ROCKER_VOLUME = "--ulimit" - name = 'ulimit' - @classmethod - def get_name(cls): - return cls.name + @staticmethod + def get_name(): + return 'ulimit' def get_docker_args(self, cliargs): - args = [''] - return ' '.join(args) + return '' @staticmethod def register_arguments(parser, defaults): From c07010a61f3bdc6c6bf8fdc378fcda220bbfdf5b Mon Sep 17 00:00:00 2001 From: Felipe Padula Sanches Date: Wed, 9 Oct 2024 12:26:30 +1300 Subject: [PATCH 03/11] Add ulimit extension to setup.py --- setup.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/setup.py b/setup.py index 4d66983..7fe8106 100644 --- a/setup.py +++ b/setup.py @@ -43,7 +43,7 @@ 'console_scripts': [ 'rocker = rocker.cli:main', 'detect_docker_image_os = rocker.cli:detect_image_os', - ], + ], 'rocker.extensions': [ 'cuda = rocker.nvidia_extension:Cuda', 'devices = rocker.extensions:Devices', @@ -66,8 +66,9 @@ 'user = rocker.extensions:User', 'volume = rocker.volume_extension:Volume', 'x11 = rocker.nvidia_extension:X11', + 'ulimit = rocker.ulimit_extension:Ulimit', ] - }, + }, 'author': 'Tully Foote', 'author_email': 'tfoote@osrfoundation.org', 'keywords': ['Docker'], @@ -91,4 +92,3 @@ } setup(**kwargs) - From 99e8c48650875a80d958527b26599ea46480166b Mon Sep 17 00:00:00 2001 From: Felipe Padula Sanches Date: Wed, 9 Oct 2024 13:37:19 +1300 Subject: [PATCH 04/11] Add expected format to help message --- src/rocker/ulimit_extension.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/rocker/ulimit_extension.py b/src/rocker/ulimit_extension.py index 055e422..0583ab6 100644 --- a/src/rocker/ulimit_extension.py +++ b/src/rocker/ulimit_extension.py @@ -14,13 +14,11 @@ from argparse import ArgumentTypeError import os -from rocker.extensions import RockerExtension +from rocker.extensions import RockerExtension, name_to_argument class Ulimit(RockerExtension): - - ARG_DOCKER_ULIMIT = "--ulimit" - ARG_ROCKER_VOLUME = "--ulimit" + EXPECTED_FORMAT = "TYPE=SOFT_LIMIT[:HARD_LIMIT]" @staticmethod def get_name(): @@ -31,8 +29,10 @@ def get_docker_args(self, cliargs): @staticmethod def register_arguments(parser, defaults): - parser.add_argument(Ulimit.ARG_ROCKER_VOLUME, + parser.add_argument(name_to_argument(Ulimit.get_name()), type=str, nargs='+', action='append', + metavar=Ulimit.EXPECTED_FORMAT, + default=defaults.get(Ulimit.get_name(), None), help='ulimit options to add into the container.') From acb54ecdfcd00bfddf293d5663da8cc9981531de Mon Sep 17 00:00:00 2001 From: Felipe Padula Sanches Date: Wed, 9 Oct 2024 13:44:08 +1300 Subject: [PATCH 05/11] Add flag processing routine --- src/rocker/ulimit_extension.py | 33 +++++++++++++++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/src/rocker/ulimit_extension.py b/src/rocker/ulimit_extension.py index 0583ab6..105650a 100644 --- a/src/rocker/ulimit_extension.py +++ b/src/rocker/ulimit_extension.py @@ -13,11 +13,17 @@ # limitations under the License. from argparse import ArgumentTypeError -import os +import re from rocker.extensions import RockerExtension, name_to_argument class Ulimit(RockerExtension): + """ + A RockerExtension to handle ulimit settings for Docker containers. + + This extension allows specifying ulimit options in the format TYPE=SOFT_LIMIT[:HARD_LIMIT] + and validates the format before passing them as Docker arguments. + """ EXPECTED_FORMAT = "TYPE=SOFT_LIMIT[:HARD_LIMIT]" @staticmethod @@ -25,7 +31,30 @@ def get_name(): return 'ulimit' def get_docker_args(self, cliargs): - return '' + args = [''] + ulimits = [x for sublist in cliargs[Ulimit.get_name()] for x in sublist] + for ulimit in ulimits: + if self.arg_format_is_valid(ulimit): + args.append(f"--ulimit {ulimit}") + else: + raise ArgumentTypeError( + f"Error processing {Ulimit.get_name()} flag '{ulimit}': expected format" + f" {Ulimit.EXPECTED_FORMAT}") + return ' '.join(args) + + def arg_format_is_valid(self, arg: str): + """ + Validate the format of the ulimit argument. + + Args: + arg (str): The ulimit argument to validate. + + Returns: + bool: True if the format is valid, False otherwise. + """ + ulimit_format = r'(\w+)=(\w+)(:\w+)?$' + match = re.match(ulimit_format, arg) + return match is not None @staticmethod def register_arguments(parser, defaults): From db75af0321baa585ec8a620f1e127588aeaa4363 Mon Sep 17 00:00:00 2001 From: Felipe Padula Sanches Date: Wed, 9 Oct 2024 14:54:22 +1300 Subject: [PATCH 06/11] Add tests for ulimit extension --- test/test_ulimit.py | 66 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 test/test_ulimit.py diff --git a/test/test_ulimit.py b/test/test_ulimit.py new file mode 100644 index 0000000..a5efae2 --- /dev/null +++ b/test/test_ulimit.py @@ -0,0 +1,66 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +import os +import unittest + +from rocker.ulimit_extension import Ulimit + + +class UlimitTest(unittest.TestCase): + """Unit tests for the Ulimit class.""" + + def setUp(self): + self._instance = Ulimit() + self._curr_path = os.path.abspath(os.path.curdir) + self._virtual_path = "/path/in/container" + + def _is_arg_translation_ok(self, mock_cliargs, expected): + docker_args = self._instance.get_docker_args({self._instance.get_name(): [mock_cliargs]}) + print(f"DEBUG: Expected docker_args: {expected}") + print(f"DEBUG: Resulted docker_args: {docker_args}") + self.assertTrue(docker_args == expected) + + def test_args_single_soft(self): + """Test single soft limit argument.""" + mock_cliargs = ["rtprio=99"] + expected = " --ulimit rtprio=99" + self._is_arg_translation_ok(mock_cliargs, expected) + + def test_args_multiple_soft(self): + """Test multiple soft limit arguments.""" + mock_cliargs = ["rtprio=99", "memlock=102400"] + expected = " --ulimit rtprio=99 --ulimit memlock=102400" + self._is_arg_translation_ok(mock_cliargs, expected) + + def test_args_single_hard(self): + """Test single hard limit argument.""" + mock_cliargs = ["nofile=1024:524288"] + expected = " --ulimit nofile=1024:524288" + self._is_arg_translation_ok(mock_cliargs, expected) + + def test_args_multiple_hard(self): + """Test multiple hard limit arguments.""" + mock_cliargs = ["nofile=1024:524288", "rtprio=90:99"] + expected = " --ulimit nofile=1024:524288 --ulimit rtprio=90:99" + self._is_arg_translation_ok(mock_cliargs, expected) + + def test_args_multiple_mix(self): + """Test multiple mixed limit arguments.""" + mock_cliargs = ["rtprio=99", "memlock=102400", "nofile=1024:524288"] + expected = " --ulimit rtprio=99 --ulimit memlock=102400 --ulimit nofile=1024:524288" + self._is_arg_translation_ok(mock_cliargs, expected) From 0a8331eb79325dadfe0ada639d28729a8050fc9b Mon Sep 17 00:00:00 2001 From: Felipe Padula Sanches Date: Wed, 9 Oct 2024 14:56:40 +1300 Subject: [PATCH 07/11] Remove unused imports from test --- test/test_ulimit.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/test/test_ulimit.py b/test/test_ulimit.py index a5efae2..94b8a7c 100644 --- a/test/test_ulimit.py +++ b/test/test_ulimit.py @@ -15,7 +15,6 @@ # specific language governing permissions and limitations # under the License. -import os import unittest from rocker.ulimit_extension import Ulimit @@ -26,8 +25,6 @@ class UlimitTest(unittest.TestCase): def setUp(self): self._instance = Ulimit() - self._curr_path = os.path.abspath(os.path.curdir) - self._virtual_path = "/path/in/container" def _is_arg_translation_ok(self, mock_cliargs, expected): docker_args = self._instance.get_docker_args({self._instance.get_name(): [mock_cliargs]}) From 2558e893a1b057d420bb3bf0bb12f4457c397853 Mon Sep 17 00:00:00 2001 From: Felipe Padula Sanches Date: Thu, 10 Oct 2024 10:32:16 +1300 Subject: [PATCH 08/11] Move asset check outside of arg checking function --- test/test_ulimit.py | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/test/test_ulimit.py b/test/test_ulimit.py index 94b8a7c..ca27bca 100644 --- a/test/test_ulimit.py +++ b/test/test_ulimit.py @@ -16,6 +16,7 @@ # under the License. import unittest +from argparse import ArgumentTypeError from rocker.ulimit_extension import Ulimit @@ -27,37 +28,43 @@ def setUp(self): self._instance = Ulimit() def _is_arg_translation_ok(self, mock_cliargs, expected): - docker_args = self._instance.get_docker_args({self._instance.get_name(): [mock_cliargs]}) - print(f"DEBUG: Expected docker_args: {expected}") - print(f"DEBUG: Resulted docker_args: {docker_args}") - self.assertTrue(docker_args == expected) + is_ok = False + try: + docker_args = self._instance.get_docker_args( + {self._instance.get_name(): [mock_cliargs]}) + is_ok = docker_args == expected + print(f"DEBUG: Expected docker_args: {expected}") + print(f"DEBUG: Resulted docker_args: {docker_args}") + except ArgumentTypeError: + print("DEBUG: Incorrect argument format") + return is_ok def test_args_single_soft(self): """Test single soft limit argument.""" mock_cliargs = ["rtprio=99"] expected = " --ulimit rtprio=99" - self._is_arg_translation_ok(mock_cliargs, expected) + self.assertTrue(self._is_arg_translation_ok(mock_cliargs, expected)) def test_args_multiple_soft(self): """Test multiple soft limit arguments.""" mock_cliargs = ["rtprio=99", "memlock=102400"] expected = " --ulimit rtprio=99 --ulimit memlock=102400" - self._is_arg_translation_ok(mock_cliargs, expected) + self.assertTrue(self._is_arg_translation_ok(mock_cliargs, expected)) def test_args_single_hard(self): """Test single hard limit argument.""" mock_cliargs = ["nofile=1024:524288"] expected = " --ulimit nofile=1024:524288" - self._is_arg_translation_ok(mock_cliargs, expected) + self.assertTrue(self._is_arg_translation_ok(mock_cliargs, expected)) def test_args_multiple_hard(self): """Test multiple hard limit arguments.""" mock_cliargs = ["nofile=1024:524288", "rtprio=90:99"] expected = " --ulimit nofile=1024:524288 --ulimit rtprio=90:99" - self._is_arg_translation_ok(mock_cliargs, expected) + self.assertTrue(self._is_arg_translation_ok(mock_cliargs, expected)) def test_args_multiple_mix(self): """Test multiple mixed limit arguments.""" mock_cliargs = ["rtprio=99", "memlock=102400", "nofile=1024:524288"] expected = " --ulimit rtprio=99 --ulimit memlock=102400 --ulimit nofile=1024:524288" - self._is_arg_translation_ok(mock_cliargs, expected) + self.assertTrue(self._is_arg_translation_ok(mock_cliargs, expected)) From 4661f47d008fec5e668d5ddddccbf1a6e26eadfa Mon Sep 17 00:00:00 2001 From: Felipe Padula Sanches Date: Thu, 10 Oct 2024 10:43:39 +1300 Subject: [PATCH 09/11] Add more test cases --- test/test_ulimit.py | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/test/test_ulimit.py b/test/test_ulimit.py index ca27bca..530d378 100644 --- a/test/test_ulimit.py +++ b/test/test_ulimit.py @@ -68,3 +68,33 @@ def test_args_multiple_mix(self): mock_cliargs = ["rtprio=99", "memlock=102400", "nofile=1024:524288"] expected = " --ulimit rtprio=99 --ulimit memlock=102400 --ulimit nofile=1024:524288" self.assertTrue(self._is_arg_translation_ok(mock_cliargs, expected)) + + def test_args_wrong_single_soft(self): + """Test if single soft limit argument is wrong.""" + mock_cliargs = ["rtprio99"] + expected = " --ulimit rtprio99" + self.assertFalse(self._is_arg_translation_ok(mock_cliargs, expected)) + + def test_args_wrong_multiple_soft(self): + """Test if multiple soft limit arguments are wrong.""" + mock_cliargs = ["rtprio=99", "memlock102400"] + expected = " --ulimit rtprio=99 --ulimit memlock=102400" + self.assertFalse(self._is_arg_translation_ok(mock_cliargs, expected)) + + def test_args_wrong_single_hard(self): + """Test if single hard limit arguments are wrong.""" + mock_cliargs = ["nofile=1024:524288:"] + expected = " --ulimit nofile=1024:524288" + self.assertFalse(self._is_arg_translation_ok(mock_cliargs, expected)) + + def test_args_wrong_multiple_hard(self): + """Test if multiple hard limit arguments are wrong.""" + mock_cliargs = ["nofile1024524288", "rtprio=90:99"] + expected = " --ulimit nofile=1024:524288 --ulimit rtprio=90:99" + self.assertFalse(self._is_arg_translation_ok(mock_cliargs, expected)) + + def test_args_wrong_multiple_mix(self): + """Test if multiple mixed limit arguments are wrong.""" + mock_cliargs = ["rtprio=:", "memlock102400", "nofile1024:524288:"] + expected = " --ulimit rtprio=99 --ulimit memlock=102400 --ulimit nofile=1024:524288" + self.assertFalse(self._is_arg_translation_ok(mock_cliargs, expected)) From 4be243630c5c15d1e3ca5148e2ab2396796e8540 Mon Sep 17 00:00:00 2001 From: Felipe Padula Sanches Date: Fri, 18 Oct 2024 12:23:13 +1300 Subject: [PATCH 10/11] Return tuple instead of bool for better unit test report feedback --- test/test_ulimit.py | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/test/test_ulimit.py b/test/test_ulimit.py index 530d378..4db8a6d 100644 --- a/test/test_ulimit.py +++ b/test/test_ulimit.py @@ -29,72 +29,72 @@ def setUp(self): def _is_arg_translation_ok(self, mock_cliargs, expected): is_ok = False + message_string = "" try: docker_args = self._instance.get_docker_args( {self._instance.get_name(): [mock_cliargs]}) is_ok = docker_args == expected - print(f"DEBUG: Expected docker_args: {expected}") - print(f"DEBUG: Resulted docker_args: {docker_args}") + message_string = f"Expected: '{expected}', got: '{docker_args}'" except ArgumentTypeError: - print("DEBUG: Incorrect argument format") - return is_ok + message_string = "Incorrect argument format" + return (is_ok, message_string) def test_args_single_soft(self): """Test single soft limit argument.""" mock_cliargs = ["rtprio=99"] expected = " --ulimit rtprio=99" - self.assertTrue(self._is_arg_translation_ok(mock_cliargs, expected)) + self.assertTrue(*self._is_arg_translation_ok(mock_cliargs, expected)) def test_args_multiple_soft(self): """Test multiple soft limit arguments.""" mock_cliargs = ["rtprio=99", "memlock=102400"] expected = " --ulimit rtprio=99 --ulimit memlock=102400" - self.assertTrue(self._is_arg_translation_ok(mock_cliargs, expected)) + self.assertTrue(*self._is_arg_translation_ok(mock_cliargs, expected)) def test_args_single_hard(self): """Test single hard limit argument.""" mock_cliargs = ["nofile=1024:524288"] expected = " --ulimit nofile=1024:524288" - self.assertTrue(self._is_arg_translation_ok(mock_cliargs, expected)) + self.assertTrue(*self._is_arg_translation_ok(mock_cliargs, expected)) def test_args_multiple_hard(self): """Test multiple hard limit arguments.""" mock_cliargs = ["nofile=1024:524288", "rtprio=90:99"] expected = " --ulimit nofile=1024:524288 --ulimit rtprio=90:99" - self.assertTrue(self._is_arg_translation_ok(mock_cliargs, expected)) + self.assertTrue(*self._is_arg_translation_ok(mock_cliargs, expected)) def test_args_multiple_mix(self): """Test multiple mixed limit arguments.""" mock_cliargs = ["rtprio=99", "memlock=102400", "nofile=1024:524288"] expected = " --ulimit rtprio=99 --ulimit memlock=102400 --ulimit nofile=1024:524288" - self.assertTrue(self._is_arg_translation_ok(mock_cliargs, expected)) + self.assertTrue(*self._is_arg_translation_ok(mock_cliargs, expected)) def test_args_wrong_single_soft(self): """Test if single soft limit argument is wrong.""" mock_cliargs = ["rtprio99"] expected = " --ulimit rtprio99" - self.assertFalse(self._is_arg_translation_ok(mock_cliargs, expected)) + self.assertFalse(*self._is_arg_translation_ok(mock_cliargs, expected)) def test_args_wrong_multiple_soft(self): """Test if multiple soft limit arguments are wrong.""" mock_cliargs = ["rtprio=99", "memlock102400"] expected = " --ulimit rtprio=99 --ulimit memlock=102400" - self.assertFalse(self._is_arg_translation_ok(mock_cliargs, expected)) + self.assertFalse(*self._is_arg_translation_ok(mock_cliargs, expected)) def test_args_wrong_single_hard(self): """Test if single hard limit arguments are wrong.""" mock_cliargs = ["nofile=1024:524288:"] expected = " --ulimit nofile=1024:524288" - self.assertFalse(self._is_arg_translation_ok(mock_cliargs, expected)) + self.assertFalse(*self._is_arg_translation_ok(mock_cliargs, expected)) def test_args_wrong_multiple_hard(self): """Test if multiple hard limit arguments are wrong.""" mock_cliargs = ["nofile1024524288", "rtprio=90:99"] expected = " --ulimit nofile=1024:524288 --ulimit rtprio=90:99" - self.assertFalse(self._is_arg_translation_ok(mock_cliargs, expected)) + self.assertFalse(*self._is_arg_translation_ok(mock_cliargs, expected)) def test_args_wrong_multiple_mix(self): """Test if multiple mixed limit arguments are wrong.""" mock_cliargs = ["rtprio=:", "memlock102400", "nofile1024:524288:"] expected = " --ulimit rtprio=99 --ulimit memlock=102400 --ulimit nofile=1024:524288" - self.assertFalse(self._is_arg_translation_ok(mock_cliargs, expected)) + self.assertFalse(*self._is_arg_translation_ok(mock_cliargs, expected)) From 4e6475a1b0507330a0bab565c187931f045ca408 Mon Sep 17 00:00:00 2001 From: Tully Foote Date: Tue, 29 Oct 2024 15:56:35 -0700 Subject: [PATCH 11/11] Alphabetize listing --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 7fe8106..2cc1d9d 100644 --- a/setup.py +++ b/setup.py @@ -63,10 +63,10 @@ 'pulse = rocker.extensions:PulseAudio', 'rmw = rocker.rmw_extension:RMW', 'ssh = rocker.ssh_extension:Ssh', + 'ulimit = rocker.ulimit_extension:Ulimit', 'user = rocker.extensions:User', 'volume = rocker.volume_extension:Volume', 'x11 = rocker.nvidia_extension:X11', - 'ulimit = rocker.ulimit_extension:Ulimit', ] }, 'author': 'Tully Foote',