[MediaWiki-commits] [Gerrit] operations/puppet[production]: labstore: avoid the hardcoding of eth0/eth1

2017-06-08 Thread Madhuvishy (Code Review) 
Madhuvishy has submitted this change and it was merged. ( 
https://gerrit.wikimedia.org/r/356109 )

Change subject: labstore: avoid the hardcoding of eth0/eth1
..


labstore: avoid the hardcoding of eth0/eth1

Instead of hardcoding eth0 and eth1 in a bunch of places, use
$monitor_iface (for eth0) and $data_iface (eth1) as parameters to
role::labs::nfs::secondary and pass them to included classes as needed.

We could use $facts['interface_primary'] for $monitor_iface (instead of
eth0), but since we have to hardcode the value for $data_iface, that
would be of limited value.

Change-Id: I761da8f8d2e07e596c2d81a69d48543dc88ece2a
---
M modules/labstore/files/monitor/check_drbd_cluster_ip
M modules/labstore/manifests/monitoring/secondary.pp
M modules/role/manifests/labs/nfs/secondary.pp
M modules/role/templates/labs/nfs/nfs-manage.sh.erb
4 files changed, 30 insertions(+), 22 deletions(-)

Approvals:
  Madhuvishy: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/modules/labstore/files/monitor/check_drbd_cluster_ip 
b/modules/labstore/files/monitor/check_drbd_cluster_ip
index 47b4f5f..51a6fad 100644
--- a/modules/labstore/files/monitor/check_drbd_cluster_ip
+++ b/modules/labstore/files/monitor/check_drbd_cluster_ip
@@ -4,18 +4,17 @@
 import sys
 
 
-def check_cluster_ip(node, role, ip):
+def check_cluster_ip(node, role, interface, ip):
 """
-If role is primary, check that cluster ip is assigned to the eth0 
interface,
+If role is primary, check that cluster IP is assigned to the interface,
 else make sure it is not.
 :param node: string
 :param role: string
 :param ip: string
 :returns: boolean
 """
-# Check if the ip string is present in the list of ipv4 IPs assigned to the
-# eth0 interface
-ip_assigned = ip in str(subprocess.check_output(['/bin/ip', '-4', 'a', 
'list', 'eth0']))
+# Check if the ip is present in the list of IPv4 IPs assigned to interface
+ip_assigned = ip in str(subprocess.check_output(['/bin/ip', '-4', 'a', 
'list', interface]))
 if (role == 'primary' and ip_assigned) or \
 (role == 'secondary' and not ip_assigned):
 print('Cluster IP assignment OK')
@@ -32,10 +31,11 @@
  to DRBD primary')
 parser.add_argument('node', help='Hostname of node being checked')
 parser.add_argument('role', help='Expected drbd role, primary|secondary')
+parser.add_argument('interface', help='Interface with an IP assigned')
 parser.add_argument('ip', help='Cluster IP assigned to primary node')
 args = parser.parse_args()
 
-if not check_cluster_ip(args.node, args.role, args.ip):
+if not check_cluster_ip(args.node, args.role, args.interface, args.ip):
 sys.exit(1)
 
 
diff --git a/modules/labstore/manifests/monitoring/secondary.pp 
b/modules/labstore/manifests/monitoring/secondary.pp
index c58c0b5..035f350 100644
--- a/modules/labstore/manifests/monitoring/secondary.pp
+++ b/modules/labstore/manifests/monitoring/secondary.pp
@@ -7,7 +7,7 @@
 # - check that cluster ip is assigned to DRBD primary
 # - NFS is being served over cluster IP
 
-class labstore::monitoring::secondary($drbd_role, $cluster_ip, $resource = 
'all') {
+class labstore::monitoring::secondary($drbd_role, $cluster_iface, $cluster_ip, 
$resource = 'all') {
 
 sudo::user { 'nagios_check_drbd':
 user   => 'nagios',
@@ -54,7 +54,7 @@
 
 nrpe::monitor_service { 'check_drbd_cluster_ip':
 description  => 'DRBD Cluster IP assignment',
-nrpe_command => "/usr/bin/sudo /usr/local/sbin/check_drbd_cluster_ip 
${::hostname} ${drbd_role} ${cluster_ip}",
+nrpe_command => "/usr/bin/sudo /usr/local/sbin/check_drbd_cluster_ip 
${::hostname} ${drbd_role} ${cluster_iface} ${cluster_ip}",
 require  => File['/usr/local/sbin/check_drbd_cluster_ip'],
 }
 
diff --git a/modules/role/manifests/labs/nfs/secondary.pp 
b/modules/role/manifests/labs/nfs/secondary.pp
index 2c102f9..90bc994 100644
--- a/modules/role/manifests/labs/nfs/secondary.pp
+++ b/modules/role/manifests/labs/nfs/secondary.pp
@@ -1,4 +1,7 @@
-class role::labs::nfs::secondary($monitor = 'eth0') {
+class role::labs::nfs::secondary(
+  $monitor_iface = 'eth0',
+  $data_iface= 'eth1',
+) {
 
 system::role { 'role::labs::nfs::secondary':
 description => 'NFS secondary share cluster',
@@ -10,10 +13,12 @@
 include role::labs::db::maintain_dbusers
 
 # Enable RPS to balance IRQs over CPUs
-interface::rps { $monitor: }
+interface::rps { 'monitor':
+interface => $monitor_iface,
+}
 
-interface::manual{ 'eth1':
-interface => 'eth1',
+interface::manual{ 'data':
+interface => $data_iface,
 }
 
 if $::hostname == 'labstore1005' {
@@ -21,10 +26,10 @@
 $drbd_role = 'primary'
 
 interface::ip { 'drbd-replication':
-interface => 'eth1',
+

[MediaWiki-commits] [Gerrit] operations/puppet[production]: labstore: avoid the hardcoding of eth0/eth1

2017-05-29 Thread Faidon Liambotis (Code Review) 
Faidon Liambotis has uploaded a new change for review. ( 
https://gerrit.wikimedia.org/r/356109 )

Change subject: labstore: avoid the hardcoding of eth0/eth1
..

labstore: avoid the hardcoding of eth0/eth1

Instead of hardcoding eth0 and eth1 in a bunch of places, use
$monitor_iface (for eth0) and $data_iface (eth1) as parameters to
role::labs::nfs::secondary and pass them to included classes as needed.

We could use $facts['interface_primary'] for $monitor_iface (instead of
eth0), but since we have to hardcode the value for $data_iface, that
would be of limited value.

Change-Id: I761da8f8d2e07e596c2d81a69d48543dc88ece2a
---
M modules/labstore/files/monitor/check_drbd_cluster_ip
M modules/labstore/manifests/monitoring/secondary.pp
M modules/role/manifests/labs/nfs/secondary.pp
M modules/role/templates/labs/nfs/nfs-manage.sh.erb
4 files changed, 29 insertions(+), 21 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/operations/puppet 
refs/changes/09/356109/1

diff --git a/modules/labstore/files/monitor/check_drbd_cluster_ip 
b/modules/labstore/files/monitor/check_drbd_cluster_ip
index 47b4f5f..6ed247c 100644
--- a/modules/labstore/files/monitor/check_drbd_cluster_ip
+++ b/modules/labstore/files/monitor/check_drbd_cluster_ip
@@ -4,7 +4,7 @@
 import sys
 
 
-def check_cluster_ip(node, role, ip):
+def check_cluster_ip(node, role, interface, ip):
 """
 If role is primary, check that cluster ip is assigned to the eth0 
interface,
 else make sure it is not.
@@ -13,9 +13,8 @@
 :param ip: string
 :returns: boolean
 """
-# Check if the ip string is present in the list of ipv4 IPs assigned to the
-# eth0 interface
-ip_assigned = ip in str(subprocess.check_output(['/bin/ip', '-4', 'a', 
'list', 'eth0']))
+# Check if the ip is present in the list of IPv4 IPs assigned to interface
+ip_assigned = ip in str(subprocess.check_output(['/bin/ip', '-4', 'a', 
'list', interface]))
 if (role == 'primary' and ip_assigned) or \
 (role == 'secondary' and not ip_assigned):
 print('Cluster IP assignment OK')
@@ -32,10 +31,11 @@
  to DRBD primary')
 parser.add_argument('node', help='Hostname of node being checked')
 parser.add_argument('role', help='Expected drbd role, primary|secondary')
+parser.add_argument('interface', help='Interface with an IP assigned')
 parser.add_argument('ip', help='Cluster IP assigned to primary node')
 args = parser.parse_args()
 
-if not check_cluster_ip(args.node, args.role, args.ip):
+if not check_cluster_ip(args.node, args.role, args.interface, args.ip):
 sys.exit(1)
 
 
diff --git a/modules/labstore/manifests/monitoring/secondary.pp 
b/modules/labstore/manifests/monitoring/secondary.pp
index c58c0b5..035f350 100644
--- a/modules/labstore/manifests/monitoring/secondary.pp
+++ b/modules/labstore/manifests/monitoring/secondary.pp
@@ -7,7 +7,7 @@
 # - check that cluster ip is assigned to DRBD primary
 # - NFS is being served over cluster IP
 
-class labstore::monitoring::secondary($drbd_role, $cluster_ip, $resource = 
'all') {
+class labstore::monitoring::secondary($drbd_role, $cluster_iface, $cluster_ip, 
$resource = 'all') {
 
 sudo::user { 'nagios_check_drbd':
 user   => 'nagios',
@@ -54,7 +54,7 @@
 
 nrpe::monitor_service { 'check_drbd_cluster_ip':
 description  => 'DRBD Cluster IP assignment',
-nrpe_command => "/usr/bin/sudo /usr/local/sbin/check_drbd_cluster_ip 
${::hostname} ${drbd_role} ${cluster_ip}",
+nrpe_command => "/usr/bin/sudo /usr/local/sbin/check_drbd_cluster_ip 
${::hostname} ${drbd_role} ${cluster_iface} ${cluster_ip}",
 require  => File['/usr/local/sbin/check_drbd_cluster_ip'],
 }
 
diff --git a/modules/role/manifests/labs/nfs/secondary.pp 
b/modules/role/manifests/labs/nfs/secondary.pp
index 2c102f9..90bc994 100644
--- a/modules/role/manifests/labs/nfs/secondary.pp
+++ b/modules/role/manifests/labs/nfs/secondary.pp
@@ -1,4 +1,7 @@
-class role::labs::nfs::secondary($monitor = 'eth0') {
+class role::labs::nfs::secondary(
+  $monitor_iface = 'eth0',
+  $data_iface= 'eth1',
+) {
 
 system::role { 'role::labs::nfs::secondary':
 description => 'NFS secondary share cluster',
@@ -10,10 +13,12 @@
 include role::labs::db::maintain_dbusers
 
 # Enable RPS to balance IRQs over CPUs
-interface::rps { $monitor: }
+interface::rps { 'monitor':
+interface => $monitor_iface,
+}
 
-interface::manual{ 'eth1':
-interface => 'eth1',
+interface::manual{ 'data':
+interface => $data_iface,
 }
 
 if $::hostname == 'labstore1005' {
@@ -21,10 +26,10 @@
 $drbd_role = 'primary'
 
 interface::ip { 'drbd-replication':
-interface => 'eth1',
+interface => $data_iface,
 address   => '192.168.0.2',
 prefixlen =>