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/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 Optional<String> extension;
+    private Optional<String> dateformat;
+    private Optional<SingularityExecutorLogrotateFrequency> logrotateFrequencyOverride;
 
     @JsonCreator
     public static SingularityExecutorLogrotateAdditionalFile fromString(String value) {
-        return new SingularityExecutorLogrotateAdditionalFile(value, Optional.<String>absent(), Optional.<String>absent());
+        return new SingularityExecutorLogrotateAdditionalFile(value, null, null, null);
     }
 
     @JsonCreator
     public SingularityExecutorLogrotateAdditionalFile(@JsonProperty("filename") String filename,
-        @JsonProperty("extension") Optional<String> extension,
-        @JsonProperty("dateformat") Optional<String> dateformat) {
+        @JsonProperty("extension") String extension,
+        @JsonProperty("dateformat") String dateformat,
+        @JsonProperty("logrotateFrequencyOverride") SingularityExecutorLogrotateFrequency logrotateFrequencyOverride) {
         this.filename = filename;
-        this.extension = extension;
-        this.dateformat = dateformat;
+        this.extension = Optional.fromNullable(extension);
+        this.dateformat = Optional.fromNullable(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/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 Optional.of(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/LogrotateTemplateContext.java

@@ -88,7 +88,8 @@ public List<LogrotateAdditionalFile> getExtrasFiles() {
         new LogrotateAdditionalFile(
           taskDefinition.getTaskDirectoryPath().resolve(additionalFile.getFilename()).toString(),
           additionalFile.getExtension().or(Strings.emptyToNull(Files.getFileExtension(additionalFile.getFilename()))),
-          dateformat
+          dateformat,
+            additionalFile.getLogrotateFrequencyOverride()
       ));
     }
 

SingularityExecutor/src/main/resources/logrotate.conf.hbs

@@ -36,6 +36,7 @@ notifempty
 
 {{#if extrasFiles}}
 {{#each extrasFiles}}{{{filename}}} {
+  {{{ logrotateFrequencyOverride }}}
   dateformat -{{{ dateformat }}}{{#if extension}}.{{{ extension}}}{{/if}}
   missingok
 {{#if useFileAttributes}}

SingularityExecutor/src/test/java/com/hubspot/singularity/executor/config/SingularityExecutorConfigurationTest.java

@@ -1,18 +1,26 @@
 package com.hubspot.singularity.executor.config;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
 
 import java.io.File;
 import java.lang.reflect.Field;
+import java.util.ArrayList;
+import java.util.List;
 
 import javax.validation.Validator;
 
 import org.junit.Test;
 
 import com.fasterxml.jackson.databind.ObjectMapper;
+import com.github.jknack.handlebars.Handlebars;
+import com.github.jknack.handlebars.Template;
 import com.google.common.base.Optional;
 import com.google.common.base.Throwables;
+import com.hubspot.singularity.executor.SingularityExecutorLogrotateFrequency;
+import com.hubspot.singularity.executor.models.LogrotateAdditionalFile;
+import com.hubspot.singularity.executor.models.LogrotateTemplateContext;
 import com.hubspot.singularity.runner.base.config.SingularityRunnerBaseModule;
 import com.hubspot.singularity.runner.base.config.SingularityRunnerConfigurationProvider;
 
@@ -62,4 +70,34 @@ private SingularityExecutorConfiguration loadConfig(String file)  {
         }
     }
 
+    @Test
+    public void itOverridesGlobalLogrotateFrequency() throws Exception {
+        Handlebars handlebars = new Handlebars();
+        Template template = handlebars.compile(SingularityExecutorModule.LOGROTATE_TEMPLATE);
+
+        LogrotateTemplateContext context = mock(LogrotateTemplateContext.class);
+
+        doReturn(SingularityExecutorLogrotateFrequency.WEEKLY.getLogrotateValue()).when(context).getLogrotateFrequency();
+
+        List<LogrotateAdditionalFile> testExtraFiles = new ArrayList<>();
+        testExtraFiles.add(new LogrotateAdditionalFile("/tmp/testfile.txt", "txt", "%Y%m%d",
+            Optional.of(SingularityExecutorLogrotateFrequency.DAILY)));
+
+        doReturn(testExtraFiles).when(context).getExtrasFiles();
+
+        // This sample output template, when copied into a staged Mesos slave and run with `logrotate -d <configFileName>`
+        // confirms that a testfile.txt at the /tmp/testfile.txt will be cycled daily instead of weekly
+        String text = template.apply(context);
+
+        // Assert that our config has both weekly and daily scopes, and that daily occurs second (thus overrides weekly
+        // in the /tmp/testfile.txt YAML object).
+
+        // Admittedly, YAML serialization would be better, but given this code doesn't actually test much without
+        // a binary of `logrotate` to run against, it's not a big deal.
+        assertThat(text.contains("weekly")).isTrue();
+        assertThat(text.contains("daily")).isTrue();
+        assertThat(text.indexOf("daily")).isGreaterThan(text.indexOf("weekly"));
+
+    }
+
 }