Correctly render cron entries for size-based logrotation.

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

@@ -35,36 +35,21 @@ public List<LogrotateForceConfig> getLogrotateForceConfigs() {
           String frequencySpecificLogrotateConfPath = frequencyWithLogrotateConfPath.getValue();
 
           return new LogrotateForceConfig(
-            configuration.getLogrotateCommand(),
             frequencySpecificLogrotateConfPath,
-            frequency.getCronSchedule().get(),
-            configuration.isIgnoreLogrotateOutput() ? "> /dev/null 2>&1" : ""
+            frequency.getCronSchedule().get()
           );
         }
       )
       .collect(Collectors.toList());
   }
 
   public static class LogrotateForceConfig {
-    private final String logrotateCommand;
     private final String logrotateForceConfigPath;
     private final String cronSchedule;
-    private final String outputRedirect;
 
-    public LogrotateForceConfig(
-      String logrotateCommand,
-      String logrotateForceConfigPath,
-      String cronSchedule,
-      String outputRedirect
-    ) {
-      this.logrotateCommand = logrotateCommand;
+    public LogrotateForceConfig(String logrotateForceConfigPath, String cronSchedule) {
       this.logrotateForceConfigPath = logrotateForceConfigPath;
       this.cronSchedule = cronSchedule;
-      this.outputRedirect = outputRedirect;
-    }
-
-    public String getLogrotateCommand() {
-      return logrotateCommand;
     }
 
     public String getLogrotateForceConfigPath() {
@@ -74,10 +59,6 @@ public String getLogrotateForceConfigPath() {
     public String getCronSchedule() {
       return cronSchedule;
     }
-
-    public String getOutputRedirect() {
-      return outputRedirect;
-    }
   }
 
   public String getLogrotateStateFile() {
@@ -87,4 +68,12 @@ public String getLogrotateStateFile() {
   public String getLogrotateSizeBasedConfig() {
     return logrotateSizeBasedConfig;
   }
+
+  public String getOutputRedirect() {
+    return configuration.isIgnoreLogrotateOutput() ? "> /dev/null 2>&1" : "";
+  }
+
+  public String getLogrotateCommand() {
+    return configuration.getLogrotateCommand();
+  }
 }

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

@@ -1,5 +1,5 @@
 {{#each logrotateForceConfigs}}
-{{{ cronSchedule }}} root {{{ logrotateCommand }}} -f -s {{{ logrotateStateFile }}} {{{ logrotateForceConfigPath }}} {{{ outputRedirect }}}
+{{{ cronSchedule }}} root {{{ ../logrotateCommand }}} -f -s {{{ ../logrotateStateFile }}} {{{ logrotateForceConfigPath }}} {{{ ../outputRedirect }}}
 {{/each}}
 
 {{#if logrotateSizeBasedConfig}}

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

@@ -216,23 +216,14 @@ public void itProperlyGeneratesCronConfigsForLogrotate() throws Exception {
 
     doReturn(
         ImmutableList.of(
+          new LogrotateForceConfig("/etc/logrotate.d/hourly/task.HOURLY", "0 * * * *"),
           new LogrotateForceConfig(
-            "logrotate",
-            "/etc/logrotate.d/hourly/task.HOURLY",
-            "0 * * * *",
-            "> /dev/null 2>&1"
-          ),
-          new LogrotateForceConfig(
-            "logrotate",
             "/etc/logrotate.d/hourly/task.EVERY_MINUTE",
-            "* * * * *",
-            "> /dev/null 2>&1"
+            "* * * * *"
           ),
           new LogrotateForceConfig(
-            "logrotate",
             "/etc/logrotate.d/hourly/task.EVERY_FIVE_MINUTES",
-            "*/5 * * * *",
-            "> /dev/null 2>&1"
+            "*/5 * * * *"
           )
         )
       )
@@ -243,6 +234,8 @@ public void itProperlyGeneratesCronConfigsForLogrotate() throws Exception {
     doReturn("/etc/logrotate.d/hourly/task.sizebased")
       .when(context)
       .getLogrotateSizeBasedConfig();
+    doReturn("logrotate").when(context).getLogrotateCommand();
+    doReturn("> /dev/null 2>&1").when(context).getOutputRedirect();
 
     String renderedConfig = cronConfigTemplate.apply(context);
     assertThat(renderedConfig)
@@ -258,6 +251,8 @@ public void itProperlyGeneratesCronConfigsForLogrotate() throws Exception {
         "*/5 * * * * root logrotate -f -s /statefile /etc/logrotate.d/hourly/task.EVERY_FIVE_MINUTES > /dev/null 2>&1"
       );
     assertThat(renderedConfig)
-      .contains("*/5 * * * * root  -s /statefile /etc/logrotate.d/hourly/task.sizebased");
+      .contains(
+        "*/5 * * * * root logrotate -s /statefile /etc/logrotate.d/hourly/task.sizebased > /dev/null 2>&1"
+      );
   }
 }