voxpupuli / puppet-rabbitmq

RabbitMQ Puppet Module
http://forge.puppetlabs.com/puppet/rabbitmq
Apache License 2.0
171 stars 500 forks source link

resources fail to prefetch when rabbitmq is not intended to be installed (via --noop or --tags) #961

Closed bugfood closed 11 months ago

bugfood commented 1 year ago

Affected Puppet, Ruby, OS and module versions/distributions

How to reproduce (e.g Puppet code you use)

mkdir modules
git clone https://github.com/voxpupuli/puppet-rabbitmq modules/rabbitmq
git clone https://github.com/puppetlabs/puppetlabs-stdlib modules/stdlib
git clone https://github.com/voxpupuli/puppet-systemd.git modules/systemd
git clone https://github.com/voxpupuli/puppet-archive modules/archive

cat > rabbit.pp << EOF
class { 'rabbitmq':
    erlang_cookie => 'asdf',
}

rabbitmq_user { 'foo':
    password => 'bar',
}

notify { 'foo':
    message => 'bar',
    require => Rabbitmq_user['foo'],
    tag     => 'message',
}
EOF

sudo puppet apply --modulepath modules rabbit.pp --tags message

What are you seeing

Error: Could not prefetch rabbitmq_user provider 'rabbitmqctl': Command rabbitmqctl is missing

What behaviour did you expect instead

Since rabbitmq resources do not have the message, tag, they should neither be applied nor fail.

Output log

Warning: This function is deprecated, please use stdlib::ensure_packages instead. at ["/home/chickey/modules/rabbitmq/manifests/install/rabbitmqadmin.pp", 14]:["/home/chickey/modules/rabbitmq/manifests/init.pp", 567]
   (location: /home/chickey/modules/stdlib/lib/puppet/functions/deprecation.rb:35:in `deprecation')
Notice: Compiled catalog for <redacted> in environment production in 0.41 seconds
Error: Could not prefetch rabbitmq_user provider 'rabbitmqctl': Command rabbitmqctl is missing
Notice: /Stage[main]/Main/Notify[foo]: Dependency Rabbitmq_user[foo] has failures: true
Warning: /Stage[main]/Main/Notify[foo]: Skipping because of failed dependencies
Notice: Applied catalog in 0.55 seconds

Any additional information you'd like to impart

I will submit a draft PR with a POC workaround for this. The commit will include a more technical explanation. I don't really know what a proper fix would be.

Original discussion in puppet-community slack: https://puppetcommunity.slack.com/archives/C0W298S9G/p1699468482674569

Thanks, Corey

wyardley commented 1 year ago

Since the notify resource that does have the tag requires rhe user resource, isn’t it expected that puppet will try to check if it exists

bugfood commented 1 year ago

Since the notify resource that does have the tag requires rhe user resource, isn’t it expected that puppet will try to check if it exists

I don't really know what is expected, but I don't think that there is any need to check the status of a resource that isn't going to be applied.

The use of tags "filters out" resources without the specified tag(s), whether or not they are depended upon by other resources that do have a specified tag. Here you can see an example of a manifest that would create a file, but I filter that out via tags, so the file does not get created.

file { '/foo':
    ensure => 'file',
}

notify { 'bar':
    message => 'baz',
    require => File['/foo'],
    tag     => 'message',
}
$ puppet apply test.pp --tags message
Notice: Compiled catalog for <redacted> in environment production in 0.01 seconds
Notice: baz
Notice: /Stage[main]/Main/Notify[bar]/message: defined 'message' as 'baz'
Notice: Applied catalog in 0.02 seconds

Going back to the example with rabbitmq_user, in a --noop or --tags run, puppet cannot check the status of the rabbitmq user, but since puppet isn't going to apply the rabbitmq user anyway, it should be ok to let the check assume the user does not exist.

At least, that's my thinking. I don't know what standard practice is for puppet modules in this sort of situation. This is the first time I've encountered this problem, but I suppose most other modules don't need to install a package in order to check resource status.

bugfood commented 1 year ago

If I were to add an applicable tag to the rabbitmq_user, then that would cause puppet to fail, even with my workaround in place.

Error: /Stage[main]/Main/Rabbitmq_user[foo]/ensure: change from 'absent' to 'present' failed: Command rabbitmqctl is missing

That's expected, and it's a pathological situation: there's no way to apply the rabbitmq user if rabbitmq isn't even installed. Thus, it's good for this to still fail. The rabbitmq user has an actual functional dependency upon the rabbitmq package (and service); not just an ordering dependency.

wyardley commented 1 year ago

I am not an expert on how this "should" work; I guess to me it makes sense that if the notify "requires" a user that does not exist, it should not apply, even in the case of a noop or applying via a tag that the resource it depends on doesn't have. Even though it wouldn't try to create it if it didn't exist, it would need to check its status to be required.

but I suppose most other modules don't need to install a package in order to check resource status.

The file resource is a bit different since it's presumably implemented natively within puppet, but I would other modules that both depended on a package to be installed, and used types / providers could exhibit similar behavior.

bugfood commented 1 year ago

I can see the logic in that. I don't think that puppet quite works that way with respect to tags, though. When tags are specified, puppet does not automatically apply the dependencies; the onus is on the user to apply tags judiciously to ensure that any actual functional dependencies are properly tagged.

If puppet did automatically apply the dependencies, then it would break the use of tags in conjunction with run stages. Since a resource in a later run stage indirectly requires all resources in earlier run stages, attempting to selectively apply a resource via a tag would end up applying all the resources in earlier run stages.

The combination of run stages and tags is exactly what happened to us when discovering this bug. We have a resource in our final run stage which is tagged for application in a selective --tags run.