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

[BUG] Environment variable setting doesnt work #59

Closed
B1ue-W01f opened this issue Jun 24, 2021 · 3 comments
Closed

[BUG] Environment variable setting doesnt work #59

B1ue-W01f opened this issue Jun 24, 2021 · 3 comments
Labels
bug Something isn't working released

Comments

@B1ue-W01f
Copy link

B1ue-W01f commented Jun 24, 2021 •
edited

Your setup

Running on debian.

Formula commit hash / release tag

n/a

Versions reports (master & minion)

n/a

Pillar / config used

prometheus:
  wanted:
    component:
      - node_exporter
  pkg:
    component:
      node_exporter:
        name: prometheus-node-exporter
        environ:
          args:
            collector.systemd: true
            collector.filesystem.ignored-mount-points: '^/(var|media|dev|run)($|/)'

Bug details

Describe the bug

The macro within the files directory does not export the correct function to allow the environ state to operate when an environ arg is set. See:

https://github.com/saltstack-formulas/prometheus-formula/blob/fa96aabba76128ebca85b76631bf04ec8daaeb90/prometheus/config/environ.sls

Expecting to import 'concat_environ' which does not exist in the referenced file.

Steps to reproduce the bug

Run a highstate with the above pillar

Expected behaviour

Environ arguments should be set
@B1ue-W01f B1ue-W01f added the bug Something isn't working label Jun 24, 2021
@myii
Copy link
Member

myii commented Jun 24, 2021

Reproduced locally using Kitchen with the pillar modified as above for default-debian-10-master-py3:

       local:
           Data failed to compile:
       ----------
           Rendering SLS 'base:prometheus.config.environ' failed: Jinja variable the template 'prometheus/files/macros.jinja' (imported on line 7) does not export the requested
 name 'concat_environ'
>>>>>> ------Exception-------
>>>>>> Class: Kitchen::ActionFailed
>>>>>> Message: 1 actions failed.
>>>>>>     Converge failed on instance <default-debian-10-master-py3>.  Please see .kitchen/logs/default-debian-10-master-py3.log for more details
>>>>>> ----------------------
>>>>>> Please see .kitchen/logs/kitchen.log for more details
>>>>>> Also try running `kitchen diagnose --all` for configuration

@myii
Copy link
Member

myii commented Jun 24, 2021

From our discussions in the #formulas channel on Slack, we're pretty sure that concat_environ needs to be changed to concat_args in all places.

B1ue-W01f pushed a commit to B1ue-W01f/prometheus-formula that referenced this issue Jun 25, 2021
fix: added guidance and reverted incorrected changes from prior commits
a4dfb87 
Added inspec checks for environment files and specifically prometheus
and node_exporter args. Provided comments throughout the key reference
points for users to signpost the differing approaches to args used along
with more clearly identifying the difference between archive and repo
approach. Tests appear to be working on both approaches though updates
have been focused at repo install method.

Fixes: saltstack-formulas#59
@B1ue-W01f B1ue-W01f mentioned this issue Jun 25, 2021
Merged
19 tasks
B1ue-W01f pushed a commit to B1ue-W01f/prometheus-formula that referenced this issue Jun 27, 2021
fix: added handle for alternative argument opts header
4de3ebd 
Centos and oraclelinux repositories for prometheus include bespoke headers
in the environment files (e.g. Debian: ARGS=, Centos: PROMETHEUS_OPTS=
ALERTMANAGER_OPTS=). This has been added as a default pillar with osmap
variances.
Additionally archlinux repo install was failing so added basic support -
an issue still remains for the prometheus app itself due to the service
file included in the arch repo hardcoding some config options - resulting
in the possibility to duplicate arguments resulting in a service error.
The prometheus service currently does not start due to permissions not being
applied to a data folder. The added config.storage begins to solve this and
ensures alignment on all platforms but would result in a duplicate config
entry as above. Prometheus on arch therefore needs more work but the exporter
installs now work.

Resolves: saltstack-formulas#59
B1ue-W01f pushed a commit to B1ue-W01f/prometheus-formula that referenced this issue Jun 29, 2021
fix: removed erroneus dash (-)
52845bb 
Dash was incorrectly left alongside squid_exporter entry in
osfamilymap.

