-
-
Notifications
You must be signed in to change notification settings - Fork 28
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
RabbitMQ and Metrics #358
RabbitMQ and Metrics #358
Conversation
merge latest
merge master
Added params for metrics section in config file
…o feature/update-master
Feature/update master
…nd incorrect tests. Updated tests to support new python version. Fixed all failing builds
…nd incorrect tests. Updated tests to support new python version. Fixed all failing builds
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.
Huge work here! 👍
Thanks a lot for the PR + fix.
It would be ideal to split the fix vs feature in separated PRs for better history, separation of concern, diff, and review.
I left a few comments, but up to @rush-skills to review & approve as Puppet is not my primary thing.
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 now, except failing tests
spec/classes/profile/web_spec.rb
Outdated
@@ -149,6 +149,34 @@ | |||
}, | |||
tag: ['st2', 'st2::backend', 'st2::backend::api']) | |||
end | |||
it do | |||
is_expected.to contain_nginx__resource__location('@basic_statusError') |
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.
Should these tests only run when the basic status page is enabled?
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.
Yea working on fixing the tests. I have a pretty busy day today though so it might have to wait till tomorrow.
Thanks for fixing the tests - all looks green now after so long! |
build/centos7-puppet6/Puppetfile
Outdated
# Waiting on https://github.com/voxpupuli/puppet-rabbitmq/pull/907 to be | ||
# merged into upstream repo. | ||
mod 'puppet/rabbitmq', | ||
:git => 'https://github.com/EncoreTechnologies/puppet-rabbitmq.git', | ||
:branch => 'feature/fix-yum-repo', | ||
:default_branch => 'master' |
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.
As voxpupuli/puppet-rabbitmq#907 was merged,
looks like we can rely on the official version now 👍
@armab can you re-run tests. They all succeeded in my repo |
@bishopbm1 Sure, let's see. Thanks for bumping this! @rush-skills Is there anything else left addressing in this PR to move forward? |
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 now if tests pass
@bishopbm1 Can you also describe/document on the metrics and nginx_basic_status parts that you have added in here? Would love to try them out in my setup |
Yea i'll throw something quick in the readme |
@bishopbm1 CI is all green again 🥳 |
… Also updated other pieces of documentation for python 3.8
@rush-skills I added a simple description. I plan to build this out a lot more in coming month but that is definitely its own PR. If you message me in the ST2 channel I can help you with how we are scraping the data to get it to prometheus. |
@bishopbm1 Thanks, that helps keep the module docs still complete for that profile. I will merge this once CI passes. |
Added Metrics configuration options to to setup and configure metrics endpoint needed for metrics collection. Will be using this and adding documentation on setting up Prometheus.
RabbitMQ and erlang changes the repository styles so added some configuration options so if they change again we can more easily update. PackageCloud documentation:
https://packagecloud.io/rabbitmq/erlang/install#bash-rpm
https://packagecloud.io/install/repositories/rabbitmq/erlang/config_file.repo?os=rhel&dist=8&source=script
Relies on the folllowing PR for additional fixes:
voxpupuli/puppet-rabbitmq#907
While waiting for that to be merged this repo and branch can be used:
https://github.com/EncoreTechnologies/puppet-rabbitmq/tree/feature/fix-yum-repo
This repo and branch will remain until the PR is merged.
Error: