Skip to content
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

Enable use of a prefix in the zfs tree used by iocage #34

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Defenso-QTH
Copy link
Contributor

Hello,

This PR aimes to close #33 and currently expects #32 being merged first.

I implemented a --prefix option to the activate command. This prefix is currently saved in a zfs property of the pool, like the active pool flag.
Non-regression tests are passing (see checks on fork repo or directly on Cirrus CI).

The issue that remains is that jails, even if they are exposed a dataset they can use, cannot modify the zfs properties of the host pool. So I am not sure how to proceed from there.

I am thinking of implementing a configuration file / a configuration option in /etc/rc.conf - but that would break the current iocage philosophy of working only with zfs as its source of information. We could however limit the damage by leaving it optional and only using that way of working if the file / option exists.

What do you think?


Make sure to follow and check these boxes before submitting a PR! Thank you.

Defenso-QTH and others added 3 commits September 18, 2024 14:23
Use isinstance() instead of type()

Co-authored-by: Vincent Barbaresi <vincent.barbaresi@datadoghq.com>
Instead of using pool/iocage, we can now use anything like
pool/some/prefix/iocage, which enables iocage use within jails. To do
so, use the -p / --prefix option with iocage activate. The prefix is
then stored as a ZFS property of the pool and used for all commands.
A fixture is also added to the tests to enable functional tests.
@coveralls
Copy link

coveralls commented Sep 25, 2024

Pull Request Test Coverage Report for Build 11026351515

Details

  • 5 of 154 (3.25%) changed or added relevant lines in 18 files are covered.
  • 15 unchanged lines in 9 files lost coverage.
  • Overall coverage decreased (-0.06%) to 7.741%

Changes Missing Coverage Covered Lines Changed/Added Lines %
iocage_lib/ioc_plugin.py 0 1 0.0%
iocage_cli/migrate.py 0 2 0.0%
iocage_cli/snapshot.py 0 2 0.0%
iocage_lib/ioc_debug.py 0 2 0.0%
iocage_lib/ioc_list.py 0 2 0.0%
iocage_lib/cache.py 0 3 0.0%
iocage_lib/ioc_clean.py 0 4 0.0%
iocage_lib/zfs.py 1 5 20.0%
iocage_cli/activate.py 0 5 0.0%
iocage_lib/ioc_fetch.py 0 5 0.0%
Files with Coverage Reduction New Missed Lines %
iocage_lib/ioc_json.py 1 6.37%
iocage_lib/cache.py 1 27.92%
iocage_cli/snapremove.py 1 0.0%
iocage_cli/migrate.py 1 0.0%
iocage_lib/ioc_clean.py 1 0.0%
iocage_lib/pools.py 2 41.03%
iocage_lib/ioc_image.py 2 0.0%
iocage_lib/ioc_create.py 3 4.56%
iocage_lib/iocage.py 3 0.0%
Totals Coverage Status
Change from base Build 10917819529: -0.06%
Covered Lines: 802
Relevant Lines: 7838

💛 - Coveralls

@dgeo
Copy link
Contributor

dgeo commented Sep 28, 2024

It's a big change, and a good idea IMHO.
Could you rebase this on master, to avoid mixing ALL feature with this one ?
I'll try to review and test this next week (hoping I'll find time to)

@dgeo
Copy link
Contributor

dgeo commented Sep 28, 2024

While thinking, why not including /iocage in iocroot variable, this may allow using arbitrary zfs to contain jails without mandatory /iocage/ parent zfs ?
Also, nowadays there are not many properties stored in zfs as it used to be in the old shell version, most data is in json files (defaults and jails configs).
I think the only config bit in zfs prop is iocage's main location.

@Defenso-QTH
Copy link
Contributor Author

Yes that's possible. I will try to make things work with the prefix approach work first and then we can improve. For the first step I preferred changing the least I could.
We could also try and do some refactoring and make sure we use either Dataset or Path objects in the different places. I feel like the current code mixes both a bit, with strings here and there, so it would make it clearer.

For configuration, I went with settings in /etc/rc.conf and getting by calling sysrc. It works fine outside a jail (see CI runs on my repo). Although we might want to check performance later and maybe cache the result.

I am still working on some issues with hierarchical jails:

  • iocage cannot manipulate devfs rulesets like it currently does on the host inside jails. I fixed this by catching the error on the call to devfs and falling back to devfs_ruleset=0 but that is not clean so I am looking at using sysctl instead like they did in bastille (see BastilleBSD/bastille@6ca8ea5).
  • iocage cannot mount the second-level jail's /var/run/log in /dev like it does in the first-level one. But I have no idea why it tries to do that anyway. Do you?

Apart from that, it works. I will do some cleaning up and try to push a new version you can play with in the week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

iocage cannot be used with hierarchical jails
3 participants