Accept custom LXC configuration #47
Conversation
Codecov Report
@@ Coverage Diff @@
## master #47 +/- ##
==========================================
+ Coverage 92.12% 92.47% +0.35%
==========================================
Files 42 42
Lines 990 997 +7
==========================================
+ Hits 912 922 +10
+ Misses 78 75 -3
Continue to review full report at Codecov.
|
|
Thanks for this PR @elventear, this looks handy. Would you mind adding documentation and tests along with this? Also, I'm wondering if the |
lxdock/container.py
Outdated
| # Get user defined lxc configs | ||
| lxc_config = self.options.get('lxc_configs', {}).copy() | ||
|
|
||
| # Overwrite any configuration settings with lxcdock defaults |
|
@hsoft I'd gladly add some tests, I looked into it but this part of the code does not seem to be tested directly in the unit tests. At least the configuration dictionary that gets passed to the client. I would appreciate pointers on how to test this. Also, no sure if we want to test this via integration too. The reason I went with |
|
Perhaps it would make more sense to have a I think you should add a test demonstrating that it is indeed possible to add a custom lxc option to a container. It could be an integration test. Also, you'll want to add this new option to the LXDock file reference documentation. |
tests/integration/test_project.py
Outdated
| @@ -14,6 +15,14 @@ | |||
| FIXTURE_ROOT = os.path.join(os.path.dirname(__file__), 'fixtures') | |||
|
|
|||
|
|
|||
| @contextlib.contextmanager | |||
There was a problem hiding this comment.
Your tests are looking good, thanks! I don't mean to be nitpicking, but I don't think you'll be needing this context manager for your tests. As long as your container names start with lxdock-test- (correct me if I'm wrong @ellmetha ), cleanup will be automatically taken care of at
lxdock/lxdock/test/testcases.py
Line 17 in bd9d011
There was a problem hiding this comment.
@elventear you can use the containername helper to construct your test container names (see:
lxdock/lxdock/test/testcases.py
Line 40 in bd9d011
tests/integration/test_project.py
Outdated
| @@ -156,3 +165,14 @@ def test_can_open_a_shell_for_a_specific_container(self, mocked_call, persistent | |||
| assert mocked_call.call_count == 1 | |||
| assert mocked_call.call_args[0][0] == \ | |||
| 'lxc exec {} -- su -m root'.format(persistent_container.lxd_name) | |||
|
|
|||
| def test_set_lxc_config(self): | |||
There was a problem hiding this comment.
I think that you should've placed this test in integration/test_container.py because it is related to a feature implemented in the Container class. 😉
docs/conf.rst
Outdated
| @@ -108,6 +108,25 @@ And then use it in your LXDock file as follows: | |||
| image: old-ubuntu | |||
| mode: local | |||
|
|
|||
| lxc_configs | |||
There was a problem hiding this comment.
IMO lxc_config would make more sense because we are considering a dictionary.
|
Sorry for the delay, life has kept me busy and just recently found time to finalize this. I have tried to address all comments. I have rebased and squashed the PR. |
Extend the lxcdock configuration schema to allow passing custom LXC configuration settings.
|
|
||
| containers: | ||
| - name: test01 | ||
| test1_key: test1_value |
There was a problem hiding this comment.
It slipped through the review. It should be removed.
There was a problem hiding this comment.
Alright! Thanks for reminding this to us!
|
Merged. Thanks @elventear! 😉 |
Provide a way to pass custom LXC parameters for a container, as an advanced setting.
The need for this arose from this bug:
https://bugs.launchpad.net/ubuntu/+source/apparmor/+bug/1575779
The only way I have to found to work around is to set the container to
raw.lxc: lxc.aa_profile=unconfined.