I’ve got a few Foreman and Smart Proxy plugins that are getting the point where they’d probably do best ending up moving into the organization on GitHub - and getting official packaging as well.
I’ve got a few other plugins that aren’t quite at the point for me to feel comfortable about promoting them as well, but I might end up throwing up another query like this in the future.
Awesome, thanks! @Gwmngilfen since I think this is the third time something like this comes up recently, perhaps we should have a page somewhere that documents the process needed in order to migrate repos to the foreman org? off the top of my mind i think of packaging, adding the foreman account as a gem owner, updating plugins list, and i’m guessing there’s a bunch of stuff i’m missing right now
Absolutely. I’ll go back and read through the device42 thread and see what went down, and then do a PR to the website. Check back with me next week if you haven’t heard anything
@ace13 for starters, see if you can find someone to do an initial code review on the DNSMasq plugins - I think @bastilian was interested …
I reviewed the code and it looks good. Couple of questions for the DHCP plugin:
I see comment “TODO: Add proper inotify listener for configs” but it looks you are using INotify, what this comment is all about?
Can you add debug level message into the notify hook, so we can debug issues around not detecting changes?
Does it work with multiple subnets? It looks so, but I see “TODO: Multiple subnets” comment.
It would be great if the plugin would “clean” the dhcpopts.conf on each startup.
It’s great, it has tests. I hope we will get DHCPv6 support in the plugin, that would be very welcome. I haven’t checked DNS module but it looks good, it’s more simple than DHCP.
I haven’t tested those, but it’s big +1 from me, thanks for this effort. A blogpost once we add those into nightlies would be awesome.
Beware if you use libvirt provider with those and you will try to manage the libvirt network (named “default”) you will run into issues. This is meant to manage separate dnsmasq instance, just to prevent confusion. There was a provider called “virsh” which made dnsmasq management possible which I wrote, but it was dropped recently as I started using ISC DHCP for development.
Indeed running a smart-proxy on my router, though I ended up not using the openwrt backend after writing it, to separate the actual configuration - in UCI - from the generated configuration by the smart-proxy.
Regarding the unfinished inotify; I grabbed some code for reading leases with inotify, never hooked it all up properly though - had some issues with inotify on my router so I couldn’t get the code to work on there though. And at the time when I was writing that, I ended up running into the issue where foreman would count a lease object as a collision against a reservation, which meant I disabled the reading of leases anyway.
The multiple subnet piece is mostly because I haven’t actually tested it at all, so they might work, but I personally expect something to go horribly wrong if multiple subnets get involved.
I’d completely missed that dnsmasq finally has DHCPv6 support as well, most definitely going to have to add support for that. Just as soon as I’ve upgraded my router to a version with said support.
Thanks for the comments and review, going to run through another cleanup pass on the DHCP plugin, set up some test cases for multiple subnets and see if I the inotify code actually works or not.
Thanks for the cleanup, code is usually the documentation
No rush with DHCPv6 but it will be definitely useful as ISC has zero support if I am not mistaken.
Let’s execute the move under theforeman organization then, @ohadlevy@Gwmngilfen - ideally not by forking but either proper rename, or we can copy and deprecate the old URL.
Transfers are easy to arrange, if we’re happy with the code. Just to confirm, is this all three plugins mentioned in the original post, or just the two DNSMasq plugins?
@ananace please get in touch on chat and we’ll get it done.
