prometheus alerts for openshift build subsystem #16495

gabemontero · 2017-09-21T21:00:33Z

https://trello.com/c/RskNHpfh/1334-5-prometheus-alerts-for-build-metrics

A WIP initial pass at alerts for the openshift build subsystem

@openshift/devex @smarterclayton @zgalor @moolitayer @mfojtik ptal, defer if bandwidth dictates, and/or pull in others as you each deem fit

Disclaimers:

I'm still debating the pros/cons of these alerts with https://docs.google.com/document/d/199PqyG3UsyXlwieHaqbGiWVa8eMWi8zzAn0YfcApr8Q/edit#heading=h.2efurbugauf in mind
still debating the template parameters / defaults for the various thresholds ... I still have a to-do to revisit with ops contacts potential default values based on their existing zabbix monitoring
still debating the severity as well
based on the activity in Update test alert metadata #16026 I did not include the miqTarget annotation

I also removed the space in the existing alert name based on how I interpreted various naming conventions.

And other than the query on the alerts URI, the extended test changes stemmed from flakiness experienced during testing that was unrelated to the addition of the alerts.

thanks

gabemontero · 2017-09-21T21:01:01Z

(5) Prometheus alerts for build metrics

smarterclayton · 2017-09-21T21:03:58Z

examples/prometheus/prometheus.yaml

@@ -30,6 +30,15 @@ parameters:
  name: SESSION_SECRET
  generate: expression
  from: "[a-zA-Z0-9]{43}"
+- description: The threshold for the active build alert


Do not add parameters here. If you can't pick good defaults we shouldn't add the alert.

Basically parameterization is the devil. No parameters. It's either a good rule, or it shouldn't be here (add it commented out).

it seems to me the duration and volume of running builds that determines whether you have a problem or not is going to be determined in part by the size of your cluster. (possibly also derived from steady state observations). The fact that there aren't absolute values for it doesn't make it a bad rule.

if your suggestion is that such rules should be commented out rather than enabled out of the box, ok, but I would expect that's going to be the case for almost all our rules. It's hard to imagine a metric who's health/sick condition isn't going to be determined in part by cluster characteristics.

Maybe what I need is a better understanding of the intent of this file?

(and is there some other file where we should be putting in rules that we want for our actual online prod cluster?)

smarterclayton · 2017-09-21T21:05:48Z

test/extended/prometheus/prometheus_builds.go

