fix purge rabbitmq_parameter

[fatpat]

Pull Request (PR) description

resource[:component_name] is set on creation (ensure => present) and deletion (ensure => absent) of the rabbitmq_parameter type but it is not set when it's removed from puppet-purge.

On the other hand, component_name is set to absent on creation, is set on deletion and on purge.

             resource[:component_name]          component_name
creation          shovel                           :absent
deletion          shovel                           shovel
purge              nil                             shovel

When deleting a parameter, replace resource[:component_name'] with resource[:component_name] || component_name.

this bug was introduced by #833 (for #832)

[wyardley]

Is there a way to test this in the unit tests somehow?

[fatpat]

Is there a way to test this in the unit tests somehow?

I don't know ... I think so but it's beyond my knowledge

to be honest, from the state of the variable in each case, the fix should be to replace resource[:component_name] with component_name but the current test fails with this change. That is why I replaced with resource[:component_name] || component_name. I think the test does not reflect the reality.

from my understanding resource[:component_name] is set when a resource is defined (and it exists in the catalog, whatever the value of the ensure parameter) while it's not set when the resource is not defined in puppet but the call is triggered from a purge

on the other hand, component_name has a valid value when the parameter exists on the system (whatever the resource is defined in puppet).

I'd like to make the test better, but I don't have time nor a good knowledge of puppet tests.

[wyardley]

So I'm not against adding this without unit tests, but I would like to understand the problem a bit better.
Can you show the relevant parts of the debug output of using this to ensure absence with both the existing and fixed code?

When I try just adjusting component_name as you mention above, this is the failing test

  2) Puppet::Type::Rabbitmq_parameter::ProviderRabbitmqctl#destroy destroy parameter
     Failure/Error: rabbitmqctl('clear_parameter', '-p', vhost, component_name, key)
     
     Mocha::ExpectationError:
       unexpected invocation: Rabbitmq_parameter[documentumShovel@/](provider=rabbitmqctl).rabbitmqctl("clear_parameter", "-p", "/", :absent, "documentumShovel")
       unsatisfied expectations:
       - expected exactly once, invoked never: Rabbitmq_parameter[documentumShovel@/](provider=rabbitmqctl).rabbitmqctl("clear_parameter", "-p", "/", "shovel", "documentumShovel")
       satisfied expectations:
       - allowed any number of times, invoked never: #<Puppet::Util::Feature:0x71e8>.root?(any_parameters)

which makes me think that value is getting set to :absent in the rabbitmqctl call?

Having the acceptance tests crash on me locally, but trying to see in #946 if that test passes or fails; I can also merge that if it is successful.

[wyardley]

ps - asking on IRC if someone can take a look at this; that may be another good place to get some help with all of this.

[wyardley]

https://github.com/voxpupuli/puppet-rabbitmq/actions/runs/6669554996/job/18127694532?pr=946
yeah, I think this test run should prove the existing behavior is broken:

       	Error: Execution of '/usr/sbin/rabbitmqctl clear_parameter -p fedhost  documentumFed' returned 2: Clearing runtime parameter "documentumFed" for component [] ...
       	Error: Parameter does not exist
       	Error: /Stage[main]/Main/Rabbitmq_parameter[documentumFed@fedhost]/ensure: change from 'present' to 'absent' failed: Execution of '/usr/sbin/rabbitmqctl clear_parameter -p fedhost  documentumFed' returned 2: Clearing runtime parameter "documentumFed" for component [] ...
       	Error: Parameter does not exist

where that value is empty. I have the acceptance tests running again locally (this can be a good quick way to test this kind of thing), so I'll see if one or both changes fixes this in the acceptance tests; if so, we can try to figure out how to make the existing unit test better.

[wyardley]

I verified that the test from #946 fails without this change (proving the bug exists), and passes with either change (with or without the ||). While I'd like to get a better fix for the unit test ideally, can you cherry-pick my commit into your branch? And if we don't hear from someone with a good plan within a day or two, we can

[fatpat]

I've cherry-picked the commit a5027a4 from #946