theforeman / smart_proxy_dhcp_dnsmasq

dnsmasq DHCP plugin for Foreman smart-proxy
GNU General Public License v3.0
7 stars 7 forks source link

Code cleanup + Upgrade to 1.22 #12

Closed ananace closed 4 years ago

ananace commented 5 years ago

Rewrote parts of the plugin to use the modern methods for unused IP checking, also now stores the subnet service as a singleton to avoid hammering the disk with every request.
Earlier, every request would instantiate a new subnet service, which would also fully reload all the config files.

Still need to do some sort of data age verification and reload, but the code works in my testing, so a PR it is.

Fixes #13

ZAM- commented 5 years ago

Any chance of this merging with the main branch soon?

ananace commented 5 years ago

As soon as it's been tested in a non-synthetic environment, I really don't want to break DHCP after all.

If you have an environment where you could easily deploy it for testing, that'd be really helpful in that regard, as it turns out that I'm going to have to do a lot of work in getting some gems to cross-compile before I'm able to use my own environment.

ZAM- commented 5 years ago

So I actually tried using your branch to test, but I don't think I did it correctly. I'm fairly new to the gem package/bundle parts of Ruby. After I replaced the changed files, and I did a _bundle update smart_proxy_dhcpdnsmasq but I don't think it pulled the necessary gem dependencies that are needed for your patch. I followed this smart-proxy plugin install doc. but it seems to be old, and I'm not sure if it's up to date.

ananace commented 5 years ago

Generally, when you want to test a non-released version of a gem, you create a bundler.d file mapping the gem name to a git reference or to a checked-out path, and then you let bundle do all the heavy lifting.

For example, in my local dev environment;

~/Projects/Ruby/smart-proxy $ cat bundler.d/dhcp_dnsmasq.rb 
gem 'smart_proxy_dhcp_dnsmasq', path: '/home/ace/Projects/Ruby/smart_proxy_dhcp_dnsmasq'

In your case, a good content for the file would probably be;

gem 'smart_proxy_dhcp_dnsmasq',
    :git => 'https://github.com/ananace/smart_proxy_dhcp_dnsmasq.git',
    :branch => 'code_cleanup'
ZAM- commented 5 years ago

Thanks for the info, that makes sense to me. I can't figure out how to make the foreman-proxy use the same ruby binary that the foreman server is using. It seems that the foreman-proxy server is using my system installed ruby instead of /usr/bin/tfm-ruby. The proxy.log says ruby 2.0.0 is being loaded and not 2.5.5. Do you know how to make it use a different version of ruby?

ananace commented 5 years ago

I believe that foreman-proxy is designed to run off of the system Ruby, so you might run into gem version issues if you try to run the RPM-installed proxy with gems from a different source.

ZAM- commented 5 years ago

When trying to bundle install this plugin, it complains about dependencies and those dependencies required Ruby 2.2+. I'm very confused if foreman-proxy uses the system ruby, but this plugin requires ruby 2.2+.

ananace commented 5 years ago

Pushed a modified dependency on rb-inotify, hopefully that helps.

ananace commented 5 years ago

Have you had any luck in testing this yet? @ZAM-

ZAM- commented 5 years ago

Hey thanks for the poke @ananace. I tried doing a bundle install, yesterday, but it was still complaining about the rb-inotify dependency and requiring Ruby v2.2+.

ZAM- commented 5 years ago

Any idea why the bundle install might still be breaking?

ananace commented 5 years ago

Might be that bundler has locked a broken version of the gem, perhaps if you remove Gemfile.lock and then do a bundle install again?

ananace commented 4 years ago

Well, this has been laying about for a while now, I'm going to merge it for now and open new PRs for any issues that testing turns out to have missed