rodjek / rspec-puppet

RSpec tests for your Puppet manifests
http://rspec-puppet.com
MIT License
364 stars 203 forks source link

Test failures with 2.3 and new Puppet 3.8.5 #353

Closed garethr closed 8 years ago

garethr commented 8 years ago

I hit an issue this morning with the Docker module in Travis where a single test (which had previously passed) failed, but only under the newly released Puppet 3.8.5 (released January 21st). This was also reported independently by another contributor https://github.com/garethr/garethr-docker/pull/396.

Running the test on it's own see's it pass. Testing with previously passing code fails. Testing with rspec-puppet master and with 2.3 also fails. As suggested by @DavidS I tried 2.2 and this allowed the test to pass.

After much spelunking I was able to get a small test case which reliably recreates the failure.

require 'spec_helper'

describe 'docker::run', :type => :define do
  let(:title) { 'sample' }
  context 'passing the required params' do
    let(:facts) { {:osfamily => 'Archlinux'} }
    let(:params) { {'command' => 'command', 'image' => 'base'} }
    initscript = '/etc/systemd/system/docker-sample.service'
    # The first 16 tests will pass. The rest will fail on Puppet 3.8.5 with
    # NoMethodError:
    # undefined method `resource' for nil:NilClass
    # /Users/garethr/.rvm/gems/ruby-2.1.5/bundler/gems/rspec-puppet-7ab588ecdd1e/lib/rspec-puppet/matchers/create_generic.rb:83:in `matches?'
    18.times do
      it { should contain_file(initscript).with_content(/docker run/).with_content(/command/) }
    end
  end
end

Note the magic number 16. Given that is the number of catalogues cached in rspec-puppet I think that's likely related. Whether this points to a bug in Puppet 3.8.5 which rspec-puppet is exposing, or whether it's rspec-puppet using a private API that's changed it's behaviour, or something else I'm unsure.

DavidS commented 8 years ago

/cc @domcleal

cardil commented 8 years ago

This was also my issue. I've already created a ticket on Puppetlabs Jira on this issue with examples: https://tickets.puppetlabs.com/browse/PUP-5743

Walkaround for now is to use Puppet 3.8.4 for rspec tests:

export PUPPET_VERSION=3.8.4

caused by NPE in https://github.com/rodjek/rspec-puppet/blob/master/lib/rspec-puppet/matchers/create_generic.rb#L83 (catalouge is nil)

daenney commented 8 years ago

As posted by @hlindberg in that ticket:

With a bit more detective work. The recently introduced cache in rspec_puppet does the wrong thing when it evicts entries from its cache. (It ends up removing the entries that should be kept as opposed to those that should be evicted. https://github.com/rodjek/rspec-puppet/blob/8a90608341077197323c6e35c042f7d1699a3cae/lib/rspec-puppet/cache.rb#L26

hlindberg commented 8 years ago

I made a new PR with a improved caching algorithm. It is however not the cause of at least one of the reported problems. The reason it works before PUP-5522 is mysterious as that change ensures that the string representing the environment name, and the environment instance set in the node do not diverge.

If rspec_puppet is trying to game the system by pretending that a node is in another environment than what it really is in, that would be prevented by the fix in 5522. Will see if I find something that is smelly in that area. (Posting here as it may trigger someone else to come up with a theory what may be wrong).

The really odd thing is that the cache tests fail because of changing object_id's. Either something mutates the cache, or the initial value used in the comparison is wrong. The tests actually get the id they compare against in a separate it clause; what if something is take down and set up again between the tests? What if tests run in a different order (which they will when things change, or when running only one tests out of all of them.

hlindberg commented 8 years ago

@kylog and I continued working on the problem, and found that rspec_puppet is sensitive to if the environment name is a string or a symbol. The change in PUP-5525 would change a string environment name set as a string to a symbol when setting the environment instance. Patching that logic in puppet made rspec_puppet's tests green. We don't yet know what to do about the problem. If there is a simple solution in rspec_puppet, or if a new puppet release is required. (sigh)

rnelson0 commented 8 years ago

354 appears to have instroduced an issue with puppet 4.3.2. See https://travis-ci.org/rnelson0/puppet-local_user/builds/104732194:

  1) local_user using minimum params should contain User[rnelson0] with comment => "rnelson0", shell => "/bin/bash", home => "/home/rnelson0", groups => ["group1", "group2"] and password_max_age => 90

     Failure/Error: it { is_expected.to create_user('rnelson0').with({

     Puppet::Error:

       Unsupported data type: 'Symbol' on node testing-worker-linux-docker-060f4cff-3202-linux-7.prod.travis-ci.org

This shows up with puppet 4.3.2 and rspec-puppet 2.3.1 regardless of ruby version, but not with puppet 3.8.5/rspec-puppet 2.3.1.

rnelson0 commented 8 years ago

And to prove it's not a fluke, I see it in other modules consistently, as in https://travis-ci.org/rnelson0/puppet-certs/jobs/104733757:

  1) certs with defaults for all parameters should contain Class[certs]

     Failure/Error: it { should contain_class('certs') }

     Puppet::Error:

       Unsupported data type: 'Symbol' on node testing-worker-linux-docker-4ecf8fb7-3188-linux-8.prod.travis-ci.org
DavidS commented 8 years ago

Augh, I thought 4.3.2 also has this fix applied, but seems like the old adage about assuming holds true.

I'll look into it first thing tomorrow morning. On Jan 25, 2016 21:05, "Rob Nelson" notifications@github.com wrote:

And to prove it's not a fluke, I see it in other modules consistently, as in https://travis-ci.org/rnelson0/puppet-certs/jobs/104733757:

1) certs with defaults for all parameters should contain Class[certs]

 Failure/Error: it { should contain_class('certs') }

 Puppet::Error:

   Unsupported data type: 'Symbol' on node testing-worker-linux-docker-4ecf8fb7-3188-linux-8.prod.travis-ci.org

— Reply to this email directly or view it on GitHub https://github.com/rodjek/rspec-puppet/issues/353#issuecomment-174662366 .

vladgh commented 8 years ago

I confirm what @rnelson0 said above.

In 2.3.1 absolutely all my tests fail with Unsupported data type: 'Symbol'

https://travis-ci.org/vladgh/puppet/builds/104784252

PS: Puppet 4.3.2, Ruby 1.9.3 / 2.1.7 / 2.3.0

daenney commented 8 years ago

cc @kylog @hlindberg

DavidS commented 8 years ago

This should now be addressed with 2.3.2

DavidS commented 8 years ago

This should now be addressed with 2.3.2

rnelson0 commented 8 years ago

Yay, thank you!


$ be gem list | grep puppet
puppet (4.3.2)
rspec-puppet (2.3.2)
$ be rspec spec/classes/build_spec.rb

10 examples, 0 failures
daenney commented 8 years ago

:clap: :tada: :fireworks: