[go: up one dir, main page]
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.

Sign up for GitHub

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

Jump to bottom

tp::install is failing when defining configuration file source in conf_hash #70

Closed
findmyname666 opened this issue Feb 19, 2020 · 9 comments
Closed

tp::install is failing when defining configuration file source in conf_hash #70

findmyname666 opened this issue Feb 19, 2020 · 9 comments

Comments

@findmyname666
Copy link
findmyname666 commented Feb 19, 2020 •
edited
Loading

Expected Behavior

To Create configuration file.

Actual Behavior

The code is failing on:

Error: Validation of File[/etc/process-exporter/config.yaml] failed: You cannot specify more than one of content, source, target (file: /etc/puppetlabs/code/environments/production/modules/tp/manifests/conf.pp, line: 339)

With added logging:

--- /etc/puppetlabs/code/environments/production/modules/tp/manifests/conf.pp	2020-02-19 15:33:25.591810484 +0000
+++ /tmp/conf.pp	2020-02-19 15:33:18.343745299 +0000
@@ -333,6 +333,8 @@
     notify       => $manage_notify,
     validate_cmd => $manage_validate_cmd,
   }
+  $config_file_parameters = pick($settings[config_file_params],{})
+  notice("path: ${manage_path},file params: ${file_params}, config file parameters: ${config_file_parameters} ")
 
   file { $manage_path:
     * => $file_params + pick($settings[config_file_params],{})

We can see that there are really source and content in var $file_params but content is empty.

Notice: Scope(Tp::Conf[prometheus-process-exporter]): path: /etc/process-exporter/config.yaml,file params: {ensure => latest, source => puppet:///site/showmax_tinydata/prometheus-process-exporter, content => , path => /etc/process-exporter/config.yaml, mode => 0644, owner => root, group => root, require => Package[prometheus-process-exporter], notify => Service[prometheus-process-exporter], validate_cmd => }, config file parameters: {}

Steps to Reproduce the Problem

  1. Pass following data to tp::install:
packages:
  prometheus-process-exporter:
    ensure: 'latest'
    data_module: 'my_tinydata'
    conf_hash:
      'prometheus-process-exporter':
         source: 'puppet:///site/my_tinydata/prometheus-process-exporter'

When i change ensure to present everything works + when i passed ensure to conf_hash:

  prometheus-process-exporter:
    ensure: 'latest'
    data_module: 'showmax_tinydata'
    conf_hash:
      prometheus-process-exporter:
        ensure: 'file'
        source: 'puppet:///modules/showmax_tinydata/prometheus-process-exporter/nginx.yaml'
  1. Execute tp::install
  $packages.each |$package_name, $attributes| {
    tp::install { $package_name:
      * => $attributes,
    }
  }

FYI my tinydata:

---
  prometheus-process-exporter::settings:
    package_name: 'prometheus-process-exporter'
    service_name: 'prometheus-process-exporter'
    config_file_path: '/etc/process-exporter/config.yaml'

Specifications

Please add this info:

  • Running on Debian9
  • TP module version 2.4.1
@alvagante
Copy link
Member

@findmyname666 curious case, maybe due to custom tinydata.
Can you tell me if changing line 338 in tp::conf https://github.com/example42/puppet-tp/blob/master/manifests/conf.pp#L338 from:
* => $file_params + pick($settings[config_file_params],{})
to
* => delete_undef_values($file_params + pick($settings[config_file_params],{}))
fixes the issue?

@findmyname666
Copy link
Author

Hi @alvagante ,
Nope still the same.
I think the issue is that it tries to set latest as ensure for file.
Even though error doesn't match it 🤷‍♂️

@alvagante
Copy link
Member

@findmyname666 oh , you're right, the problem is here: https://github.com/example42/puppet-tp/blob/master/manifests/install.pp#L294 .
Honestly I almost never used the conf_hash param in tp::install.
Have to fix it, thanks for the workaround anyway.

@findmyname666
Copy link
Author

Honestly I almost never used the conf_hash param in tp::install.
Actually i think that it is neat feature ;)

alvagante added a commit that referenced this issue Feb 19, 2020
@alvagante
Fix for #70
6157efa
@alvagante
Copy link
Member

@findmyname666 can you please check if 6157efa fixes the issue?

alvagante added a commit that referenced this issue Mar 2, 2020
@alvagante
Fix 70 (#71)
cacc4ed 
* Ensure we find tp command in task tp::test

* Fix for #70
@findmyname666
Copy link
Author

Hi @alvagante , busy time ! sorry to don't get back to you ...
i was thinking can you insert also data_module to dir_defaults please?

  $dir_defaults = {
    'ensure'        => $ensure,
    'ensure'        => tp::ensure2dir($ensure),
    'settings_hash' => $settings,
    'data_module'  => $data_module
  }

alvagante added a commit that referenced this issue Mar 5, 2020
@alvagante
#70 Propagate data_module var to tp defines used in tp::install
037d66c
@alvagante
Copy link
Member

Good catch @findmyname666 , and good that someone is using custom tinydata.
Added in the referenced commit.

@findmyname666
Copy link
Author

@alvagante can you publish it on forge?

@alvagante
Copy link
Member

@findmyname666 pushed version 2.4.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@alvagante @findmyname666