+
+				for _, sample := range metrics {
+					// first see if a metric has all the label names and label values we are looking for
+					foundCorrectLabels := true


If we don't have helpers to do this you should create them. This is pretty ugly code.

Isn't there a prometheus client library?

re: prometheus client library, at least for my interpretation of that, there is an alpha one for remote access ....see https://github.com/prometheus/client_golang#client-for-the-prometheus-http-api and https://github.com/prometheus/client_golang/blob/master/api/prometheus/v1/api.go

but the version of prometheus vendored into origin does not have the above

there is "client_golang" stuff in the prometheus vendored in, but it is non-HTTP non-remote

unless I missed something

otherwise sure I can work up some helpers ... might do it in a non-trello card PR depending on what happens with this work

smarterclayton · 2017-09-21T21:06:58Z

examples/prometheus/prometheus.yaml

+            severity: "MEDIUM"
+            message: "{{$labels.instance}} indicates at least ${PROMETHEUS_ACTIVE_BUILD_ALERT_THRESHOLD} OpenShift builds are taking more than ${PROMETHEUS_ACTIVE_BUILD_ALERT_DURATION} seconds to complete"
+        - alert: BuildFailureRate
+          expr: (count(openshift_build_terminal_phase_total{phase="failed"}) / count(openshift_build_terminal_phase_total{phase="complete"})) > ${PROMETHEUS_FAILED_BUILD_RATIO}


How would this alert get cleared? If openshift online has 9000 failed builds, do I have to wait for 9000 successful builds for the alert to go away?

pending all the other discussion points in the PR, I almost did not include this alert to begin with (it was suggested to me) .... I'm inclined to simply remove it, but will wait a bit to see what other discussion arises with it before doing so.

As the suggester of the alert: i don't know what the prometheus workflow is like, or how the alerting behaves... can you have an alert that's ignored once tripped until it resets below the alert level?

Pruning old builds would be one way for it to be reset. Or we can punt it and do an alert based on "failed builds within the last X minutes" instead, but that's going to be another alert that needs to be tuned for your cluster/expectations.

smarterclayton · 2017-09-21T21:07:28Z

examples/prometheus/prometheus.yaml

          expr: up{job="kubernetes-nodes"} == 0
          annotations:
            miqTarget: "ContainerNode"
            severity: "HIGH"
-            message: "{{$labels.instance}} is down"
+            message: "{{$labels.instance}} is down"    
+        - alert: HangingBuild


It's not hanging, it's slow.

smarterclayton · 2017-09-21T21:08:17Z

examples/prometheus/prometheus.yaml

+          expr: count(openshift_build_running_phase_start_time_seconds{} < time() - ${PROMETHEUS_ACTIVE_BUILD_ALERT_DURATION}) > ${PROMETHEUS_ACTIVE_BUILD_ALERT_THRESHOLD}
+          annotations:
+            severity: "MEDIUM"
+            message: "{{$labels.instance}} indicates at least ${PROMETHEUS_ACTIVE_BUILD_ALERT_THRESHOLD} OpenShift builds are taking more than ${PROMETHEUS_ACTIVE_BUILD_ALERT_DURATION} seconds to complete"


This is a more clear alarm. However, I'm not sure I want to check the other two in until you prove that you want these alerts on. I.e. when we upgrade to 3.7.0 I'll turn your proposed alerts on and send them to you. Then you can decide whether they work or not.

I actually probably would say we're not ready to merge any of these, and instead you should give me the alerts to turn on in production which i will hook up to your email.

Based on what I've learned as part of taking on this card (see the referenced google doc), I'm inclined to not do any of these alerts, either in this example template or manually submitting them to you (if not submitting to you is even an option), and wait until we get more practical experience with the build subsystem in production. But the card was assigned and initiating a PR to start discussions seemed the best thing to do.

On the values of the conditions / evil template parameters, for the current zabbix equivalent that ops is employing today, https://github.com/openshift/openshift-tools/blob/prod/scripts/monitoring/cron-send-stuck-builds.py#L60-L93, the "age" is not hard coded, but a variable passed in.

That said, I've re-initiated the email exchange I started with them during the build metrics work, and seeing if default values emerge (perhaps the calling to the referenced function hardcodes the value).

Let's actually base it on real data. Suggest your top 3 alerts based on recent bugs, and I'll turn them on after the 3.7.0 upgrade.

10-4 re: alerts <=> recent bugs and #16495 (comment) .... @bparees and I will start conferring on all that

for completeness, an update on what ops is currently doing with their zabbix stuff:

they are currently concerned with builds stuck in NEW|PENDING ... not slow RUNNING builds

I elaborated a bit that is really a pod scheduling issue, and might be captured by any new k8s or platform mgmt team metrics that get injected/rebased into openshift/origin

but that we could add it to our build specific stuff easy enough if need

their build alerts are at the lowest level; so a warning on the zabbix board, but nobody gets paged

bparees · 2017-09-21T21:08:22Z

examples/prometheus/prometheus.yaml

+- description: The allowable active build duration for the active build alert
+  name: PROMETHEUS_ACTIVE_BUILD_ALERT_DURATION
+  value: "360"
+- description: The allowable ration of failed to complete builds for the failed build alert


smarterclayton · 2017-09-21T22:31:27Z

So I think you guys got the hard one (being the first responders). Let's turn this around a bit.

The point of metrics is to enable you to debug and assess the health of your component in the product.

The point of alerts is to unambiguously indicate a serious problem is occurring that an engineer should react to.

The process for this is best laid out as

add a metric that you think represents your health, based on past bugs / problems / challenges in production
observe the metric in production and verify it matches your expectation
repeat by adding more metrics or refining the ones in place
define an unambiguous alert based on production data that would indicate that production is in trouble and we should react - observe that (you, that is) until you're confident it represents a real problem
take that alert and refine its parameters further
ship that alert to end users

I'd expect you guys to be in the gathering phase (between 1-4). I'd recommend you create a new section the readme and place your recommended snippets in there with some guesses and text that says what to do with them. I'll accumulate them into one of the prod clusters and then you can observe them.

We won't ship the alert until it's something you're confident we could page you on at 1am on a Saturday night and you'd be happy because it means you just saved the world.

smarterclayton · 2017-09-21T22:41:34Z

Re: parameters every time you add a parameter to a template god kills a kitten. Parameters are best used judiciously and sparingly, and making them tunable makes it the end user's problem before they are ready. Most users should be forking our config if they don't like our settings. Parameters that are proportional to cluster size can be alerted proportional to cluster size.

…

On Thu, Sep 21, 2017 at 5:01 PM, Gabe Montero ***@***.***> wrote: <https://camo.githubusercontent.com/193766a3b9959c5f4ed5cd8cf3251d015e839d51/68747470733a2f2f6769746875622e7472656c6c6f2e73657276696365732f696d616765732f6d696e692d7472656c6c6f2d69636f6e2e706e67> (5) Prometheus alerts for build metrics <https://trello.com/c/RskNHpfh/1334-5-prometheus-alerts-for-build-metrics> — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#16495 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABG_p5D2b0AH5kTgSqq4XC9gdetYpe2Xks5sks6QgaJpZM4Pf6SJ> .

bparees · 2017-09-22T19:21:05Z

add a metric that you think represents your health, based on past bugs / problems / challenges in production

Well, that was the metric that included the failure reason+start time date which was rejected as not conforming to the prometheus philosophy. Failed builds alone is not a very interesting metrics for us since there are lots of perfectly normal reasons for builds to fail.

@gabemontero is going to circle back w/ a hybrid proposal between prometheus metrics and live query-based state analysis.

Alternatively, rather than adding failure reason+timetamp labels to our "completed builds" metric, what if we introduced additional metrics like:

FailedFetchSourceCount
FailedPushImageCount
FailedLast5MinutesCount

?

(the collector would do the work of summing those things when iterating the build cache).

gabemontero · 2017-09-26T20:32:33Z

OK, I've just pushed updates in response to the various comments.

Specifics:

saved some kittens, template parameters gone
alerts removed from template, per the metrics analysis phase we are in
net, template no longer changed
updated README with some sample queries, where those queries are of the form I initially could see us executing online as part of assessing metrics / forming alerts later on
extended test massaging / simplification; comments on where various prometheus apis either are not vendored into origin yet, or not sufficient
on "top bug review" and stats that would have assisted diagnosis:
1. all builds failed because of registry issues; so no more successful builds after the registry failure, bunch of failed builds for a given reason, the rate of failed builds increasing after the registry failure
2. build pods were not getting created and scheduled properly; so a bunch of builds in new/pending
3. overwhelmed docker registry delay docker push; builds slower than expected
affect on metrics: added/modified metrics; may delete a metric, but have not yet (want to discuss); specifics:
1. broke out failed builds from the complete/cancelled/error; those three don't have failed reasons, so was able to stay at 1 label for both count/gauges, where the failed count just has the reason label, and the terminal(complete/cancelled/error) just has the phase label
2. adding a new/pending constant metric with unix time, a la the running metric; since start time is not set on those, using the api obj create time; in theory this could be a generic pod metric, but don't see those yet, and I don't want to wait; also, this lines up with ops current monitoring with pending builds via zabbix
3. for the running constant time metric, which would identify slow builds, overwhelmed docker registries, etc.; I'm now not convinced this will be useful ... what will be the too slow threshold??? I'm inclined to delete, but decided to wait for feedback before doing so.

bparees · 2017-09-26T23:01:34Z

for the running constant time metric, which would identify slow builds, overwhelmed docker registries, etc.; I'm now not convinced this will be useful ... what will be the too slow threshold??? I'm inclined to delete, but decided to wait for feedback before doing so.

let's hang onto it for now and see how it performs in the real world.

bparees

some nits but i'm in agreement w/ the general changes.

bparees · 2017-09-26T23:03:06Z

examples/prometheus/README.md

+
+> openshift_build_failed_phase_total{}
+
+Returns the latest totals for failed builds.


how is this different from the count() query above?

if the count() query above is doing a sum of all the collected data points ("yesterday there were a total of 100 failed builds in the system", "today there were a total of 101", "count=201") that doesn't seem too useful since it's going to double (or triple or quadruple..) count a lot of builds.

this query will return a multi-row table which includes count by reason; I'll push an update the clarifies that, as well as the aggregation that occurs with count (though that might need to be sum ... I'll sort that out as well).

