Rex icon indicating copy to clipboard operation
Rex copied to clipboard

Rex::Virtualization doesnt accept a hashref like intended

Open djzort opened this issue 3 years ago • 2 comments
trafficstars

Describe the bug

The intention seems to be to permit:

set virtualization => {
  type => 'LibVirt',
  %other
}

However in the above case the "type" would just be saved in to Rex::Config and that would be it.

Expected behavior

With

set virtualization => {
  type => 'LibVirt',
  %other
}

The entire hashref would be loaded

How to reproduce it

As above

Code example

set virtualization => {
  type => 'LibVirt',
  %other
}

Rex::Config->get("virtualization");

Additional context

No response

Rex version

1.13.4

Perl version

any

Operating system running rex

any

Operating system managed by rex

any

How rex was installed?

cpan client

djzort avatar Sep 09 '22 05:09 djzort

Thanks for the report and patience, @djzort!

I can't reproduce the issue, though. I have inflated your example into a full executable code like this:

use Rex;
use Data::Dumper;

my %other = ( abc => 123 );

set virtualization => { type => 'LibVirt', %other };

task 'test', sub {
    say Dumper( Rex::Config->get("virtualization") );
};

When I run rex test, then the output is correctly including the values from the %other hash as well, and as expected:

[2023-02-17 21:07:02] INFO - Running task test on <local>
$VAR1 = {
          'type' => 'LibVirt',
          'abc' => 123
        };

[2023-02-17 21:07:02] INFO - All tasks successful on all hosts

The title talks about Rex::Virtualization, but the code example is only about Rex::Config. Therefore I suspect some crucial information is still missing from this report.

Could you elaborate what are you trying to do outside the Rex::Config calls in your code that fails for you, please?


ps.: The "Perl version", "Operating system running rex", and "Operating system managed by rex fields" of the bug report form has been set to any. We've seen such assumptions proven false many times during the 12+ years of history of Rex. These fields are required in order to learn about the environment of the reporter, which proven to be important many times.

Please make sure to follow the instructions in the issue forms, and fill those fields out meaningfully next time, even if they are assumed unimportant or unrelated.

ferki avatar Feb 17 '23 20:02 ferki

I was looking at the code https://github.com/RexOps/Rex/blob/9987c289c94ef4c383aba189c3860c97b1cda00c/lib/Rex/Virtualization.pm#L18

which has been in place for 11 years, so the version didn't seem relevant at all.

i can see in virtualization.t that 'connect' is stored and retrieved, so i may not have found a bug that i thought i had.

Spelunking more, i can see set in Rex::Config here uses %SET_HANDLER to call handers https://github.com/RexOps/Rex/blob/9987c289c94ef4c383aba189c3860c97b1cda00c/lib/Rex/Config.pm#L1653

Otherwise it just saves everything

Rex::Virtualization is registering via register_config_handler which only applies the handler to config files https://github.com/RexOps/Rex/blob/9987c289c94ef4c383aba189c3860c97b1cda00c/lib/Rex/Config.pm#L1723

It would seem then that the code in Rex::Virtualization would ignore everything other than type when loading virtualization from config files.

djzort avatar Feb 25 '23 05:02 djzort