-
Notifications
You must be signed in to change notification settings - Fork 7
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
Use new connection configuration for metrics #48
Conversation
Generally looks right. I would like to see some testing output like what's been provided on other PRs though. |
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.
Looks good to me. We'll need to make sure to coordinate this change into STO as well.
Sweet looks to be dealt with here :) https://github.com/infrawatch/service-telemetry-operator/pull/93/files |
Since this is in coordination with a SG change, I think we should avoid merging this in to master for now, and do similar to what Chris was doing with all the SG, SGO, and STO PRs related to the ceilometer metrics changes should have a base branch target of something pre-master. We can then do a full integration test with all the components together, and then merge them all into master at the same time. What I'm afraid of is this merging before the other SG changes merge, and resulting in a broken set of released container images. |
AMQP1Connections parameter should be used instead of nonflexible AMQP1Url. Depends-On: infrawatch/smart-gateway#83
3be2925
to
d966d01
Compare
Passes CI 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.
Go!
"AMQP1Connections": [ | ||
{ | ||
"URL": "{{ amqp_url | default('messaging-internal-' + meta.name + '.' + meta.namespace + '.svc:5672/collectd/telemetry') }}", | ||
"DataSource": "{{ amqp_data_source | default('collectd') }}" |
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.
No problem, but just want to draw attention to the fact that when the SG3 code merges, the collectd metrics container will no longer read this configmap at all. In that case, this default will be a little confusing.
Is it worth an issue to come back to this and change it to ceilometer once the SG3 code merges? It will be weird to default to collectd in so many places, but to ceilometer here. Not sure exactly what to do, and it's minor, but thought I'd draw attention to this detail.
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.
I agree, let's open an issue and we can deal with this as part of the set of changes we need to handle as part of SG3 work. Once we merge ceilometer metrics work, then we can rebase the SG3 work on top of that, and address anything that seems off.
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.
(I have opened a new issue that refers to this comment)
* Enable deployment of metric SG for Ceilometer data Depends-On: infrawatch/smart-gateway#83 Depends-On: infrawatch/smart-gateway-operator#48 * Add smoketest for Ceilometer data * Listen on correct channel * Ceilometer smoketest tuning Makes smoketest_ceilometer_entrypoint.sh being executed during smoketest job. * Use data source setting for metrics too # Please enter the commit message for your changes. Lines starting # with '#' will be ignored, and an empty message aborts the commit. # # On branch mmagr-amqp10connections # Changes to be committed: # modified: roles/servicetelemetry/templates/manifest_smartgateway_metrics.j2 # * Finish ceilometer events smoke test * Do not use hardcoded timestamps * Finish ceilometer metrics smoketests * Increase timeout * Add container names * Validate also Ceilometer metrics SG * Update tests/smoketest/smoketest_ceilometer_entrypoint.sh * Update tests/smoketest/smoketest_collectd_entrypoint.sh Co-authored-by: Martin Magr <[email protected]> Co-authored-by: Leif Madsen <[email protected]>
* Use new connection configuration for metrics (#48) AMQP1Connections parameter should be used instead of nonflexible AMQP1Url. Depends-On: infrawatch/smart-gateway#83 * Bump default CSV to 2.0.0 * Fix file rename * Lock operator-courier to v2.1.7 Lock operator-courier to 2.1.7 until we can figure out what is wrong with our CSV/CRD setup or until the operator-courier issue noted in the related issue is resolved. Related: infrawatch/service-telemetry-operator#108 * Update roles/smartgateway/templates/ceilometer-metrics-configmap.yaml.j2 Co-authored-by: Chris Sibbitt <[email protected]> Co-authored-by: Martin Mágr <[email protected]> Co-authored-by: Chris Sibbitt <[email protected]>
* Enhance CI automation (#106) * Add ServiceTelemetry overrides Allows ServiceTelemetry overrides to be expressed via Ansible extra-vars. Adds the four main overrides you would expect in a ServiceTelemetry object, with appropriate defaults set. Will also allow passing in the service_telemetry_manifest as a whole object like what we do with the Service Telemetry Operator. * Allow for per-repo branch overrides Allow for per-repo branch overrides for the Smart Gateway Operator and Smart Gateway repositories via sgo_branch and sg_branch (respectively). * Add functionality around quickstart.sh Add some functionality that was replaced when I dropped the quickstart.sh. Adds some of this functionality back in and also adds some new stuff. * Fix syntax error * Add back quickstart.sh Adds back a quickstart.sh that simulates the same result as the old quickstart.sh * Better CSV modification support Also adds some tags to make skipping over builds for testing much easier. * Debugging ci.yml firing * Make sure namespace is set before using it * Fix syntax and documentation * Test locally first kids * Drop CI debug lines * Clean up working repo clones * Copy CSV into working directory On subsequent runs the in-place modification of the CSV can cause issues either in the development environment, or re-runs of the CI system. Copying the CSV out of the in-place repo into a working location, and then modifying in-place results in a cleaner setup. By doing the copy of the CSV files, we can drop the need to force clone the supporting repositories. Also cleans up some shell commands that were commented out now that they are being dealt with via the replace module. Removes the extra commands added to ci.yml. * Changes to infrared-openstack.sh for OSP13 (#102) * Migrate OSP16 script to OSP13 directory Uses a multi-cloud stf-connectors.yaml style configuration which directly loads the resource lists rather than a list of environment files. Uses the same script as used in OSP16 but subs out the network configuration for a vlan type setup and the latest paths for async puddle. * Working deployment of OSP13 * Migrate changes to align to existing docs Update PR to align to existing documentation and testing the group has been working on. Adjust the stf-connectors.yaml.template to better reflect what we've been testing. Deployment by default will result in presettle: true which is bad for reliability of message delivery. * Get closer alignment to OSP16 setup * Enable deployment of metric SG for Ceilometer data (#93) * Enable deployment of metric SG for Ceilometer data Depends-On: infrawatch/smart-gateway#83 Depends-On: infrawatch/smart-gateway-operator#48 * Add smoketest for Ceilometer data * Listen on correct channel * Ceilometer smoketest tuning Makes smoketest_ceilometer_entrypoint.sh being executed during smoketest job. * Use data source setting for metrics too # Please enter the commit message for your changes. Lines starting # with '#' will be ignored, and an empty message aborts the commit. # # On branch mmagr-amqp10connections # Changes to be committed: # modified: roles/servicetelemetry/templates/manifest_smartgateway_metrics.j2 # * Finish ceilometer events smoke test * Do not use hardcoded timestamps * Finish ceilometer metrics smoketests * Increase timeout * Add container names * Validate also Ceilometer metrics SG * Update tests/smoketest/smoketest_ceilometer_entrypoint.sh * Update tests/smoketest/smoketest_collectd_entrypoint.sh Co-authored-by: Martin Magr <[email protected]> Co-authored-by: Leif Madsen <[email protected]> * Implement CI updates for SG3 * Correct value for SG bridge image path * Lock operator-courier to 2.1.7 Lock operator-courier to 2.1.7 until we can figure out what is wrong with our CSV/CRD setup or until the operator-courier issue noted in the related issue is resolved. Related: #108 * Update build/stf-run-ci/tasks/main.yml Co-authored-by: Chris Sibbitt <[email protected]> * Adjust README to match run-ci.yaml methods Co-authored-by: Martin Mágr <[email protected]> Co-authored-by: Martin Magr <[email protected]> Co-authored-by: Chris Sibbitt <[email protected]>
* Changes for sg2 * Added new containers images for sg2 and bridge * Added socket-dir for them to communicate * Added more mode-specific code to deployment template * Removed now-unused metrics-configmap * Use best bridge args from testing * Updated CSV and container values for new SG * Updated core image name & scorecard * fixed "replaced" line in csv * Adjust template to support ceilometer metrics too * Update description in CSV (#53) Update the CSV description so that we could pass upstream Community Operator requirements should we ever publish there. * amqpDataSource should be amqp_data_source * Misplaced conditionals broke volume * Setup SG3 CI system (#59) * Use new connection configuration for metrics (#48) AMQP1Connections parameter should be used instead of nonflexible AMQP1Url. Depends-On: infrawatch/smart-gateway#83 * Bump default CSV to 2.0.0 * Fix file rename * Lock operator-courier to v2.1.7 Lock operator-courier to 2.1.7 until we can figure out what is wrong with our CSV/CRD setup or until the operator-courier issue noted in the related issue is resolved. Related: infrawatch/service-telemetry-operator#108 * Update roles/smartgateway/templates/ceilometer-metrics-configmap.yaml.j2 Co-authored-by: Chris Sibbitt <[email protected]> Co-authored-by: Martin Mágr <[email protected]> Co-authored-by: Chris Sibbitt <[email protected]> Co-authored-by: Chris Sibbitt <[email protected]> Co-authored-by: Martin Mágr <[email protected]>
* Changes to match new internal SG metrics names * Changing exported_instance to host * plugin_instance relabelling no longer being done in sg2 * type becomes type_instance * Applied relevant changes to the new alerts * Fixing plugin vs type instance on memory alarm * Adjusted order of labels in smoketest * Need longer interval for rates with default 10s scrape interval * OCP metrics label change from pod_name to pod * Raise the linkCapacity of QDR->bridge * Adds a new edgeListener on 5673 with linkCapacity 25000 * Adjusts metrics SG bridge to connect to new listener * Minimizes presettled metrics loss during throughput testing * Should also provide performance improvements in unsettled mode bursts * Other SG modes can keep using 5672 until we converge the SG code * New CRD and CSV for v1.0.3 * Updated scorecard paths * Added amqpDataSource to metrics SG template * This is so I can test changes here: infrawatch/smart-gateway-operator#54 * In preparation to merge with: https://github.com/infrawatch/service-telemetry-operator/pull/93/files#diff-33675527951f20f9727fb4be5c84a746R8 * Putting quickstart.sh back until build/run-ci.yaml is a true replacement * No ability to deploy published artifacts without building * No support for ephemeral storage * Adding back quickstart configs * Setup SG3 CI system (#107) * Enhance CI automation (#106) * Add ServiceTelemetry overrides Allows ServiceTelemetry overrides to be expressed via Ansible extra-vars. Adds the four main overrides you would expect in a ServiceTelemetry object, with appropriate defaults set. Will also allow passing in the service_telemetry_manifest as a whole object like what we do with the Service Telemetry Operator. * Allow for per-repo branch overrides Allow for per-repo branch overrides for the Smart Gateway Operator and Smart Gateway repositories via sgo_branch and sg_branch (respectively). * Add functionality around quickstart.sh Add some functionality that was replaced when I dropped the quickstart.sh. Adds some of this functionality back in and also adds some new stuff. * Fix syntax error * Add back quickstart.sh Adds back a quickstart.sh that simulates the same result as the old quickstart.sh * Better CSV modification support Also adds some tags to make skipping over builds for testing much easier. * Debugging ci.yml firing * Make sure namespace is set before using it * Fix syntax and documentation * Test locally first kids * Drop CI debug lines * Clean up working repo clones * Copy CSV into working directory On subsequent runs the in-place modification of the CSV can cause issues either in the development environment, or re-runs of the CI system. Copying the CSV out of the in-place repo into a working location, and then modifying in-place results in a cleaner setup. By doing the copy of the CSV files, we can drop the need to force clone the supporting repositories. Also cleans up some shell commands that were commented out now that they are being dealt with via the replace module. Removes the extra commands added to ci.yml. * Changes to infrared-openstack.sh for OSP13 (#102) * Migrate OSP16 script to OSP13 directory Uses a multi-cloud stf-connectors.yaml style configuration which directly loads the resource lists rather than a list of environment files. Uses the same script as used in OSP16 but subs out the network configuration for a vlan type setup and the latest paths for async puddle. * Working deployment of OSP13 * Migrate changes to align to existing docs Update PR to align to existing documentation and testing the group has been working on. Adjust the stf-connectors.yaml.template to better reflect what we've been testing. Deployment by default will result in presettle: true which is bad for reliability of message delivery. * Get closer alignment to OSP16 setup * Enable deployment of metric SG for Ceilometer data (#93) * Enable deployment of metric SG for Ceilometer data Depends-On: infrawatch/smart-gateway#83 Depends-On: infrawatch/smart-gateway-operator#48 * Add smoketest for Ceilometer data * Listen on correct channel * Ceilometer smoketest tuning Makes smoketest_ceilometer_entrypoint.sh being executed during smoketest job. * Use data source setting for metrics too # Please enter the commit message for your changes. Lines starting # with '#' will be ignored, and an empty message aborts the commit. # # On branch mmagr-amqp10connections # Changes to be committed: # modified: roles/servicetelemetry/templates/manifest_smartgateway_metrics.j2 # * Finish ceilometer events smoke test * Do not use hardcoded timestamps * Finish ceilometer metrics smoketests * Increase timeout * Add container names * Validate also Ceilometer metrics SG * Update tests/smoketest/smoketest_ceilometer_entrypoint.sh * Update tests/smoketest/smoketest_collectd_entrypoint.sh Co-authored-by: Martin Magr <[email protected]> Co-authored-by: Leif Madsen <[email protected]> * Implement CI updates for SG3 * Correct value for SG bridge image path * Lock operator-courier to 2.1.7 Lock operator-courier to 2.1.7 until we can figure out what is wrong with our CSV/CRD setup or until the operator-courier issue noted in the related issue is resolved. Related: #108 * Update build/stf-run-ci/tasks/main.yml Co-authored-by: Chris Sibbitt <[email protected]> * Adjust README to match run-ci.yaml methods Co-authored-by: Martin Mágr <[email protected]> Co-authored-by: Martin Magr <[email protected]> Co-authored-by: Chris Sibbitt <[email protected]> Co-authored-by: Chris Sibbitt <[email protected]> Co-authored-by: Martin Mágr <[email protected]> Co-authored-by: Martin Magr <[email protected]>
AMQP1Connections parameter should be used instead of nonflexible AMQP1Url.
Depends-On: infrawatch/smart-gateway#83