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

Updates to manage LWRP #212

Merged
merged 3 commits into from Oct 15, 2015

Conversation

Projects
None yet
2 participants
@jmanero-r7
Copy link
Contributor

commented Jun 23, 2015

Set cookbook attribute for template in manage provider

If the cookbook attribute is not defined for a template in an LWRP, the resource will search in the instantiating cookbook, not the defining (datadog in this case) cookbook, for its source.

Restart notifications in manage provider

The manage resource issued restart notifications to the datadog-agent service regardless of the node['datadog']['agent_start'] attribute. This could cause the service to start even though the desired state was stopped.

jmanero-r7 added some commits Jun 23, 2015

Add cookbook attribute to monitor resource, and pass to template in c…
…reate action.

This allows the datadog_monitor resource to be used in other cookbooks. Currently it fails as the template resource in the create action does not search for templates in the datadog cookbook.
Defining this as an attribute with  as the default also allows custom templates to be passed from user cookbooks.
Guard restart notifications on the node attribute.
This keeps datadog_monitor resources from starting the dd-agent when the service should remain stopped
seems to return the cookbook that the resource is instantiated in. T…
…his does not help us as a default value.
@miketheman

This comment has been minimized.

Copy link
Collaborator

commented Jul 4, 2015

@jmanero-r7 That's a good point about the cookbook path. Most of the time, we've directed use of the datadog::<integrationname> recipes, which will typically execute the datadog_monitor resource at that time.
This was one way to solve the possibility of a node having multiple datadog_monitor invocations for the same integration in differing recipes, overwriting the same config file and restarting the agent multiple times unnecessarily per run.

By building up the instances hash on the node object, and including the recipe once, that solved this particular problem.

Can you show an example of using datadog_monitor external to the cookbook that does this safely?

I like the restart action addition - that's a good one to have. Can you try adding some test cases for it?

@jmanero-r7

This comment has been minimized.

Copy link
Contributor Author

commented Jul 14, 2015

Thanks @miketheman. I'll try to get a test added when I have some free time.

This was one way to solve the possibility of a node having multiple datadog_monitor invocations for the same integration in differing recipes, overwriting the same config file and restarting the agent multiple times unnecessarily per run.

IMO It's really the responsibility of the consumer to handle conflicts if they define a resource. The Chef runtime will generate warnings during the compile phase if multiple resources with the same identity (e.g. datadog_monitor[cassandra]) are defined. A previously defined resource can be fetched and modified later in the compile phase if the consumer really wants to. For example (not that this is a good idea, but...):

resources('datadog_monitor[cassandra]').instances.push(:host => 'localhost', :port=> 1234)

Multiple restarts are a non-issue as the notification is :delayed. Since Chef 11, these get queued and de-duplicated, so the service[datadog-agent] resource will only have its :restart action run once regardless of how many times notifies :restart, 'service[datadog-agent]' is triggered.

@miketheman miketheman added the feature label Aug 19, 2015

@miketheman miketheman added this to the Next minor milestone Aug 19, 2015

miketheman added a commit that referenced this pull request Oct 15, 2015

Merge pull request #212 from rapid7/master
Updates to manage LWRP

@miketheman miketheman merged commit 5889e13 into DataDog:master Oct 15, 2015

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 100.0%
Details
@miketheman

This comment has been minimized.

Copy link
Collaborator

commented Oct 15, 2015

@jmanero-r7 Thanks!

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.