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.

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

Accept none as valid init_style #399

Merged
merged 3 commits into from
Nov 28, 2019

Conversation

alexjfisher
Copy link
Member

This is a more generic alternative to the manage_systemd_unit part of #303.

install_method already accepts none, so I don't think it looks too wonky.

@alexjfisher alexjfisher requested a review from anarcat November 28, 2019 10:01
Copy link

@anarcat anarcat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think this is a great idea. But there are bits missing.

  1. you should also cover the incantations of init_style in config.pp for this to be complete
  2. all exporters need to have such a init_style parameter (postfix, for example, doesn't have one in Add postfix exporter #396 ) that can be fixed by tweaking the init_style at the prometheus class level, although I'm not sure I like that idea after all... but see Add postfix exporter #396 for a discussion on that.
  3. service_reload fails here because there's no case for none in the selector:
Error: Could not retrieve catalog from remote server: Error 500 on SERVER: Server Error: Evaluation Error: No matching entry for selector parameter with value 'none' (file: /srv/puppet.torproject.org/stages/production/3rdparty/modules/prometheus/manifests/service_reload.pp, line: 9, column: 26) on node hetzner-nbg1-01.torproject.org

The formed is fixed in 6478155 and the latter in c1adc1c.

@anarcat anarcat mentioned this pull request Nov 28, 2019
@alexjfisher alexjfisher changed the title Accept none as valid daemon init_style Accept none as valid init_style Nov 28, 2019
Copy link

@anarcat anarcat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awesome, thanks for the changes, now it's good! :)

@alexjfisher alexjfisher merged commit 9a42f87 into voxpupuli:master Nov 28, 2019
@alexjfisher alexjfisher deleted the init_style_none branch November 28, 2019 17:48
Rovanion pushed a commit to Rovanion/puppet-prometheus that referenced this pull request May 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants