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

Fixes #366 - Add ability to pin datadog-agent versions to install for multiple platforms #368

Merged
merged 7 commits into from Oct 18, 2016

Conversation

Projects
None yet
3 participants
@mlcooper
Copy link
Contributor

commented Oct 6, 2016

No description provided.

mlcooper and others added some commits Oct 6, 2016

@iancward

This comment has been minimized.

Copy link
Contributor

commented Oct 17, 2016

@olivielpeau since Fedora and RHEL pull from the same repository, we figured it would be better to have both platform families use the same attribute key for the agent version. However, we could just as easily remove that logic, which would require a user to set a separate fedora and rhel attribute.
Let @mlcooper and I know which way makes the most sense.

@olivielpeau
Copy link
Member

left a comment

Thanks @iancward!

Having the same key for fedora and rhel makes sense to me too.

I've added a few comments on the spec tests, could you address them? The code/logic looks good to me.

) do |node|
node.set['datadog'] = {
'api_key' => 'somethingnotnil',
'agent_version' => '4.4.0-200'

This comment has been minimized.

Copy link
@olivielpeau

olivielpeau Oct 17, 2016

Member

nit: could you use a version with an epoch here since the platform is ubuntu? (1:5.9.0-1)

ChefSpec::SoloRunner.new(
:platform => 'windows',
:version => '2012R2',
:file_cache_path => '/tmp'

This comment has been minimized.

Copy link
@olivielpeau

olivielpeau Oct 17, 2016

Member

I'd prefer a "realistic" path on Windows (C:/chef/cache for instance, see https://github.com/DataDog/chef-datadog/blob/v2.6.0/spec/dd-agent_spec.rb#L162)

# But we should probably assert the full default attribute somewhere...
it 'installs agent 4.4.0' do
expect(chef_run.remote_file(temp_file).source.to_s)
.to match(/ddagent-cli-4.4.0.msi/)

This comment has been minimized.

Copy link
@olivielpeau
end

temp_file = ::File.join('/tmp', 'ddagent-cli.msi')

This comment has been minimized.

Copy link
@olivielpeau

olivielpeau Oct 17, 2016

Member

could you add the common example it_behaves_like 'windows Datadog Agent' here?

(you'll also need to set the env var ProgramData like here: https://github.com/DataDog/chef-datadog/blob/v2.6.0/spec/dd-agent_spec.rb#L158)

@olivielpeau olivielpeau added this to the 2.7.0 milestone Oct 17, 2016

@olivielpeau olivielpeau added the feature label Oct 17, 2016

mlcooper added some commits Oct 18, 2016

@mlcooper

This comment has been minimized.

Copy link
Contributor Author

commented Oct 18, 2016

Hi @olivielpeau please let us know if you're okay with the changes made per your feedback. Thanks!

@olivielpeau

This comment has been minimized.

Copy link
Member

commented Oct 18, 2016

Looks great, thanks @mlcooper and @iancward!

Merging

@olivielpeau olivielpeau merged commit 8d8a873 into DataDog:master Oct 18, 2016

2 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.