-
Notifications
You must be signed in to change notification settings - Fork 808
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
feat(monitoredDeploy): Completed event should include details of success/failure #3214
Conversation
...ix/spinnaker/orca/clouddriver/pipeline/servergroup/strategies/MonitoredDeployStrategy.groovy
Outdated
Show resolved
Hide resolved
.../com/netflix/spinnaker/orca/clouddriver/tasks/monitoreddeploy/NotifyDeployCompletedTask.java
Outdated
Show resolved
Hide resolved
...ix/spinnaker/orca/clouddriver/pipeline/servergroup/strategies/MonitoredDeployStrategy.groovy
Outdated
Show resolved
Hide resolved
...oovy/com/netflix/spinnaker/orca/clouddriver/tasks/cluster/ClusterSizePreconditionTask.groovy
Outdated
Show resolved
Hide resolved
...ntmonitor/src/main/java/com/netflix/spinnaker/orca/deploymentmonitor/models/RequestBase.java
Outdated
Show resolved
Hide resolved
Once I merge the rollback pr #3222 you can add the failure/rollback flag |
.../com/netflix/spinnaker/orca/clouddriver/tasks/monitoreddeploy/NotifyDeployCompletedTask.java
Outdated
Show resolved
Hide resolved
e472453
to
2081651
Compare
orca-core/src/main/java/com/netflix/spinnaker/orca/pipeline/model/Stage.java
Outdated
Show resolved
Hide resolved
...va/com/netflix/spinnaker/orca/deploymentmonitor/models/MonitoredDeployInternalStageData.java
Outdated
Show resolved
Hide resolved
.../com/netflix/spinnaker/orca/clouddriver/tasks/monitoreddeploy/NotifyDeployCompletedTask.java
Outdated
Show resolved
Hide resolved
orca-core/src/main/java/com/netflix/spinnaker/orca/pipeline/model/Stage.java
Outdated
Show resolved
Hide resolved
...ix/spinnaker/orca/clouddriver/pipeline/servergroup/strategies/MonitoredDeployStrategy.groovy
Outdated
Show resolved
Hide resolved
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.
See my comments in-line and let me know if you have any questions
...etflix/spinnaker/orca/clouddriver/tasks/monitoreddeploy/NotifyDeployCompletedTaskSpec.groovy
Outdated
Show resolved
Hide resolved
...etflix/spinnaker/orca/clouddriver/tasks/monitoreddeploy/NotifyDeployCompletedTaskSpec.groovy
Outdated
Show resolved
Hide resolved
.../com/netflix/spinnaker/orca/clouddriver/tasks/monitoreddeploy/NotifyDeployCompletedTask.java
Outdated
Show resolved
Hide resolved
.../com/netflix/spinnaker/orca/clouddriver/tasks/monitoreddeploy/NotifyDeployCompletedTask.java
Outdated
Show resolved
Hide resolved
…ouddriver/tasks/monitoreddeploy/NotifyDeployCompletedTaskSpec.groovy Co-Authored-By: Mark Vulfson <markvu@live.com>
…ouddriver/tasks/monitoreddeploy/NotifyDeployCompletedTask.java Co-Authored-By: Mark Vulfson <markvu@live.com>
return TaskResult.ofStatus(ExecutionStatus.SUCCEEDED); | ||
} | ||
|
||
private DeploymentCompletedRequest.DeploymentStatus convertStageStatus(Boolean failedStatus) { |
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.
nit: you can make this boolean now
private DeploymentCompletedRequest.DeploymentStatus convertStageStatus(Boolean failedStatus) { | |
private DeploymentCompletedRequest.DeploymentStatus convertStageStatus(boolean failedStatus) { |
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.
lgtm, thanks for your diligence on this!
Completed event (see NotifyDeployCompletedTask in orca) should populate the status and rollback fields of the DeploymentCompletedRequest.
Example:
deploymentStatus: “success” / ”failure”,
rollback: "failure" / "success" / "not_performed"