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
[dd-agent] Add attribute to allow agent downgrade #359
Conversation
Despite the Foodcritic warning, it works. If you don't find a way to fix it, I'm fine with silencing this error in this specific case. Tested on Ubuntu 15.04 and Centos 7.0. 👍 |
3e2d26d
to
544e665
Compare
@degemer The Foodcritic warning kind of makes sense, so I separated the Also added spec tests on the new attribute |
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.
A BC comment, I know you like them. No comment otherwise, and CI is broken on the url
issue...
version dd_agent_version | ||
action node['datadog']['agent_package_action'] # default is :install | ||
allow_downgrade node['datadog']['agent_allow_downgrade'] | ||
end |
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.
If we want to be really cautious, we could add a default with the old resource.
List of all the Linux platform_family
: https://github.com/chef/ohai/blob/af061dd99e771197b9aaec8c63a42b07a72662f2/lib/ohai/plugins/linux/platform.rb
I don't see anything missing, but...
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.
Discussed this offline: since we're doing the same test in recipes/repository.rb
, the cookbook doesn't work on other platforms anyway, so no need to fall back on a default here.
@@ -298,6 +306,98 @@ def set_env_var(name, value) | |||
end | |||
end | |||
|
|||
context 'package downgrade' do |
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.
🍰
Still not allowed by default. Switching the attribute to `true` allows downgrades: * Straightforward for rhel/fedora: the yum and rpm package providers support a parameter that does just that * Uses `--force-yes` on debian: there's no direct option on the apt provider. `apt-get` supports a more fine-grained option (`--allow-downgrades`) since version `1.1` of `apt`, but it ships with ubuntu starting from `16.04` only, so we stick to using `--force-yes`.
`allow_downgrade` is available on `yum_package` but not on the generic `package` resource, so it makes sense to designate the provider explicitely. Also: * update the spec tests to adapt to this change * test that the `datadog-agent-base` package is removed
544e665
to
f04306f
Compare
CI is 💚 , merging, thanks @degemer! |
Still not allowed by default. Switching the attribute to
true
allows downgrades:support a parameter that does just that
--force-yes
on debian because:provider
apt-get
supports a more fine-grained option(
--allow-downgrades
) since version1.1
ofapt
, but it ships withubuntu starting from
16.04
only, so we stick to using--force-yes
. It could be nice to switch the option depending on the version ofapt
, but it'd add some complexity to the logic.Notes on downgrading packages with
apt
With
apt
, downgrading can be dangerous for packages that have dependencies/on which other packages depend. Fromdpkg
's manpage, on downgrades:This shouldn't be the case of the
datadog-agent
package so downgrading should be relatively safe.Also, using
--force-yes
is equivalent to allowingremove-essential
,downgrades
andchange-held-packages
. None of these should be dangerous when applied to thedatadog-agent
package only. For reference see the manpage ofapt
1.2
: http://manpages.ubuntu.com/manpages/xenial/en/man8/apt-get.8.html