-
Notifications
You must be signed in to change notification settings - Fork 262
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
[windows] Add option to support EXE Agent installer #410
Conversation
The EXE installer handles per-user to per-machine upgrades gracefully, including edges cases that the present cookbook can't reasonably handle on its own. For this reason, we add the option to use the exe installer when needed (i.e. for upgrades from Agent <= 5.10.1 to Agent >= 5.12.0). Also added related documentation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, modulo the one comment.
attributes/default.rb
Outdated
@@ -111,6 +111,13 @@ | |||
# Expected checksum to validate correct agent installer is downloaded (Windows only) | |||
default['datadog']['windows_agent_checksum'] = nil | |||
|
|||
# Set to `true` to use the EXE installer on Windows, recommended to gracefully handle upgrades from per-user | |||
# to per-machine installs for all known use cases. We recommend setting this option to `true` for Agent upgrades from |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I'd say "all known use cases". That's pretty broad :)
918a8db
to
2c165ab
Compare
[skip ci]
recipes/_install-windows.rb
Outdated
@@ -25,28 +25,48 @@ | |||
end | |||
|
|||
# If no version is specified, select the latest package | |||
dd_agent_msi = dd_agent_version ? "ddagent-cli-#{dd_agent_version}.msi" : 'ddagent-cli.msi' | |||
temp_file = ::File.join(Chef::Config[:file_cache_path], 'ddagent-cli.msi') | |||
dd_agent_installer_basename = dd_agent_version ? "ddagent-cli-#{dd_agent_version}" : 'ddagent-cli' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: let's not forget to follow this naming convention when uploading the new Windows packages. (default name is different)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
at this point the default installer URL is not different for the MSI (and the EXE has the same URL, except that it has an .exe
extension).
In general though if we're concerned about the user experience of automatic upgrades failing when the default latest package is downloaded (shouldn't happen with this cookbook but could happen with user-maintained scripts) then we may consider using a different URL for the new installer.
The URL to the latest installer changes in 5.12.0 from `ddagent-cli` to `ddagent-cli-latest` so that unattended upgrades don't make chef runs fail. Reflect this here. The new URLs work only since 5.12.0, so this change should only be released once the 5.12.0 installers (msi and exe) are generally available at these URLs
f6c4249
to
0507f83
Compare
Tested the change on Chef 12.2 and 12.18, works well on clean installs and upgrades. Updated the spec tests assuming they run on Chef >= 12.6. This should be removed (and the whole recipe should be cleaned up) when we stop supporting Chef < 12.6 on Windows.
@degemer I've added a few commits since your review, could you have one last look at the changes? :) |
The EXE installer handles per-user to per-machine upgrades
gracefully, including edges cases that the present cookbook can't
reasonably handle on its own.
For this reason, we add the option to use the exe installer when needed
(i.e. for upgrades from Agent <= 5.10.1 to Agent >= 5.12.0).
Also added related documentation