Resolves: saltstack-formulas#59
@B1ue-W01f B1ue-W01f mentioned this issue Jun 29, 2021
Open
B1ue-W01f pushed a commit to B1ue-W01f/prometheus-formula that referenced this issue Jun 30, 2021
fix: rework to implement environment variables handling
e52f804 
Developed environ.sh.jinja and added test pillar data to default
Corrected prometheus.config.environ
Switched default test pillar to use none archive - due to deployment of custom service
Disabled a number of exporters following switch from archive due to failing - to be reviewed
Corrected prometheus environ_file location

Resolves: saltstack-formulas#59
B1ue-W01f pushed a commit to B1ue-W01f/prometheus-formula that referenced this issue Jun 30, 2021
fix: added guidance and reverted incorrected changes from prior commits
0ca247a 
Added inspec checks for environment files and specifically prometheus
and node_exporter args. Provided comments throughout the key reference
points for users to signpost the differing approaches to args used along
with more clearly identifying the difference between archive and repo
approach. Tests appear to be working on both approaches though updates
have been focused at repo install method.

Fixes: saltstack-formulas#59
B1ue-W01f pushed a commit to B1ue-W01f/prometheus-formula that referenced this issue Jun 30, 2021
fix: added handle for alternative argument opts header
076869a 
Centos and oraclelinux repositories for prometheus include bespoke headers
in the environment files (e.g. Debian: ARGS=, Centos: PROMETHEUS_OPTS=
ALERTMANAGER_OPTS=). This has been added as a default pillar with osmap
variances.
Additionally archlinux repo install was failing so added basic support -
an issue still remains for the prometheus app itself due to the service
file included in the arch repo hardcoding some config options - resulting
in the possibility to duplicate arguments resulting in a service error.
The prometheus service currently does not start due to permissions not being
applied to a data folder. The added config.storage begins to solve this and
ensures alignment on all platforms but would result in a duplicate config
entry as above. Prometheus on arch therefore needs more work but the exporter
installs now work.

Resolves: saltstack-formulas#59
B1ue-W01f pushed a commit to B1ue-W01f/prometheus-formula that referenced this issue Jun 30, 2021
fix: removed erroneus dash (-)
ef8a3a9 
Dash was incorrectly left alongside squid_exporter entry in
osfamilymap.

Resolves: saltstack-formulas#59
saltstack-formulas-travis pushed a commit that referenced this issue Jul 9, 2021
@semantic-release-bot
chore(release): 5.5.1 [skip ci]
8e96162 
## [5.5.1](v5.5.0...v5.5.1) (2021-07-09)

### Bug Fixes

* added guidance and reverted incorrected changes from prior commits ([0ca247a](0ca247a)), closes [#59](#59)
* added guidance and reverted incorrected changes from prior commits ([a4dfb87](a4dfb87)), closes [#59](#59)
* added handle for alternative argument opts header ([076869a](076869a)), closes [#59](#59)
* added handle for alternative argument opts header ([4de3ebd](4de3ebd)), closes [#59](#59)
* removed erroneus dash (-) ([ef8a3a9](ef8a3a9)), closes [#59](#59)
* removed erroneus dash (-) ([52845bb](52845bb)), closes [#59](#59)
* revert to use of macro.jinja for arg handling ([f2261f9](f2261f9))
* revert to use of macro.jinja for arg handling ([696bee0](696bee0))
* rework to implement environment variables handling ([e52f804](e52f804)), closes [#59](#59)
* rework to implement environment variables handling ([eea5b40](eea5b40)), closes [#59](#59) [#59](#59)
* switched test config entry that wasnt available for deb9 ([5c1d8b6](5c1d8b6))
* switched test config entry that wasnt available for deb9 ([4635ca7](4635ca7))

### Continuous Integration

* **3003.1:** update inc. AlmaLinux, Rocky & `rst-lint` [skip ci] ([5550397](5550397))
* **kitchen+gitlab:** remove Ubuntu 16.04 & Fedora 32 (EOL) [skip ci] ([fa96aab](fa96aab))
@saltstack-formulas-travis
Copy link

🎉 This issue has been resolved in version 5.5.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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

No branches or pull requests
3 participants
@myii @B1ue-W01f @saltstack-formulas-travis