add DragonFlyBSD support#904
Conversation
goneri
force-pushed
the
add-DragonFlyBSD-support_24047
branch
3 times, most recently
from
545a40d to
abfe17e
Compare
|
Note: you can try the DragonFly BSD 6.0.0 image from https://bsd-cloud-image.org/ |
goneri
force-pushed
the
add-DragonFlyBSD-support_24047
branch
2 times, most recently
from
d3a9a46 to
b3754c5
Compare
TheRealFalcon
left a comment
TheRealFalcon
left a comment
There was a problem hiding this comment.
Hey @goneri , thanks for this addition. Overall, this looks very good. I left some comments inline along with some comments for additions and testing here.
Should we add dragonfly here?
https://github.com/canonical/cloud-init/blob/master/cloudinit/distros/__init__.py#L47
and here?
https://github.com/canonical/cloud-init/blob/master/tools/render-cloudcfg#L7
Additionally, we would like some unit testing to go along with this addition.
Can we get unit testing to make sure it is detected correctly? Similar to:
https://github.com/canonical/cloud-init/blob/master/cloudinit/tests/test_util.py#L464
Along with testing for the new find_devs implementation? Simlar to:
https://github.com/canonical/cloud-init/blob/master/tests/unittests/test_util.py#L890
or perhaps:
https://github.com/canonical/cloud-init/blob/master/tests/unittests/test_datasource/test_nocloud.py#L292
goneri
force-pushed
the
add-DragonFlyBSD-support_24047
branch
from
8b110fd to
2635885
Compare
TheRealFalcon
left a comment
TheRealFalcon
left a comment
There was a problem hiding this comment.
Thanks for addressing my inline comments. Did you see my overall comments on the review though? It looks like they still need to be addressed. We still need a couple unit tests, and there might be a place or two you would want to add the distro.
Previous comment:
Should we add dragonfly here?
https://github.com/canonical/cloud-init/blob/master/cloudinit/distros/__init__.py#L47
and here?
https://github.com/canonical/cloud-init/blob/master/tools/render-cloudcfg#L7
Additionally, we would like some unit testing to go along with this addition.
Can we get unit testing to make sure it is detected correctly? Similar to:
https://github.com/canonical/cloud-init/blob/master/cloudinit/tests/test_util.py#L464
Along with testing for the new find_devs implementation? Simlar to:
https://github.com/canonical/cloud-init/blob/master/tests/unittests/test_util.py#L890
or perhaps:
https://github.com/canonical/cloud-init/blob/master/tests/unittests/test_datasource/test_nocloud.py#L292
|
Oh indeed, I totally missed the comment. I will work on that. Thank you for the review! |
I see I also need to add OpenBSD and NetBSD. Can I do that in another iteration?
I use I'm working on the unit-tests. |
goneri
force-pushed
the
add-DragonFlyBSD-support_24047
branch
from
6345d85 to
fd8c5e0
Compare
|
Hey @goneri , I noticed some new commits. Are you looking for a re-review yet? |
|
I've yet to add a test in test_nocloud.py. But yes, an early feedback is welcome. |
- Mostly based on FreeBSD, the main exception is that `find_devs_with_on_freebsd` does not work. - Since we cannot get the CDROM or the partition labels, `find_devs_with_on_dragonflybsd()` has a more naive approach and returns all the block devices.
Grow GPT partition and resize the hammer2 FS. You need to include a growpart script like this one in the image: https://github.com/virt-lightning/dragonflybsd-cloud-images/blob/master/growpart Signed-off-by: Gonéri Le Bouder <goneri@lebouder.net>
goneri
force-pushed
the
add-DragonFlyBSD-support_24047
branch
from
c4a1fd2 to
b2930e7
Compare
|
Regarding nocloud, I don't think we need any special unittest since 72f6eb0. They current code base relies on |
|
Thank you @TheRealFalcon! |
2 participants
find_devs_with_on_freebsddoes not work.find_devs_with_on_dragonflybsd()has a more naive approach andreturns all the block devices.