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

[BUG] pkg.component.prometheus.service.args arguments is not working #61

Open
MurzNN opened this issue Jun 29, 2021 · 9 comments
Open
Labels
bug Something isn't working

Comments

@MurzNN
Copy link

MurzNN commented Jun 29, 2021

Formula commit hash / release tag

56d444b8133d04a53d38c42be3de78c2724de3e92c00ee99450d7c18c7f05011

Versions reports (master & minion)

Salt Version:
          Salt: 3003
 
Dependency Versions:
          cffi: 1.14.2
      cherrypy: Not Installed
      dateutil: 2.7.3
     docker-py: 4.1.0
         gitdb: 2.0.6
     gitpython: 3.0.7
        Jinja2: 2.10.1
       libgit2: Not Installed
      M2Crypto: Not Installed
          Mako: Not Installed
       msgpack: 0.6.2
  msgpack-pure: Not Installed
  mysql-python: Not Installed
     pycparser: 2.20
      pycrypto: Not Installed
  pycryptodome: 3.6.1
        pygit2: Not Installed
        Python: 3.8.5 (default, May 27 2021, 13:30:53)
  python-gnupg: 0.4.5
        PyYAML: 5.3.1
         PyZMQ: 18.1.1
         smmap: 2.0.5
       timelib: Not Installed
       Tornado: 4.5.3
           ZMQ: 4.3.2
 
System Versions:
          dist: ubuntu 20.04 focal
        locale: utf-8
       machine: x86_64
       release: 5.4.0-72-generic
        system: Linux
       version: Ubuntu 20.04 focal

Pillar / config used

prometheus:
  pkg:
    use_upstream_repo: true
    use_upstream_archive: false
    component:
      prometheus:
        service:
          args:
            web.listen-address: 192.168.1.1:9191
            enable-feature: remote-write-receiver

Bug details

Describe the bug

I trying to enable remote write receiver, and set custom listen address and port. I took example from pillar.example and fill needed values, applied changes without errors, but as result - my config is not applied.

Changes to other part of pillar, for example, scrape_interval: 16s, applied successfully.

Steps to reproduce the bug

  1. Add to the pillar in section pkg.component.prometheus.service.args some args, eg:
          args:
            web.listen-address: 192.168.1.1:9191
            enable-feature: remote-write-receiver
  1. Apply changes via state.apply
  2. See that prometheus process don't receive this arguments.
    File /lib/systemd/system/prometheus.service also don't contain my custom args.

Expected behaviour

Custom args must be applied via adding to /lib/systemd/system/prometheus.service file or via some other way.

@MurzNN MurzNN added the bug Something isn't working label Jun 29, 2021
@B1ue-W01f
Copy link

See #59

Theres an open pull request solving this issue.

@MurzNN
Copy link
Author

MurzNN commented Jun 29, 2021

Thanks, I saw that issue, but it is for environment variables, but my - for arguments, but if the solution is same - that's good!

@B1ue-W01f
Copy link

Yeah prometheus can have these config options passed into the service file or the environ file. Previously it was partially implemented, but ultimately not working for either.

When the pull request is complete, this will work through the environ piller in the same format as you gave above, i.e.:

prometheus:
  pkg:
    use_upstream_repo: true
    use_upstream_archive: false
    component:
      prometheus:
        environ:
          args:
            web.listen-address: 192.168.1.1:9191
            enable-feature: remote-write-receiver

Im based off the repo my pull is from and the above pillar deploys the correct config.

@MurzNN
Copy link
Author

MurzNN commented Jul 12, 2021

I updated to v5.5.1, but this still not working, here is my part of the pillar:

      prometheus:
        version: v2.28.0
        archive:
          source_hash: d2d0ded275090a13208114b633abe41c40e14c83ff5ec07230d79b4da1d8e701
        service:
          args:
            web.listen-address: localhost:9091
            enable-feature: remote-write-receiver
        environ:
          args:
            web.listen-address: localhost:9092
            enable-feature: remote-write-receiver2

It applies successfully, but in /etc/systemd/system/multi-user.target.wants/prometheus.service I still don't see those arguments appeared:

ExecStart=/opt/prometheus/prometheus-v2.28.0/prometheus --config.file /etc/prometheus/prometheus.yml

Where can be the problem?

@B1ue-W01f
Copy link

You wont see them in the service because as mentioned the formula does not handle service args - it never did.

service: args: web.listen-address: localhost:9091 enable-feature: remote-write-receiver
<<Does nothing.

Because of the environ section, you should however see that the service file makes reference to an environment file (that's how your package repo should install it, though some OS's don't include it for some reason) and that environment file will include those args.

On debian it will be: /etc/default/prometheus

@MurzNN
Copy link
Author

MurzNN commented Jul 12, 2021

I tried both (server and environ) to see something as working :)
In /etc/default/prometheus I see no changes too, seems because I use use_upstream_repo: false? In Debian repo there is too old version of prometheus, so I have to install from upstream archive. So this feature will not work with use_upstream_archive mode?

@B1ue-W01f
Copy link

B1ue-W01f commented Jul 12, 2021

The formula is essentially split in two based on the use of 'use_upstream_archive'.

The 'use_upstream_archive' essentially ignores all of the code I updated, so I have no idea if it should work or not.

If you look at the archive section of the formula, it looks like its supposed to, with this being the key bit:

env: {{ p.pkg.component[name]['service'].get('env', [])|tojson }}

So perhaps its looking for a different pillar again.

@Ahummeling
Copy link
Contributor

I'm running into problems with this as well, specifically for the --storage.tsdb.retention.time flag.
After going through the formula for a while I noticed something interesting the in the way the ExecStart directive of the prometheus (component) unit file gets constructed:

{%- if name in ('node_exporter', 'consul_exporter') or 'config_file' not in p.pkg.component[name] %}
                 {%- set args = [] %}
                 {%- for param, value in p.pkg.component.get(name).get('service').get('args', {}).items() %}
                    {%- if value is not none %}
                      {% do args.append("--" ~ param ~ "=" ~ value ) %}
                    {%- else %}
                      {% do args.append("--" ~ param ) %}
                    {%- endif %}
                 {%- endfor %}
        start: {{ p.pkg.component[name]['path'] }}/{{ name }} {{ args|join(' ') }}
               {%- else %}
        start: {{ p.pkg.component[name]['path'] }}/{{ name }} --config.file {{ p.pkg.component[name]['config_file'] }}  # noqa 204
               {%- endif %}

If I am not mistaken, omitting all args from the start variable here, we would never be able to add commandline flags to the prometheus service, but Prometheus docs seem to suggest that there are some settings that can only be configured through the commandline flags.
Admittedly, the variables will be added to the env file just fine when configured to do so, but the prometheus executable seems to start the TSDB before loading this env file, and as a result, certain flags (such as the retention time) cannot be modified. The only way to set them, is by providing the appropriate command line flags.

I ended up creating sort of a dirty fix, but I'll open a PR anyway, maybe it can end up being useful in some way.
pr: #74

@gabrielSoudry
Copy link

Same problem as above, with the feature flag agent

        service:
          args:
            web.listen-address: localhost:9091
            enable-feature: agent

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants