puppet-prometheus icon indicating copy to clipboard operation
puppet-prometheus copied to clipboard

Configuration of prometheus::server fails when looking up configname

Open madelaney opened this issue 7 years ago • 24 comments

Affected Puppet, Ruby, OS and module versions/distributions

  • Puppet: 4.10.12
  • Ruby: ruby 2.1.9p490 (2016-03-30 revision 54437) [x86_64-linux]
  • Distribution: Ubuntu 16.04.5 LTS
  • Module version: v6.2.0

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

node {
include ::prometheus
include ::prometheus::server
}

What are you seeing

The puppet catalog fails to compile.

What behaviour did you expect instead

I'd expect the catalog to compile and run.

Output log

Error: Could not retrieve catalog from remote server: Error 500 on SERVER: Server Error: Evaluation Error: Error while evaluating a Function Call, Lookup of key 'prometheus::server::configname' failed: Data Provider type mismatch: Got String when a hash-like object was expected to access value using 'distro' from key 'os.distro.codename' at /etc/puppetlabs/code/environments/production/manifests/site.pp:41:1 on node <machinename>

Any additional information you'd like to impart

I also tried to follow the example to see if that would help but it was all for not.

madelaney avatar Aug 20 '18 15:08 madelaney

Hi @madelaney, can you share your site.pp? At least the code around line 41.

bastelfreak avatar Aug 20 '18 18:08 bastelfreak

@bastelfreak Sure. This will be a two part, for full context:

site.pp:41 is hiera_include('classes', [])

The classes array is

---
classes:
- prometheus
- prometheus::server

madelaney avatar Aug 20 '18 18:08 madelaney

I don't think that the error relates to this module. However I can add an acceptance test for this later.

bastelfreak avatar Aug 20 '18 18:08 bastelfreak

Um. I can play around with this, but I'm not sure I have enough configuration for prometheus to cause an issue. I will admit, this is my first foray into prometheus so I do claim ignorance on the subject matter.

madelaney avatar Aug 20 '18 18:08 madelaney

There is a similar issue reported to the puppet-ntp (see MODULES-4103). This seems to point to an issue on my side, and to your point, not this module.

Unlink the aforementioned issue, I'm on a newer version of Puppet/Facter

sudo /opt/puppetlabs/bin/puppet agent --version
4.10.12
sudo /opt/puppetlabs/bin/facter --version
3.6.10 (commit ecf783f2ac05eb026d820524df45ee3dbf01bfe1)

The root cause seems to come from the hiera lookup file.

For what it's worth, I'm also using the NTP module (v7.2.0) but I don't see the error listed in MODULES-4103.

madelaney avatar Aug 20 '18 18:08 madelaney

After some digging, this is a facter bug where the variable $os cannot be overloaded. Would you be open to a P.R. that replaces the variable $os with $os_lc? If not, do you have another term in mind?

madelaney avatar Aug 20 '18 22:08 madelaney

@madAndroid which version of facter do you run? The bundled version in Puppet 4.10 already supports hashes. Can you do this on the cmdline please:

which facter
facter --version
facter -p os

bastelfreak avatar Aug 26 '18 06:08 bastelfreak

@bastelfreak , I think you're mention had the wrong user (maybe?).

If so, here's the data you requested:

which facter
facter: Command not found.
Exit 1
/opt/puppetlabs/bin/facter --version
3.6.10 (commit ecf783f2ac05eb026d820524df45ee3dbf01bfe1)
/opt/puppetlabs/bin/facter -p os
2018-08-31 09:00:58.005141 WARN  puppetlabs.facter - skipping external facts for "/home/mdelaney/.puppetlabs/opt/puppet/cache/facts.d": No such file or directory
{
  architecture => "amd64",
  distro => {
    codename => "xenial",
    description => "Ubuntu 16.04.5 LTS",
    id => "Ubuntu",
    release => {
      full => "16.04",
      major => "16.04"
    }
  },
  family => "Debian",
  hardware => "x86_64",
  name => "Ubuntu",
  release => {
    full => "16.04",
    major => "16.04"
  },
  selinux => {
    enabled => false
  }
}

madelaney avatar Aug 31 '18 13:08 madelaney

Sorry, I wanted to highlight you. Do you somewhere reference the os.distro.codename? this seems to come from your codebase, not from our module. Do you use it in your global hiera.yaml or somewhere in the site.pp or are there more values for the classes hiera key?

bastelfreak avatar Aug 31 '18 22:08 bastelfreak

I do not, I do have the following (which is similar) but I do not have os.distro.codename exactly.

        {
          'name'  => 'per-os defaults',
          'paths' => [
            'osfamily/%{facts.os.name}/%{facts.os.distro.codename}/defaults.yaml',
            'osfamily/%{facts.os.name}/%{facts.os.architecture}.yaml',
            'osfamily/%{facts.os.name}/defaults.yaml'
          ]
        },

madelaney avatar Sep 04 '18 14:09 madelaney

this code references the s.distro.codename fact so I assume that is the root cause.

bastelfreak avatar Sep 04 '18 14:09 bastelfreak

Okay, I'll do some digging and try to get to the bottom of it. Sorry for the spam.

madelaney avatar Sep 05 '18 14:09 madelaney

