-
Notifications
You must be signed in to change notification settings - Fork 25
Change rlimit nproc resource limit to apply to the autotester process #587
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
Merged
Merged
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
11d96bf
Refactor setting resource limits
ch-iv b549d2e
Make nproc resource limit relative to the autotest user
ch-iv 446731d
Use a comprehension for `get_resource_settings`
ch-iv 0d1b3ed
Add back rlimit validation and write tests
ch-iv da91973
Add back rlimit validation and write tests
ch-iv fb7b204
Merge remote-tracking branch 'origin/fix-rlimit-nproc' into fix-rlimi…
ch-iv 9121be2
Move resource setting to the tester
ch-iv b747df9
Merge branch 'master' into fix-rlimit-nproc
ch-iv bcf2174
Add resource settings to tester init
ch-iv c61532f
Revert fake linebreak
ch-iv 903d963
Convert inline comments to docstrings
ch-iv 327e2bd
Update Changelog.md
ch-iv 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 hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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,81 @@ | ||
| import unittest | ||
| from unittest.mock import patch, MagicMock | ||
| import resource | ||
|
|
||
| from ..config import _Config | ||
| from ..utils import validate_rlimit, get_resource_settings | ||
|
|
||
|
|
||
| class TestValidateRlimit(unittest.TestCase): | ||
| def test_normal_limits(self): | ||
| """Test validate_rlimit with normal positive values.""" | ||
| self.assertEqual(validate_rlimit(100, 200, 150, 250), (100, 200)) | ||
| self.assertEqual(validate_rlimit(200, 300, 100, 250), (100, 250)) | ||
|
|
||
| def test_soft_limit_exceeding_hard_limit(self): | ||
| """Test validate_rlimit where soft limit would exceed hard limit.""" | ||
| self.assertEqual(validate_rlimit(500, 400, 300, 350), (300, 350)) | ||
|
|
||
| def test_infinity_values(self): | ||
| """Test validate_rlimit with -1 (resource.RLIM_INFINITY) values.""" | ||
| self.assertEqual(validate_rlimit(-1, 200, 100, 150), (100, 150)) | ||
| self.assertEqual(validate_rlimit(100, -1, 150, 200), (100, 200)) | ||
| self.assertEqual(validate_rlimit(-1, -1, 100, 200), (100, 200)) | ||
| self.assertEqual(validate_rlimit(100, 200, -1, 150), (100, 150)) | ||
| self.assertEqual(validate_rlimit(100, 200, 150, -1), (100, 200)) | ||
| self.assertEqual(validate_rlimit(100, 200, -1, -1), (100, 200)) | ||
|
|
||
| def test_both_negative(self): | ||
| """Test validate_rlimit where both config and current are negative.""" | ||
| self.assertEqual(validate_rlimit(-1, -1, -1, -1), (-1, -1)) | ||
|
|
||
| def test_mixed_negative_cases(self): | ||
| """Test validate_rlimit with various mixed cases with negative values.""" | ||
| self.assertEqual(validate_rlimit(-1, 200, -1, 300), (-1, 200)) | ||
| self.assertEqual(validate_rlimit(100, -1, -1, -1), (100, -1)) | ||
|
|
||
|
|
||
| class TestGetResourceSettings(unittest.TestCase): | ||
| @patch("resource.getrlimit") | ||
| def test_empty_config(self, _): | ||
| """Test get_resource_settings with an empty config.""" | ||
| config = _Config() | ||
| config.get = MagicMock(return_value={}) | ||
|
|
||
| self.assertEqual(get_resource_settings(config), []) | ||
|
|
||
| @patch("resource.getrlimit") | ||
| def test_with_config_values(self, mock_getrlimit): | ||
| """Test get_resource_settings with config containing values.""" | ||
| config = _Config() | ||
| rlimit_settings = {"nofile": (1024, 2048), "nproc": (30, 60)} | ||
|
|
||
| # Setup config.get to return our rlimit_settings when called with "rlimit_settings" | ||
| config.get = lambda key, default=None: rlimit_settings if key == "rlimit_settings" else default | ||
|
|
||
| # Setup mock for resource.getrlimit to return different values | ||
| mock_getrlimit.side_effect = lambda limit: { | ||
| resource.RLIMIT_NOFILE: (512, 1024), | ||
| resource.RLIMIT_NPROC: (60, 90), | ||
| }[limit] | ||
|
|
||
| expected = [(resource.RLIMIT_NOFILE, (512, 1024)), (resource.RLIMIT_NPROC, (30, 60))] | ||
|
|
||
| self.assertEqual(get_resource_settings(config), expected) | ||
|
|
||
| @patch("resource.getrlimit") | ||
| def test_with_infinity_values(self, mock_getrlimit): | ||
| """Test get_resource_settings with some infinity (-1) values in the mix.""" | ||
| config = _Config() | ||
| rlimit_settings = {"nofile": (1024, -1), "nproc": (-1, 60)} | ||
|
|
||
| config.get = lambda key, default=None: rlimit_settings if key == "rlimit_settings" else default | ||
|
|
||
| mock_getrlimit.side_effect = lambda limit: { | ||
| resource.RLIMIT_NOFILE: (512, 1024), | ||
| resource.RLIMIT_NPROC: (60, 90), | ||
| }[limit] | ||
|
|
||
| expected = [(resource.RLIMIT_NOFILE, (512, 1024)), (resource.RLIMIT_NPROC, (60, 60))] | ||
|
|
||
| self.assertEqual(get_resource_settings(config), expected) |
This file contains hidden or 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,60 @@ | ||
| import unittest | ||
| from unittest.mock import patch, call | ||
| import resource | ||
| from typing import Type | ||
|
|
||
| from ..testers.specs import TestSpecs | ||
| from ..testers.tester import Tester, Test | ||
|
|
||
|
|
||
| class MockTester(Tester): | ||
| def __init__( | ||
| self, | ||
| specs: TestSpecs, | ||
| test_class: Type[Test] | None = Test, | ||
| resource_settings: list[tuple[int, tuple[int, int]]] | None = None, | ||
| ) -> None: | ||
| super().__init__(specs, test_class, resource_settings) | ||
|
|
||
| def run(self) -> None: | ||
| pass | ||
|
|
||
|
|
||
| class TestResourceLimits(unittest.TestCase): | ||
|
|
||
| @patch("resource.setrlimit") | ||
| def test_set_resource_limits_single_limit(self, mock_setrlimit): | ||
| """Test setting a single resource limit.""" | ||
| # Arrange | ||
| tester = MockTester(specs=TestSpecs(), resource_settings=[(resource.RLIMIT_CPU, (10, 20))]) | ||
|
|
||
| # Act | ||
| tester.set_resource_limits(tester.resource_settings) | ||
|
|
||
| # Assert | ||
| mock_setrlimit.assert_called_once_with(resource.RLIMIT_CPU, (10, 20)) | ||
|
|
||
| @patch("resource.setrlimit") | ||
| def test_set_resource_limits_multiple_limits(self, mock_setrlimit): | ||
| """Test setting multiple resource limits.""" | ||
| # Arrange | ||
| tester = MockTester( | ||
| specs=TestSpecs(), | ||
| resource_settings=[ | ||
| (resource.RLIMIT_CPU, (10, 20)), | ||
| (resource.RLIMIT_NOFILE, (1024, 2048)), | ||
| (resource.RLIMIT_AS, (1024 * 1024 * 100, 1024 * 1024 * 200)), | ||
| ], | ||
| ) | ||
|
|
||
| # Act | ||
| tester.set_resource_limits(tester.resource_settings) | ||
|
|
||
| # Assert | ||
| expected_calls = [ | ||
| call(resource.RLIMIT_CPU, (10, 20)), | ||
| call(resource.RLIMIT_NOFILE, (1024, 2048)), | ||
| call(resource.RLIMIT_AS, (1024 * 1024 * 100, 1024 * 1024 * 200)), | ||
| ] | ||
| mock_setrlimit.assert_has_calls(expected_calls, any_order=False) | ||
| self.assertEqual(mock_setrlimit.call_count, 3) | ||
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.
Please omit the initializer entirely (as it isn't doing anything other than forwarding the arguments)
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.
Could you please clarify how you want me to do this, because I've tried a few things, but none of them worked.
I've tried:
In the above code I get
TypeError: Can't instantiate abstract class MockTester without an implementation for abstract method '__init__'.I've also tried:
Which raises
AttributeError: 'MockTester' object has no attribute 'resource_settings'.Then I asked chat, which suggested to do:
But I still get
TypeError: Can't instantiate abstract class MockTester without an implementation for abstract method '__init__'I think this is because the
Tester.__init__method is defined as abstract, so it has to be implemented in theMockTesterclass. However,Tester.__init__also does some setup, so all classes inhering fromTestermust either callTester.__init__or reimplement that setup logic.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.
@ch-iv you're right about this, I had missed the
@abstractmethoddecorator on the initializer. I'm good to leave this as-is!