got it, thanks. (I think you explained that to me before in my cube, sorry i needed the refresher)

bparees · 2017-09-26T23:05:31Z

pkg/build/metrics/prometheus/metrics.go

+	activeBuildCount          = "running_phase_start_time_seconds"
+	activeBuildCountQuery     = buildSubsystem + separator + activeBuildCount
+	newPendingBuildCount      = "new_pending_phase_creation_time_seconds"
+	newPendingBuildCountQuery = buildSubsystem + separator + newPendingBuildCount
 )

 var (
 	// decided not to have a separate counter for failed builds, which have reasons,


comment is no longer accurate

sure isn't :-) ... will remove

bparees · 2017-09-26T23:08:13Z

pkg/build/metrics/prometheus/metrics.go

+	}
+	for reason, count := range reasons {
+		addCountGauge(ch, failedBuildCountDesc, reason, float64(count))
+	}
 	addCountGauge(ch, terminalBuildCountDesc, errorPhase, float64(error))


these should probably be counted under failedBuilds with a reason of "Build pod encountered an error" or something.

(errored builds are closer to "failed" builds than they are to "completed+canceled" builds).

yeah I went back and forth on this, considering each point you just mentioned vs. the fact that in the runtime, following a guru search, the StatusReason field in the build is never set with an error build

