Add deployment failure reason to event. #6011
Conversation
Summary: Currently we do not send the reason for a deployment failure along with the event. This makes debugging hard. This change introduces an optional reason message.
There was a problem hiding this comment.
I'm building your change at jenkins-marathon-pipelines-PR-6011-1.
There was a problem hiding this comment.
✔ Build of #6011 completed successfully.
See details at jenkins-marathon-pipelines-PR-6011-1.
You can create a DC/OS with your patched Marathon by creating a new pull
request with the following changes in buildinfo.json:
"url": "https://s3.amazonaws.com/downloads.mesosphere.io/marathon/builds/1.6.320-130b3bb34/marathon-1.6.320-130b3bb34.tgz",
"sha1": "1eda8f81bcda91b0fe86f48b28ff3185019a9c8e"You can run system integration test changes of this PR against Marathon
master by triggering this Jenkins job with the Pull_Request_id 6011.
The job then reports back to this PR.
＼\ ٩( ᐛ )و /／
| @@ -16,6 +16,11 @@ get: | |||
| description: | |||
| Specify subscribed event types. | |||
| You can specify this parameter multiple times with different values. | |||
| plan-format: | |||
There was a problem hiding this comment.
Is this part of this PR? Could it be that you meant to include the reason field?
There was a problem hiding this comment.
This is a clean up. Boy Scout rule. We never followed up.
There was a problem hiding this comment.
It could be a separate PR, but I am totally fine with it being here 👍
|
|
||
| event.reason match { | ||
| case Some(reason) => | ||
| serializedEvent ++ Json.obj("reason" -> reason) |
There was a problem hiding this comment.
Please document the reason field in the RAML docs
There was a problem hiding this comment.
I don't think we have the event types in RAML anywhere. Do we?
| @@ -147,7 +147,7 @@ class DeploymentManagerActor( | |||
| sender() ! cancelDeployment(plan.id) | |||
|
|
|||
| case DeploymentFinished(plan, result) => | |||
| runningDeployments.remove(plan.id).map { deploymentInfo => | |||
| runningDeployments.remove(plan.id).foreach { deploymentInfo => | |||
|
I created a DC/OS build with this build of Marathon dcos/dcos#2457. |
Summary:
Currently we do not send the reason for a deployment failure along with
the event. This makes debugging hard. This change introduces an optional
reason message.