(PE-10956) windows upgrade #66

highb · 2015-09-25T21:43:20Z

Adds a PE-hosted MSI option for upgrading pre-2015 agents.

highb · 2015-09-25T21:43:43Z

Spec tests need an update.

jpartlow · 2015-09-29T00:07:23Z

manifests/prepare/package.pp

+      $file_dest = "C:\\Program Files\\Puppet Labs\\Puppet Enterprise\\packages\\${package_file_name}"
+    } else {
+      $source = "puppet:///pe_packages/${pe_server_version}/${::platform_tag}/${package_file_name}"
+      $dest = "/opt/puppetlabs/packages"


Ya, this was on my list of things to clean up. @highb what do you think about pulling this back into params, and then either plumbing it through alongside $package_file_name or referencing it as something like $pupppet_agent::params::local_packages_dir. Either way, we can then replace all the /opt/puppetlabs/packages references.

And then I think the reference to puppet_agent::prepare:: package::file_dest in install.pp just becomes this variable + package_file_name.

@jpartlow Yeah I think that makes more sense.

highb · 2015-10-06T23:34:58Z

I was going crazy about this-- be rake spec locally showed all of the specs passing, so I couldn't figure out what was going on. Looking at the actual Travis failure...

be rake test
---> syntax:manifests
---> syntax:templates
---> syntax:hiera:yaml
manifests/osfamily/windows.pp - WARNING: unquoted resource title on line 7
manifests/params.pp - WARNING: double quoted string containing no variables on line 29
manifests/params.pp - WARNING: double quoted string containing no variables on line 51

I didn't even know there was a rake test Fixing the WARNINGs now.

highb · 2015-10-06T23:38:19Z

OK, latest push should resolve the linter errors and this should finally be good to review and/or merge?

jpartlow · 2015-10-07T15:36:49Z

Looks like it's having issues with Puppet 3.8. Most of the specs and the module itself are constrained around the Puppet version.

highb · 2015-10-07T17:38:02Z

@jpartlow Fixed! :)

jpartlow · 2015-12-04T20:23:13Z

I think we just need a test run of a 3.8.3->2015.3.0 upgrade with a windows agent.

highb · 2015-12-08T23:44:23Z

Rebased this commit off the OSX/Solaris changes that just went in.

highb · 2015-12-09T01:28:33Z

Confirmed rebased code upgrades Win2012 agents by

Installing 3.8.3 on centos 7
Installing 3.8.3 on Win2012
Verifying mco/puppet runs work
Upgrading to 2015.3 on centos 7 master
Installing windows pe_repo
Blocking access to pm.puppetlabs.com on windows agent
Classifying puppet_agent with puppet_agent class
puppet run on windows agent, verified mco ping

jpartlow · 2015-12-11T18:11:40Z

rake test is exiting with linting failures

jpartlow · 2015-12-11T18:29:41Z

@highb there's something wrong with the merge on this branch:

https://github.com/highb/puppetlabs-puppet_agent/blob/highb-master-pe-10956_windows_upgrade/manifests/init.pp#L37

I'm not sure how you were able to verify it? Did you do a rebase after testing?

highb · 2015-12-15T19:44:37Z

@jpartlow That stray merge hash isn't in my local copy and when I force push, there are no changes. I'm re-running specs and tests locally to figure out what is happening.

highb · 2015-12-15T19:45:58Z

retest this please

highb · 2015-12-15T19:49:52Z

@jpartlow Also https://github.com/highb/puppetlabs-puppet_agent/blob/highb-master-pe-10956_windows_upgrade/manifests/init.pp#L37 is the wrong branch. I accidentally pushed that. The real branch is: https://github.com/highb/puppetlabs-puppet_agent/blob/master-pe-10956_windows_upgrade/manifests/init.pp

highb · 2015-12-15T19:53:01Z

Oh, I've been running rake spec not rake test. I'll fix the linter errors.

jpartlow · 2015-12-15T20:19:55Z

@highb Are your local changes synced with your remote? I'm still seeing the <<<< in the branch from your remote.

highb · 2015-12-15T20:32:33Z

@jpartlow Yeah, my changes should be synced. Make sure you're not pointing to the other (broken) branch I accidentally pushed highb-master-pe-10956_windows_upgrade

