From 4b7b78917f41bca0ea9304656ca4d9ae213badb1 Mon Sep 17 00:00:00 2001 From: Brian Cipriano Date: Fri, 21 Feb 2020 15:31:59 -0800 Subject: [PATCH] Allow log root directory to be specified via command line flag. (#627) --- .../java/com/imageworks/spcue/dao/JobDao.java | 3 +- .../spcue/dao/oracle/JobDaoJdbc.java | 4 +- .../spcue/dao/postgres/JobDaoJdbc.java | 4 +- .../spcue/service/JobManagerService.java | 13 ++++++- .../com/imageworks/spcue/util/JobLogUtil.java | 37 ++++++------------- .../spring/applicationContext-service.xml | 3 ++ cuebot/src/main/resources/opencue.properties | 4 +- .../spcue/test/dao/oracle/JobDaoTests.java | 11 ++++-- .../spcue/test/dao/oracle/LayerDaoTests.java | 7 +++- .../spcue/test/dao/postgres/JobDaoTests.java | 11 ++++-- .../test/dao/postgres/LayerDaoTests.java | 8 ++-- .../spcue/test/util/JobLogUtilTests.java | 34 ++++++++++++----- cuebot/src/test/resources/opencue.properties | 2 + 13 files changed, 86 insertions(+), 55 deletions(-) diff --git a/cuebot/src/main/java/com/imageworks/spcue/dao/JobDao.java b/cuebot/src/main/java/com/imageworks/spcue/dao/JobDao.java index ad8b4b882..a8669a7f8 100644 --- a/cuebot/src/main/java/com/imageworks/spcue/dao/JobDao.java +++ b/cuebot/src/main/java/com/imageworks/spcue/dao/JobDao.java @@ -37,6 +37,7 @@ import com.imageworks.spcue.ShowInterface; import com.imageworks.spcue.TaskEntity; import com.imageworks.spcue.grpc.job.JobState; +import com.imageworks.spcue.util.JobLogUtil; public interface JobDao { @@ -115,7 +116,7 @@ public interface JobDao { * * @param j */ - void insertJob(JobDetail j); + void insertJob(JobDetail j, JobLogUtil jobLogUtil); /** * Finds a Job from its name. This method returns only diff --git a/cuebot/src/main/java/com/imageworks/spcue/dao/oracle/JobDaoJdbc.java b/cuebot/src/main/java/com/imageworks/spcue/dao/oracle/JobDaoJdbc.java index 9c642289c..7559ad0d3 100644 --- a/cuebot/src/main/java/com/imageworks/spcue/dao/oracle/JobDaoJdbc.java +++ b/cuebot/src/main/java/com/imageworks/spcue/dao/oracle/JobDaoJdbc.java @@ -441,9 +441,9 @@ public boolean updateJobFinished(JobInterface job) { "VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)"; @Override - public void insertJob(JobDetail j) { + public void insertJob(JobDetail j, JobLogUtil jobLogUtil) { j.id = SqlUtil.genKeyRandom(); - j.logDir = JobLogUtil.getJobLogPath(j); + j.logDir = jobLogUtil.getJobLogPath(j); if (j.minCoreUnits < 100) { j.minCoreUnits = 100; } getJdbcTemplate().update(INSERT_JOB, diff --git a/cuebot/src/main/java/com/imageworks/spcue/dao/postgres/JobDaoJdbc.java b/cuebot/src/main/java/com/imageworks/spcue/dao/postgres/JobDaoJdbc.java index f65ef1fc4..2b7ac5456 100644 --- a/cuebot/src/main/java/com/imageworks/spcue/dao/postgres/JobDaoJdbc.java +++ b/cuebot/src/main/java/com/imageworks/spcue/dao/postgres/JobDaoJdbc.java @@ -439,9 +439,9 @@ public boolean updateJobFinished(JobInterface job) { "VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)"; @Override - public void insertJob(JobDetail j) { + public void insertJob(JobDetail j, JobLogUtil jobLogUtil) { j.id = SqlUtil.genKeyRandom(); - j.logDir = JobLogUtil.getJobLogPath(j); + j.logDir = jobLogUtil.getJobLogPath(j); if (j.minCoreUnits < 100) { j.minCoreUnits = 100; } getJdbcTemplate().update(INSERT_JOB, diff --git a/cuebot/src/main/java/com/imageworks/spcue/service/JobManagerService.java b/cuebot/src/main/java/com/imageworks/spcue/service/JobManagerService.java index 56af6fd0d..caff9719c 100644 --- a/cuebot/src/main/java/com/imageworks/spcue/service/JobManagerService.java +++ b/cuebot/src/main/java/com/imageworks/spcue/service/JobManagerService.java @@ -81,6 +81,7 @@ public class JobManagerService implements JobManager { private FilterManager filterManager; private GroupDao groupDao; private FacilityDao facilityDao; + private JobLogUtil jobLogUtil; @Transactional(propagation = Propagation.REQUIRED, readOnly=true) public boolean isJobComplete(JobInterface job) { @@ -242,7 +243,7 @@ public JobDetail createJob(BuildableJob buildableJob) { resolveFacility(job); - jobDao.insertJob(job); + jobDao.insertJob(job, jobLogUtil); jobDao.insertEnvironment(job, buildableJob.env); for (BuildableLayer buildableLayer: buildableJob.getBuildableLayers()) { @@ -366,7 +367,7 @@ public void markFrameAsDepend(FrameInterface frame) { */ @Transactional(propagation = Propagation.NEVER) public void createJobLogDirectory(JobDetail job) { - if (!JobLogUtil.createJobLogDirectory(job.logDir)) { + if (!jobLogUtil.createJobLogDirectory(job.logDir)) { throw new JobLaunchException("error launching job, unable to create log directory"); } } @@ -623,5 +624,13 @@ public HostDao getHostDao() { public void setHostDao(HostDao hostDao) { this.hostDao = hostDao; } + + public JobLogUtil getJobLogUtil() { + return jobLogUtil; + } + + public void setJobLogUtil(JobLogUtil jobLogUtil) { + this.jobLogUtil = jobLogUtil; + } } diff --git a/cuebot/src/main/java/com/imageworks/spcue/util/JobLogUtil.java b/cuebot/src/main/java/com/imageworks/spcue/util/JobLogUtil.java index 2dff4291a..c270f4e28 100644 --- a/cuebot/src/main/java/com/imageworks/spcue/util/JobLogUtil.java +++ b/cuebot/src/main/java/com/imageworks/spcue/util/JobLogUtil.java @@ -16,39 +16,31 @@ */ - package com.imageworks.spcue.util; -import java.io.File; - -import org.springframework.beans.factory.annotation.Value; +import com.imageworks.spcue.JobDetail; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.core.env.Environment; import org.springframework.stereotype.Component; -import com.imageworks.spcue.JobDetail; +import java.io.File; @Component public class JobLogUtil { - public static String jobLogRootDir; + @Autowired + private Environment env; - public static boolean createJobLogDirectory(String path) { + public boolean createJobLogDirectory(String path) { File f = new File(path); f.mkdir(); f.setWritable(true, false); return f.isDirectory(); } - public static boolean shotLogDirectoryExists(String show, String shot) { - return new File(getJobLogDir(show, shot)).exists(); - } - - public static boolean jobLogDirectoryExists(JobDetail job) { - return new File(job.logDir).exists(); - } - - public static String getJobLogDir(String show, String shot) { + public String getJobLogDir(String show, String shot) { StringBuilder sb = new StringBuilder(512); - sb.append(jobLogRootDir); + sb.append(getJobLogRootDir()); sb.append("/"); sb.append(show); sb.append("/"); @@ -57,7 +49,7 @@ public static String getJobLogDir(String show, String shot) { return sb.toString(); } - public static String getJobLogPath(JobDetail job) { + public String getJobLogPath(JobDetail job) { StringBuilder sb = new StringBuilder(512); sb.append(getJobLogDir(job.showName, job.shot)); sb.append("/"); @@ -67,13 +59,8 @@ public static String getJobLogPath(JobDetail job) { return sb.toString(); } - public static String getJobLogRootDir() { - return jobLogRootDir; - } - - @Value("${log.frameLogDirRoot}") - public void setJobLogRootDir(String logRootDir) { - jobLogRootDir = logRootDir; + public String getJobLogRootDir() { + return env.getRequiredProperty("log.frame-log-root", String.class); } } diff --git a/cuebot/src/main/resources/conf/spring/applicationContext-service.xml b/cuebot/src/main/resources/conf/spring/applicationContext-service.xml index d14274470..443188236 100644 --- a/cuebot/src/main/resources/conf/spring/applicationContext-service.xml +++ b/cuebot/src/main/resources/conf/spring/applicationContext-service.xml @@ -159,6 +159,8 @@ + + @@ -170,6 +172,7 @@ + diff --git a/cuebot/src/main/resources/opencue.properties b/cuebot/src/main/resources/opencue.properties index e51f8872a..7a5d16f14 100644 --- a/cuebot/src/main/resources/opencue.properties +++ b/cuebot/src/main/resources/opencue.properties @@ -36,7 +36,9 @@ grpc.rqd_cache_expiration=30 messaging.enabled=false # Root directory for which logs will be stored. See com/imageworks/spcue/util/JobLogUtil.java. -log.frameLogDirRoot=${CUE_FRAME_LOG_DIR:/shots} +# Override this via environment variable (CUE_FRAME_LOG_DIR) or command line flag +# (--log.frame-log-root). Command line flag will be preferred if both are provided. +log.frame-log-root=${CUE_FRAME_LOG_DIR:/shots} # Maximum number of jobs to query. dispatcher.job_query_max=20 diff --git a/cuebot/src/test/java/com/imageworks/spcue/test/dao/oracle/JobDaoTests.java b/cuebot/src/test/java/com/imageworks/spcue/test/dao/oracle/JobDaoTests.java index 0be9fb676..b4bb33a40 100644 --- a/cuebot/src/test/java/com/imageworks/spcue/test/dao/oracle/JobDaoTests.java +++ b/cuebot/src/test/java/com/imageworks/spcue/test/dao/oracle/JobDaoTests.java @@ -99,6 +99,9 @@ public class JobDaoTests extends AbstractTransactionalJUnit4SpringContextTests @Resource DepartmentDao departmentDao; + @Resource + JobLogUtil jobLogUtil; + private static String ROOT_FOLDER = "A0000000-0000-0000-0000-000000000000"; private static String ROOT_SHOW = "00000000-0000-0000-0000-000000000000"; private static String JOB_NAME = "pipe-dev.cue-testuser_shell_v1"; @@ -117,11 +120,11 @@ public JobDetail insertJob() { JobDetail job = this.buildJobDetail(); job.groupId = ROOT_FOLDER; job.showId = ROOT_SHOW; - job.logDir = JobLogUtil.getJobLogPath(job); + job.logDir = jobLogUtil.getJobLogPath(job); job.deptId = departmentDao.getDefaultDepartment().getId(); job.facilityId = facilityDao.getDefaultFacility().getId(); job.state = JobState.PENDING; - jobDao.insertJob(job); + jobDao.insertJob(job, jobLogUtil); return job; } @@ -156,10 +159,10 @@ public void testInsertJob() { JobDetail job = this.buildJobDetail(); job.groupId = ROOT_FOLDER; job.showId = ROOT_SHOW; - job.logDir = JobLogUtil.getJobLogPath(job); + job.logDir = jobLogUtil.getJobLogPath(job); job.deptId = departmentDao.getDefaultDepartment().getId(); job.facilityId= facilityDao.getDefaultFacility().getId(); - jobDao.insertJob(job); + jobDao.insertJob(job, jobLogUtil); assertNotNull(job.id); } diff --git a/cuebot/src/test/java/com/imageworks/spcue/test/dao/oracle/LayerDaoTests.java b/cuebot/src/test/java/com/imageworks/spcue/test/dao/oracle/LayerDaoTests.java index 6cd033158..74bacf61c 100644 --- a/cuebot/src/test/java/com/imageworks/spcue/test/dao/oracle/LayerDaoTests.java +++ b/cuebot/src/test/java/com/imageworks/spcue/test/dao/oracle/LayerDaoTests.java @@ -95,6 +95,9 @@ public class LayerDaoTests extends AbstractTransactionalJUnit4SpringContextTests @Resource FacilityDao facilityDao; + @Resource + JobLogUtil jobLogUtil; + private static String ROOT_FOLDER = "A0000000-0000-0000-0000-000000000000"; private static String ROOT_SHOW = "00000000-0000-0000-0000-000000000000"; private static String LAYER_NAME = "pass_1"; @@ -116,10 +119,10 @@ public LayerDetail getLayer() { JobDetail job = spec.getJobs().get(0).detail; job.groupId = ROOT_FOLDER; job.showId = ROOT_SHOW; - job.logDir = JobLogUtil.getJobLogPath(job); + job.logDir = jobLogUtil.getJobLogPath(job); job.deptId = departmentDao.getDefaultDepartment().getId(); job.facilityId = facilityDao.getDefaultFacility().getId(); - jobDao.insertJob(job); + jobDao.insertJob(job, jobLogUtil); LayerDetail lastLayer= null; String limitId = limitDao.createLimit(LIMIT_NAME, LIMIT_MAX_VALUE); diff --git a/cuebot/src/test/java/com/imageworks/spcue/test/dao/postgres/JobDaoTests.java b/cuebot/src/test/java/com/imageworks/spcue/test/dao/postgres/JobDaoTests.java index ce7452c5f..12761ff14 100644 --- a/cuebot/src/test/java/com/imageworks/spcue/test/dao/postgres/JobDaoTests.java +++ b/cuebot/src/test/java/com/imageworks/spcue/test/dao/postgres/JobDaoTests.java @@ -99,6 +99,9 @@ public class JobDaoTests extends AbstractTransactionalJUnit4SpringContextTests @Resource DepartmentDao departmentDao; + @Resource + JobLogUtil jobLogUtil; + private static String ROOT_FOLDER = "A0000000-0000-0000-0000-000000000000"; private static String ROOT_SHOW = "00000000-0000-0000-0000-000000000000"; private static String JOB_NAME = "pipe-dev.cue-testuser_shell_v1"; @@ -117,11 +120,11 @@ public JobDetail insertJob() { JobDetail job = this.buildJobDetail(); job.groupId = ROOT_FOLDER; job.showId = ROOT_SHOW; - job.logDir = JobLogUtil.getJobLogPath(job); + job.logDir = jobLogUtil.getJobLogPath(job); job.deptId = departmentDao.getDefaultDepartment().getId(); job.facilityId = facilityDao.getDefaultFacility().getId(); job.state = JobState.PENDING; - jobDao.insertJob(job); + jobDao.insertJob(job, jobLogUtil); return job; } @@ -156,10 +159,10 @@ public void testInsertJob() { JobDetail job = this.buildJobDetail(); job.groupId = ROOT_FOLDER; job.showId = ROOT_SHOW; - job.logDir = JobLogUtil.getJobLogPath(job); + job.logDir = jobLogUtil.getJobLogPath(job); job.deptId = departmentDao.getDefaultDepartment().getId(); job.facilityId= facilityDao.getDefaultFacility().getId(); - jobDao.insertJob(job); + jobDao.insertJob(job, jobLogUtil); assertNotNull(job.id); } diff --git a/cuebot/src/test/java/com/imageworks/spcue/test/dao/postgres/LayerDaoTests.java b/cuebot/src/test/java/com/imageworks/spcue/test/dao/postgres/LayerDaoTests.java index 085f8d84d..0b2c6f9c4 100644 --- a/cuebot/src/test/java/com/imageworks/spcue/test/dao/postgres/LayerDaoTests.java +++ b/cuebot/src/test/java/com/imageworks/spcue/test/dao/postgres/LayerDaoTests.java @@ -89,13 +89,15 @@ public class LayerDaoTests extends AbstractTransactionalJUnit4SpringContextTests @Resource JobLauncher jobLauncher; - @Resource DepartmentDao departmentDao; @Resource FacilityDao facilityDao; + @Resource + JobLogUtil jobLogUtil; + private static String ROOT_FOLDER = "A0000000-0000-0000-0000-000000000000"; private static String ROOT_SHOW = "00000000-0000-0000-0000-000000000000"; private static String LAYER_NAME = "pass_1"; @@ -117,10 +119,10 @@ public LayerDetail getLayer() { JobDetail job = spec.getJobs().get(0).detail; job.groupId = ROOT_FOLDER; job.showId = ROOT_SHOW; - job.logDir = JobLogUtil.getJobLogPath(job); + job.logDir = jobLogUtil.getJobLogPath(job); job.deptId = departmentDao.getDefaultDepartment().getId(); job.facilityId = facilityDao.getDefaultFacility().getId(); - jobDao.insertJob(job); + jobDao.insertJob(job, jobLogUtil); LayerDetail lastLayer= null; String limitId = limitDao.createLimit(LIMIT_NAME, LIMIT_MAX_VALUE); diff --git a/cuebot/src/test/java/com/imageworks/spcue/test/util/JobLogUtilTests.java b/cuebot/src/test/java/com/imageworks/spcue/test/util/JobLogUtilTests.java index 339ad5d55..c2cbc843a 100644 --- a/cuebot/src/test/java/com/imageworks/spcue/test/util/JobLogUtilTests.java +++ b/cuebot/src/test/java/com/imageworks/spcue/test/util/JobLogUtilTests.java @@ -15,37 +15,53 @@ */ - package com.imageworks.spcue.test.util; -import junit.framework.TestCase; -import org.junit.Before; - import com.imageworks.spcue.JobDetail; +import com.imageworks.spcue.config.TestAppConfig; import com.imageworks.spcue.util.JobLogUtil; +import org.junit.Before; +import org.junit.Test; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.junit4.AbstractTransactionalJUnit4SpringContextTests; +import org.springframework.test.context.support.AnnotationConfigContextLoader; + +import javax.annotation.Resource; + +import static org.junit.Assert.assertEquals; + +@ContextConfiguration(classes = TestAppConfig.class, loader = AnnotationConfigContextLoader.class) +public class JobLogUtilTests extends AbstractTransactionalJUnit4SpringContextTests { + + @Resource + private JobLogUtil jobLogUtil; -public class JobLogUtilTests extends TestCase { + private String logRoot; @Before public void setUp() { - JobLogUtil.jobLogRootDir = "/shots"; + // This value should match what's defined in test/resources/opencue.properties. + logRoot = "/arbitraryLogDirectory"; } + @Test public void testGetJobLogRootDir() { - assertEquals(JobLogUtil.getJobLogRootDir(), "/shots"); + assertEquals(logRoot, jobLogUtil.getJobLogRootDir()); } + @Test public void testGetJobLogDir() { - assertEquals(JobLogUtil.getJobLogDir("show", "shot"), "/shots/show/shot/logs"); + assertEquals(logRoot + "/show/shot/logs", jobLogUtil.getJobLogDir("show", "shot")); } + @Test public void testGetJobLogPath() { JobDetail jobDetail = new JobDetail(); jobDetail.id = "id"; jobDetail.name = "name"; jobDetail.showName = "show"; jobDetail.shot = "shot"; - assertEquals(JobLogUtil.getJobLogPath(jobDetail), "/shots/show/shot/logs/name--id"); + assertEquals(logRoot + "/show/shot/logs/name--id", jobLogUtil.getJobLogPath(jobDetail)); } } diff --git a/cuebot/src/test/resources/opencue.properties b/cuebot/src/test/resources/opencue.properties index d002f5b12..0a59548ce 100644 --- a/cuebot/src/test/resources/opencue.properties +++ b/cuebot/src/test/resources/opencue.properties @@ -19,6 +19,8 @@ grpc.rqd_cache_size=500 # RQD Channel Cache Expiration in Minutes grpc.rqd_cache_expiration=30 +log.frame-log-root=/arbitraryLogDirectory + dispatcher.job_query_max=20 dispatcher.job_lock_expire_seconds=2 dispatcher.job_lock_concurrency_level=3