Fix legacy Facts

[cocker-cc]

Since Puppet 4 “structured Facts” are available.
The “non-strucured Facts” are legacy and deprecated. So this Commit is a Fix for that.

The legacy Facts are kept in Rspec-Tests for now, but can be deleted in near Future.

[chouetz]

Will trigger the CI before final approval

[chouetz]

Eventually this should be removed, correct?
And the same for other spec files.

[cocker-cc]

I planned to remove the legacy Facts from the Rspec-Tests, as soon as the Tests succeed. But currently they don't.

Honestly, I don't know, whats wrong. In my own Projects I am used to use the Facts from the Gem facterdb like so:

  on_supported_os.each do |os, os_facts|
    context "on #{os}" do
      let(:facts) { os_facts }

      it { is_expected.to compile.with_all_deps }
    end
  end

os_facts comes out of facterdb.
on_supported_os.each iterates over all OS-Releases stated in metadata.json. So you don't have to specify the Facts yourself for each OS's Context.

But here, the Tests are designed differently, which leaves me a bit lost…
Perhaps you find the Problem. I am a bit short of Time right now.

I invested an Amount of Time in this Module. More PRs are already prepared:

• migrate to PDK 3 with most recent Version of PDK-Templates
• fix all the Linters' Complaints

[chouetz]

@cocker-cc the CI errors are fixed on #791.
You can do the updates or cherry-pick my commits (the second one is merge of main branch because there were additional errors see #793) to be able to merge you PR
Many thanks in advance

[cocker-cc]

Wow, that's cool. I cherry-picked, rebased against main and force-pushed. Checks seem okay now.

[cocker-cc]

I'd like to remove the legacy Facts from the Rspec-Tests now in the next Step.

[chouetz]

you can maybe remove in a next PR, seems it raises new errors :/

[chouetz]

I think I understand the failures in CI:
As you define $facts['os']['name'] which is used in other module, you can run into conditions that were ignored in the past.
As a consequence there is maybe a minimum definition that is expected by default (ie do not define ['os']['name'] without ['os']['release'])

[cocker-cc]

Please don't squash, when merging.

[chouetz]

Suggested change

osfamily: 'redhat',

You probably missed this one

[cocker-cc]

Actually not. The Test fails in this Case.
If I specify…

          facts_to_tags: ['os.family', 'facts_array'],

… then datadog_agent::tag5 tries to find it via…

     $value = getvar($tag_name)

… and $os.family does not exist, in Contrast to getvar('facts.os.family'). (see getvar())

But there could be Help: stdlib got a Function called fact(). So this could be changed to…

  if $lookup_fact {
    $value = fact($tag_name)

… but not in this Change.

[chouetz]

Please don't squash, when merging.

It's the repository policy, can I ask you why you require this? The change is pretty uniform.

[cocker-cc]

Please don't squash, when merging.

It's the repository policy, can I ask you why you require this? The change is pretty uniform.

Even though I find this Policy stupid, for God's Sake then let it squash.