Remove redundant JobLogs class (elastic/elasticsearch#348)

Now we no longer have a log directory per job we don't need the functionality
to delete those directories.

Fixes elastic/elasticsearch#346

Original commit: elastic/x-pack-elasticsearch@a18beb0519

elasticsearch/src/main/java/org/elasticsearch/xpack/prelert/PrelertPlugin.java

@@ -54,7 +54,6 @@ import org.elasticsearch.xpack.prelert.action.ValidateDetectorAction;
 import org.elasticsearch.xpack.prelert.action.ValidateTransformAction;
 import org.elasticsearch.xpack.prelert.action.ValidateTransformsAction;
 import org.elasticsearch.xpack.prelert.job.data.DataProcessor;
-import org.elasticsearch.xpack.prelert.job.logs.JobLogs;
 import org.elasticsearch.xpack.prelert.job.manager.AutodetectProcessManager;
 import org.elasticsearch.xpack.prelert.job.manager.JobManager;
 import org.elasticsearch.xpack.prelert.job.metadata.JobAllocator;
@@ -135,7 +134,6 @@ public class PrelertPlugin extends Plugin implements ActionPlugin {
     public List<Setting<?>> getSettings() {
         return Collections.unmodifiableList(
                 Arrays.asList(USE_NATIVE_PROCESS_OPTION,
-                        JobLogs.DONT_DELETE_LOGS_SETTING,
                         ProcessCtrl.DONT_PERSIST_MODEL_STATE_SETTING,
                         ProcessCtrl.MAX_ANOMALY_RECORDS_SETTING,
                         StatusReporter.ACCEPTABLE_PERCENTAGE_DATE_PARSE_ERRORS_SETTING,

elasticsearch/src/main/java/org/elasticsearch/xpack/prelert/job/logs/JobLogs.java

@@ -1,122 +0,0 @@
-/*
- * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
- * or more contributor license agreements. Licensed under the Elastic License;
- * you may not use this file except in compliance with the Elastic License.
- */
-package org.elasticsearch.xpack.prelert.job.logs;
-
-import org.apache.logging.log4j.Logger;
-import org.elasticsearch.common.logging.Loggers;
-import org.elasticsearch.common.settings.Setting;
-import org.elasticsearch.common.settings.Setting.Property;
-import org.elasticsearch.common.settings.Settings;
-import org.elasticsearch.env.Environment;
-import org.elasticsearch.xpack.prelert.PrelertPlugin;
-import org.elasticsearch.xpack.prelert.job.messages.Messages;
-
-import java.io.IOException;
-import java.nio.file.DirectoryStream;
-import java.nio.file.Files;
-import java.nio.file.Path;
-import java.util.Locale;
-
-/**
- * Manage job logs
- */
-public class JobLogs {
-    private static final Logger LOGGER = Loggers.getLogger(JobLogs.class);
-    /**
-     * If this system property is set the log files aren't deleted when the job
-     * is.
-     */
-    public static final Setting<Boolean> DONT_DELETE_LOGS_SETTING = Setting.boolSetting("preserve.logs", false, Property.NodeScope);
-
-    private boolean m_DontDelete;
-
-    public JobLogs(Settings settings)
-    {
-        m_DontDelete = DONT_DELETE_LOGS_SETTING.get(settings);
-    }
-
-    /**
-     * Delete all the log files and log directory associated with a job.
-     *
-     * @param jobId
-     *            the jobId
-     * @return true if success.
-     */
-    public boolean deleteLogs(Environment env, String jobId) {
-        return deleteLogs(env.logsFile().resolve(PrelertPlugin.NAME), jobId);
-    }
-
-    /**
-     * Delete all the files in the directory
-     *
-     * <pre>
-     * logDir / jobId
-     * </pre>
-     *
-     * .
-     *
-     * @param logDir
-     *            The base directory of the log files
-     * @param jobId
-     *            the jobId
-     */
-    public boolean deleteLogs(Path logDir, String jobId) {
-        if (m_DontDelete) {
-            return true;
-        }
-
-        Path logPath = sanitizePath(logDir.resolve(jobId), logDir);
-
-        LOGGER.info(String.format(Locale.ROOT, "Deleting log files %s/%s", logDir, jobId));
-
-        try (DirectoryStream<Path> directoryStream = Files.newDirectoryStream(logPath)) {
-            for (Path logFile : directoryStream) {
-                try {
-                    Files.delete(logFile);
-                } catch (IOException e) {
-                    String msg = "Cannot delete log file " + logDir + ". ";
-                    msg += (e.getCause() != null) ? e.getCause().getMessage() : e.getMessage();
-                    LOGGER.warn(msg);
-                }
-            }
-        } catch (IOException e) {
-            String msg = "Cannot open the log directory " + logDir + ". ";
-            msg += (e.getCause() != null) ? e.getCause().getMessage() : e.getMessage();
-            LOGGER.warn(msg);
-        }
-
-        // delete the directory
-        try {
-            Files.delete(logPath);
-        } catch (IOException e) {
-            String msg = "Cannot delete log directory " + logDir + ". ";
-            msg += (e.getCause() != null) ? e.getCause().getMessage() : e.getMessage();
-            LOGGER.warn(msg);
-            return false;
-        }
-
-        return true;
-    }
-
-    /**
-     * Normalize a file path resolving .. and . directories and check the
-     * resulting path is below the rootDir directory.
-     *
-     * Throws an exception if the path is outside the logs directory e.g.
-     * logs/../lic/license resolves to lic/license and would throw
-     */
-    public Path sanitizePath(Path filePath, Path rootDir) {
-        Path normalizedPath = filePath.normalize();
-        Path rootPath = rootDir.normalize();
-        if (normalizedPath.startsWith(rootPath) == false) {
-            String msg = Messages.getMessage(Messages.LOGFILE_INVALID_PATH, filePath);
-            LOGGER.warn(msg);
-            throw new IllegalArgumentException(msg);
-        }
-
-        return normalizedPath;
-    }
-}

elasticsearch/src/main/java/org/elasticsearch/xpack/prelert/job/manager/JobManager.java

@@ -31,7 +31,6 @@ import org.elasticsearch.xpack.prelert.job.JobStatus;
 import org.elasticsearch.xpack.prelert.job.ModelSnapshot;
 import org.elasticsearch.xpack.prelert.job.SchedulerState;
 import org.elasticsearch.xpack.prelert.job.audit.Auditor;
-import org.elasticsearch.xpack.prelert.job.logs.JobLogs;
 import org.elasticsearch.xpack.prelert.job.messages.Messages;
 import org.elasticsearch.xpack.prelert.job.metadata.Allocation;
 import org.elasticsearch.xpack.prelert.job.metadata.PrelertMetadata;
@@ -245,37 +244,20 @@ public class JobManager {
     public void deleteJob(DeleteJobAction.Request request, ActionListener<DeleteJobAction.Response> actionListener) {
         String jobId = request.getJobId();
         LOGGER.debug("Deleting job '" + jobId + "'");
-        // NORELEASE: Should also delete the running process
+        // NORELEASE: Should first gracefully stop any running process
         ActionListener<Boolean> delegateListener = new ActionListener<Boolean>() {
             @Override
             public void onResponse(Boolean jobDeleted) {
-                jobProvider.deleteJobRelatedIndices(request.getJobId(), new ActionListener<Boolean>() {
+                if (jobDeleted) {
-                    @Override
+                    jobProvider.deleteJobRelatedIndices(request.getJobId(), actionListener);
-                    public void onResponse(Boolean indicesDeleted) {
+                    // NORELEASE: This is not the place the audit log
-
+                    // (indexes a document), because this method is
-                        new JobLogs(settings).deleteLogs(env, jobId);
+                    // executed on the cluster state update task thread and any
-                        // NORELEASE: This is not the place the audit
+                    // action performed on that thread should be quick.
-                        // log
+                    //audit(jobId).info(Messages.getMessage(Messages.JOB_AUDIT_DELETED));
-                        // (indexes a document), because this method is
+                } else {
-                        // executed on
+                    actionListener.onResponse(new DeleteJobAction.Response(false));
-                        // the cluster state update task thread and any
+                }
-                        // action performed on that thread should be
-                        // quick.
-                        // audit(jobId).info(Messages.getMessage(Messages.JOB_AUDIT_DELETED));
-
-                        // Also I wonder if we need to audit log infra
-                        // structure in prelert as when we merge into
-                        // xpack
-                        // we can use its audit trailing. See:
-                        // https://github.com/elastic/prelert-legacy/issues/48
-                        actionListener.onResponse(new DeleteJobAction.Response(jobDeleted && indicesDeleted));
-                    }
-
-                    @Override
-                    public void onFailure(Exception e) {
-                        actionListener.onFailure(e);
-                    }
-                });
             }
 
             @Override

elasticsearch/src/main/java/org/elasticsearch/xpack/prelert/job/persistence/ElasticsearchJobProvider.java

@@ -45,6 +45,7 @@ import org.elasticsearch.search.sort.FieldSortBuilder;
 import org.elasticsearch.search.sort.SortBuilder;
 import org.elasticsearch.search.sort.SortBuilders;
 import org.elasticsearch.search.sort.SortOrder;
+import org.elasticsearch.xpack.prelert.action.DeleteJobAction;
 import org.elasticsearch.xpack.prelert.job.CategorizerState;
 import org.elasticsearch.xpack.prelert.job.DataCounts;
 import org.elasticsearch.xpack.prelert.job.Job;
@@ -281,7 +282,7 @@ public class ElasticsearchJobProvider implements JobProvider
     }
 
     @Override
-    public void deleteJobRelatedIndices(String jobId, ActionListener<Boolean> listener) {
+    public void deleteJobRelatedIndices(String jobId, ActionListener<DeleteJobAction.Response> listener) {
         if (indexExists(jobId) == false) {
             listener.onFailure(ExceptionsHelper.missingJobException(jobId));
             return;
@@ -294,7 +295,7 @@ public class ElasticsearchJobProvider implements JobProvider
             client.admin().indices().delete(deleteIndexRequest, new ActionListener<DeleteIndexResponse>() {
                 @Override
                 public void onResponse(DeleteIndexResponse deleteIndexResponse) {
-                    listener.onResponse(deleteIndexResponse.isAcknowledged());
+                    listener.onResponse(new DeleteJobAction.Response(deleteIndexResponse.isAcknowledged()));
                 }
 
                 @Override

elasticsearch/src/main/java/org/elasticsearch/xpack/prelert/job/persistence/JobProvider.java

@@ -7,6 +7,7 @@ package org.elasticsearch.xpack.prelert.job.persistence;
 
 
 import org.elasticsearch.action.ActionListener;
+import org.elasticsearch.xpack.prelert.action.DeleteJobAction;
 import org.elasticsearch.xpack.prelert.job.DataCounts;
 import org.elasticsearch.xpack.prelert.job.Job;
 import org.elasticsearch.xpack.prelert.job.ModelSizeStats;
@@ -116,5 +117,5 @@ public interface JobProvider extends JobResultsProvider {
      * Delete all the job related documents from the database.
      */
     // TODO: should live together with createJobRelatedIndices (in case it moves)?
-    void deleteJobRelatedIndices(String jobId, ActionListener<Boolean> listener);
+    void deleteJobRelatedIndices(String jobId, ActionListener<DeleteJobAction.Response> listener);
 }

elasticsearch/src/test/java/org/elasticsearch/xpack/prelert/job/logs/JobLogsTests.java

@@ -1,69 +0,0 @@
-/*
- * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
- * or more contributor license agreements. Licensed under the Elastic License;
- * you may not use this file except in compliance with the Elastic License.
- */
-package org.elasticsearch.xpack.prelert.job.logs;
-
-import org.elasticsearch.common.io.PathUtils;
-import org.elasticsearch.common.settings.Settings;
-import org.elasticsearch.env.Environment;
-import org.elasticsearch.test.ESTestCase;
-import org.elasticsearch.xpack.prelert.job.messages.Messages;
-import org.hamcrest.core.StringStartsWith;
-
-import java.io.IOException;
-import java.nio.file.Path;
-
-public class JobLogsTests extends ESTestCase {
-
-    public void testOperationsNotAllowedWithInvalidPath() throws IOException {
-        Path pathOutsideLogsDir = PathUtils.getDefaultFileSystem().getPath("..", "..", "..", "etc");
-
-        Settings settings = Settings.builder().put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toString()).build();
-        Environment env = new Environment(
-                settings);
-
-        // delete
-        try {
-            JobLogs jobLogs = new JobLogs(settings);
-            jobLogs.deleteLogs(env, pathOutsideLogsDir.toString());
-            fail();
-        } catch (IllegalArgumentException e) {
-            assertThat(e.getMessage(), StringStartsWith.startsWith("Invalid log file path."));
-        }
-    }
-
-    public void testSanitizePath_GivenInvalid() {
-
-        Settings settings = Settings.builder().put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toString()).build();
-        Path filePath = PathUtils.getDefaultFileSystem().getPath("/opt", "prelert", "../../etc");
-        try {
-            Path rootDir = PathUtils.getDefaultFileSystem().getPath("/opt", "prelert");
-            new JobLogs(settings).sanitizePath(filePath, rootDir);
-            fail();
-        } catch (IllegalArgumentException e) {
-            assertEquals(Messages.getMessage(Messages.LOGFILE_INVALID_PATH, filePath), e.getMessage());
-        }
-    }
-
-    public void testSanitizePath() {
-
-        Settings settings = Settings.builder().put(Environment.PATH_HOME_SETTING.getKey(), createTempDir().toString()).build();
-        Path filePath = PathUtils.getDefaultFileSystem().getPath("/opt", "prelert", "logs", "logfile.log");
-        Path rootDir = PathUtils.getDefaultFileSystem().getPath("/opt", "prelert", "logs");
-        Path normalized = new JobLogs(settings).sanitizePath(filePath, rootDir);
-        assertEquals(filePath, normalized);
-
-        Path logDir = PathUtils.getDefaultFileSystem().getPath("./logs");
-        Path filePathStartingDot = logDir.resolve("farequote").resolve("logfile.log");
-        normalized = new JobLogs(settings).sanitizePath(filePathStartingDot, logDir);
-        assertEquals(filePathStartingDot.normalize(), normalized);
-
-        Path filePathWithDotDot = PathUtils.getDefaultFileSystem().getPath("/opt", "prelert", "logs", "../logs/logfile.log");
-        rootDir = PathUtils.getDefaultFileSystem().getPath("/opt", "prelert", "logs");
-        normalized = new JobLogs(settings).sanitizePath(filePathWithDotDot, rootDir);
-
-        assertEquals(filePath, normalized);
-    }
-}