Giuseppe Lavagetto has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/403440 )
Change subject: base::resolving: explicitly pass arguments
......................................................................
base::resolving: explicitly pass arguments
Instead of relying on top-scope variable overriding each other, do the
following:
* Remove the conditional on $nameserver_override in the resolv.conf
template
* Add $nameservers as a local variable, which defaults to $::nameservers
* Pass the value from profile::base in case it's defined in hiera, and
have care to exclude the IP of the current node from the list to avoid
self-dependencies
* Remove all the $nameservers_override from the node definitions and add
those to per-site, per-role hiera
Change-Id: Ib0926a8966db2066d87a9ddea4265ed741c07437
---
M hieradata/role/codfw/lvs/balancer.yaml
M hieradata/role/codfw/recursor.yaml
M hieradata/role/eqiad/lvs/balancer.yaml
M hieradata/role/eqiad/recursor.yaml
M hieradata/role/esams/lvs/balancer.yaml
M hieradata/role/ulsfo/lvs/balancer.yaml
M manifests/site.pp
M modules/base/manifests/resolving.pp
M modules/base/spec/classes/resolving_spec.rb
M modules/base/templates/resolv.conf.erb
M modules/profile/manifests/base.pp
11 files changed, 51 insertions(+), 53 deletions(-)
Approvals:
Giuseppe Lavagetto: Looks good to me, approved
jenkins-bot: Verified
diff --git a/hieradata/role/codfw/lvs/balancer.yaml
b/hieradata/role/codfw/lvs/balancer.yaml
index 5fd35e0..19d968c 100644
--- a/hieradata/role/codfw/lvs/balancer.yaml
+++ b/hieradata/role/codfw/lvs/balancer.yaml
@@ -8,3 +8,11 @@
- public1-b-codfw
- public1-c-codfw
- public1-d-codfw
+# lvs200[25] are LVS balancers for the codfw recursive DNS IP,
+# so they need to use the recursive DNS backends directly
+# (acamar and achernar) with fallback to eqiad
+# (doing this for all lvs for now, see T103921)
+profile::base::nameservers:
+ - '208.80.153.12' # acamar
+ - '208.80.153.42' # achenar
+ - '208.80.154.254' # eqiad lvs
diff --git a/hieradata/role/codfw/recursor.yaml
b/hieradata/role/codfw/recursor.yaml
index d3fb55e..7f5238b 100644
--- a/hieradata/role/codfw/recursor.yaml
+++ b/hieradata/role/codfw/recursor.yaml
@@ -1,3 +1,8 @@
profile::bird::neighbors_list:
- 208.80.153.192 # cr1-codfw loopback
- 208.80.153.193 # cr2-codfw loopback
+profile::base::nameservers:
+# - '208.80.153.12' # acamar
+# - '208.80.153.42' # achenar
+ - '208.80.153.254' # codfw lvs
+ - '208.80.154.254' # eqiad lvs
diff --git a/hieradata/role/eqiad/lvs/balancer.yaml
b/hieradata/role/eqiad/lvs/balancer.yaml
index 4b1764e..b14b9ba 100644
--- a/hieradata/role/eqiad/lvs/balancer.yaml
+++ b/hieradata/role/eqiad/lvs/balancer.yaml
@@ -8,3 +8,11 @@
- public1-b-eqiad
- public1-c-eqiad
- public1-d-eqiad
+# lvs100[25] are LVS balancers for the eqiad recursive DNS IP,
+# so they need to use the recursive DNS backends directly
+# (chromium and hydrogen) with fallback to codfw
+# (doing this for all lvs for now, see T103921)
+profile::base::nameservers:
+ - '208.80.154.50' # hydrogen
+ - '208.80.154.157' # chromium
+ - '208.80.153.254' # codfw lvs
diff --git a/hieradata/role/eqiad/recursor.yaml
b/hieradata/role/eqiad/recursor.yaml
index 446f990..d560dc0 100644
--- a/hieradata/role/eqiad/recursor.yaml
+++ b/hieradata/role/eqiad/recursor.yaml
@@ -1,3 +1,8 @@
profile::bird::neighbors_list:
- 208.80.154.196 # cr1-eqiad loopback
- 208.80.154.197 # cr2-eqiad loopback
+profile::base::nameservers:
+# - '208.80.154.50' # hydrogen
+# - '208.80.154.157' # chromium
+ - '208.80.154.254' # eqiad lvs
+ - '208.80.153.254' # codfw lvs
diff --git a/hieradata/role/esams/lvs/balancer.yaml
b/hieradata/role/esams/lvs/balancer.yaml
index c4e9150..3b1d667 100644
--- a/hieradata/role/esams/lvs/balancer.yaml
+++ b/hieradata/role/esams/lvs/balancer.yaml
@@ -1,3 +1,11 @@
profile::pybal::config_host: conf1003.eqiad.wmnet
profile::lvs::tagged_subnets:
- public1-esams
+# lvs300[24] are LVS balancers for the esams recursive DNS IP,
+# so they need to use the recursive DNS backends directly
+# (nescio and maerlant) with fallback to eqiad
+# (doing this for all lvs for now, see T103921)
+profile::base::nameservers:
+ - '91.198.174.106' # nescio
+ - '91.198.174.122' # maerlant
+ - '208.80.154.254' # eqiad lvs
diff --git a/hieradata/role/ulsfo/lvs/balancer.yaml
b/hieradata/role/ulsfo/lvs/balancer.yaml
index ada9df4..d4976fb 100644
--- a/hieradata/role/ulsfo/lvs/balancer.yaml
+++ b/hieradata/role/ulsfo/lvs/balancer.yaml
@@ -1,2 +1,7 @@
profile::pybal::config_host: conf2003.codfw.wmnet
profile::lvs::tagged_subnets: []
+# ns override for all lvs for now, see T103921
+profile::base::nameservers:
+ - '208.80.153.12' # acamar
+ - '208.80.153.42' # achenar
+ - '208.80.154.254' # eqiad lvs
diff --git a/manifests/site.pp b/manifests/site.pp
index b5b4d43..8803b67 100644
--- a/manifests/site.pp
+++ b/manifests/site.pp
@@ -12,17 +12,11 @@
node 'acamar.wikimedia.org' {
role(recursor)
- # use achernar (directly) + eqiad LVS (avoid self-dep)
- $nameservers_override = [ '208.80.153.42', '208.80.154.254' ]
-
interface::add_ip6_mapped { 'main': }
}
node 'achernar.wikimedia.org' {
role(recursor)
-
- # use acamar (directly) + eqiad LVS (avoid self-dep)
- $nameservers_override = [ '208.80.153.12', '208.80.154.254' ]
interface::add_ip6_mapped { 'main': }
}
@@ -142,9 +136,6 @@
# DNS recursor
node 'chromium.wikimedia.org' {
role(recursor)
-
- # use hydrogen (directly) + codfw LVS (avoid self-dep)
- $nameservers_override = [ '208.80.154.50', '208.80.153.254' ]
interface::add_ip6_mapped { 'main': }
}
@@ -702,6 +693,7 @@
role(authdns::server)
# use eqiad LVS + codfw LVS (avoid self-dep)
+ # TODO: this was probably wrong, and surely has no effect on the catalog.
Remove?
$nameservers_override = [ '208.80.154.254', '208.80.153.254' ]
interface::add_ip6_mapped { 'main': }
@@ -894,10 +886,6 @@
# DNS recursor
node 'hydrogen.wikimedia.org' {
role(recursor)
-
- # use chromium (directly) + codfw LVS (avoid self-dep)
- $nameservers_override = [ '208.80.154.157', '208.80.153.254' ]
-
interface::add_ip6_mapped { 'main': }
}
@@ -1276,12 +1264,6 @@
}
node /lvs100[1-6]\.wikimedia\.org/ {
-
- # lvs100[25] are LVS balancers for the eqiad recursive DNS IP,
- # so they need to use the recursive DNS backends directly
- # (chromium and hydrogen) with fallback to codfw
- # (doing this for all lvs for now, see T103921)
- $nameservers_override = [ '208.80.154.157', '208.80.154.50',
'208.80.153.254' ]
role(lvs::balancer)
lvs::interface_tweaks {
@@ -1293,13 +1275,6 @@
}
node /^lvs10(0[789]|10)\.eqiad\.wmnet$/ {
-
- # lvs1008,10 are LVS balancers for the eqiad recursive DNS IP,
- # so they need to use the recursive DNS backends directly
- # (chromium and hydrogen) with fallback to codfw
- # (doing this for all lvs for now, see T103921)
- $nameservers_override = [ '208.80.154.157', '208.80.154.50',
'208.80.153.254' ]
-
role(lvs::balancer)
lvs::interface_tweaks {
@@ -1316,11 +1291,6 @@
# codfw lvs
node /lvs200[1-6]\.codfw\.wmnet/ {
- # lvs200[25] are LVS balancers for the codfw recursive DNS IP,
- # so they need to use the recursive DNS backends directly
- # (acamar and achernar) with fallback to eqiad
- # (doing this for all lvs for now, see T103921)
- $nameservers_override = [ '208.80.153.12', '208.80.153.42',
'208.80.154.254' ]
role(lvs::balancer)
lvs::interface_tweaks {
'eth0': bnx2x => true, txqlen => 10000;
@@ -1332,12 +1302,6 @@
# ESAMS lvs servers
node /^lvs300[1-4]\.esams\.wmnet$/ {
- # lvs300[24] are LVS balancers for the esams recursive DNS IP,
- # so they need to use the recursive DNS backends directly
- # (nescio and maerlant) with fallback to eqiad
- # (doing this for all lvs for now, see T103921)
- $nameservers_override = [ '91.198.174.106', '91.198.174.122',
'208.80.154.254' ]
-
role(lvs::balancer)
lvs::interface_tweaks {
'eth0': bnx2x => true, txqlen => 20000;
@@ -1350,9 +1314,6 @@
# ULSFO lvs servers
node /^lvs400[567]\.ulsfo\.wmnet$/ {
- # ns override for all lvs for now, see T103921
- $nameservers_override = [ '208.80.153.12', '208.80.153.42',
'208.80.154.254' ]
-
role(lvs::balancer)
lvs::interface_tweaks {
'eth0': bnx2x => true, txqlen => 10000;
diff --git a/modules/base/manifests/resolving.pp
b/modules/base/manifests/resolving.pp
index 86fccc3..7d2d8ad 100644
--- a/modules/base/manifests/resolving.pp
+++ b/modules/base/manifests/resolving.pp
@@ -1,9 +1,10 @@
class base::resolving (
$domain_search = $::domain,
$labs_additional_domains = [],
+ $nameservers = $::nameservers,
){
- if ! $::nameservers {
- fail('Variable $::nameservers is not defined!')
+ if ! $nameservers {
+ fail('Variable $nameservers is not defined!')
}
if $::realm == 'labs' {
diff --git a/modules/base/spec/classes/resolving_spec.rb
b/modules/base/spec/classes/resolving_spec.rb
index 24754d4..b0ddbce 100644
--- a/modules/base/spec/classes/resolving_spec.rb
+++ b/modules/base/spec/classes/resolving_spec.rb
@@ -1,11 +1,11 @@
require 'spec_helper'
describe 'base::resolving' do
- it 'requires $::nameservers' do
+ it 'requires $nameservers' do
should compile.and_raise_error(
- /Variable \$::nameservers is not defined!/)
+ /Variable \$nameservers is not defined!/)
end
- context "when nameservers are defined" do
+ context "when \$::nameservers are defined" do
let(:facts) { {'domain' => 'example.com'} }
let(:node_params){ {'nameservers' => ['1.2.3.4'], 'realm' =>
'production'}}
it { is_expected.to compile.with_all_deps }
@@ -19,7 +19,7 @@
expect(content).to match(/search example.com\noptions timeout:1
attempts:3\nnameserver 1.2.3.4\n/)
end
context "when nameservers are overridden" do
- let(:node_params){ super().merge({'nameservers_override' =>
['2.2.2.2'] })}
+ let(:params) { {'nameservers' => ['2.2.2.2']}}
it "nameserver is changed in resolv.conf" do
content = catalogue.resource('file',
'/etc/resolv.conf').send(:parameters)[:content]
diff --git a/modules/base/templates/resolv.conf.erb
b/modules/base/templates/resolv.conf.erb
index 2fdb7f6..debabe3 100644
--- a/modules/base/templates/resolv.conf.erb
+++ b/modules/base/templates/resolv.conf.erb
@@ -3,14 +3,9 @@
#### as template('base/resolv.conf.erb')
#####################################################################
# Resolver configuration for site <%= @site %>
+<%- ip = scope.lookupvar('::ipaddress') -%>
search <%= Array(@domain_search).join(' ') %>
options timeout:1 attempts:3
-<% if @nameservers_override.nil? -%>
-<% @nameservers.each do |nameserver| -%>
+<% @nameservers.select{|ns| ns != ip }.each do |nameserver| -%>
nameserver <%= nameserver %>
-<% end -%>
-<% else -%>
-<% @nameservers_override.each do |nameserver| -%>
-nameserver <%= nameserver %>
-<% end -%>
<% end -%>
diff --git a/modules/profile/manifests/base.pp
b/modules/profile/manifests/base.pp
index dd38d70..fbdaa84 100644
--- a/modules/profile/manifests/base.pp
+++ b/modules/profile/manifests/base.pp
@@ -6,6 +6,7 @@
$use_apt_proxy = hiera('profile::base::use_apt_proxy', true),
$purge_apt_sources = hiera('profile::base::purge_apt_sources', false),
$domain_search = hiera('profile::base::domain_search', $::domain),
+ $nameservers = hiera('profile::base::nameservers', $::nameservers), #
lint:ignore:wmf_styleguide
$remote_syslog = hiera('profile::base::remote_syslog',
['syslog.eqiad.wmnet', 'syslog.codfw.wmnet']),
$remote_syslog_tls = hiera('profile::base::remote_syslog_tls', []),
$notifications_enabled = hiera('profile::base::notifications_enabled',
'1'),
@@ -54,6 +55,7 @@
class { '::base::resolving':
domain_search => $domain_search,
+ nameservers => $nameservers,
}
class { '::rsyslog': }
--
To view, visit https://gerrit.wikimedia.org/r/403440
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib0926a8966db2066d87a9ddea4265ed741c07437
Gerrit-PatchSet: 5
Gerrit-Project: operations/puppet
Gerrit-Branch: production
Gerrit-Owner: Giuseppe Lavagetto <glavage...@wikimedia.org>
Gerrit-Reviewer: Gehel <guillaume.leder...@wikimedia.org>
Gerrit-Reviewer: Giuseppe Lavagetto <glavage...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
