Coverage, Syntax, And Chef

In Why I Don’t Track Test Coverage I explained why I don’t think coverage measures the quality of my tests. There’s a counter argument, though: 100% coverage means every line of code runs when the tests run, so it’s impossible to break prod with a misplaced comma. I think this counter argument is wrong when you’re working in Chef (I think it’s wrong in Python too but I’ll cover that in a separate post).

When Chef runs it processes each recipe into a list of actions (the ‘compile’ phase) and then it does those actions on the system (the ‘converge’ phase). The compile phase will (usually) execute every line of every recipe that’s in the run_list or is included with include_recipe. Both ChefSpec and kitchen/Serverspec take Chef through its compile phase, so in simple cases a syntax error will make both fail before the system is touched.

There are three (anti-)patterns in Chef that I know of that can sneak changes to system state past the compiler even when there are syntax errors:

#1 Raw Ruby

Chef recipes are Ruby files. You can put any valid Ruby code in them. You could do this:

File.delete('/etc/my_app/config.ini')

Ruby deletes config.ini as soon as it hits this line, before the rest of the compile finishes. If there’s a syntax problem later in the code you’ll still get an error but you’ll already have edited the system. The fallout of incomplete Chef client runs can get ugly (more on that another time).

Imagine if the tests for a Jenkins cookbook deleted the Jenkins config file. Then, a side-effect like this could take down a build server that does the innocent task of running ChefSpecs (which are only supposed to simulate Chef’s actions). It’s also surprisingly easy to accidentally hide this from the tests using #2 or #3 from below, which can cause incomplete Chef runs in production.

If you have side-effects like this in your code, replace them with a Chef resource (file with the :delete action in this case), write a custom resource, extract them into a gem that’s run before Chef runs, etc. Chef shouldn’t touch the state of the system before its converge phase.

#2 Ruby Conditions

Foodcritic, the linter for Chef, warns you not to do this:

if node['foo'] == 'bar'
  service 'apache' do
    action :enable
  end
end

Their argument is that you should use Guards, the library’s built-in feature:

service 'apache' do
  action :enable
  only_if { node['foo'] == 'bar' }
end

That’s a great argument, but there’s one more: with a Ruby condition, the resource won’t be compiled unless node[‘foo’] == ‘bar’. That means that unless you have a test where this is set, the compiler will never touch this resource and a syntax error won’t make the tests fail.

If you follow foodcritic’s recommendation, conditional resources will always be compiled (but may not be converged) and syntax errors will fail early without you doing any work.

#3 Conditional Includes

These technically belong with the other Ruby conditions, but they’re extra-nasty so I’m dedicating a section to them.

If you do this:

if node['foo'] == 'bar'
  include_recipe 'my_cookbook::my_recipe'
end

The resources in my_recipe will only be compiled if foo is set to bar in the node object. This is like putting a Ruby condition around every resource in my_recipe.

It gets worse if your condition is processed in the converge phase. For example, you could do an include in a ruby_block:

block 'run_my_recipe' do
  if File.directory?('/etc/my_app')
    run_context.include_recipe 'my_cookbook::my_recipe'
  end
end

Even if /etc/my_app exists, my_recipe won’t be compiled until Chef enters the converge phase and reaches the run_my_recipe resource. I bet you that nobody reading your cookbook will realize that it changes Chef’s “compile then converge” order into “compile some stuff then converge some stuff then compile the rest then converge the rest”. This is likely to bite you. Plus, now you have to start looking at mocks to make sure the tests exercise all your recipes. My advice is to avoid this pattern. Maybe there’s some special situation I haven’t found, but the few cases of converge-time conditional includes that I’ve seen have been hacks.

Conditional includes are usually a symptom of using Chef for something it’s not designed for. Chef is designed to converge the host where it’s running to a specific state. Its resources do a great job of detecting if that state is already present and skipping their actions if it is. If you have lots of resources that aren’t safe to run multiple times and that Chef isn’t automatically skipping then you should take a step back and make sure Chef is the right tool. Your standard approach should be to include all the recipes that may need to run and write each recipe to guard its resources from running when they shouldn’t.

 

If you write your Chef cookbooks well then you get 100% syntax coverage for free if you’ve written even one test. You can focus on exercising the logic of your recipes. Leave it to the robots to catch those misplaced commas.

Thanks for reading!

Adam

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out /  Change )

Google+ photo

You are commenting using your Google+ account. Log Out /  Change )

Twitter picture

You are commenting using your Twitter account. Log Out /  Change )

Facebook photo

You are commenting using your Facebook account. Log Out /  Change )

Connecting to %s