Faidon Liambotis has uploaded a new change for review. ( 
https://gerrit.wikimedia.org/r/377676 )

Change subject: salt: change roots/runner_dir from Array to String
......................................................................

salt: change roots/runner_dir from Array to String

In the salt::master class, these parameters are defined like that:
- $salt_runner_dirs    = ['/srv/runners']
- $salt_state_roots    = { 'base' => ['/srv/salt'] }
- $salt_file_roots     = { 'base' => ['/srv/salt'] }
- $salt_pillar_roots   = { 'base' => ['/srv/pillars'] }
- $salt_module_roots   = { 'base' => ['/srv/salt/_modules'] }
- $salt_returner_roots = { 'base' => ['/srv/salt/_returners'] }

We actually never use more than one element in those arrays for any of
these anywhere, and the manifests are actually pretty broken if one
attempts to do that (e.g. they're being interpolated in some places and
passed as arguments to defined() in salt::master_environment), something
that the future parser caught.

Rather than attempt to fix the manifests to expect arrays everywhere,
convert those arrays into strings and call it a day. The days of those
manifests are fortunately numbered.

Change-Id: Iab10c654d8631e61e0f3a929d4d52189e53b0d81
---
M modules/role/manifests/salt/masters/labs.pp
M modules/role/manifests/salt/masters/labs/project_master.pp
M modules/role/manifests/salt/masters/production.pp
M modules/salt/manifests/master.pp
M modules/salt/templates/master.erb
5 files changed, 33 insertions(+), 33 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/operations/puppet 
refs/changes/76/377676/1

diff --git a/modules/role/manifests/salt/masters/labs.pp 
b/modules/role/manifests/salt/masters/labs.pp
index ac8cf55..98adb6b 100644
--- a/modules/role/manifests/salt/masters/labs.pp
+++ b/modules/role/manifests/salt/masters/labs.pp
@@ -3,14 +3,14 @@
 
     $puppet_master = hiera('labs_puppet_master')
 
-    $salt_state_roots    = { 'base' => ['/srv/salt'] }
-    $salt_file_roots     = { 'base' => ['/srv/salt'] }
-    $salt_pillar_roots   = { 'base' => ['/srv/pillars'] }
-    $salt_module_roots   = { 'base' => ['/srv/salt/_modules'] }
-    $salt_returner_roots = { 'base' => ['/srv/salt/_returners'] }
+    $salt_state_roots    = { 'base' => '/srv/salt' }
+    $salt_file_roots     = { 'base' => '/srv/salt' }
+    $salt_pillar_roots   = { 'base' => '/srv/pillars' }
+    $salt_module_roots   = { 'base' => '/srv/salt/_modules' }
+    $salt_returner_roots = { 'base' => '/srv/salt/_returners' }
 
     class { 'salt::master':
-        salt_runner_dirs    => ['/srv/runners'],
+        salt_runner_dir     => '/srv/runners',
         salt_file_roots     => $salt_file_roots,
         salt_pillar_roots   => $salt_pillar_roots,
         salt_worker_threads => '50',
diff --git a/modules/role/manifests/salt/masters/labs/project_master.pp 
b/modules/role/manifests/salt/masters/labs/project_master.pp
index 0b0262b..2a2ad9d 100644
--- a/modules/role/manifests/salt/masters/labs/project_master.pp
+++ b/modules/role/manifests/salt/masters/labs/project_master.pp
@@ -3,14 +3,14 @@
 # filtertags: labs-project-servermon labs-project-debdeploy 
labs-project-integration labs-project-ttmserver
 class role::salt::masters::labs::project_master {
 
-    $salt_state_roots    = { 'base' => ['/srv/salt'] }
-    $salt_file_roots     = { 'base' => ['/srv/salt'] }
-    $salt_pillar_roots   = { 'base' => ['/srv/pillars'] }
-    $salt_module_roots   = { 'base' => ['/srv/salt/_modules'] }
-    $salt_returner_roots = { 'base' => ['/srv/salt/_returners'] }
+    $salt_state_roots    = { 'base' => '/srv/salt' }
+    $salt_file_roots     = { 'base' => '/srv/salt' }
+    $salt_pillar_roots   = { 'base' => '/srv/pillars' }
+    $salt_module_roots   = { 'base' => '/srv/salt/_modules' }
+    $salt_returner_roots = { 'base' => '/srv/salt/_returners' }
 
     class { 'salt::master':
-        salt_runner_dirs    => ['/srv/runners'],
+        salt_runner_dir     => '/srv/runners',
                 # For simplicity of test/dev we trust all of labs
                 # to run deploy module calls, but rely on security groups
                 # to secure this.
diff --git a/modules/role/manifests/salt/masters/production.pp 
b/modules/role/manifests/salt/masters/production.pp
index 3b3ebf8..fd63d74 100644
--- a/modules/role/manifests/salt/masters/production.pp
+++ b/modules/role/manifests/salt/masters/production.pp
@@ -1,15 +1,15 @@
 class role::salt::masters::production {
 
-    $salt_state_roots    = { 'base' => ['/srv/salt'] }
-    $salt_file_roots     = { 'base' => ['/srv/salt'] }
-    $salt_pillar_roots   = { 'base' => ['/srv/pillars'] }
-    $salt_module_roots   = { 'base' => ['/srv/salt/_modules'] }
-    $salt_returner_roots = { 'base' => ['/srv/salt/_returners'] }
+    $salt_state_roots    = { 'base' => '/srv/salt' }
+    $salt_file_roots     = { 'base' => '/srv/salt' }
+    $salt_pillar_roots   = { 'base' => '/srv/pillars' }
+    $salt_module_roots   = { 'base' => '/srv/salt/_modules' }
+    $salt_returner_roots = { 'base' => '/srv/salt/_returners' }
 
 
     # TODO: do not hardcode salt_peer_run
     class { 'salt::master':
-        salt_runner_dirs    => ['/srv/runners'],
+        salt_runner_dir     => '/srv/runners',
         salt_peer_run       => {
             'tin.eqiad.wmnet'              => ['deploy.*'],
             'naos.codfw.wmnet'             => ['deploy.*'],
diff --git a/modules/salt/manifests/master.pp b/modules/salt/manifests/master.pp
index ed85cbc..5cc8e61 100644
--- a/modules/salt/manifests/master.pp
+++ b/modules/salt/manifests/master.pp
@@ -2,9 +2,7 @@
     $salt_version='installed',
     $salt_interface=undef,
     $salt_worker_threads=undef,
-    $salt_runner_dirs=['/srv/runners'],
-    $salt_file_roots={'base'=>['/srv/salt']},
-    $salt_pillar_roots={'base'=>['/srv/pillar']},
+    $salt_runner_dir='/srv/runners',
     $salt_ext_pillar={},
     $salt_reactor_root='/srv/reactors',
     $salt_reactor = {},
@@ -12,9 +10,11 @@
     $salt_peer={},
     $salt_peer_run={},
     $salt_nodegroups={},
-    $salt_state_roots={'base'=>['/srv/salt']},
-    $salt_module_roots={'base'=>['/srv/salt/_modules']},
-    $salt_returner_roots={'base'=>['/srv/salt/_returners']},
+    $salt_file_roots={'base'=>'/srv/salt'},
+    $salt_pillar_roots={'base'=>'/srv/pillar'},
+    $salt_state_roots={'base'=>'/srv/salt'},
+    $salt_module_roots={'base'=>'/srv/salt/_modules'},
+    $salt_returner_roots={'base'=>'/srv/salt/_returners'},
 ){
     package { 'salt-master':
         ensure => $salt_version,
@@ -35,14 +35,14 @@
         require => Package['salt-master'],
     }
 
-    file { $salt_runner_dirs:
+    file { $salt_runner_dir:
         ensure => directory,
         mode   => '0755',
         owner  => 'root',
         group  => 'root',
     }
 
-    file { "${salt_runner_dirs}/keys.py":
+    file { "${salt_runner_dir}/keys.py":
         ensure => present,
         source => 'puppet:///modules/salt/keys.py',
         mode   => '0755',
diff --git a/modules/salt/templates/master.erb 
b/modules/salt/templates/master.erb
index e4dae06..e9cedb8 100644
--- a/modules/salt/templates/master.erb
+++ b/modules/salt/templates/master.erb
@@ -199,7 +199,7 @@
 
 # Add any additional locations to look for master runners
 #runner_dirs: []
-<% if @salt_runner_dirs %>runner_dirs: [<%= @salt_runner_dirs.map { |i| "'" + 
i.to_s + "'" }.join(",") %>]<% end %>
+<% if @salt_runner_dir %>runner_dirs: [<%= @salt_runner_dir %>]<% end %>
 
 # Enable Cython for master side modules
 #cython_enable: False
@@ -273,10 +273,10 @@
 #  base:
 #    - /srv/salt
 <% if @salt_file_roots != {} %>
-file_roots:<% @salt_file_roots.sort.each do |salt_environment, salt_roots| %>
-  <%= salt_environment %>:<% salt_roots.sort.each do |salt_root| %>
+file_roots:<% @salt_file_roots.sort.each do |salt_environment, salt_root| %>
+  <%= salt_environment %>:
     - <%= salt_root %>
-<% end %><% end %><% end %>
+<% end %><% end %>
 
 # The hash_type is the hash to use when discovering the hash of a file on
 # the master server, the default is md5, but sha1, sha224, sha256, sha384
@@ -344,10 +344,10 @@
 #  base:
 #    - /srv/pillar
 <% if @salt_pillar_roots != {} %>
-pillar_roots:<% @salt_pillar_roots.sort.each do |salt_environment, salt_roots| 
%>
-  <%= salt_environment %>:<% salt_roots.sort.each do |salt_root| %>
+pillar_roots:<% @salt_pillar_roots.sort.each do |salt_environment, salt_root| 
%>
+  <%= salt_environment %>:
     - <%= salt_root %>
-<% end %><% end %><% end %>
+<% end %><% end %>
 
 # ext_pillar:
 #   - hiera: /etc/hiera.yaml

-- 
To view, visit https://gerrit.wikimedia.org/r/377676
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Iab10c654d8631e61e0f3a929d4d52189e53b0d81
Gerrit-PatchSet: 1
Gerrit-Project: operations/puppet
Gerrit-Branch: production
Gerrit-Owner: Faidon Liambotis <fai...@wikimedia.org>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to