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

(MODULES-1014) Adding noop mode option #153

Merged
merged 2 commits into from
Jun 19, 2014

Conversation

petems
Copy link

@petems petems commented Jun 2, 2014

This is a bit of a test the waters pull-request.

So there's a ticket to add a noop option detection (https://tickets.puppetlabs.com/browse/MODULES-1014), I'm thinking it'd look something like this? If people are cool with this I can add it to all the non-idempotent stuff. Any thoughts?

@apenney
Copy link

apenney commented Jun 5, 2014

We like the idea of noop!

underscorgan pushed a commit that referenced this pull request Jun 19, 2014
(MODULES-1014) Adding noop mode option
@underscorgan underscorgan merged commit 5a61e81 into puppetlabs:master Jun 19, 2014
@petems
Copy link
Author

petems commented Jun 19, 2014

I'll have a Beaker test for this as soon as voxpupuli/beaker#313 is fixed! 👍

@petems petems deleted the MODULES-1014/add-noop-mode branch June 19, 2014 18:33
@PeterJCLaw
Copy link

Hi,

Not sure if this is quite the right place to post this (apologies if not -- please do let me know where I should put it instead), but I'm getting an issue post this commit on Fedora 17 using puppet 2.7.21. The error is: Could not evaluate: undefined method noop?' for ensure:Puppet::Type::Vcsrepo::Ensure`. I believe that the rest of my submodules are up to date with the latest versions of things.

Is there some other dependency I've missed?

Thanks,
Peter

@sodabrew
Copy link

Hm, that may be a bug with this PR. @petems The beaker issue 313 linked above is now resolved, do you have a moment to write up a test for this noop mode?

@sodabrew
Copy link

@PeterJCLaw If you patch in #176 does it resolve the problem for you?

@petems
Copy link
Author

petems commented Jun 24, 2014

I've got a beaker test with the new :noop => true option cooking, will be done soon 👍

@petems
Copy link
Author

petems commented Jun 24, 2014

Ok, looks like it's a live issue even with the latest change:

require 'spec_helper_acceptance'

tmpdir = default.tmpdir('vcsrepo')

describe 'remove a repo' do
  it 'creates a blank repo' do
    pp = <<-EOS
    vcsrepo { "#{tmpdir}/testrepo_deleted":
      ensure   => present,
      provider => git,
    }
    EOS
    apply_manifest(pp, :catch_failures => true)
  end

  it 'does not remove a repo' do
    pp = <<-EOS
    vcsrepo { "#{tmpdir}/testrepo_deleted":
      ensure   => absent,
      provider => git,
      force    => true,
    }
    EOS

    apply_manifest(pp, :catch_failures => true, :noop => true, :verbose => false)
  end

  describe file("#{tmpdir}/testrepo_deleted") do
    it { should be_directory }
  end
end
Warning: Could not retrieve fact fqdn
        Notice: Compiled catalog for centos-64-x64 in environment production in 0.05 seconds
        Warning: Found multiple default providers for vcsrepo: dummy, git; using dummy
        Info: Applying configuration version '1403617998'
        Error: /Stage[main]/Main/Vcsrepo[/tmp/vcsrepo.Pd7w2p/testrepo_deleted]: Could not evaluate: undefined method `noop?' for #<Puppet::Type::Vcsrepo::Ensure:0x7fa6215762e0>

@PeterJCLaw
Copy link

I probably should have mentioned upfront that this is with an existing config which uses force => true, but which doesn't use noop at all.
I will give it a go with #176 added shortly.

@PeterJCLaw
Copy link

Hi,
Adding #176 made things work for me. Let me know if you'd like a separate bug report, but I'm guessing that that PR was triggered by someone else finding the same issue.
Thanks for the quick responses,
Peter

cegeka-jenkins pushed a commit to cegeka/puppet-vcsrepo that referenced this pull request Jan 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants