Default attributes set to nil can override attributes set at the same level

[alq666]

In one instance, using our cookbook with chef solo causes the Redis attributes to be set to nil by our cookbook thereby causing the Redis cookbook to run with nil attributes. There are workaround for this, e.g. set the Redis attributes at a higher priority than that of our cookbook but we're not playing nice.

Suggested solution #1: namespace our attributes to read from the Redis attributes with overriding them.

Default['datadog']['redis']['server']=default['redis']['server'] || nil

In the proper ruby way (making sure the above won't bomb)

Suggested solution #2: write a lightweight provider for our stuff (nice and idiomatic) so that you can say

Datadog_monitor(Redis, server, port)

And the rest is taken care of for you (nice in that it will hide our transition to split configuration files)

[alq666]

@miketheman your feedback is welcome.

[miketheman]

I like suggestion No.2 - this makes defining a datadog config very conf.d-style and awesome.

However, let me float No.3 (not using hashes, as they link to GH Issues...):

Instead of defining attributes that are "owned" by other namespaces and potentially causing overrides, maybe look at inspecting whether the attribute exists first?

Here's a short repl session in shef:

chef > node.attribute?(:platform)
 => true 
chef > node.attribute?(:redis)
 => false 
chef > node[:redis] = {:server => "localhost"}
 => {:server=>"localhost"} 
chef > node[:redis][:server]
 => "localhost" 
chef > node.attribute?(:redis)
 => true

So as you can see, we might want to inspect the existence of an attribute (or set) in a template before placing code. Code references are here, and I chose to use the alias attribute to be clear that we're looking for a node's attribute.

Something like:

<% if node.attribute?('redis') %>
# redis, from http://community.opscode.com/cookbooks/redis
redis_urls: <%= node['redis']['server']['addr']%>:<%= node['redis']['server']['port'] %>
<% end %>

Or better yet - look for the server subset:

chef > node.attribute?(:redis) && node[:redis].attribute?(:server)
 => true 

It may be exhaustive to test for every attribute, but it's kind of a safe assumption that "if you're using a well-known cookbook to deploy the software, then you'll have these set of attributes", and this allows the DD cookbook to not have to predefine them.

But to be clear - I like No.2 the best - but it's gotta be a little open-ended to allow for any variation of parameters.

[alq666]

Option 3 makes sense. It'll minimize the disruption.
On Sep 27, 2012 10:56 PM, "Mike Fiedler" [email protected] wrote:

I like suggestion No.2 - this makes defining a datadog config very
conf.d-style and awesome.

However, let me float No.3 (not using hashes, as they link to GH
Issues...):

Instead of defining attributes that are "owned" by other namespaces and
potentially causing overrides, maybe look at inspecting whether the
attribute exists first?

Here's a short repl session in shef:

chef > node.attribute?(:platform)
=> true chef > node.attribute?(:redis)
=> false chef > node[:redis] = {:server => "localhost"}
=> {:server=>"localhost"} chef > node[:redis][:server]
=> "localhost" chef > node.attribute?(:redis)
=> true

So as you can see, we might want to inspect the existence of an attribute
(or set) in a template before placing code. Code references are herehttp://rubydoc.info/gems/chef/Chef/Node/Attribute#has_key%3F-instance_method,
and I chose to use the alias attribute to be clear that we're looking for
a node's attribute.

Something like:

<% if node.attribute?('redis') %># redis, from http://community.opscode.com/cookbooks/redisredis_urls: <%= node['redis']['server']['addr']%>:<%= node['redis']['server']['port'] %><% end %>

Or better yet - look for the server subset:

chef > node.attribute?(:redis) && node[:redis].attribute?(:server)
=> true

It may be exhaustive to test for every attribute, but it's kind of a safe
assumption that "if you're using a well-known cookbook to deploy the
software, then you'll have these set of attributes", and this allows the DD
cookbook to not have to predefine them.

But to be clear - I like No.2 the best - but it's gotta be a little
open-ended to allow for any variation of parameters.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/23#issuecomment-8964031.

[zimbatm]

Solution 1 seems the most solid-one.

The biggest issue from re-using other cookbooks's values (apart that the current impl. breaks my redis cookbook) is that you then don't have the choice if you want to monitor or not. Client machines might have the node[:redis] value set but I don't really want to monitor the server from all the client hosts.

Solution 3 is nice too but it's more work. The datadog client needs to understand split config files (eg. /etc/datadog.d/*.conf) so that each LWRP resource can set it's monitor independently (to avoid ordering issues in the cookbook evaluation).

[miketheman]

@zimbatm while adding namespace for attributes would work, it is very much duplication of effort and code. As to splitting the agent configs, this is already in progress (or done?) over in the agent repo.

You raise a good point about clients monitoring server, and that should probably be worked out as well, and can probably be handled with split agent configs.

I envision this working similar to the apache2 cookbook, where you can provide a list of components you want configs for (in a role or such) as well as the default attributes you want to provide to those configs.

[miketheman]

@zimbatm Here's the references I wrote about:

A conf.d-style approach in the agent: DataDog/dd-agent#194

Apache2 cookbooks module inclusion: https://github.com/opscode-cookbooks/apache2/blob/master/attributes/default.rb#L140-149

[ansel1]

So others can find this more quickly than I did, the manifestation of this may be something like:

NoMethodError
-------------
undefined method `[]=' for nil:NilClass

Cookbook Trace:
---------------
  /tmp/vagrant-chef-1/chef-solo-1/cookbooks/mysql/attributes/server.rb:81:in `from_file'

datadog is near the beginning of my run list. During compilation phase, it sets default['mysql']['server'] = nil, then mysql cookbooks attributes are compiled, which tries default['mysql']['server']['packages'] = %w{mysql-server}, and fails.

[miketheman]

I believe this has been resolved in master branch, to be released soon in the upcoming 1.0.0 release, completed by #46.

Thanks everyone for surfacing your thoughts and helping figure out what the approach should be!