feat(travis): Support for log_complete from Travis API (#449)

* feat(travis): Support for log_complete from Travis API

Travis added a new property in version 2.2.9 (also available on travis-ci.org) that tells us if the log is complete or not. This allows us to be more efficient in the way we fetch artifacts and properties from Travis. It is not backwards compatible, so we have to keep the old way of doing it around for a while.

Each Travis master will be queried at Igor startup for some simple feature detection to determine wether to use the legacy method or the new method.

A few interfaces has been changed to use GenericBuild instead of `buildNumber`. This allows for some reduction in complexity for a few of the providers that don't use `buildNumber` internally, as we don't have to query the master for `buildId` multiple times.

If log_complete is false, we fall back to use the legacy log fetching. The reason is that the travis log endpoint actually returns the complete logs for a while before the flag turns to `true`. If we only wait for the flag, the delay between a finished build and the Spinnaker pipeline triggering can become very long. On the flip side, we can use the flag to avoid downloading the logs to check for builds where we know the log is completed. This gives us the best of both worlds (ideally Travis should fix this in the API, but I'm not holding my breath).

Also contains some spring cleaning of the TravisClient and the TravisService, it contained a lot of unused methods.

* Don't purge builds from the tracking cache before the log is ready

igor-web/src/main/groovy/com/netflix/spinnaker/igor/build/BuildController.groovy

@@ -74,7 +74,7 @@ class BuildController {
             return null
 
         try {
-            build.genericGitRevisions = buildService.getGenericGitRevisions(job, buildNumber)
+            build.genericGitRevisions = buildService.getGenericGitRevisions(job, build)
         } catch (Exception e) {
             log.error("could not get scm results for {} / {} / {}", kv("master", master), kv("job", job), kv("buildNumber", buildNumber), e)
         }
@@ -108,7 +108,7 @@ class BuildController {
         def buildService = getBuildService(master)
         GenericBuild build = jobStatus(buildService, master, job, buildNumber)
         if (build && buildService instanceof BuildProperties && artifactExtractor != null) {
-            build.properties = buildService.getBuildProperties(job, buildNumber, propertyFile)
+            build.properties = buildService.getBuildProperties(job, build, propertyFile)
             return artifactExtractor.extractArtifacts(build)
         }
         return Collections.emptyList()
@@ -240,7 +240,8 @@ class BuildController {
         def buildService = getBuildService(master)
         if (buildService instanceof BuildProperties) {
             BuildProperties buildProperties = (BuildProperties) buildService
-            return buildProperties.getBuildProperties(job, buildNumber, fileName)
+          def genericBuild = buildService.getGenericBuild(job, buildNumber)
+          return buildProperties.getBuildProperties(job, genericBuild, fileName)
         }
         return Collections.emptyMap()
     }

igor-web/src/main/groovy/com/netflix/spinnaker/igor/build/model/GenericGitRevision.java

@@ -5,10 +5,12 @@
 import lombok.Builder;
 import lombok.EqualsAndHashCode;
 import lombok.Getter;
+import lombok.experimental.Wither;
 
 @Getter
 @EqualsAndHashCode(of = "sha1")
 @Builder
+@Wither
 @JsonInclude(JsonInclude.Include.NON_NULL)
 public class GenericGitRevision {
   private String name;

igor-web/src/main/groovy/com/netflix/spinnaker/igor/gitlabci/service/GitlabCiService.java

@@ -63,7 +63,7 @@ public BuildServiceProvider getBuildServiceProvider() {
   }
 
   @Override
-  public List<GenericGitRevision> getGenericGitRevisions(String job, int buildNumber) {
+  public List<GenericGitRevision> getGenericGitRevisions(String job, GenericBuild build) {
     throw new UnsupportedOperationException();
   }
 

igor-web/src/main/groovy/com/netflix/spinnaker/igor/jenkins/service/JenkinsService.java

@@ -254,19 +254,18 @@ public JobConfig getJobConfig(String jobName) {
   }
 
   @Override
-  @SuppressWarnings("unchecked")
-  public Map<String, Object> getBuildProperties(String job, int buildNumber, String fileName) {
+  public Map<String, Object> getBuildProperties(String job, GenericBuild build, String fileName) {
     if (StringUtils.isEmpty(fileName)) {
       return new HashMap<>();
     }
     Map<String, Object> map = new HashMap<>();
     try {
-      String path = getArtifactPathFromBuild(job, buildNumber, fileName);
+      String path = getArtifactPathFromBuild(job, build.getNumber(), fileName);
       try (InputStream propertyStream =
-          this.getPropertyFile(job, buildNumber, path).getBody().in()) {
+          this.getPropertyFile(job, build.getNumber(), path).getBody().in()) {
         if (fileName.endsWith(".yml") || fileName.endsWith(".yaml")) {
           Yaml yml = new Yaml(new SafeConstructor());
-          map = (Map<String, Object>) yml.load(propertyStream);
+          map = yml.load(propertyStream);
         } else if (fileName.endsWith(".json")) {
           map = objectMapper.readValue(propertyStream, new TypeReference<Map<String, Object>>() {});
         } else {
@@ -331,8 +330,8 @@ public BuildServiceProvider getBuildServiceProvider() {
   }
 
   @Override
-  public List<GenericGitRevision> getGenericGitRevisions(String job, int buildNumber) {
-    ScmDetails scmDetails = getGitDetails(job, buildNumber);
+  public List<GenericGitRevision> getGenericGitRevisions(String job, GenericBuild build) {
+    ScmDetails scmDetails = getGitDetails(job, build.getNumber());
     return scmDetails.genericGitRevisions();
   }
 }

igor-web/src/main/groovy/com/netflix/spinnaker/igor/service/BuildOperations.java

@@ -31,10 +31,10 @@ public interface BuildOperations extends BuildService {
    * Get a list of the Spinnaker representation of the Git commits relevant for the given build
    *
    * @param job The name of the job
-   * @param buildNumber The build number
+   * @param build The build
    * @return A list of git revisions relevant for the build
    */
-  List<GenericGitRevision> getGenericGitRevisions(String job, int buildNumber);
+  List<GenericGitRevision> getGenericGitRevisions(String job, GenericBuild build);
 
   /**
    * Return all information of a given build

igor-web/src/main/groovy/com/netflix/spinnaker/igor/service/BuildProperties.java

@@ -16,6 +16,7 @@
 
 package com.netflix.spinnaker.igor.service;
 
+import com.netflix.spinnaker.igor.build.model.GenericBuild;
 import java.util.Map;
 import javax.annotation.Nullable;
 
@@ -29,10 +30,10 @@ public interface BuildProperties {
    * Get build properties defined in the given build
    *
    * @param job The name of the job
-   * @param buildNumber The build number
+   * @param build The build
    * @param fileName The file name containing the properties. Not all providers require this
    *     parameter.
    * @return A map of properties
    */
-  Map<String, ?> getBuildProperties(String job, int buildNumber, @Nullable String fileName);
+  Map<String, ?> getBuildProperties(String job, GenericBuild build, @Nullable String fileName);
 }

igor-web/src/main/groovy/com/netflix/spinnaker/igor/wercker/WerckerService.groovy

@@ -80,7 +80,7 @@ class WerckerService implements BuildOperations {
     }
 
     @Override
-    List<GenericGitRevision> getGenericGitRevisions(final String job, final int buildNumber) {
+    List<GenericGitRevision> getGenericGitRevisions(final String job, final GenericBuild build) {
         return null
     }
 

igor-web/src/main/java/com/netflix/spinnaker/igor/concourse/service/ConcourseService.java

@@ -113,14 +113,12 @@ public Collection<Job> getJobs() {
   }
 
   @Override
-  public List<GenericGitRevision> getGenericGitRevisions(String jobPath, int buildNumber) {
-    GenericBuild build = getGenericBuild(jobPath, buildNumber);
+  public List<GenericGitRevision> getGenericGitRevisions(String jobPath, GenericBuild build) {
     return build == null ? emptyList() : build.getGenericGitRevisions();
   }
 
   @Override
-  public Map<String, ?> getBuildProperties(String jobPath, int buildNumber, String fileName) {
-    GenericBuild build = getGenericBuild(jobPath, buildNumber);
+  public Map<String, ?> getBuildProperties(String jobPath, GenericBuild build, String fileName) {
     return build == null ? emptyMap() : build.getProperties();
   }
 

igor-web/src/main/java/com/netflix/spinnaker/igor/config/TravisConfig.java

@@ -24,6 +24,7 @@
 import com.netflix.spinnaker.igor.service.BuildServices;
 import com.netflix.spinnaker.igor.travis.TravisCache;
 import com.netflix.spinnaker.igor.travis.client.TravisClient;
+import com.netflix.spinnaker.igor.travis.client.model.v3.Root;
 import com.netflix.spinnaker.igor.travis.service.TravisService;
 import com.squareup.okhttp.OkHttpClient;
 import java.util.ArrayList;
@@ -34,8 +35,7 @@
 import java.util.function.Function;
 import java.util.stream.Collectors;
 import javax.validation.Valid;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
+import lombok.extern.slf4j.Slf4j;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
 import org.springframework.boot.context.properties.EnableConfigurationProperties;
 import org.springframework.context.annotation.Bean;
@@ -53,8 +53,8 @@
 @Configuration
 @ConditionalOnProperty("travis.enabled")
 @EnableConfigurationProperties(com.netflix.spinnaker.igor.config.TravisProperties.class)
+@Slf4j
 public class TravisConfig {
-  private Logger log = LoggerFactory.getLogger(getClass());
 
   @Bean
   public Map<String, TravisService> travisMasters(
@@ -81,6 +81,18 @@ public Map<String, TravisService> travisMasters(
                               host.getAddress(),
                               igorConfigurationProperties.getClient().getTimeout(),
                               objectMapper);
+
+                      boolean useLegacyLogFetching = false;
+                      try {
+                        Root root = client.getRoot();
+                        useLegacyLogFetching = !root.hasLogCompleteAttribute();
+                        if (useLegacyLogFetching) {
+                          log.info(
+                              "It seems Travis Enterprise is older than version 2.2.9. Will use legacy log fetching.");
+                        }
+                      } catch (Exception e) {
+                        log.warn("Could not query Travis API to check API compability", e);
+                      }
                       return travisService(
                           travisName,
                           host.getBaseUrl(),
@@ -89,9 +101,10 @@ public Map<String, TravisService> travisMasters(
                           client,
                           travisCache,
                           artifactDecorator,
-                          (travisProperties == null ? null : travisProperties.getRegexes()),
+                          travisProperties.getRegexes(),
                           travisProperties.getBuildMessageKey(),
-                          host.getPermissions().build());
+                          host.getPermissions().build(),
+                          useLegacyLogFetching);
                     })
                 .collect(Collectors.toMap(TravisService::getGroupKey, Function.identity()));
 
@@ -109,7 +122,8 @@ private static TravisService travisService(
       Optional<ArtifactDecorator> artifactDecorator,
       Collection<String> artifactRexeges,
       String buildMessageKey,
-      Permissions permissions) {
+      Permissions permissions,
+      boolean legacyLogFetching) {
     return new TravisService(
         travisHostId,
         baseUrl,
@@ -120,7 +134,8 @@ private static TravisService travisService(
         artifactDecorator,
         artifactRexeges,
         buildMessageKey,
-        permissions);
+        permissions,
+        legacyLogFetching);
   }
 
   public static TravisClient travisClient(String address, int timeout, ObjectMapper objectMapper) {

igor-web/src/main/java/com/netflix/spinnaker/igor/travis/TravisBuildMonitor.java

@@ -30,19 +30,21 @@
 import com.netflix.spinnaker.igor.history.model.GenericBuildContent;
 import com.netflix.spinnaker.igor.history.model.GenericBuildEvent;
 import com.netflix.spinnaker.igor.model.BuildServiceProvider;
-import com.netflix.spinnaker.igor.polling.*;
+import com.netflix.spinnaker.igor.polling.CommonPollingMonitor;
+import com.netflix.spinnaker.igor.polling.DeltaItem;
+import com.netflix.spinnaker.igor.polling.LockService;
+import com.netflix.spinnaker.igor.polling.PollContext;
+import com.netflix.spinnaker.igor.polling.PollingDelta;
 import com.netflix.spinnaker.igor.service.BuildServices;
 import com.netflix.spinnaker.igor.travis.client.model.Repo;
 import com.netflix.spinnaker.igor.travis.client.model.v3.TravisBuildState;
 import com.netflix.spinnaker.igor.travis.client.model.v3.V3Build;
-import com.netflix.spinnaker.igor.travis.client.model.v3.V3Job;
 import com.netflix.spinnaker.igor.travis.service.TravisBuildConverter;
 import com.netflix.spinnaker.igor.travis.service.TravisService;
 import com.netflix.spinnaker.security.AuthenticatedRequest;
 import java.time.Instant;
 import java.time.temporal.ChronoUnit;
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -212,21 +214,15 @@ private List<BuildDelta> processBuilds(
     List<BuildDelta> results = new ArrayList<>();
     builds.stream()
         .map(build -> setTracking(build, master))
-        .filter(filterNewBuildsPredicate())
+        .filter(filterNewBuildsPredicate(master))
         .forEach(
             build -> {
               String branchedRepoSlug = build.branchedRepoSlug();
               int cachedBuild =
                   buildCache.getLastBuild(master, branchedRepoSlug, build.getState().isRunning());
               GenericBuild genericBuild =
                   TravisBuildConverter.genericBuild(build, travisService.getBaseUrl());
-              List<Integer> jobIds =
-                  build.getJobs() != null
-                      ? build.getJobs().stream().map(V3Job::getId).collect(Collectors.toList())
-                      : Collections.emptyList();
-              if (build.getNumber() > cachedBuild
-                  && !build.spinnakerTriggered()
-                  && travisService.isLogReady(jobIds)) {
+              if (build.getNumber() > cachedBuild && !build.spinnakerTriggered()) {
                 BuildDelta delta =
                     new BuildDelta()
                         .setBranchedRepoSlug(branchedRepoSlug)
@@ -248,7 +244,7 @@ private List<BuildDelta> processBuilds(
   }
 
   private V3Build setTracking(V3Build build, String master) {
-    if (!filterNewBuildsPredicate().test(build)) {
+    if (!filterNewBuildsPredicate(master).test(build)) {
       buildCache.setTracking(
           master, build.getRepository().getSlug(), build.getId(), getPollInterval() * 5);
       log.debug("({}) tracking set up for {}", kv("master", master), build.toString());
@@ -264,10 +260,7 @@ private void sendEventForBuild(
     if (!buildDelta.getBuild().spinnakerTriggered()) {
       if (echoService.isPresent()) {
         log.info(
-            "({}) pushing event for :"
-                + branchedSlug
-                + ":"
-                + String.valueOf(buildDelta.getBuild().getNumber()),
+            "({}) pushing event for :" + branchedSlug + ":" + buildDelta.getBuild().getNumber(),
             kv("master", master));
 
         GenericProject project = new GenericProject(branchedSlug, buildDelta.getGenericBuild());
@@ -334,16 +327,18 @@ private int buildCacheJobTTLSeconds() {
     return (int) TimeUnit.DAYS.toSeconds(travisProperties.getCachedJobTTLDays());
   }
 
-  private Predicate<V3Build> filterNewBuildsPredicate() {
+  private Predicate<V3Build> filterNewBuildsPredicate(String master) {
+    final TravisService travisService = (TravisService) buildServices.getService(master);
     /*
     NewBuildGracePeriodSeconds is here because the travis API needs some time in order to fully represent the build in
     the api. This can be overridden by travis.newBuildGracePeriod.
     */
     Instant threshold =
         Instant.now().minus(travisProperties.getNewBuildGracePeriodSeconds(), ChronoUnit.SECONDS);
     return build ->
-        !build.getState().isRunning()
-            || (build.getFinishedAt() != null && build.getFinishedAt().isBefore(threshold));
+        (!build.getState().isRunning()
+                || (build.getFinishedAt() != null && build.getFinishedAt().isBefore(threshold)))
+            && travisService.isLogReady(build);
   }
 
   private List<Repo> filterOutOldBuilds(List<Repo> repos) {