Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(788)

Issue 29511584: #2762 - Fix provisioning of geoip to filter servers

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 week, 5 days ago by f.nicolaisen
Modified:
1 week, 1 day ago
Reviewers:
f.lopez, mathias
Base URL:
https://hg1/infrastructure
Visibility:
Public.

Description

#2762 - Fix provisioning of geoip to filter servers

Patch Set 1 #

Patch Set 2 : split the absent package into a legacy class #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+17 lines, -0 lines) Patch
M hiera/roles/filterserver.yaml View 1 1 chunk +2 lines, -0 lines 2 comments Download
A modules/adblockplus/manifests/legacy/filterserver.pp View 1 1 chunk +15 lines, -0 lines 1 comment Download

Messages

Total messages: 9
f.nicolaisen
1 week, 5 days ago (2017-08-10 09:57:30 UTC) #1
mathias
LGTM. Not ideal to place the package/purged resource in the same context where one can ...
1 week, 5 days ago (2017-08-10 17:30:40 UTC) #2
f.lopez
On 2017/08/10 17:30:40, mathias wrote: > LGTM. Not ideal to place the package/purged resource in ...
1 week, 5 days ago (2017-08-10 18:04:02 UTC) #3
mathias
On 2017/08/10 18:04:02, f.lopez wrote: > Maybe in the legacy class? So include some adblockplus::legacy::geoip ...
1 week, 5 days ago (2017-08-10 18:19:21 UTC) #4
f.lopez
On 2017/08/10 18:19:21, mathias wrote: > On 2017/08/10 18:04:02, f.lopez wrote: > > Maybe in ...
1 week, 5 days ago (2017-08-10 18:21:43 UTC) #5
f.lopez
On 2017/08/10 18:21:43, f.lopez wrote: > On 2017/08/10 18:19:21, mathias wrote: > > On 2017/08/10 ...
1 week, 5 days ago (2017-08-10 18:22:45 UTC) #6
mathias
On 2017/08/10 18:22:45, f.lopez wrote: > > Or you include geoip class normally in hiera ...
1 week, 5 days ago (2017-08-10 18:24:40 UTC) #7
f.nicolaisen
On 2017/08/10 18:24:40, mathias wrote: > On 2017/08/10 18:22:45, f.lopez wrote: > > > Or ...
1 week, 4 days ago (2017-08-11 13:40:45 UTC) #8
mathias
1 week, 1 day ago (2017-08-14 08:04:33 UTC) #9
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?
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 87257f5