jpartlow · 2015-12-15T21:35:58Z

@highb ok, that's sorted out on my end, so that just leaves the lint.

joshcooper · 2015-12-15T21:53:08Z

manifests/params.pp

+      $package_name = 'puppet-agent'
+      $service_names = ['puppet', 'mcollective']
+
+      $local_packages_dir = 'C:\\Program Files\\Puppet Labs\\Puppet Enterprise\\packages'


This may not be the correct directory in situations where the user has installed puppet elsewhere, or when running 32-bit puppet on 64-bit windows. I think there's an env_windows_installdir fact that you can use: https://github.com/puppetlabs/puppet_for_the_win/blob/master/conf/windows/stage/bin/environment.bat#L8 /cc @Iristyle

Ah, that's helpful.

Another thought is to use C:\ProgramData\PuppetLabs instead (I think there's a PE common_appdata fact for that). If you write to C:\Program Files then those files will prevent puppet from uninstalling cleanly (it will leave behind the new files and any parent directories). It's not a big deal, but not ideal. Also if you are running puppet as "non-root", puppet may not have permission to write to C:\Program Files, but it can write to C:\ProgramData

highb · 2015-12-15T22:47:16Z

I'm going to run through upgrading a w2012 agent with this patch again.

highb · 2015-12-16T00:06:01Z

Worked on upgrading a 3.8.3 agent to 2015.3, with one error in the run:
Could not evaluate: Cannot get status of mcollective, error was: The specified service does not exist as an installed service. - OpenService: The specified service does not exist as an installed service. Wrapped exception: The specified service does not exist as an installed service. - OpenService: The specified service does not exist as an installed service.

But subsequent runs can find mco/pxp-agent just fine and mco ping/runonce from the console work fine.

highb · 2015-12-16T19:38:49Z

Tests are failing on puppet 3.8.1. Fun. Looking into what is going on there.

Prior to this commit the specs were not up to date with the latest functionality from the module, specifically, the new PE windows upgrade, which required some path changes.

Prior to this commit I decided to check for a null `::puppet_agent::service_names`, but I've since realized that should never really be null at this point in the module. This commit reverts the change I made to add a null check to `::puppet_agent::service_names`.

These syntax errors were causing `rake test` to fail, so they have been fixed in this commit.

Prior to this commit we were hardcoding the location of the puppet install to `C:\Program Files\Puppet Labs\Puppet Enterprise`, but we can't count on the location being set there. This commit changes the location to use `common_appdata` which even non-root users should have write access to.

highb · 2016-01-25T18:17:36Z

Rebased everything, I'll run this through some testing.

highb · 2016-01-25T21:23:11Z

Looks like Puppet might be having trouble dealing with the Windows service changing underneath it?

highb · 2016-01-25T21:24:12Z

Aside from the strange service error on the first run with puppet_agent, the upgrade seems to work fine.

highb · 2016-01-25T21:27:51Z

Seems like it might be a race condition with the MSI finishing install because post puppet run I have no problem interacting with the puppet and mcollective services:

MikaelSmith · 2016-01-25T21:34:29Z

/cc @puppetlabs/windows

MikaelSmith · 2016-01-25T21:35:18Z

Oh, shouldn't be trying to manage the service for Windows installs. The actual install happens with a script scheduled to run after Puppet finishes, and the MSI will ensure the service is started.

MikaelSmith · 2016-01-25T21:36:06Z

manifests/init.pp

+    class { '::puppet_agent::install':
+      package_file_name => $_package_file_name,
+    } ->
+    class { '::puppet_agent::service': }


This should be skipped or not do anything on Windows.

highb · 2016-01-25T21:36:27Z

Ah, alright. I was starting to dive in to start /wait and how that works, but just leaving it all to the MSI is much easier. ;)

Prior to this commit we were attempting to manage the puppet/mcollective services on Windows systems, which resulted in errors in the puppet run due to the MSI being in the middle of configuring those services. This commit removes management of the puppet/mco services on windows systems, because the MSI is handling that already and attempting to manage it in puppet just creates a race condition.

highb · 2016-01-25T22:42:33Z

OK, tested this out on a mono 3.8.3->2015.3.2-rc upgrade with the change to not manage the service on windows, and I'm no longer seeing the error, as expected. I'd call this ready for someone to test/review again.

