theforeman / foreman_default_hostgroup

A plugin to set the default hostgroup when hosts are created.
GNU General Public License v3.0
11 stars 29 forks source link

added ability to set a hostgroup by matching the hostname with regex #4

Closed lotherk closed 10 years ago

lotherk commented 10 years ago

This adds the functionallity to add hosts to hostgroups matching their hostname on a regex pattern.

There is currently only a simple yaml file to create the matching map, this should be moved in the hostgroup section or something.

I think the best place would be if we would add some new fields into the hostgroup UI like in the puppetclasses where you can add match values.

GregSutcliffe commented 10 years ago

@lotherk this is good stuff, just a few small tweaks and we can get it merged. Moving to including the map in SETTINGS is probably the messiest part, ping me if you need a hand.

GregSutcliffe commented 10 years ago

@lotherk oh, we'll need tests too - again, I can help if you need a hand.

lotherk commented 10 years ago

@GregSutcliffe I've added tests and added force_hostgroup_match to the settings page to force matching on hosts that already have a hostgroup. default is false. I've also added tests. Would be good if you'd take a look on them. :-)

GregSutcliffe commented 10 years ago

@lotherk Looking good. There's a lot of duplicated code in the tests that we can probably extract to a helper, but otherwise this is shaping up nicely.

I'm off to FOSDEM in a couple of hours, so I'll pick this up with you next week somethime :)

GregSutcliffe commented 10 years ago

@lotherk ok, conference season is over, let's pick this up. I'll try to take a look at the current state in the next day or two :)

lotherk commented 10 years ago

NOW the commits are squashed. ;-)

GregSutcliffe commented 10 years ago

@lotherk I totally forgot about this, sorry! I'll try to find some time to retest it this week.

lotherk commented 10 years ago

haha, no problem. ;-)

GregSutcliffe commented 10 years ago

I've just created a develop branch to match my other plugins - any chance you could rebase against that? The only change is we have to use find_by_title for Hostgroups now.

I'm testing the actual patch now, comments to follow :)

GregSutcliffe commented 10 years ago

Works great! Tests pass too :)

Maybe I missed it, but I don't see a way to say for a given regex - i.e unset the hostgroup. Maybe we could use a special string? Say

:map
  "-nil-" => /^.*\.blankhosts\.com$/

What do you think?

GregSutcliffe commented 10 years ago

Merged as 1418c02c7494e0c64bbfac4e8236c9cdcecb4a3e - thanks for the hard work!