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

rabbitmq_vhost resource not idempotent with a description #1006

Open
d1nuc0m opened this issue Jun 12, 2024 · 5 comments
Open

rabbitmq_vhost resource not idempotent with a description #1006

d1nuc0m opened this issue Jun 12, 2024 · 5 comments
Labels
bug Something isn't working

Comments

@d1nuc0m
Copy link

d1nuc0m commented Jun 12, 2024

Affected Puppet, Ruby, OS and module versions/distributions

  • Puppet: independent
  • Ruby: independent
  • Distribution: independent
  • Module version: tested on v13.2.0

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
Copy link
Contributor

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
Copy link
Contributor

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:

if supports_metadata?
raise Puppet::Error, "Cannot parse invalid vhost line: #{line}" unless \
(matches = line.match(%r{^(\S+)\t+(.*?)\t+(undefined|quorum|classic|stream)?\t+\[(.*?)\]$}i))
name, description, default_queue_type, tags = matches.captures
# RMQ returns 'undefined' as default_queue_type if it has never been set
default_queue_type = nil if default_queue_type == 'undefined'
new(ensure: :present, name: name, description: description, default_queue_type: default_queue_type, tags: tags.split(%r{,\s*}))
else
raise Puppet::Error, "Cannot parse invalid vhost line: #{line}" unless line =~ %r{^(\S+)$}
new(ensure: :present, name: Regexp.last_match(1))
)

@d1nuc0m
Copy link
Author

d1nuc0m commented Jun 13, 2024

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
Copy link
Contributor

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.

return if @property_hash.empty? || !supports_metadata? || !exists?

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).

@wyardley wyardley added the bug Something isn't working label Jun 13, 2024
@d1nuc0m
Copy link
Author

d1nuc0m commented Jun 14, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants