make bpm optional - #321
Conversation
When system-metrics-release is deployed as an add on requiring bpm is problematic as the version of bpm in the add on must exactly match the version in the deployment manifest. Making it optional for now, allowing for a transition period.
| bosh_metrics_forwarder_metrics_only: | ||
| description: "reduces metrics emitted only to the metrics emitted by the bosh system metrics forwarder" | ||
| default: false | ||
| clock_drift_enabled: |
There was a problem hiding this comment.
I remember I had to fix a merge conflict when BPM was enabled in a previous PR and I attempted to add the clock drift flag in - does this bring back something or make it so that we'd have to add this enabled flag somewhere else, or is it ok?
There was a problem hiding this comment.
I guess you had to fix merge conflicts as @mkocher was working on job spec changes. This change doesn't change the clock_drift properties
jorbaum
left a comment
There was a problem hiding this comment.
Looks good. It sounds like you have good reasons for bringing this back. It could also help to be more specific about the use case here. Considering chpst is still missing on Resolute stemcell.
I also fixed up the readme while I was adding documentation for this change.
I can see no readme change. Maybe you forgot to add this?
| When true, run this job under BPM. BPM is required on Resolute Raccoon | ||
| stemcells; support for running without BPM will be removed in a future | ||
| version. | ||
| default: false |
There was a problem hiding this comment.
I wonder if using a default of false is a good idea here. Especially since you mention non-bpm will be dropped in a future release.
| bosh_metrics_forwarder_metrics_only: | ||
| description: "reduces metrics emitted only to the metrics emitted by the bosh system metrics forwarder" | ||
| default: false | ||
| clock_drift_enabled: |
There was a problem hiding this comment.
I guess you had to fix merge conflicts as @mkocher was working on job spec changes. This change doesn't change the clock_drift properties
| When true, run this job under BPM. BPM is required on Resolute Raccoon | ||
| stemcells; support for running without BPM will be removed in a future | ||
| version. | ||
| default: false |
There was a problem hiding this comment.
Nice. You've thought about not breaking the current behavior <3
When system-metrics-release is deployed as an add on requiring bpm is problematic as the version of bpm in the add on must exactly match the version in the deployment manifest. Making it optional for now, allowing for a transition period.
I also fixed up the readme while I was adding documentation for this change.
Type of change
Testing performed?
Checklist:
mainbranch, or relevant version branchIf you have any questions, or want to get attention for a PR or issue please reach out on the #logging-and-metrics channel in the cloudfoundry slack