-
Notifications
You must be signed in to change notification settings - Fork 51
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
Check for Amazon in osfamilymap to create pkg repo #67
Conversation
f29a51b
to
08e5265
Compare
prometheus/osfamilymap.yaml
Outdated
@@ -443,3 +448,7 @@ MacOS: | |||
archive: | |||
source_hash: 15132494523c2b6a89e09b2da63452c8fe587fb82fcc3fd21cc75a4aa2766644 | |||
|
|||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove the extra lines..
prometheus/osfamilymap.yaml
Outdated
@@ -206,9 +206,14 @@ RedHat: | |||
args: | |||
config.file: /etc/prometheus/blackbox.yml | |||
repo: | |||
{%- if grains.os == 'Amazon' %} | |||
{%- set releasever = salt['cmd.run']("rpm -E '%{rhel}'") %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indent 4 spaces, instead of 2 spaces, to line up with the block above (suggestion, not rule).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, two suggested updates
* Add check for Amazon Linux to osfamilymap.yaml so that the RPM package repository is properly generated. Resolves saltstack-formulas#65
Updated. Not sure how those extra lines ended up in there. Thx for pointing that and the spacing out. Should be all good now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would be so much neater if there was an osmap.yaml
available but that's not present unfortunately.
The testing should be updated though:
prometheus-formula/.gitlab-ci.yml
Lines 170 to 172 in 2c572d0
default-amazonlinux-2-master-py3: {extends: '.test_instance'} | |
# repo-amazonlinux-2-master-py3: {extends: '.test_instance'} | |
# amazonlinux-2-master-py3: {extends: '.test_instance'} |
- Please comment out line 170 and then uncomment line 172, which will test both the
default
andrepo
suites for Amazon.
* this change sets things to test both the default and repo suites for Amazon Linux. Resolves saltstack-formulas#65
is |
well, for the archive install, it's supposed to verify that.. I'm confused. |
@mdschmitt This will resolve your problem. Change the first line of this block: prometheus-formula/prometheus/clientlibs/install.sls Lines 31 to 36 in 2c572d0
To: - {%- if grains.get('osfinger', '') in ['Oracle Linux Server-8'] %}
+ {%- if grains.get('osfinger', '') in ['Oracle Linux Server-8', 'Amazon Linux-2'] %} |
I'll admit, I could've found that out eventually, but I really appreciate your help. Updated and pushed. Thank you! |
...and pipeline success! |
@mdschmitt You're welcome. And thank you for your excellent contribution! |
🎉 This PR is included in version 5.5.2 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Fixes #65