-
Notifications
You must be signed in to change notification settings - Fork 5
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
Add role to deploy cASO #205
Conversation
8532698
to
8629430
Compare
I've updated this following Mark's review comments in: #160 |
host {% raw %}{{ elasticsearch_address }} | ||
{% endraw %} | ||
port {% raw %}{{ elasticsearch_port }} | ||
{% endraw %} |
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.
Are you sure these should be raw?
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.
Hmm, good point. This does look wrong, I just copied this across from the original patch.
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.
The bizaare thing is that this works as is:
<match apel.events>
@type copy
<store>
@type elasticsearch
host 192.168.33.2
port 9200
logstash_format true
logstash_prefix apel
flush_interval 15s
reload_connections false
reconnect_on_error true
</store>
</match>
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 is without:
<match apel.events>
@type copy
<store>
@type elasticsearch
host 192.168.33.2
port 9200
logstash_format true
logstash_prefix apel
flush_interval 15s
reload_connections false
reconnect_on_error true
</store>
</match>
I'll remove it anyway for neatness.
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.
Done
I guess it will fail if the inventory is missing the |
If this was an upstream patch I would point out the lack of |
Good point. I think I just need to add stuff like:
to any of the tasks outside of the caso role. |
You can check how it was done for the libvirt-exporter change on Xena: |
Wouldn't we never get there, since the play would not match in site.yml? |
Good point, it's a new role rather than an extension of an existing one. |
I think this would fail though: https://github.com/stackhpc/kolla-ansible/pull/205/files#diff-d33e12c3e3423adcb3eca3138dbc35ff4a5c224a307f38d088fce52a1ca9b56aR77 as it is outside the role? Edit: confirmed an issue here:
Edit2: forgot to not set enable_caso. |
cASO is an OpenStack accounting extractor. For more detail see: https://github.com/IFCA/caso By default, cASO is configured to output to Fluentd via TCP. The accounting information can then be shipped off to ElasticSearch. (cherry picked from commit d8ab00f) (cherry picked from commit 67ee60e) Change-Id: Ib81ec443bbb14d494d7c81dff617b371a1b68c52
8629430
to
1add752
Compare
I've removed the unnecessary raw tags and confirmed that a missing caso group doesn't break any tasks (provided that you do not set |
cASO is an OpenStack accounting extractor. For more detail see:
https://github.com/IFCA/caso
By default, cASO is configured to output to Fluentd via TCP. The
accounting information can then be shipped off to ElasticSearch.
(cherry picked from commit d8ab00f)
(cherry picked from commit 67ee60e)
Change-Id: Ib81ec443bbb14d494d7c81dff617b371a1b68c52