From 384848e92d5f94f36f721ea282bc1de84cfd7ecf Mon Sep 17 00:00:00 2001 From: Jonathan LaBroad Date: Tue, 29 Aug 2017 12:38:29 -0400 Subject: [PATCH 1/8] Adding python_version parameter for Windows minions Ignoring non-Windows Salt parameters Get the correct minion file for ver >= 2017.x.x --- plugins/provisioners/salt/bootstrap-salt.ps1 | 25 ++++++++++++++++---- plugins/provisioners/salt/config.rb | 3 +++ plugins/provisioners/salt/provisioner.rb | 17 +++++++------ 3 files changed, 33 insertions(+), 12 deletions(-) diff --git a/plugins/provisioners/salt/bootstrap-salt.ps1 b/plugins/provisioners/salt/bootstrap-salt.ps1 index 0a05d6517..d0655ad42 100644 --- a/plugins/provisioners/salt/bootstrap-salt.ps1 +++ b/plugins/provisioners/salt/bootstrap-salt.ps1 @@ -1,5 +1,6 @@ Param( [string]$version, + [string]$pythonVersion = "2", [string]$runservice, [string]$minion, [string]$master @@ -14,6 +15,11 @@ If ($version -notmatch "2\d{3}\.\d{1,2}\.\d+(\-\d{1})?"){ $version = '2016.11.3' } +If ($pythonVersion -notmatch "\d+") { + $pythonVersion = "2" + Write-Host "Defaulting to minion Python version $pythonVersion" +} + If ($runservice.ToLower() -eq "true"){ Write-Host "Service is set to run." [bool]$runservice = $True @@ -49,11 +55,20 @@ If ([IntPtr]::Size -eq 4) { } # Download minion setup file -Write-Host "Downloading Salt minion installer Salt-Minion-$version-$arch-Setup.exe" -$webclient = New-Object System.Net.WebClient -$url = "https://repo.saltstack.com/windows/Salt-Minion-$version-$arch-Setup.exe" -$file = "C:\tmp\salt.exe" -$webclient.DownloadFile($url, $file) +$possibleFilenames = @("Salt-Minion-$version-$arch-Setup.exe", "Salt-Minion-$version-Py$pythonVersion-$arch-Setup.exe") +foreach ($minionFilename in $possibleFilenames) { + try { + Write-Host "Downloading Salt minion installer $minionFilename" + $webclient = New-Object System.Net.WebClient + $url = "https://repo.saltstack.com/windows/$minionFilename" + $file = "C:\tmp\salt.exe" + $webclient.DownloadFile($url, $file) + break + } + catch { + Write-Host "Unable to download $minionFilename" + } +} # Install minion silently diff --git a/plugins/provisioners/salt/config.rb b/plugins/provisioners/salt/config.rb index b42cfb00d..4e15a7f3f 100644 --- a/plugins/provisioners/salt/config.rb +++ b/plugins/provisioners/salt/config.rb @@ -34,6 +34,7 @@ module VagrantPlugins attr_accessor :no_minion attr_accessor :bootstrap_options attr_accessor :version + attr_accessor :python_version attr_accessor :run_service attr_accessor :master_id @@ -64,6 +65,7 @@ module VagrantPlugins @masterless = UNSET_VALUE @minion_id = UNSET_VALUE @version = UNSET_VALUE + @python_version = UNSET_VALUE @run_service = UNSET_VALUE @master_id = UNSET_VALUE end @@ -89,6 +91,7 @@ module VagrantPlugins @masterless = false if @masterless == UNSET_VALUE @minion_id = nil if @minion_id == UNSET_VALUE @version = nil if @version == UNSET_VALUE + @python_version = nil if @python_version == UNSET_VALUE @run_service = nil if @run_service == UNSET_VALUE @master_id = nil if @master_id == UNSET_VALUE diff --git a/plugins/provisioners/salt/provisioner.rb b/plugins/provisioners/salt/provisioner.rb index deefa1428..57db562ad 100644 --- a/plugins/provisioners/salt/provisioner.rb +++ b/plugins/provisioners/salt/provisioner.rb @@ -121,7 +121,7 @@ module VagrantPlugins options = "%s -F -c %s" % [options, config_dir] end - if @config.seed_master && @config.install_master + if @config.seed_master && @config.install_master && @machine.config.vm.communicator != :winrm seed_dir = "/tmp/minion-seed-keys" @machine.communicate.sudo("mkdir -p -m777 #{seed_dir}") @config.seed_master.each do |name, keyfile| @@ -132,27 +132,27 @@ module VagrantPlugins options = "#{options} -k #{seed_dir}" end - if configure && !install + if configure && !install && @machine.config.vm.communicator != :winrm options = "%s -C" % options end - if @config.install_master + if @config.install_master && @machine.config.vm.communicator != :winrm options = "%s -M" % options end - if @config.install_syndic + if @config.install_syndic && @machine.config.vm.communicator != :winrm options = "%s -S" % options end - if @config.no_minion + if @config.no_minion && @machine.config.vm.communicator != :winrm options = "%s -N" % options end - if @config.install_type + if @config.install_type && @machine.config.vm.communicator != :winrm options = "%s %s" % [options, @config.install_type] end - if @config.install_args + if @config.install_args && @machine.config.vm.communicator != :winrm options = "%s %s" % [options, @config.install_args] end @@ -267,6 +267,9 @@ module VagrantPlugins if @config.version options += " -version %s" % @config.version end + if @config.python_version + options += " -pythonVersion %s" % @config.python_version + end if @config.run_service @machine.env.ui.info "Salt minion will be stopped after installing." options += " -runservice %s" % @config.run_service From 8e3831b810a970da79547486b23a57f1b901b32c Mon Sep 17 00:00:00 2001 From: Jonathan LaBroad Date: Tue, 29 Aug 2017 13:13:24 -0400 Subject: [PATCH 2/8] Update default minion version to latest (Python 2) Perform check to determine to include python version or not --- plugins/provisioners/salt/bootstrap-salt.ps1 | 26 +++++++++----------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/plugins/provisioners/salt/bootstrap-salt.ps1 b/plugins/provisioners/salt/bootstrap-salt.ps1 index d0655ad42..53bdd18bd 100644 --- a/plugins/provisioners/salt/bootstrap-salt.ps1 +++ b/plugins/provisioners/salt/bootstrap-salt.ps1 @@ -12,7 +12,7 @@ $startupType = "Manual" # Version to install - default to latest if there is an issue If ($version -notmatch "2\d{3}\.\d{1,2}\.\d+(\-\d{1})?"){ - $version = '2016.11.3' + $version = '2017.7.1' } If ($pythonVersion -notmatch "\d+") { @@ -55,21 +55,17 @@ If ([IntPtr]::Size -eq 4) { } # Download minion setup file -$possibleFilenames = @("Salt-Minion-$version-$arch-Setup.exe", "Salt-Minion-$version-Py$pythonVersion-$arch-Setup.exe") -foreach ($minionFilename in $possibleFilenames) { - try { - Write-Host "Downloading Salt minion installer $minionFilename" - $webclient = New-Object System.Net.WebClient - $url = "https://repo.saltstack.com/windows/$minionFilename" - $file = "C:\tmp\salt.exe" - $webclient.DownloadFile($url, $file) - break - } - catch { - Write-Host "Unable to download $minionFilename" - } +$minionFilename = "Salt-Minion-$version-$arch-Setup.exe" +$versionYear = [regex]::Match($version, "\d+").Value +If ([convert]::ToInt32($versionYear) -ge 2017) +{ + $minionFilename = "Salt-Minion-$version-Py$pythonVersion-$arch-Setup.exe" } - +Write-Host "Downloading Salt minion installer $minionFilename" +$webclient = New-Object System.Net.WebClient +$url = "https://repo.saltstack.com/windows/$minionFilename" +$file = "C:\tmp\salt.exe" +$webclient.DownloadFile($url, $file) # Install minion silently Write-Host "Installing Salt minion..." From cec589ecd9f6d6a996dc5ae15f3084da5eff5a42 Mon Sep 17 00:00:00 2001 From: Jonathan LaBroad Date: Tue, 29 Aug 2017 13:17:56 -0400 Subject: [PATCH 3/8] Updating documentation --- website/source/docs/provisioning/salt.html.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/website/source/docs/provisioning/salt.html.md b/website/source/docs/provisioning/salt.html.md index 1f3ea1e4e..17c95bb4e 100644 --- a/website/source/docs/provisioning/salt.html.md +++ b/website/source/docs/provisioning/salt.html.md @@ -63,7 +63,7 @@ on this machine. Not supported on Windows guest machines. `false`. Not supported on Windows guest machines. * `install_type` (stable | git | daily | testing) - Whether to install from a -distribution's stable package manager, git tree-ish, daily ppa, or testing repository. +distribution's stable package manager, git tree-ish, daily ppa, or testing repository. Not supported on Windows guest machines. * `install_args` (string, default: "develop") - When performing a git install, you can specify a branch, tag, or any treeish. Not supported on Windows. @@ -75,7 +75,9 @@ distribution's stable package manager, git tree-ish, daily ppa, or testing repos * `bootstrap_options` (string) - Additional command-line options to pass to the bootstrap script. -* `version` (string, default: "2016.11.3") - Version of minion to be installed. Only supported on Windows guest machines. +* `version` (string, default: "2017.7.1") - Version of minion to be installed. Only supported on Windows guest machines. + +* `python_version` (string, default: "2") - Python version of minion to be installed. Only valid for minion versions >= 2017.7.1. Only supported on Windows guest machines. ## Minion Options These only make sense when `no_minion` is `false`. From 8e2c66d8a81160addd2831b3ff360d29c6c5fa6d Mon Sep 17 00:00:00 2001 From: Jonathan LaBroad Date: Tue, 29 Aug 2017 13:23:53 -0400 Subject: [PATCH 4/8] Reverting whitespace change --- plugins/provisioners/salt/bootstrap-salt.ps1 | 1 + 1 file changed, 1 insertion(+) diff --git a/plugins/provisioners/salt/bootstrap-salt.ps1 b/plugins/provisioners/salt/bootstrap-salt.ps1 index 53bdd18bd..0b8355860 100644 --- a/plugins/provisioners/salt/bootstrap-salt.ps1 +++ b/plugins/provisioners/salt/bootstrap-salt.ps1 @@ -67,6 +67,7 @@ $url = "https://repo.saltstack.com/windows/$minionFilename" $file = "C:\tmp\salt.exe" $webclient.DownloadFile($url, $file) + # Install minion silently Write-Host "Installing Salt minion..." #Wait for process to exit before continuing... From 56861296fa69b7db9e59380005a44cde9185a3f9 Mon Sep 17 00:00:00 2001 From: Jonathan LaBroad Date: Thu, 5 Oct 2017 06:25:36 -0400 Subject: [PATCH 5/8] Added tests and validation for python_version parameter --- plugins/provisioners/salt/config.rb | 8 ++++ templates/locales/en.yml | 2 + .../plugins/provisioners/salt/config_test.rb | 43 +++++++++++++++++++ website/source/docs/provisioning/salt.html.md | 2 +- 4 files changed, 54 insertions(+), 1 deletion(-) diff --git a/plugins/provisioners/salt/config.rb b/plugins/provisioners/salt/config.rb index c10bdcbf0..c4af8933b 100644 --- a/plugins/provisioners/salt/config.rb +++ b/plugins/provisioners/salt/config.rb @@ -163,6 +163,14 @@ module VagrantPlugins errors << I18n.t("vagrant.provisioners.salt.args_array") end + if @python_version && @python_version.is_a?(String) && !@python_version.scan(/\D/).empty? + errors << I18n.t("vagrant.provisioners.salt.python_version") + end + + if @python_version && !(@python_version.is_a?(Integer) || @python_version.is_a?(String)) + errors << I18n.t("vagrant.provisioners.salt.python_version") + end + return {"salt provisioner" => errors} end end diff --git a/templates/locales/en.yml b/templates/locales/en.yml index a953478c8..98193171e 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -2444,6 +2444,8 @@ en: You must accept keys when running highstate with master! args_array: |- You must set this value as an array. + python_version: |- + You must set this as an integer or string that represents an integer. pushes: file: diff --git a/test/unit/plugins/provisioners/salt/config_test.rb b/test/unit/plugins/provisioners/salt/config_test.rb index 2571bdbb2..c3e0a0123 100644 --- a/test/unit/plugins/provisioners/salt/config_test.rb +++ b/test/unit/plugins/provisioners/salt/config_test.rb @@ -114,5 +114,48 @@ describe VagrantPlugins::Salt::Config do expect(result[error_key]).to be_empty end end + + context "python_version" do + it "is valid if is set and not missing" do + subject.python_version = "2" + subject.finalize! + + result = subject.validate(machine) + expect(result[error_key]).to be_empty + end + + it "can be a string" do + subject.python_version = "2" + subject.finalize! + + result = subject.validate(machine) + expect(result[error_key]).to be_empty + end + + it "can be an integer" do + subject.python_version = 2 + subject.finalize! + + result = subject.validate(machine) + expect(result[error_key]).to be_empty + end + + it "is not a number that is not an integer" do + subject.python_version = 2.7 + subject.finalize! + + result = subject.validate(machine) + expect(result[error_key]).to_not be_empty + end + + it "is not a string that does not parse to an integer" do + subject.python_version = '2.7' + subject.finalize! + + result = subject.validate(machine) + expect(result[error_key]).to_not be_empty + end + + end end end diff --git a/website/source/docs/provisioning/salt.html.md b/website/source/docs/provisioning/salt.html.md index 09cd71707..f84633305 100644 --- a/website/source/docs/provisioning/salt.html.md +++ b/website/source/docs/provisioning/salt.html.md @@ -77,7 +77,7 @@ distribution's stable package manager, git tree-ish, daily ppa, or testing repos * `version` (string, default: "2017.7.1") - Version of minion to be installed. Only supported on Windows guest machines. -* `python_version` (string, default: "2") - Python version of minion to be installed. Only valid for minion versions >= 2017.7.1. Only supported on Windows guest machines. +* `python_version` (string, default: "2") - Major Python version of minion to be installed. Only valid for minion versions >= 2017.7.0. Only supported on Windows guest machines. ## Minion Options These only make sense when `no_minion` is `false`. From 79365f378928c55987ed4828ea78717a813a4aaa Mon Sep 17 00:00:00 2001 From: Jonathan LaBroad Date: Thu, 5 Oct 2017 06:55:53 -0400 Subject: [PATCH 6/8] Test that linux flags don't get passed to Windows Salt minion bootstrap --- .../provisioners/salt/provisioner_test.rb | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/test/unit/plugins/provisioners/salt/provisioner_test.rb b/test/unit/plugins/provisioners/salt/provisioner_test.rb index 8ebf3b846..d128fdf0a 100644 --- a/test/unit/plugins/provisioners/salt/provisioner_test.rb +++ b/test/unit/plugins/provisioners/salt/provisioner_test.rb @@ -30,7 +30,22 @@ describe VagrantPlugins::Salt::Provisioner do end describe "#provision" do - + context "minion" do + it "does not add linux-only bootstrap flags when on windows" do + additional_windows_options = "-these options -should -remain" + allow(config).to receive(:seed_master).and_return(true) + allow(config).to receive(:install_master).and_return(true) + allow(config).to receive(:install_syndic).and_return(true) + allow(config).to receive(:no_minion).and_return(true) + allow(config).to receive(:install_type).and_return('stable') + allow(config).to receive(:install_args).and_return('develop') + allow(config).to receive(:verbose).and_return(true) + allow(machine.config.vm).to receive(:communicator).and_return(:winrm) + allow(config).to receive(:bootstrap_options).and_return(additional_windows_options) + result = subject.bootstrap_options(true, true, "C:\\salttmp") + expect(result.strip).to eq(additional_windows_options) + end + end end describe "#call_highstate" do From 64f8d918889e9fed5ec46eadf930bb10cf9c6c28 Mon Sep 17 00:00:00 2001 From: Jonathan LaBroad Date: Thu, 5 Oct 2017 07:33:57 -0400 Subject: [PATCH 7/8] Add slightly more clarity in test case --- test/unit/plugins/provisioners/salt/provisioner_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/plugins/provisioners/salt/provisioner_test.rb b/test/unit/plugins/provisioners/salt/provisioner_test.rb index d128fdf0a..0cbd7b38c 100644 --- a/test/unit/plugins/provisioners/salt/provisioner_test.rb +++ b/test/unit/plugins/provisioners/salt/provisioner_test.rb @@ -32,7 +32,7 @@ describe VagrantPlugins::Salt::Provisioner do describe "#provision" do context "minion" do it "does not add linux-only bootstrap flags when on windows" do - additional_windows_options = "-these options -should -remain" + additional_windows_options = "-only -these options -should -remain" allow(config).to receive(:seed_master).and_return(true) allow(config).to receive(:install_master).and_return(true) allow(config).to receive(:install_syndic).and_return(true) From 73349d6ed63b13b583bce8542f99b79e704d869e Mon Sep 17 00:00:00 2001 From: Jonathan LaBroad Date: Thu, 5 Oct 2017 07:40:54 -0400 Subject: [PATCH 8/8] Whitespace --- test/unit/plugins/provisioners/salt/config_test.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/test/unit/plugins/provisioners/salt/config_test.rb b/test/unit/plugins/provisioners/salt/config_test.rb index c3e0a0123..fb9e10b69 100644 --- a/test/unit/plugins/provisioners/salt/config_test.rb +++ b/test/unit/plugins/provisioners/salt/config_test.rb @@ -155,7 +155,6 @@ describe VagrantPlugins::Salt::Config do result = subject.validate(machine) expect(result[error_key]).to_not be_empty end - end end end