Updated logrotate frequency to allow for an override

SingularityBase/src/main/java/com/hubspot/singularity/executor/SingularityExecutorLogrotateFrequency.java

@@ -4,9 +4,9 @@
 
 public enum SingularityExecutorLogrotateFrequency {
     HOURLY("daily", Optional.of("0 * * * *")),  // we have to use the "daily" frequency because not all versions of logrotate support "hourly"
-    DAILY("daily", Optional.<String>absent()),
-    WEEKLY("weekly", Optional.<String>absent()),
-    MONTHLY("monthly", Optional.<String>absent());
+    DAILY("daily", Optional.absent()),
+    WEEKLY("weekly", Optional.absent()),
+    MONTHLY("monthly", Optional.absent());
 
     private final String logrotateValue;
     private final Optional<String> cronSchedule;

SingularityExecutor/src/main/java/com/hubspot/singularity/executor/TemplateManager.java

@@ -25,19 +25,22 @@ public class TemplateManager {
   private final Template runnerTemplate;
   private final Template environmentTemplate;
   private final Template logrotateTemplate;
+  private final Template logrotateHourlyTemplate;
   private final Template logrotateCronTemplate;
   private final Template dockerTemplate;
 
   @Inject
   public TemplateManager(@Named(SingularityExecutorModule.RUNNER_TEMPLATE) Template runnerTemplate,
                          @Named(SingularityExecutorModule.ENVIRONMENT_TEMPLATE) Template environmentTemplate,
                          @Named(SingularityExecutorModule.LOGROTATE_TEMPLATE) Template logrotateTemplate,
+                         @Named(SingularityExecutorModule.LOGROTATE_HOURLY_TEMPLATE) Template logrotateHourlyTemplate,
                          @Named(SingularityExecutorModule.LOGROTATE_CRON_TEMPLATE) Template logrotateCronTemplate,
                          @Named(SingularityExecutorModule.DOCKER_TEMPLATE) Template dockerTemplate
                          ) {
     this.runnerTemplate = runnerTemplate;
     this.environmentTemplate = environmentTemplate;
     this.logrotateTemplate = logrotateTemplate;
+    this.logrotateHourlyTemplate = logrotateHourlyTemplate;
     this.logrotateCronTemplate = logrotateCronTemplate;
     this.dockerTemplate = dockerTemplate;
   }
@@ -54,6 +57,10 @@ public void writeLogrotateFile(Path destination, LogrotateTemplateContext logRot
     writeTemplate(destination, logrotateTemplate, logRotateContext);
   }
 
+  public void writeHourlyLogrotateFile(Path destination, LogrotateTemplateContext logRotateContext) {
+    writeTemplate(destination, logrotateHourlyTemplate, logRotateContext);
+  }
+
   public boolean writeCronEntryForLogrotate(Path destination, LogrotateCronTemplateContext logrotateCronTemplateContext) {
     writeTemplate(destination, logrotateCronTemplate, logrotateCronTemplateContext);
     final File destinationFile = destination.toFile();

SingularityExecutor/src/main/java/com/hubspot/singularity/executor/config/SingularityExecutorConfiguration.java

@@ -99,6 +99,10 @@ public class SingularityExecutorConfiguration extends BaseRunnerConfiguration {
   @JsonProperty
   private String logrotateConfDirectory = "/etc/logrotate.d";
 
+  @NotEmpty
+  @JsonProperty
+  private String logrotateHourlyConfDirectory = "/etc/logrotate.d/hourly";
+
   @NotEmpty
   @JsonProperty
   private String logrotateToDirectory = "logs";
@@ -328,6 +332,10 @@ public String getLogrotateConfDirectory() {
     return logrotateConfDirectory;
   }
 
+  public String getLogrotateHourlyConfDirectory() {
+    return logrotateHourlyConfDirectory;
+  }
+
   public String getLogrotateToDirectory() {
     return logrotateToDirectory;
   }

SingularityExecutor/src/main/java/com/hubspot/singularity/executor/config/SingularityExecutorLogrotateAdditionalFile.java

@@ -3,24 +3,28 @@
 import com.fasterxml.jackson.annotation.JsonCreator;
 import com.fasterxml.jackson.annotation.JsonProperty;
 import com.google.common.base.Optional;
+import com.hubspot.singularity.executor.SingularityExecutorLogrotateFrequency;
 
 public class SingularityExecutorLogrotateAdditionalFile {
     private final String filename;
     private final Optional<String> extension;
     private final Optional<String> dateformat;
+    private final Optional<SingularityExecutorLogrotateFrequency> logrotateFrequencyOverride;
 
     @JsonCreator
     public static SingularityExecutorLogrotateAdditionalFile fromString(String value) {
-        return new SingularityExecutorLogrotateAdditionalFile(value, Optional.<String>absent(), Optional.<String>absent());
+        return new SingularityExecutorLogrotateAdditionalFile(value, Optional.absent(), Optional.absent(), null);
     }
 
     @JsonCreator
     public SingularityExecutorLogrotateAdditionalFile(@JsonProperty("filename") String filename,
-        @JsonProperty("extension") Optional<String> extension,
-        @JsonProperty("dateformat") Optional<String> dateformat) {
+            @JsonProperty("extension") Optional<String> extension,
+            @JsonProperty("dateformat") Optional<String> dateformat,
+            @JsonProperty("logrotateFrequencyOverride") SingularityExecutorLogrotateFrequency logrotateFrequencyOverride) {
         this.filename = filename;
         this.extension = extension;
         this.dateformat = dateformat;
+        this.logrotateFrequencyOverride = Optional.fromNullable(logrotateFrequencyOverride);
     }
 
     public String getFilename() {
@@ -34,4 +38,9 @@ public Optional<String> getExtension() {
     public Optional<String> getDateformat() {
         return dateformat;
     }
+
+    public Optional<SingularityExecutorLogrotateFrequency> getLogrotateFrequencyOverride() {
+        return logrotateFrequencyOverride;
+    }
+
 }

SingularityExecutor/src/main/java/com/hubspot/singularity/executor/config/SingularityExecutorModule.java

@@ -30,6 +30,7 @@ public class SingularityExecutorModule extends AbstractModule {
   public static final String RUNNER_TEMPLATE = "runner.sh";
   public static final String ENVIRONMENT_TEMPLATE = "deploy.env";
   public static final String LOGROTATE_TEMPLATE = "logrotate.conf";
+  public static final String LOGROTATE_HOURLY_TEMPLATE = "logrotate.hourly.conf";
   public static final String LOGROTATE_CRON_TEMPLATE = "logrotate.cron";
   public static final String DOCKER_TEMPLATE = "docker.sh";
   public static final String LOCAL_DOWNLOAD_HTTP_CLIENT = "SingularityExecutorModule.local.download.http.client";
@@ -72,6 +73,13 @@ public Template providesLogrotateTemplate(Handlebars handlebars) throws IOExcept
     return handlebars.compile(LOGROTATE_TEMPLATE);
   }
 
+  @Provides
+  @Singleton
+  @Named(LOGROTATE_HOURLY_TEMPLATE)
+  public Template providesLogrotateHourlyTemplate(Handlebars handlebars) throws IOException {
+    return handlebars.compile(LOGROTATE_HOURLY_TEMPLATE);
+  }
+
   @Provides
   @Singleton
   @Named(LOGROTATE_CRON_TEMPLATE)

SingularityExecutor/src/main/java/com/hubspot/singularity/executor/models/LogrotateAdditionalFile.java

@@ -1,14 +1,19 @@
 package com.hubspot.singularity.executor.models;
 
+import com.google.common.base.Optional;
+import com.hubspot.singularity.executor.SingularityExecutorLogrotateFrequency;
+
 public class LogrotateAdditionalFile {
     private final String filename;
     private final String extension;
     private final String dateformat;
+    private final Optional<SingularityExecutorLogrotateFrequency> logrotateFrequencyOverride;
 
-    public LogrotateAdditionalFile(String filename, String extension, String dateformat) {
+    public LogrotateAdditionalFile(String filename, String extension, String dateformat, Optional<SingularityExecutorLogrotateFrequency> logrotateFrequencyOverride) {
         this.filename = filename;
         this.extension = extension;
         this.dateformat = dateformat;
+        this.logrotateFrequencyOverride = logrotateFrequencyOverride;
     }
 
     public String getFilename() {
@@ -23,12 +28,18 @@ public String getDateformat() {
         return dateformat;
     }
 
+    public String getLogrotateFrequencyOverride() {
+        return logrotateFrequencyOverride.isPresent() ?
+            logrotateFrequencyOverride.get().getLogrotateValue() : "";
+    }
+
     @Override
     public String toString() {
         return "LogrotateAdditionalFile{" +
             "filename='" + filename + '\'' +
             ", extension='" + extension + '\'' +
             ", dateformat='" + dateformat + '\'' +
+            ", frequency='" + logrotateFrequencyOverride + '\'' +
             '}';
     }
 }

SingularityExecutor/src/main/java/com/hubspot/singularity/executor/models/LogrotateCronTemplateContext.java

@@ -15,7 +15,7 @@ public class LogrotateCronTemplateContext {
     public LogrotateCronTemplateContext(SingularityExecutorConfiguration configuration, SingularityExecutorTaskDefinition taskDefinition, SingularityExecutorLogrotateFrequency logrotateFrequency) {
         this.logrotateCommand = configuration.getLogrotateCommand();
         this.logrotateStateFile = taskDefinition.getLogrotateStateFilePath().toString();
-        this.logrotateConfig = Paths.get(configuration.getLogrotateConfDirectory()).resolve(taskDefinition.getTaskId()).toString();
+        this.logrotateConfig = Paths.get(configuration.getLogrotateHourlyConfDirectory()).resolve(taskDefinition.getTaskId()).toString();
         this.cronSchedule = logrotateFrequency.getCronSchedule().get();
     }
 

SingularityExecutor/src/main/java/com/hubspot/singularity/executor/models/LogrotateTemplateContext.java

@@ -2,10 +2,12 @@
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.stream.Collectors;
 
 import com.google.common.base.Optional;
 import com.google.common.base.Strings;
 import com.google.common.io.Files;
+import com.hubspot.singularity.executor.SingularityExecutorLogrotateFrequency;
 import com.hubspot.singularity.executor.config.SingularityExecutorConfiguration;
 import com.hubspot.singularity.executor.config.SingularityExecutorLogrotateAdditionalFile;
 import com.hubspot.singularity.executor.task.SingularityExecutorTaskDefinition;
@@ -69,10 +71,30 @@ public String getCompressExt() {
   }
 
   /**
-   * Extra files for logrotate to rotate. If these do not exist logrotate will continue without error.
+   * Extra files for logrotate to rotate (non-hourly). If these do not exist logrotate will continue without error.
    * @return filenames to rotate.
    */
   public List<LogrotateAdditionalFile> getExtrasFiles() {
+    return getAllExtraFiles().stream()
+        .filter(p -> !p.getLogrotateFrequencyOverride().equals(SingularityExecutorLogrotateFrequency.HOURLY.getLogrotateValue()))
+        .collect(Collectors.toList());
+  }
+
+  /**
+   * Extra files for logrotate to rotate hourly. If these do not exist logrotate will continue without error.
+   * @return filenames to rotate.
+   */
+  public List<LogrotateAdditionalFile> getExtrasFilesHourly() {
+    return getAllExtraFiles().stream()
+        .filter(p -> p.getLogrotateFrequencyOverride().equals(SingularityExecutorLogrotateFrequency.HOURLY.getLogrotateValue()))
+        .collect(Collectors.toList());
+  }
+
+  public boolean isGlobalLogrotateHourly() {
+    return configuration.getLogrotateFrequency().getLogrotateValue().equals(SingularityExecutorLogrotateFrequency.HOURLY.getLogrotateValue());
+  }
+
+  private List<LogrotateAdditionalFile> getAllExtraFiles() {
     final List<SingularityExecutorLogrotateAdditionalFile> original = configuration.getLogrotateAdditionalFiles();
     final List<LogrotateAdditionalFile> transformed = new ArrayList<>(original.size());
 
@@ -85,11 +107,12 @@ public List<LogrotateAdditionalFile> getExtrasFiles() {
       }
 
       transformed.add(
-        new LogrotateAdditionalFile(
-          taskDefinition.getTaskDirectoryPath().resolve(additionalFile.getFilename()).toString(),
-          additionalFile.getExtension().or(Strings.emptyToNull(Files.getFileExtension(additionalFile.getFilename()))),
-          dateformat
-      ));
+          new LogrotateAdditionalFile(
+              taskDefinition.getTaskDirectoryPath().resolve(additionalFile.getFilename()).toString(),
+              additionalFile.getExtension().or(Strings.emptyToNull(Files.getFileExtension(additionalFile.getFilename()))),
+              dateformat,
+              additionalFile.getLogrotateFrequencyOverride()
+          ));
     }
 
     return transformed;

SingularityExecutor/src/main/java/com/hubspot/singularity/executor/task/SingularityExecutorTaskLogManager.java

@@ -1,5 +1,6 @@
 package com.hubspot.singularity.executor.task;
 
+import java.io.File;
 import java.lang.ProcessBuilder.Redirect;
 import java.nio.file.FileAlreadyExistsException;
 import java.nio.file.Files;
@@ -15,11 +16,12 @@
 import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableList;
 import com.hubspot.singularity.SingularityS3FormatHelper;
+import com.hubspot.singularity.SingularityS3UploaderFile;
 import com.hubspot.singularity.SingularityTaskId;
 import com.hubspot.singularity.executor.SingularityExecutorLogrotateFrequency;
 import com.hubspot.singularity.executor.TemplateManager;
 import com.hubspot.singularity.executor.config.SingularityExecutorConfiguration;
-import com.hubspot.singularity.SingularityS3UploaderFile;
+import com.hubspot.singularity.executor.config.SingularityExecutorLogrotateAdditionalFile;
 import com.hubspot.singularity.executor.models.LogrotateCronTemplateContext;
 import com.hubspot.singularity.executor.models.LogrotateTemplateContext;
 import com.hubspot.singularity.runner.base.configuration.SingularityRunnerBaseConfiguration;
@@ -93,15 +95,50 @@ private boolean writeS3MetadataFileForRotatedFiles(boolean finished) {
   }
 
   private void writeLogrotateFile() {
-    log.info("Writing logrotate configuration file to {}", getLogrotateConfPath());
+    log.info("Writing non-hourly logrotate configuration file to {}", getLogrotateConfPath());
     templateManager.writeLogrotateFile(getLogrotateConfPath(), new LogrotateTemplateContext(configuration, taskDefinition));
 
-    if (logrotateFrequency.getCronSchedule().isPresent()) {
-      log.info("Writing logrotate cron entry with schedule '{}' to {}", logrotateFrequency.getCronSchedule().get(), getLogrotateCronPath());
-      templateManager.writeCronEntryForLogrotate(getLogrotateCronPath(), new LogrotateCronTemplateContext(configuration, taskDefinition, logrotateFrequency));
+    // Get the frequency and cron schedule for an additional file with an HOURLY schedule, if there is any
+    Optional<SingularityExecutorLogrotateFrequency> additionalFileFrequency = getAdditionalHourlyFileFrequency();
+
+    // if any additional file or the global setting has an hourly rotation, write a separate rotate config and force rotate using a cron schedule
+    if (additionalFileFrequency.isPresent() && additionalFileFrequency.get().getCronSchedule().isPresent() || logrotateFrequency.getCronSchedule().isPresent()) {
+      File hourlyLogrotateDir = new File(configuration.getLogrotateHourlyConfDirectory());
+      if (!hourlyLogrotateDir.exists()) {
+        if (!hourlyLogrotateDir.mkdir()) {
+          log.warn("Could not create hourly logrotate directory at {}", configuration.getLogrotateHourlyConfDirectory());
+        }
+      }
+      log.info("Writing hourly logrotate configuration file to {}", getLogrotateHourlyConfPath());
+      templateManager.writeHourlyLogrotateFile(getLogrotateHourlyConfPath(), new LogrotateTemplateContext(configuration, taskDefinition));
+
+      SingularityExecutorLogrotateFrequency freq = additionalFileFrequency.isPresent() && additionalFileFrequency.get().getCronSchedule().isPresent() ?
+          additionalFileFrequency.get() : logrotateFrequency;
+
+      String cronScheduleString = freq.getCronSchedule().isPresent() ? freq.getCronSchedule().get() : "Error in cron schedule"; // This should never evaluate to false
+
+      log.info("Writing logrotate cron entry with schedule '{}' to {}", cronScheduleString, getLogrotateCronPath());
+      templateManager.writeCronEntryForLogrotate(getLogrotateCronPath(), new LogrotateCronTemplateContext(configuration, taskDefinition, freq));
     }
   }
 
+  /**
+   *
+   * @return Frequency (and contained cron schedule) of the hourly rotation additional file
+   */
+  private Optional<SingularityExecutorLogrotateFrequency> getAdditionalHourlyFileFrequency() {
+
+    for (SingularityExecutorLogrotateAdditionalFile file : configuration.getLogrotateAdditionalFiles()) {
+      if (file.getLogrotateFrequencyOverride().isPresent() &&
+          file.getLogrotateFrequencyOverride().get().equals(SingularityExecutorLogrotateFrequency.HOURLY) &&
+          file.getLogrotateFrequencyOverride().get().getCronSchedule().isPresent()) {
+
+        return Optional.of(file.getLogrotateFrequencyOverride().get());
+      }
+    }
+    return Optional.absent();
+  }
+
   @SuppressFBWarnings
   public boolean teardown() {
     boolean writeTailMetadataSuccess = writeTailMetadata(true);
@@ -130,7 +167,7 @@ public boolean teardown() {
 
       return writeTailMetadataSuccess && removeLogRotateFileSuccess && writeS3MetadataForLogrotatedFilesSuccess && writeS3MetadataForNonLogRotatedFileSuccess;
     } else {
-      return false;
+      return removeLogrotateFile();
     }
   }
 
@@ -162,17 +199,40 @@ private void copyLogTail() {
   public boolean removeLogrotateFile() {
     boolean deleted = false;
     try {
-      deleted = Files.deleteIfExists(getLogrotateConfPath());
-      if (logrotateFrequency.getCronSchedule().isPresent()) {
-        boolean cronDeleted = Files.deleteIfExists(getLogrotateCronPath());
-        deleted = deleted || cronDeleted;
+      if (Files.exists(getLogrotateConfPath())) {
+        deleted = Files.deleteIfExists(getLogrotateConfPath());
+        log.debug("Deleted {} : {}", getLogrotateConfPath(), deleted);
+      } else {
+        deleted = true;
+      }
+    } catch (Throwable t){
+      log.debug("Couldn't delete {}", getLogrotateConfPath(), t);
+      return false;
+    }
+
+    Optional<SingularityExecutorLogrotateFrequency> additionalFileFreq = getAdditionalHourlyFileFrequency();
+    try {
+      if ((additionalFileFreq.isPresent() && additionalFileFreq.get().getCronSchedule().isPresent()) || logrotateFrequency.getCronSchedule().isPresent()) {
+        boolean hourlyConfDeleted = !Files.exists(getLogrotateHourlyConfPath()) || Files.deleteIfExists(getLogrotateHourlyConfPath());
+        log.debug("Deleted {} : {}", getLogrotateHourlyConfPath(), deleted);
+        deleted = deleted && hourlyConfDeleted;
       }
     } catch (Throwable t) {
-      log.trace("Couldn't delete {}", getLogrotateConfPath(), t);
+      log.debug("Couldn't delete {}", getLogrotateHourlyConfPath(), t);
       return false;
     }
-    log.trace("Deleted {} : {}", getLogrotateConfPath(), deleted);
-    return true;
+
+    try {
+      if ((additionalFileFreq.isPresent() && additionalFileFreq.get().getCronSchedule().isPresent()) || logrotateFrequency.getCronSchedule().isPresent()) {
+        boolean cronDeleted = !Files.exists(getLogrotateCronPath()) || Files.deleteIfExists(getLogrotateCronPath());
+        log.debug("Deleted {} : {}", getLogrotateCronPath(), deleted);
+        deleted = deleted && cronDeleted;
+      }
+    } catch (Throwable t) {
+      log.debug("Couldn't delete {}", getLogrotateCronPath(), t);
+      return false;
+    }
+    return deleted;
   }
 
   public boolean manualLogrotate() {
@@ -247,6 +307,10 @@ public Path getLogrotateConfPath() {
     return Paths.get(configuration.getLogrotateConfDirectory()).resolve(taskDefinition.getTaskId());
   }
 
+  public Path getLogrotateHourlyConfPath() {
+    return Paths.get(configuration.getLogrotateHourlyConfDirectory()).resolve(taskDefinition.getTaskId());
+  }
+
   public Path getLogrotateCronPath() {
     return Paths.get(configuration.getCronDirectory()).resolve(taskDefinition.getTaskId() + ".logrotate");
   }