voxpupuli / puppet-rabbitmq

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

rabbitmq_vhost resource not idempotent with a description #1006

Open d1nuc0m opened 5 months ago

d1nuc0m commented 5 months ago

Affected Puppet, Ruby, OS and module versions/distributions

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

rabbitmq_vhost { 'myvhost':
    ensure      => present,
    description => 'A description here',
  }

What are you seeing

At every agent run, a change is reported because of the vhost description, even if this has not been modified

What behaviour did you expect instead

Report changes only if there have been modifications

Output log

Notice: /Stage[main]/Main/Node[rabbitmq.example.com]/Rabbitmq_vhost[myvhost]/description: description changed to 'A description here' (corrective)

Any additional information you'd like to impart

\

wyardley commented 5 months ago

I think I see at least one issue... question: what's the RabbitMQ behavior you see? I can reproduce this in the acceptance tests where the description is not supported yet. If the RMQ version is < 3.11, however, the description will not get set anyway (since it's apparently unsupported).

wyardley commented 5 months ago

It seems like the provider code is mostly smart enough to not try and set it in the case where supports_metadata? is false (for example, here: https://github.com/voxpupuli/puppet-rabbitmq/blob/381dcc0f9ade24629407d3579ccdb605d9d6e6bd/lib/puppet/provider/rabbitmq_vhost/rabbitmqctl.rb#L46-L57)

d1nuc0m commented 5 months ago

What's the RabbitMQ behavior you see? I can reproduce this in the acceptance tests where the description is not supported yet. If the RMQ version is < 3.11, however, the description will not get set anyway (since it's apparently unsupported).

Nothing happens, I'm still on RMQ 3.9, because of OpenStack, I'd expect it to throw an error/warning that description is unsupported

wyardley commented 5 months ago

Got it. Not sure if OpenStack will let you use repos_ensure = true and the erlang module to install a later version?

A warning and / or a docs fix may be feasible.

For example, removing the condition for !supports_metadata? here, adding a conditional that does the warning first and then returns might do the trick. https://github.com/voxpupuli/puppet-rabbitmq/blob/4ac20983b31b0c5811a59669c79d08df2bbc7ddf/lib/puppet/provider/rabbitmq_vhost/rabbitmqctl.rb#L101

I was able to create an acceptance test that reproduces the issue, however I don't know how to suppress the idempotency issue caused by Puppet thinking it wants to update the description (will be happy to review PRs if someone else wants to put one in, though).

The simplest fix is going to be to not try and set the description (maybe leaving it commented out in the code with a note).

d1nuc0m commented 5 months ago

Got it. Not sure if OpenStack will let you use repos_ensure = true and the erlang module to install a later version?

It is possible but it would be an "untested" configuration

The simplest fix is going to be to not try and set the description (maybe leaving it commented out in the code with a note).

Yes, for now I'll go this route