I think, I have a similar issue:

Error: Evaluation Error: Error while evaluating a Resource Statement, Lookup of key 'prometheus::node_exporter::download_extension' failed: Data Provider type mismatch: Got String when a hash-like object was expected to access value using 'family' from key 'os.family' at /data/puppetrepo/manifests/init.pp:29:5 on node testnode.domain.net

the manifest at that line contains:

    class { 'prometheus::node_exporter':
      version => '0.17.0',
    }
# which facter
/opt/puppetlabs/bin/facter

# facter -p os
{
  architecture => "x86_64",
  family => "RedHat",
  hardware => "x86_64",
  name => "CentOS",
  release => {
    full => "6.9",
    major => "6",
    minor => "9"
  },
  selinux => {
    enabled => false
  }
}

# puppet agent --version
4.10.9
# facter -v
3.6.8 (commit 25dace2536f6bd339b70b385c7e90e1951870671)
# hiera -v
3.3.2

First, I assumed it has something to do with the lookup in hiera.yaml in the module. https://github.com/voxpupuli/puppet-prometheus/blob/757a5b4d835603c65a7590cc5698bcd0b91a64e4/hiera.yaml#L12

But even after replacing the hiera lookup with hardcoded values, I get the same error... but I can't find where else it it referenced in the module. As soon I remove the above class reference, it works.

spali avatar Jan 16 '19 17:01 spali

@spali which version of the module do you use? Did you also test the master branch? Can you share the whole /data/puppetrepo/manifests/init.pp file?

bastelfreak avatar Jan 16 '19 21:01 bastelfreak

I used 6.4.0. I can reproduce it with master. But not if I use a test manifest with only the above call. I will further dig into it, how it can be reproduced or if I have some weird bug somewhere.

Side note: found another bug on CentOS with sysv init system. https://github.com/voxpupuli/puppet-prometheus/pull/290

spali avatar Jan 17 '19 09:01 spali

Got my error... I had in the main hiera.yaml a reference of "osfamily/%{os.family}.yaml". As soon as I replaced it with "osfamily/%{facts.os.family}.yaml" it worked... Don't ask me why it only occurs if I add this module :(

spali avatar Jan 17 '19 09:01 spali

ah, interesting. Thanks for debugging!

bastelfreak avatar Jan 18 '19 00:01 bastelfreak

@bastelfreak

I'm a bit confused why this issue has been closed, because the problem still exists. When including prometheus::server from a profile that error pops up with puppet-agent 5.5.18.

The issue seems to stem from the fact that the module defines a parameter $os which apparently conflicts with the facter-provided os-fact. Renaming the parameter to - say - os_ls makes puppet compile a catalog flawlessly.

I really believe that this is related to a bug in facter (as @madelaney suggested in his comment: https://github.com/voxpupuli/puppet-prometheus/issues/254#issuecomment-414484794) and/or hiera, but given that the module claims compatibility with the puppet-version mentioned above, wouldn't it anyway make sense to use a different name for the parameter?

Regards, Patrick

aptituz avatar Mar 06 '20 14:03 aptituz

I closed this because our tests cannot reproduce this and people reported that they had an issue within their own codebase :)

Renaming the variable might help, but that would be a breaking change and we still cannot reproduce the error.

  • Can you please tell us your facter version?
  • Which version of the module do you use?
  • Do you use the Puppet AIO packages / on which OS are you?
  • Please show the Puppet/hiera code you use for this module

bastelfreak avatar Mar 08 '20 10:03 bastelfreak

Doing this (without any other calls to prometheus) - breaks it for us - on latest puppet 6.

class {'prometheus::node_exporter': install_method => 'package', package_name => 'obmondo-node-exporter', package_ensure => ensure_latest($enable), bin_dir => '/ops/obmondo/nagios/plugins', bin_name => 'node_exporter', collectors_disable => ['loadavg', 'mdadm'], extra_options => '--collector.ntp.server ntp1.orange.intra', }

KlavsKlavsen avatar Nov 13 '20 13:11 KlavsKlavsen

Was actually caused by same bug - as previous poster.. we had one place in a hiera.yaml referring to %{os.family} instead of %{facts.os.family} - so great bug to get flushed out, in our own code :)

KlavsKlavsen avatar Nov 13 '20 13:11 KlavsKlavsen

always happy to help :D

bastelfreak avatar Nov 13 '20 22:11 bastelfreak

Having same issue as originally reported (Got string when hash-like object was expected...). Determined that it seems to be a conflict with the module's use of $os conflicting with the $os fact. I've resolved in my environment by forking the module and refactoring the $os variable as $os_type. Once the variable name was changed, the errors went away. I'm happy to submit a PR if the team will consider this solution.

My environment is puppet server: CentOS 7.9.2009 puppet 7.5.0 facter 4.0.52 puppetserver 7.1.0

prometheus server: Ubuntu 20.04.2 puppet 7.5.0 facter 4.0.52

lonebrave avatar Jul 05 '21 09:07 lonebrave

Puppet 7 EOL is 2 month away. With Puppet 8 this should not be an issue since $os not automatically refer to a fact.

So can we close this? Or are there other reasons to rename the variable everywhere?

TheMeier avatar Dec 31 '24 15:12 TheMeier