jpartlow · 2016-01-27T18:43:27Z

@highb which Windows platform were you testing? I'm seeing failures upgrading 2008r2 64. (I'm updating the ticket with details.)

Ok the ticket has the details for reproduction and a Beaker log. In summary I'm seeing:

qubl1edmr2gecx6.delivery.puppetlabs.net (windows2008r2-64-1) 10:35:34$ cmd.exe /c puppet resource service puppet ensure=stopped
Error: Could not run: No such file or directory @ rb_sysopen - C:/ProgramData/PuppetLabs/puppet/cache/state/graphs/relationships.dot

after what seems like an otherwise successful puppet_agent upgrade of the windows node.

Prior to this commit our call to msiexec would not block until the msi finished. This resulted in any commands dependent on puppet that were run after a puppet_agent upgrade puppet run would fail due to puppet still being in the middle of upgrading. This commit adds a `start /wait` to the msiexec so that, hopefully, we will block until the MSI finishes running and puppet is actually upgraded.

Iristyle · 2016-01-29T15:54:06Z

templates/install_puppet.bat.erb

@@ -6,7 +6,7 @@ FOR /F "tokens=*" %%A IN ('tasklist /FI "PID eq %AGENT_PID%" /NH') DO set _task=
 echo %_task% | findstr "No tasks are running" >nul
 IF NOT %errorlevel% == 0 ( GOTO wait_for_pid )

-msiexec.exe /qn /norestart /i "<%= @_msi_location %>" /l*v "<%= @_logfile %>"
+start /wait msiexec.exe /qn /norestart /i "<%= @_msi_location %>" /l*v "<%= @_logfile %>"


For reference, Beaker is always a good place to look to see how we install the Puppet MSI - https://github.com/puppetlabs/beaker/blob/f3d87f2b3463c2c49e1dd2b3e34b028336e7151e/lib/beaker/dsl/install_utils/windows_utils.rb#L18-L39

You've fixed the problem now with the /wait ... but I don't see the exit status being returned - i.e. exit /B %errorlevel%

Prior to this commit when re-installing the MSI on windows systems, it would automatically revert the Puppet server setting to whatever was defined when puppet was initially installed. If that value was configured to be 'puppet' because no argument was given (as is the case in beaker) then it would default back to 'puppet', even though we had set the server to the appropriate master with beaker post-MSI install. This commit gets around that surprising behavior by always providing the currently configured Puppet server value to the MSI.

highb · 2016-01-29T23:51:54Z

templates/install_puppet.bat.erb

@@ -6,7 +6,7 @@ FOR /F "tokens=*" %%A IN ('tasklist /FI "PID eq %AGENT_PID%" /NH') DO set _task=
 echo %_task% | findstr "No tasks are running" >nul
 IF NOT %errorlevel% == 0 ( GOTO wait_for_pid )

-msiexec.exe /qn /norestart /i "<%= @_msi_location %>" /l*v "<%= @_logfile %>"
+start /wait msiexec.exe /qn /norestart /i "<%= @_msi_location %>" /l*v "<%= @_logfile %>" PUPPET_MASTER_SERVER="<%= @_puppet_master %>"


D'oh: note to future me, the settings::server is being read on the master. https://docs.puppetlabs.com/puppet/latest/reference/lang_facts_and_builtin_vars.html#puppet-master-variables So if I want to get the existing value, I should write a custom fact. :/

edit: This is fixed with the puppet_master_server fact added in 9b349a5

highb · 2016-02-01T20:45:53Z

Tested with this change https://github.com/puppetlabs/pe_acceptance_tests/pull/836 and it works, but I need to change the use of settings::server to a custom fact that reads the server value from the config.

Prior to this commit we were setting the agent's `server` setting to whatever `settings::server` was set to, which in a compile master scenario, would be the MoM. This certainly isn't what a user would want to have happen in production. This commit creates a `puppet_master_server` fact populated with the value of `Puppet.settings['server']` on the agent and uses that as the argument to the windows MSI upgrade so we don't override the existing config value with the default of `puppet` or whatever was written to the registry on install.

jpartlow · 2016-02-01T23:10:28Z

Is the problem that puppet.conf is being overwritten, or just the server setting is being overwritten? If just the later, then this fix may be fine.

