Skip to content

Commit

Permalink
fix(error handling): mark executions failed (#638)
Browse files Browse the repository at this point in the history
* fix(error handling): mark executions failed

We need a way to indicate failures in materializing a pipeline.
If `echo` fails to materialize a pipeline during a trigger the failure needs to be recorded
in `orca` otherwise the failure appears "silent" to the user because the UI is unable to
show any errors unless they are associated with an execution.
This is especially bad if, say, there is a typo in the properties file in the jenkins trigger
Starting the pipeline appears to do nothing and the user is confused.

This change captures errors during artifact resolution and sends them to `orca` to capture for the user
>NOTE: depends on related `orca` PR: spinnaker/orca#3126
  • Loading branch information
marchello2000 committed Sep 4, 2019
1 parent ed001d9 commit 7e33063
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ public class Pipeline {

@JsonProperty Object appConfig;

@JsonProperty String errorMessage;

Trigger trigger;

@JsonPOJOBuilder(withPrefix = "")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,11 @@ private Optional<Pipeline> withMatchingTrigger(ManualEvent manualEvent, Pipeline
String application = content.getApplication();
String pipelineNameOrId = content.getPipelineNameOrId();
if (pipelineMatches(application, pipelineNameOrId, pipeline)) {
return Optional.of(buildTrigger(pipelineCache.refresh(pipeline), content.getTrigger()));
try {
return Optional.of(buildTrigger(pipelineCache.refresh(pipeline), content.getTrigger()));
} catch (Exception e) {
return Optional.of(pipeline.withErrorMessage(e.toString()));
}
} else {
return Optional.empty();
}
Expand Down Expand Up @@ -126,26 +130,49 @@ protected Pipeline buildTrigger(Pipeline pipeline, Trigger manualTrigger) {
String master = manualTrigger.getMaster();
String job = manualTrigger.getJob();
Integer buildNumber = manualTrigger.getBuildNumber();

ArrayList<String> pipelineErrors = new ArrayList<>();

if (buildInfoService.isPresent()
&& StringUtils.isNoneEmpty(master, job)
&& buildNumber != null) {
BuildEvent buildEvent = buildInfoService.get().getBuildEvent(master, job, buildNumber);
trigger =
trigger
.withBuildInfo(buildInfoService.get().getBuildInfo(buildEvent))
.withProperties(
buildInfoService
.get()
.getProperties(buildEvent, manualTrigger.getPropertyFile()));
artifacts.addAll(
buildInfoService.get().getArtifactsFromBuildEvent(buildEvent, manualTrigger));
try {
trigger = trigger.withBuildInfo(buildInfoService.get().getBuildInfo(buildEvent));
} catch (Exception e) {
pipelineErrors.add("Could not get build from build server: " + e.toString());
}

try {
trigger =
trigger.withProperties(
buildInfoService.get().getProperties(buildEvent, manualTrigger.getPropertyFile()));
} catch (Exception e) {
pipelineErrors.add("Could not get property file from build server: " + e.toString());
}

try {
artifacts.addAll(
buildInfoService.get().getArtifactsFromBuildEvent(buildEvent, manualTrigger));
} catch (Exception e) {
pipelineErrors.add("Could not get all artifacts from build server: " + e.toString());
}
}

try {
if (artifactInfoService.isPresent()
&& !CollectionUtils.isEmpty(manualTrigger.getArtifacts())) {
List<Artifact> resolvedArtifacts = resolveArtifacts(manualTrigger.getArtifacts());
artifacts.addAll(resolvedArtifacts);
// update the artifacts on the manual trigger with the resolved artifacts
trigger = trigger.withArtifacts(convertToListOfMaps(resolvedArtifacts));
}
} catch (Exception e) {
pipelineErrors.add("Could not resolve artifacts: " + e.toString());
}

if (artifactInfoService.isPresent() && !CollectionUtils.isEmpty(manualTrigger.getArtifacts())) {
List<Artifact> resolvedArtifacts = resolveArtifacts(manualTrigger.getArtifacts());
artifacts.addAll(resolvedArtifacts);
// update the artifacts on the manual trigger with the resolved artifacts
trigger = trigger.withArtifacts(convertToListOfMaps(resolvedArtifacts));
if (!pipelineErrors.isEmpty()) {
pipeline = pipeline.withErrorMessage(String.join("\n", pipelineErrors));
}

return pipeline
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.netflix.spinnaker.echo.pipelinetriggers.monitor;

import com.google.common.base.Strings;
import com.netflix.spectator.api.Id;
import com.netflix.spectator.api.Registry;
import com.netflix.spinnaker.echo.events.EchoEventListener;
Expand Down Expand Up @@ -74,12 +75,17 @@ private void triggerMatchingPipelines(T event) {
try {
List<Pipeline> matchingPipelines = eventHandler.getMatchingPipelines(event, pipelineCache);
matchingPipelines.stream()
.filter(p -> Strings.isNullOrEmpty(p.getErrorMessage()))
.map(pipelinePostProcessorHandler::process)
.forEach(
p -> {
recordMatchingPipeline(p);
pipelineInitiator.startPipeline(p, PipelineInitiator.TriggerSource.EVENT);
});

matchingPipelines.stream()
.filter(p -> !Strings.isNullOrEmpty(p.getErrorMessage()))
.forEach(pipelineInitiator::recordPipelineFailure);
} catch (TimeoutException e) {
log.error("Failed to get pipeline configs", e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import com.netflix.spinnaker.security.AuthenticatedRequest;
import java.util.Collection;
import java.util.Map;
import retrofit.client.Response;
import retrofit.http.Body;
import retrofit.http.GET;
import retrofit.http.POST;
Expand All @@ -31,6 +32,9 @@ public interface OrcaService {
@POST("/orchestrate")
TriggerResponse trigger(@Body Pipeline pipeline);

@POST("/fail")
Response recordFailure(@Body Pipeline pipeline);

@POST("/plan")
Map plan(@Body Map pipelineConfig, @Query("resolveArtifacts") boolean resolveArtifacts);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,10 @@ public enum TriggerSource {
EVENT
}

public void recordPipelineFailure(Pipeline pipeline) {
orca.recordFailure(pipeline);
}

public void startPipeline(Pipeline pipeline, TriggerSource triggerSource) {
if (enabled) {
try {
Expand Down

0 comments on commit 7e33063

Please sign in to comment.