Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(MODULES-10653) Failed to upgrade agent using puppet task #494

Merged
merged 1 commit into from
Jun 10, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 38 additions & 9 deletions task_spec/spec/acceptance/init_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,29 +40,31 @@ def target_platform
else
'5.5.3'
end
# test the agent isn't already installed and that the version task works

# Test the agent isn't already installed and that the version task works
results = run_task('puppet_agent::version', 'target', {})
results.each do |res|
expect(res).to include('status' => 'success')
expect(res['result']['version']).to eq(nil)
end

# Try to install an older version
# Try to install an older puppet5 version
results = run_task('puppet_agent::install', 'target', { 'collection' => 'puppet5',
'version' => puppet_5_version,
'stop_service' => true })
results.each do |res|
expect(res).to include('status' => 'success')
end

# It installed a version older than latest
# It installed a version older than latest puppet5
results = run_task('puppet_agent::version', 'target', {})
results.each do |res|
expect(res).to include('status' => 'success')
expect(res['result']['version']).to eq(puppet_5_version)
expect(res['result']['source']).to be
end

# Check that puppet agent service has been stopped due to 'stop_service' parameter set to true
service = if target_platform =~ /win/
run_command('c:/"program files"/"puppet labs"/puppet/bin/puppet resource service puppet', 'target')
else
Expand All @@ -71,21 +73,23 @@ def target_platform
output = service[0]['result']['stdout']
expect(output).to include("ensure => 'stopped'")

# Run with no argument upgrades
# Try to upgrade with no specific version given in parameter
# Expect nothing to happen and receive a message regarding this
results = run_task('puppet_agent::install', 'target', { 'collection' => 'puppet5' })
results.each do |res|
expect(res).to include('status' => 'success')
expect(res['result']['_output']).to match(%r{Version parameter not defined and agent detected. Nothing to do.})
end

# Verify that it did nothing
# Verify that the version didn't change
results = run_task('puppet_agent::version', 'target', {})
results.each do |res|
expect(res).to include('status' => 'success')
expect(res['result']['version']).to eq(puppet_5_version)
expect(res['result']['source']).to be
end

# Run with latest upgrades
# Upgrade to latest puppet5 version
results = run_task('puppet_agent::install', 'target', { 'collection' => 'puppet5', 'version' => 'latest' })
results.each do |res|
expect(res).to include('status' => 'success')
Expand All @@ -100,19 +104,44 @@ def target_platform
expect(res['result']['source']).to be
end

# Upgrade from puppet5 to puppet6
# Puppet Agent can't be upgraded on Windows nodes while 'puppet agent' service or 'pxp-agent' service are running
if target_platform =~ /win/
# Try to upgrade from puppet5 to puppet6 but fail due to puppet agent service already running
results = run_task('puppet_agent::install', 'target', { 'collection' => 'puppet6', 'version' => 'latest' })
results.each do |res|
expect(res).to include('status' => 'failure')
expect(res['result']['_error']['msg']).to match(%r{Puppet Agent upgrade cannot be done while Puppet services are still running.})
end

# Manually stop the puppet agent service
service = run_command('c:/"program files"/"puppet labs"/puppet/bin/puppet resource service puppet ensure=stopped', 'target')
output = service[0]['result']['stdout']
expect(output).to include("ensure => 'stopped'")
end

# Succesfully upgrade from puppet5 to puppet6
results = run_task('puppet_agent::install', 'target', { 'collection' => 'puppet6', 'version' => 'latest' })
results.each do |res|
expect(res).to include('status' => 'success')
end

# Verify that it upgraded
installed_version = nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this for?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this set to nil?

Copy link
Contributor Author

@luchihoratiu luchihoratiu Jun 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's there to simply avoid an undefined local variable or method error. Its role is to store the currently installed puppet agent version (since we've previously installed the latest puppet6 version) and it's used below, in the next step, when trying to install the same version again.

I have no doubt that there are better/more elegant approaches to testing this. I just tried to be minimalist with the changes and use the existing steps as much as possible. If there are any concerns, I'm always open to improvement ideas 😃.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. I did not see that was used below.

results = run_task('puppet_agent::version', 'target', {})
results.each do |res|
expect(res).to include('status' => 'success')
expect(res['result']['version']).not_to match(%r{^5\.\d+\.\d+})
expect(res['result']['version']).to match(%r{^6\.\d+\.\d+})
installed_version = res['result']['version']
expect(installed_version).not_to match(%r{^5\.\d+\.\d+})
expect(installed_version).to match(%r{^6\.\d+\.\d+})
expect(res['result']['source']).to be
end

# Try installing the same version again
# Expect nothing to happen and receive a message regarding this
results = run_task('puppet_agent::install', 'target', { 'collection' => 'puppet6', 'version' => installed_version })
results.each do |res|
expect(res).to include('status' => 'success')
expect(res['result']['_output']).to match(%r{Puppet Agent #{installed_version} detected. Nothing to do.})
end
end
end
44 changes: 43 additions & 1 deletion tasks/install_powershell.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,45 @@ function Test-PuppetInstalled {
}
}

function Test-PuppetInstalledVersion {
$rootPath = 'HKLM:\SOFTWARE\Puppet Labs\Puppet'

$reg = Get-ItemProperty -Path $rootPath -ErrorAction SilentlyContinue
if ($null -ne $reg) {
if ($null -ne $reg.RememberedInstallDir64) {
$loc = $reg.RememberedInstallDir64+'VERSION'
} elseif ($null -ne $reg.RememberedInstallDir) {
$loc = $reg.RememberedInstallDir+'VERSION'
}
}

if ($null -ne $loc) {
$installedVersion = Get-Content -Path $loc -ErrorAction SilentlyContinue
if ($installedVersion -eq $version) {
RETURN $true
}
}

RETURN $false
}

function Test-RunningServices {
$puppetAgentService = Get-Service -DisplayName 'Puppet Agent' -ErrorAction SilentlyContinue
$pxpAgentService = Get-Service -DisplayName 'Puppet PXP Agent' -ErrorAction SilentlyContinue

if ($puppetAgentService.Status -eq 'Running' -or $pxpAgentService.Status -eq 'Running') {
RETURN $true
}

RETURN $false
}

if ($version) {
if (Test-PuppetInstalledVersion) {
Write-Output "Puppet Agent ${version} detected. Nothing to do."
Exit
}

if ($version -eq "latest") {
$msi_name = "puppet-agent-${arch}-latest.msi"
} else {
Expand All @@ -43,13 +81,17 @@ if ($version) {
}
else {
if (Test-PuppetInstalled) {
Write-Output "Puppet Agent detected and no version specified. Nothing to do."
Write-Output "Version parameter not defined and agent detected. Nothing to do."
Exit
}

$msi_name = "puppet-agent-${arch}-latest.msi"
}

if (Test-RunningServices) {
Write-Error "Puppet Agent upgrade cannot be done while Puppet services are still running."
}

# Change windows_source only if the collection is a nightly build, and the source was not explicitly specified.
if (($collection -like '*nightly*') -And -Not ($PSBoundParameters.ContainsKey('windows_source'))) {
$windows_source = 'https://nightlies.puppet.com/downloads'
Expand Down