bottom line, I'm not convinced ... unless there is more than one reason for builds in error state, it is only wasteful to add a label reason for it ... we shouldn't use prometheus labels to clarify like this

I'm saying we treat "Error" as the reason. Treat Failed and Error builds the same, when applying the reason label for the Error builds, just manufacture a reason.

In other words i want the "failedBuilds" metrics to include the count of Error builds, because otherwise they may be ignored under "terminated" builds as not interesting.

I see ... ok, we should make the reason string consistent in look/feel to the others ... I'll go with BuildPodError .... there already BuildPodDeleted and BuildPodExists

sounds good.

bparees · 2017-09-26T23:10:18Z

pkg/build/metrics/prometheus/metrics.go

 	case buildapi.BuildPhaseFailed:
-		failed++
+		// currently only failed builds have reasons
+		reasons[string(b.Status.Reason)] = 1
 	case buildapi.BuildPhaseError:
 		error++


you may live to regret this variable name choice.

ah ... a golang thing perhaps ... good catch / I'll change

bparees · 2017-09-26T23:12:15Z

pkg/build/metrics/prometheus/metrics.go

+		e, cc, cp, r := bc.collectBuild(ch, b)
+		for key, value := range r {
+			count := reasons[key]
+			count = count + value


count:=reasons[value]+value ?

or better reasons[key]=reasons[key]+value?

yep ... iterated a bit on this piece and didn't finish clean up .. thx

gabemontero · 2017-09-26T23:52:17Z

cmd test run looks to have hit flake #16468 though that was presumably fixed / closed by @bparees

then, another cmd test did a oc delete all -l app=helloworld, which resulted in buildconfig "ruby-sample-build" deleted and then Error from server (NotFound): builds "ruby-sample-build-1" not found ... seems like maybe a race condition in deleting bc's and builds with owner ref's to the bc's

both of course unrelated to this change

gabemontero · 2017-09-27T00:47:41Z

responses to sept 26 comments from @bparees pushed

bparees · 2017-09-27T00:52:43Z

/lgtm

but will
/hold

to give @smarterclayton a last opportunity to raise concerns before we merge this tomorrow. ( @gabemontero you can remove the WIP label)

gabemontero · 2017-09-27T19:54:14Z

Thought it just compiled for me locally, so not obvious what is up:

gmontero ~/go/src/github.com/openshift/origin  (build-alert)$ hack/build-go.sh test/extended/extended.test
++ Building go targets for linux/amd64: test/extended/extended.test
[INFO] hack/build-go.sh exited with code 0 after 00h 00m 19s

looking ...

gabemontero · 2017-09-27T20:01:57Z

a rebase, make clean, then recompile now reproduces compile issue

gabemontero · 2017-09-27T20:05:02Z

yep ... 31fe387#diff-5e0dc194b78051f414a42e52f4caf7dd merged yesterday, changed things up

gabemontero · 2017-09-27T21:10:29Z

rebase after @jim-minter pr merge pushed

at least for the cmd test failures that were available ... again appear to be random flakes ... was able to execute same tests locally

bparees · 2017-09-27T21:11:56Z

/lgtm
/unhold

gabemontero · 2017-09-27T21:45:02Z

cmd test failures same as last time:

=== BEGIN TEST CASE ===
test/cmd/authentication.sh:101: executing 'oc get --raw /metrics --as=user3' expecting success and text 'apiserver_request_latencies'
FAILURE after 0.213s: test/cmd/authentication.sh:101: executing 'oc get --raw /metrics --as=user3' expecting success and text 'apiserver_request_latencies': the command returned the wrong error code; the output content test failed
There was no output from the command.
Standard error from the command:
Unable to connect to the server: unexpected EOF
=== END TEST CASE ===

and

=== BEGIN TEST CASE ===
hack/test-cmd.sh:126: executing 'oc delete project 'cmd-authentication'' expecting success
FAILURE after 0.177s: hack/test-cmd.sh:126: executing 'oc delete project 'cmd-authentication'' expecting success: the command returned the wrong error code
There was no output from the command.
Standard error from the command:
The connection to the server 172.17.0.2:28443 was refused - did you specify the right host or port?
=== END TEST CASE ===

But I can run make test-cmd, hack/test-cmd.sh test/cmd/authentication.sh locally ... not sure how to debug, but will wait to see what is up with other tests when logs are available.

gabemontero · 2017-09-27T22:05:22Z

Found it:

panic: assignment to entry in nil map
goroutine 186758 [running]:
github.com/openshift/origin/pkg/build/metrics/prometheus.(*buildCollector).collectBuild(0x10ea7470, 0xc429102000, 0xc43682d200, 0x0, 0x1, 0x0)
	/go/src/github.com/openshift/origin/_output/local/go/src/github.com/openshift/origin/pkg/build/metrics/prometheus/metrics.go:146 +0x1ab
github.com/openshift/origin/pkg/build/metrics/prometheus.(*buildCollector).Collect(0x10ea7470, 0xc429102000)
	/go/src/github.com/openshift/origin/_output/local/go/src/github.com/openshift/origin/pkg/build/metrics/prometheus/metrics.go:99 +0x17c
github.com/openshift/origin/vendor/github.com/prometheus/client_golang/prometheus.(*Registry).Gather.func2(0xc435ab4920, 0xc429102000, 0xefc10a0, 0x10ea7470)
	/go/src/github.com/openshift/origin/_output/local/go/src/github.com/openshift/origin/vendor/github.com/prometheus/client_golang/prometheus/registry.go:382 +0x61
created by github.com/openshift/origin/vendor/github.com/prometheus/client_golang/prometheus.(*Registry).Gather
	/go/src/github.com/openshift/origin/_output/local/go/src/github.com/openshift/origin/vendor/github.com/prometheus/client_golang/prometheus/registry.go:383 +0x2ec

will need to push another update.

Not sure yet why it happens on ci and not locally, but given the nil ref / panic, does not matter.

gabemontero · 2017-09-27T22:09:27Z

must be some sort of golang / version / compile diff thingy

…uild subsystem

bparees · 2017-09-27T22:15:51Z

/lgtm

openshift-merge-robot · 2017-09-27T22:16:03Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bparees, gabemontero

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

~~examples/prometheus/OWNERS~~ [bparees]
~~pkg/build/OWNERS~~ [bparees]
~~test/extended/OWNERS~~ [bparees]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

gabemontero · 2017-09-27T22:32:31Z

the cmd tests are still running, but the authenticate.sh tests that failed before have passed

gabemontero · 2017-09-28T02:13:28Z

unrelated cmd /test cmd

gabemontero · 2017-09-28T02:14:53Z

/test cmd

gabemontero · 2017-09-28T04:42:15Z

/test extended_networking_minimal

openshift-bot · 2017-09-28T09:28:24Z

/retest

Please review the full test history for this PR and help us cut down flakes.

openshift-merge-robot · 2017-09-28T12:58:16Z

Automatic merge from submit-queue.

openshift-merge-robot assigned smarterclayton and stevekuznetsov Sep 21, 2017

openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Sep 21, 2017

gabemontero added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 21, 2017

smarterclayton reviewed Sep 21, 2017

View reviewed changes

bparees reviewed Sep 21, 2017

View reviewed changes

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 24, 2017

gabemontero force-pushed the build-alert branch from e07d075 to 838a344 Compare September 24, 2017 16:56

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 24, 2017

gabemontero force-pushed the build-alert branch from 838a344 to 2126108 Compare September 26, 2017 20:32

bparees requested changes Sep 26, 2017

View reviewed changes

gabemontero force-pushed the build-alert branch from 2126108 to dda3ba5 Compare September 27, 2017 00:47

openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 27, 2017

openshift-ci-robot assigned bparees Sep 27, 2017

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 27, 2017

openshift-merge-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 27, 2017

gabemontero removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 27, 2017

gabemontero force-pushed the build-alert branch from aa4eeb8 to c209a95 Compare September 27, 2017 20:09

openshift-merge-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed lgtm Indicates that a PR is ready to be merged. labels Sep 27, 2017

gabemontero force-pushed the build-alert branch from c209a95 to 456dce5 Compare September 27, 2017 21:09

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 27, 2017

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 27, 2017

bparees removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 27, 2017

metrics, readme changes to prep for prometheus alerts for openshift b…

cf023ef

…uild subsystem

gabemontero force-pushed the build-alert branch from 456dce5 to cf023ef Compare September 27, 2017 22:10

openshift-merge-robot removed the lgtm Indicates that a PR is ready to be merged. label Sep 27, 2017

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 27, 2017

bparees added the kind/bug Categorizes issue or PR as related to a bug. label Sep 28, 2017

openshift-merge-robot merged commit f86e504 into openshift:master Sep 28, 2017

gabemontero deleted the build-alert branch September 28, 2017 13:56


		> openshift_build_failed_phase_total{}

		Returns the latest totals for failed builds.

prometheus alerts for openshift build subsystem #16495

prometheus alerts for openshift build subsystem #16495

Conversation

gabemontero commented Sep 21, 2017

gabemontero commented Sep 21, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gabemontero Sep 21, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smarterclayton Sep 21, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smarterclayton commented Sep 21, 2017

smarterclayton commented Sep 21, 2017 via email

bparees commented Sep 22, 2017

gabemontero commented Sep 26, 2017

bparees commented Sep 26, 2017

bparees left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gabemontero commented Sep 26, 2017 • edited Loading

gabemontero commented Sep 27, 2017

bparees commented Sep 27, 2017

gabemontero commented Sep 27, 2017

gabemontero commented Sep 27, 2017

gabemontero commented Sep 27, 2017

gabemontero commented Sep 27, 2017

bparees commented Sep 27, 2017

gabemontero commented Sep 27, 2017

gabemontero commented Sep 27, 2017

gabemontero commented Sep 27, 2017

bparees commented Sep 27, 2017

openshift-merge-robot commented Sep 27, 2017

gabemontero commented Sep 27, 2017

gabemontero commented Sep 28, 2017

gabemontero commented Sep 28, 2017

gabemontero commented Sep 28, 2017

openshift-bot commented Sep 28, 2017

openshift-merge-robot commented Sep 28, 2017

gabemontero Sep 21, 2017 •

edited

Loading

smarterclayton Sep 21, 2017 •

edited

Loading

gabemontero commented Sep 26, 2017 •

edited

Loading