#2762 - Fix provisioning of geoip to filter servers

#2762 - Fix provisioning of geoip to filter servers

Replaced by https://codereview.adblockplus.org/29527816/

[mathias, 2017-08-10 17:30:40]

LGTM. Not ideal to place the package/purged resource in the same context where
one can freely configure which package to use, but currently I don't know of any
low hanging fruit to make it better.

[f.lopez, 2017-08-10 18:04:02]

On 2017/08/10 17:30:40, mathias wrote:
> LGTM. Not ideal to place the package/purged resource in the same context where
> one can freely configure which package to use, but currently I don't know of
any
> low hanging fruit to make it better.

Maybe in the legacy class?

[mathias, 2017-08-10 18:19:21]

On 2017/08/10 18:04:02, f.lopez wrote:
> Maybe in the legacy class?

So include some adblockplus::legacy::geoip class which in turn defines the
package and includes class geoip? Yeah a bit over the top maybe, but sure
that'll work.

[f.lopez, 2017-08-10 18:21:43]

On 2017/08/10 18:19:21, mathias wrote:
> On 2017/08/10 18:04:02, f.lopez wrote:
> > Maybe in the legacy class?
> 
> So include some adblockplus::legacy::geoip class which in turn defines the
> package and includes class geoip? Yeah a bit over the top maybe, but sure
> that'll work.

Or you include geoip class normally in hiera but introduce an
abp::legacy::filterserver class which ensure geoip-database is uninstalled,
since we still want to use geoip class normally but only uninstalle
geoip-database package from "old" servers?

[f.lopez, 2017-08-10 18:22:45]

On 2017/08/10 18:21:43, f.lopez wrote:
> On 2017/08/10 18:19:21, mathias wrote:
> > On 2017/08/10 18:04:02, f.lopez wrote:
> > > Maybe in the legacy class?
> > 
> > So include some adblockplus::legacy::geoip class which in turn defines the
> > package and includes class geoip? Yeah a bit over the top maybe, but sure
> > that'll work.
> 
> Or you include geoip class normally in hiera but introduce an
> abp::legacy::filterserver class which ensure geoip-database is uninstalled,
> since we still want to use geoip class normally but only uninstalle
> geoip-database package from "old" servers?

I meant geoip-database-contrib

[mathias, 2017-08-10 18:24:40]

On 2017/08/10 18:22:45, f.lopez wrote:
> > Or you include geoip class normally in hiera but introduce an
> > abp::legacy::filterserver class which ensure geoip-database is uninstalled,
> > since we still want to use geoip class normally but only uninstalle
> > geoip-database package from "old" servers?
> 
> I meant geoip-database-contrib

Either way is fine I guess.

[f.nicolaisen, 2017-08-11 13:40:45]

On 2017/08/10 18:24:40, mathias wrote:
> On 2017/08/10 18:22:45, f.lopez wrote:
> > > Or you include geoip class normally in hiera but introduce an
> > > abp::legacy::filterserver class which ensure geoip-database is
uninstalled,
> > > since we still want to use geoip class normally but only uninstalle
> > > geoip-database package from "old" servers?
> > 
> > I meant geoip-database-contrib
> 
> Either way is fine I guess.

OK, I tried this in PS2, but noticed that if I configure only the legacy class,
it'll break because of the required resource in the other class. Is that
something we can live with?

[mathias, 2017-08-14 08:04:33]

On 2017/08/11 13:40:45, f.nicolaisen wrote:
> OK, I tried this in PS2, but noticed that if I configure only the legacy
class,
> it'll break because of the required resource in the other class. Is that
> something we can live with?

No.

https://codereview.adblockplus.org/29511584/diff/29512671/hiera/roles/filters...
File hiera/roles/filterserver.yaml (right):

https://codereview.adblockplus.org/29511584/diff/29512671/hiera/roles/filters...
hiera/roles/filterserver.yaml:6: geoip:
Since class filterserver depends on class geoip it should include the latter.

https://codereview.adblockplus.org/29511584/diff/29512671/hiera/roles/filters...
hiera/roles/filterserver.yaml:7: adblockplus::legacy::filterserver:
This class should in turn include class filterserver, it's basically the adapter
for the legacy module. You can use a root-level `filterserver::is_default: true`
to replace the parametrization above.

https://codereview.adblockplus.org/29511584/diff/29512671/modules/adblockplus...
File modules/adblockplus/manifests/legacy/filterserver.pp (right):

https://codereview.adblockplus.org/29511584/diff/29512671/modules/adblockplus...
modules/adblockplus/manifests/legacy/filterserver.pp:12: require =>
Cron['geoip'],
Why should ensuring absence require anything?