Renamed jobs variables not being picked up (#412)

* Fix bug where variables weren't being picked up by services

* Remove info from build as log files were going over 4 MB

* For real removing info this time from all the lines

genie-app/build.gradle

@@ -1,3 +1,7 @@
+//plugins {
+//    id "com.palantir.docker" version "0.9.1"
+//}
+
 apply plugin: "spring-boot"
 
 dependencies {

genie-core/src/main/java/com/netflix/genie/core/properties/JobsCleanupProperties.java

@@ -0,0 +1,34 @@
+/*
+ *
+ *  Copyright 2016 Netflix, Inc.
+ *
+ *     Licensed under the Apache License, Version 2.0 (the "License");
+ *     you may not use this file except in compliance with the License.
+ *     You may obtain a copy of the License at
+ *
+ *         http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *     Unless required by applicable law or agreed to in writing, software
+ *     distributed under the License is distributed on an "AS IS" BASIS,
+ *     WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *     See the License for the specific language governing permissions and
+ *     limitations under the License.
+ *
+ */
+package com.netflix.genie.core.properties;
+
+import lombok.Getter;
+import lombok.Setter;
+
+/**
+ * Properties related to cleanup for jobs.
+ *
+ * @author tgianos
+ * @since 3.0.0
+ */
+@Getter
+@Setter
+public class JobsCleanupProperties {
+    private boolean deleteArchiveFile = true;
+    private boolean deleteDependencies = true;
+}

genie-core/src/main/java/com/netflix/genie/core/properties/JobsProperties.java

@@ -31,6 +31,9 @@
 @Getter
 @Setter
 public class JobsProperties {
+    @NotNull
+    private JobsCleanupProperties cleanup = new JobsCleanupProperties();
+
     @NotNull
     private JobsForwardingProperties forwarding = new JobsForwardingProperties();
 

genie-core/src/test/java/com/netflix/genie/core/properties/JobsCleanupPropertiesUnitTests.java

@@ -0,0 +1,71 @@
+/*
+ *
+ *  Copyright 2016 Netflix, Inc.
+ *
+ *     Licensed under the Apache License, Version 2.0 (the "License");
+ *     you may not use this file except in compliance with the License.
+ *     You may obtain a copy of the License at
+ *
+ *         http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *     Unless required by applicable law or agreed to in writing, software
+ *     distributed under the License is distributed on an "AS IS" BASIS,
+ *     WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *     See the License for the specific language governing permissions and
+ *     limitations under the License.
+ *
+ */
+package com.netflix.genie.core.properties;
+
+import com.netflix.genie.test.categories.UnitTest;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+/**
+ * Unit tests for the cleanup properties.
+ *
+ * @author tgianos
+ * @since 3.0.0
+ */
+@Category(UnitTest.class)
+public class JobsCleanupPropertiesUnitTests {
+
+    private JobsCleanupProperties properties;
+
+    /**
+     * Setup for tests.
+     */
+    @Before
+    public void setup() {
+        this.properties = new JobsCleanupProperties();
+    }
+
+    /**
+     * Make sure properties are true by default.
+     */
+    @Test
+    public void canConstruct() {
+        Assert.assertTrue(this.properties.isDeleteArchiveFile());
+        Assert.assertTrue(this.properties.isDeleteDependencies());
+    }
+
+    /**
+     * Make sure can set whether to delete the archive file.
+     */
+    @Test
+    public void canSetDeleteArchiveFile() {
+        this.properties.setDeleteArchiveFile(false);
+        Assert.assertFalse(this.properties.isDeleteArchiveFile());
+    }
+
+    /**
+     * Make sure can set whether to delete the delete the dependencies.
+     */
+    @Test
+    public void canSetDeleteDependencies() {
+        this.properties.setDeleteDependencies(false);
+        Assert.assertFalse(this.properties.isDeleteDependencies());
+    }
+}

genie-web/src/main/java/com/netflix/genie/web/configs/ServicesConfig.java

@@ -222,7 +222,7 @@ public JobPersistenceService jobPersistenceService(
      * @param hostName         The name of the host this Genie node is running on.
      * @param jobSearchService The job search service to use to locate job information.
      * @param executor         The executor to use to run system processes.
-     * @param runAsUser        Whether jobs on this instance are run as the user or not
+     * @param jobsProperties   The jobs properties to use
      * @param eventPublisher   The application event publisher to use to publish system wide events
      * @return A job kill service instance.
      */
@@ -231,11 +231,16 @@ public JobKillService jobKillService(
         final String hostName,
         final JobSearchService jobSearchService,
         final Executor executor,
-        @Value("${genie.jobs.runAsUser.enabled:false}")
-        final boolean runAsUser,
+        final JobsProperties jobsProperties,
         final ApplicationEventPublisher eventPublisher
     ) {
-        return new LocalJobKillServiceImpl(hostName, jobSearchService, executor, runAsUser, eventPublisher);
+        return new LocalJobKillServiceImpl(
+            hostName,
+            jobSearchService,
+            executor,
+            jobsProperties.getUsers().isRunAsUserEnabled(),
+            eventPublisher
+        );
     }
 
     /**

genie-web/src/main/java/com/netflix/genie/web/controllers/GenieExceptionMapper.java

@@ -28,6 +28,7 @@
 import com.netflix.genie.core.util.MetricsConstants;
 import com.netflix.spectator.api.Counter;
 import com.netflix.spectator.api.Registry;
+import lombok.extern.slf4j.Slf4j;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.http.HttpStatus;
 import org.springframework.web.bind.annotation.ControllerAdvice;
@@ -44,6 +45,7 @@
  * @author tgianos
  * @since 3.0.0
  */
+@Slf4j
 @ControllerAdvice
 public class GenieExceptionMapper {
 
@@ -106,6 +108,7 @@ public void handleGenieException(
         } else {
             this.genieRate.increment();
         }
+        log.error(e.getLocalizedMessage(), e);
         response.sendError(e.getErrorCode(), e.getLocalizedMessage());
     }
 
@@ -131,6 +134,7 @@ public void handleConstraintViolation(
             }
         }
         this.constraintViolationRate.increment();
+        log.error(cve.getLocalizedMessage(), cve);
         response.sendError(HttpStatus.PRECONDITION_FAILED.value(), builder.toString());
     }
 }

genie-web/src/main/java/com/netflix/genie/web/tasks/job/JobCompletionService.java

@@ -31,6 +31,7 @@
 import com.netflix.genie.core.events.JobFinishedReason;
 import com.netflix.genie.core.jobs.JobConstants;
 import com.netflix.genie.core.jobs.JobDoneFile;
+import com.netflix.genie.core.properties.JobsProperties;
 import com.netflix.genie.core.services.JobPersistenceService;
 import com.netflix.genie.core.services.JobSearchService;
 import com.netflix.genie.core.services.MailService;
@@ -46,7 +47,6 @@
 import org.apache.commons.io.FileUtils;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.annotation.Qualifier;
-import org.springframework.beans.factory.annotation.Value;
 import org.springframework.core.io.Resource;
 import org.springframework.retry.support.RetryTemplate;
 import org.springframework.stereotype.Service;
@@ -82,7 +82,7 @@ public class JobCompletionService {
     private final Executor executor;
     private final boolean deleteArchiveFile;
     private final boolean deleteDependencies;
-    private final boolean isRunAsUserEnabled;
+    private final boolean runAsUserEnabled;
 
     // Metrics
     private final Registry registry;
@@ -107,9 +107,7 @@ public class JobCompletionService {
      * @param genieWorkingDir          The working directory where all job directories are created.
      * @param mailServiceImpl          An implementation of the mail service.
      * @param registry                 The metrics registry to use
-     * @param deleteArchiveFile        Flag that determines if the job archive file will be deleted after upload.
-     * @param deleteDependencies       Flag that determines if the job dependencies will be deleted after job is done.
-     * @param isRunAsUserEnabled       Flag that decides where jobs are run as user themselves or genie user.
+     * @param jobsProperties           The properties relating to running jobs
      * @param retryTemplate            Retry template for retrying remote calls
      * @throws GenieException if there is a problem
      */
@@ -121,24 +119,19 @@ public JobCompletionService(
         final Resource genieWorkingDir,
         final MailService mailServiceImpl,
         final Registry registry,
-        @Value("${genie.jobs.cleanup.deleteArchiveFile.enabled:true}")
-        final boolean deleteArchiveFile,
-        @Value("${genie.jobs.cleanup.deleteDependencies.enabled:true}")
-        final boolean deleteDependencies,
-        @Value("${genie.jobs.runAsUser.enabled:false}")
-        final boolean isRunAsUserEnabled,
+        final JobsProperties jobsProperties,
         @Qualifier("genieRetryTemplate") @NotNull final RetryTemplate retryTemplate
     ) throws GenieException {
         this.jobPersistenceService = jobPersistenceService;
         this.jobSearchService = jobSearchService;
         this.genieFileTransferService = genieFileTransferService;
         this.mailServiceImpl = mailServiceImpl;
-        this.deleteArchiveFile = deleteArchiveFile;
-        this.deleteDependencies = deleteDependencies;
-        this.isRunAsUserEnabled = isRunAsUserEnabled;
+        this.deleteArchiveFile = jobsProperties.getCleanup().isDeleteArchiveFile();
+        this.deleteDependencies = jobsProperties.getCleanup().isDeleteDependencies();
+        this.runAsUserEnabled = jobsProperties.getUsers().isRunAsUserEnabled();
 
         this.executor = new DefaultExecutor();
-        executor.setStreamHandler(new PumpStreamHandler(null, null));
+        this.executor.setStreamHandler(new PumpStreamHandler(null, null));
 
         try {
             this.baseWorkingDir = genieWorkingDir.getFile();
@@ -403,7 +396,7 @@ private void deleteApplicationDependencies(final String jobId, final File jobDir
                     );
 
                     if (appDependencyDir.exists()) {
-                        if (this.isRunAsUserEnabled) {
+                        if (this.runAsUserEnabled) {
                             final CommandLine deleteCommand = new CommandLine("sudo");
                             deleteCommand.addArgument("rm");
                             deleteCommand.addArgument("-rf");

genie-web/src/main/java/com/netflix/genie/web/tasks/node/DiskCleanupTask.java

@@ -20,6 +20,7 @@
 import com.netflix.genie.common.dto.Job;
 import com.netflix.genie.common.exceptions.GenieException;
 import com.netflix.genie.core.jobs.JobConstants;
+import com.netflix.genie.core.properties.JobsProperties;
 import com.netflix.genie.core.services.JobSearchService;
 import com.netflix.genie.web.properties.DiskCleanupProperties;
 import com.netflix.genie.web.tasks.TaskUtils;
@@ -31,7 +32,6 @@
 import org.apache.commons.io.FileUtils;
 import org.apache.commons.lang3.SystemUtils;
 import org.springframework.beans.factory.annotation.Autowired;
-import org.springframework.beans.factory.annotation.Value;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
 import org.springframework.core.io.Resource;
 import org.springframework.scheduling.TaskScheduler;
@@ -73,7 +73,7 @@ public class DiskCleanupTask implements Runnable {
      * @param scheduler        The scheduler to use to schedule the cron trigger.
      * @param jobsDir          The resource representing the location of the job directory
      * @param jobSearchService The service to find jobs with
-     * @param runAsUser        If jobs are run as the user (and thus ownership of directories is changed)
+     * @param jobsProperties   The jobs properties to use
      * @param processExecutor  The process executor to use to delete directories
      * @param registry         The metrics registry
      * @throws IOException When it is unable to open a file reference to the job directory
@@ -84,7 +84,7 @@ public DiskCleanupTask(
         @NotNull final TaskScheduler scheduler,
         @NotNull final Resource jobsDir,
         @NotNull final JobSearchService jobSearchService,
-        @Value("${genie.jobs.runAsUser.enabled:false}") final boolean runAsUser,
+        @NotNull final JobsProperties jobsProperties,
         @NotNull final Executor processExecutor,
         @NotNull final Registry registry
     ) throws IOException {
@@ -96,7 +96,7 @@ public DiskCleanupTask(
         this.properties = properties;
         this.jobsDir = jobsDir.getFile();
         this.jobSearchService = jobSearchService;
-        this.runAsUser = runAsUser;
+        this.runAsUser = jobsProperties.getUsers().isRunAsUserEnabled();
         this.processExecutor = processExecutor;
 
         this.numberOfDeletedJobDirs

genie-web/src/main/resources/application.yml

@@ -28,6 +28,9 @@ genie:
     cache:
       location: file:///tmp/genie/cache
   jobs:
+    cleanup:
+      deleteArchiveFile: true
+      deleteDependencies: true
     forwarding:
       enabled: true
       port: 8080

genie-web/src/test/groovy/com/netflix/genie/web/tasks/job/JobCompletionServiceSpec.groovy

@@ -23,6 +23,7 @@ import com.netflix.genie.common.dto.JobStatus
 import com.netflix.genie.common.exceptions.GenieServerException
 import com.netflix.genie.core.events.JobFinishedEvent
 import com.netflix.genie.core.events.JobFinishedReason
+import com.netflix.genie.core.properties.JobsProperties
 import com.netflix.genie.core.services.JobPersistenceService
 import com.netflix.genie.core.services.JobSearchService
 import com.netflix.genie.core.services.MailService
@@ -51,15 +52,20 @@ class JobCompletionServiceSpec extends Specification{
     JobCompletionService jobCompletionService;
     MailService mailService;
     GenieFileTransferService genieFileTransferService;
+    JobsProperties jobsProperties;
 
     def setup(){
         jobPersistenceService = Mock(JobPersistenceService.class)
         jobSearchService = Mock(JobSearchService.class)
         mailService = Mock(MailService.class)
         genieFileTransferService = Mock(GenieFileTransferService.class)
+        jobsProperties = new JobsProperties()
+        jobsProperties.cleanup.deleteArchiveFile = false
+        jobsProperties.cleanup.deleteDependencies = false
+        jobsProperties.users.runAsUserEnabled = false
         jobCompletionService = new JobCompletionService( jobPersistenceService, jobSearchService,
                 genieFileTransferService, new FileSystemResource("/tmp"), mailService, new NoopRegistry(),
-                false, false, false, new RetryTemplate())
+                jobsProperties, new RetryTemplate())
     }
 
     def handleJobCompletion() throws Exception{

genie-web/src/test/java/com/netflix/genie/web/configs/ServicesConfigUnitTests.java

@@ -256,7 +256,7 @@ public void canGetJobKillServiceBean() {
                 "localhost",
                 this.jobSearchService,
                 Mockito.mock(Executor.class),
-                true,
+                new JobsProperties(),
                 Mockito.mock(ApplicationEventPublisher.class)
             )
         );