highb · 2016-02-01T23:52:47Z

@jpartlow As far as I can tell, the MSI just overwrites the specific INI setting. So reading it before upgrading, then passing it in during the MSI run should preserve it. Even if it does mess up other settings, having the server correct should make it much easier to remediate any problems.

As for the settings::server problem, I've added the puppet_master_server fact to read the server value.

mwbutcher · 2016-02-03T19:40:34Z

👍 successfully tested against windows2012r2 upgrading from puppet 3.8.5 to 4.3.2

(PE-10956) windows upgrade

jpartlow reviewed Sep 29, 2015
View reviewed changes

highb force-pushed the master-pe-10956_windows_upgrade branch 2 times, most recently from 244d5d9 to 5522a92 Compare September 29, 2015 22:54

highb force-pushed the master-pe-10956_windows_upgrade branch from 9298f1f to 0f15b98 Compare October 7, 2015 17:01

highb force-pushed the master-pe-10956_windows_upgrade branch from 0f15b98 to d62fdfa Compare December 8, 2015 23:43

highb force-pushed the master-pe-10956_windows_upgrade branch from d62fdfa to 9b40529 Compare December 15, 2015 20:31

joshcooper reviewed Dec 15, 2015
View reviewed changes

highb force-pushed the master-pe-10956_windows_upgrade branch from 567eca1 to 056888e Compare December 16, 2015 00:04

highb force-pushed the master-pe-10956_windows_upgrade branch 2 times, most recently from a235715 to 4e4533c Compare December 16, 2015 21:33

Brandon High added 4 commits January 25, 2016 10:05

(PE-10956) Fix specs for windows changes

e8b2b3a

Prior to this commit the specs were not up to date with the latest functionality from the module, specifically, the new PE windows upgrade, which required some path changes.

(PE-10956) Fix linter errors on windows PR 66

c8ea36d

These syntax errors were causing `rake test` to fail, so they have been fixed in this commit.

highb force-pushed the master-pe-10956_windows_upgrade branch from 6788ade to dc14d0f Compare January 25, 2016 18:14

MikaelSmith reviewed Jan 25, 2016
View reviewed changes

highb force-pushed the master-pe-10956_windows_upgrade branch from 3d01dc2 to f4d74e5 Compare January 25, 2016 21:46

Iristyle reviewed Jan 29, 2016
View reviewed changes

highb force-pushed the master-pe-10956_windows_upgrade branch from c7a758c to c2c3ef6 Compare January 29, 2016 23:08

highb reviewed Jan 29, 2016
View reviewed changes

mwbutcher added a commit that referenced this pull request Feb 3, 2016

Merge pull request #66 from highb/master-pe-10956_windows_upgrade

f3af4eb

(PE-10956) windows upgrade

mwbutcher merged commit f3af4eb into puppetlabs:master Feb 3, 2016

(PE-10956) windows upgrade #66

(PE-10956) windows upgrade #66

Conversation

highb commented Sep 25, 2015

highb commented Sep 25, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

highb commented Oct 6, 2015

highb commented Oct 6, 2015

jpartlow commented Oct 7, 2015

highb commented Oct 7, 2015

jpartlow commented Dec 4, 2015

highb commented Dec 8, 2015

highb commented Dec 9, 2015

jpartlow commented Dec 11, 2015

jpartlow commented Dec 11, 2015

highb commented Dec 15, 2015

highb commented Dec 15, 2015

highb commented Dec 15, 2015

highb commented Dec 15, 2015

jpartlow commented Dec 15, 2015

highb commented Dec 15, 2015

jpartlow commented Dec 15, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

highb commented Dec 15, 2015

highb commented Dec 16, 2015

highb commented Dec 16, 2015

highb commented Jan 25, 2016

highb commented Jan 25, 2016

highb commented Jan 25, 2016

highb commented Jan 25, 2016

MikaelSmith commented Jan 25, 2016

MikaelSmith commented Jan 25, 2016

Choose a reason for hiding this comment

highb commented Jan 25, 2016

highb commented Jan 25, 2016

jpartlow commented Jan 27, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

highb commented Feb 1, 2016

jpartlow commented Feb 1, 2016

highb commented Feb 1, 2016

mwbutcher commented Feb 3, 2016