-
Notifications
You must be signed in to change notification settings - Fork 76
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for ulimit flag #291
Merged
Merged
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
3b9752f
Create extension file
fpadula ecf1f6e
Barebones extension
fpadula c07010a
Add ulimit extension to setup.py
fpadula 99e8c48
Add expected format to help message
fpadula acb54ec
Add flag processing routine
fpadula db75af0
Add tests for ulimit extension
fpadula 0a8331e
Remove unused imports from test
fpadula 2558e89
Move asset check outside of arg checking function
fpadula 4661f47
Add more test cases
fpadula 4be2436
Return tuple instead of bool for better unit test report feedback
fpadula 4e6475a
Alphabetize listing
tfoote File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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', | ||
|
@@ -63,11 +63,12 @@ | |
'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', | ||
] | ||
}, | ||
}, | ||
'author': 'Tully Foote', | ||
'author_email': '[email protected]', | ||
'keywords': ['Docker'], | ||
|
@@ -91,4 +92,3 @@ | |
} | ||
|
||
setup(**kwargs) | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,67 @@ | ||
# 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 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 | ||
def get_name(): | ||
return 'ulimit' | ||
|
||
def get_docker_args(self, cliargs): | ||
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): | ||
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.') |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,100 @@ | ||
# 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 unittest | ||
from argparse import ArgumentTypeError | ||
|
||
from rocker.ulimit_extension import Ulimit | ||
|
||
|
||
class UlimitTest(unittest.TestCase): | ||
"""Unit tests for the Ulimit class.""" | ||
|
||
def setUp(self): | ||
self._instance = Ulimit() | ||
|
||
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 | ||
message_string = f"Expected: '{expected}', got: '{docker_args}'" | ||
except ArgumentTypeError: | ||
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)) | ||
|
||
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)) | ||
|
||
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)) | ||
|
||
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)) | ||
|
||
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)) | ||
|
||
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)) |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for comprehensive tests. Could you change this function to return a tuple of the success and message as a tuple that can then be passed to assertFalse or assertTrue so that the message gets captured in the unit test report not just in the console output.
The return would be
(is_ok, message_string)