Closed absmith82 closed 3 years ago
@absmith82 could you check if the recently merged #7 inpacts your PR?
The merge wouldn't technically impact my PR, however, it does make the current purge.sls redundant.
The purge.sls
state makes sense only if beats:use_upstream_repo
is set to True
. So, I think the whole state could be defined within an if
statement like the following
{% if salt['pillar.get']('beats:use_upstream_repo', 'True') %}
...
{% endif %}
@absmith82 can you reply to @psmiraglia suggestion about purge.sls...
I agree with the if statement as I think this statement is correct that it would only be used in the case that upstream repo is true. I have added your conditional into the purge.sls
I just realised that Filebeat (and other Beats) uses a separate repository for OSS version (e.g. deb https://artifacts.elastic.co/packages/oss-7.x/apt stable main
). I think the "with or without oss-
" should be managed with a boolean config entry like beats:use_oss_repo
(with default to True
).
@absmith82 are you able to work on that?
The changes here are getting beyond the original scope of the pull request. That's fine, but I just wonder if we should edit the description of the pull request and change the idea of how the formula works.
For instance, rather than having jinja pull information from pillar and then setting a default to each individual value in different states, would it be better to have a defaults or map file that merges pillar so that we aren't having to write two different states, one if the OSS repo is used and one if the elastic repo is used? Then the global variables in the file could work across the entire formula.
This could be a fairly large undertaking however.
Full disclosure, I am not actively using this formula as it contains more systems than I actually use. I had originally looked at the formula for use and started testing with it and started using something that only managed filebeat as that is all that I am currently using.
Since I am no longer working with this formula, and am not setting up systems to work with it. I'll remove my pull request feel free to revive this and use any of the fixes if needed.
Added Jinja templating to the purge state to match the install repo