[HWORKS-522] Enable cancelling of git execution (#1358) (#1394)

* Enable cancelling of git execution

* Add more attributes to git log file

* Add index pattern

* Fix add index pattern

* change the sepaator

* PR review comments

* Test async

* Test changes

* Check if already cancelled

* Catch any exception and log it in the cancel execution

* Log with Level INFO

hopsworks-IT/src/test/ruby/spec/git_spec.rb

@@ -465,7 +465,7 @@
     end
     describe "Git operation" do
       after :each do
-        setVar("git_command_timeout_minutes", 60)
+        setVar("git_command_timeout_minutes", 15)
         create_session(@project[:username], "Pass123")
       end
       it "should indicate ongoing operation in the repository" do
@@ -520,6 +520,21 @@
           delete_repository(@project[:id], repository_id)
         end
       end
+      it 'should cancel a git execution' do
+        begin
+          clone_config = get_clone_config("GitHub", @project[:projectname])
+          repository_id, _ = clone_repo(@project[:id], clone_config)
+          git_status(@project[:id], repository_id)
+          expect_status_details(200)
+          execution_id = json_body[:id]
+          # cancel the execution
+          cancel_git_execution(@project[:id],  repository_id, execution_id)
+          expect_status_details(200)
+          wait_for_git_operation_completed(@project[:id], repository_id, execution_id, "Cancelled")
+        ensure
+          delete_repository(@project[:id], repository_id)
+        end
+      end
     end
   end
 end

hopsworks-IT/src/test/ruby/spec/helpers/git_helper.rb

@@ -292,6 +292,12 @@ def get_git_executions(project_id, repository_id, query="")
     get "#{ENV['HOPSWORKS_API']}/project/#{project_id}/git/repository/#{repository_id}/execution#{query}"
   end
 
+  def cancel_git_execution(project_id, repository_id, execution_id)
+    put "#{ENV['HOPSWORKS_API']}/project/#{project_id}/git/repository/#{repository_id}/execution/#{execution_id}/state", {executionState: 'Cancelled', message: 'Cancelled'}.to_json
+  end
+
+
+
   def git_file_add_or_delete(project, repository_id, repo_full_path, filename, action)
     dsname = repo_full_path.gsub("/Projects/#{project[:projectname]}", "")
     if action == "add"

hopsworks-common/src/main/java/io/hops/hopsworks/common/dao/git/GitOpExecutionFacade.java

@@ -101,6 +101,7 @@ public GitOpExecution updateState(GitOpExecution exec, GitOpExecutionState newSt
       exec.setExecutionStop(System.currentTimeMillis());
     }
     em.merge(exec);
+    em.flush();
     return exec;
   }
 }

hopsworks-common/src/main/java/io/hops/hopsworks/common/dao/git/GitPaths.java

@@ -28,7 +28,7 @@ public class GitPaths {
 
   public GitPaths(String privateDir, String secretConfig) {
     this.gitPath = privateDir + secretConfig;
-    this.logDirPath = gitPath + File.separator + "logs";
+    this.logDirPath = gitPath + File.separator + "git_logs";
     this.confDirPath = gitPath + File.separator + "conf";
     this.certificatesDirPath = gitPath + File.separator + "certificates";
     this.runDirPath = gitPath + File.separator + "run";

hopsworks-common/src/main/java/io/hops/hopsworks/common/git/AsynchronousGitCommandExecutor.java

@@ -107,7 +107,42 @@ public void execute(GitOpExecution gitOpExecution, GitPaths gitPaths) {
     }
   }
 
+  @Asynchronous
+  @TransactionAttribute(TransactionAttributeType.NOT_SUPPORTED)
+  public void cancelGitExecution(GitOpExecution execution, String message) {
+    if (execution.getState().isFinalState()) {
+      return;
+    }
+    try {
+      gitOpExecutionFacade.updateState(execution, GitOpExecutionState.CANCELLED, message);
+      GitRepository repository = execution.getRepository();
+      int maxTries = 10; // wait time if the container is not launched
+      while (maxTries > 0 && org.opensearch.common.Strings.isNullOrEmpty(repository.getCid())) {
+        Optional<GitRepository> optional = gitRepositoryFacade.findById(repository.getId());
+        if (optional.isPresent()) {
+          repository = optional.get();
+        }
+        try {
+          Thread.sleep(100);
+        } catch (InterruptedException e) {
+          LOGGER.log(Level.INFO, "Interrupted while waiting for the git container to start");
+        }
+        maxTries--;
+      }
+      gitCommandOperationUtil.shutdownCommandService(repository, execution);
+    } catch (Exception e) {
+      LOGGER.log(Level.INFO, "Error when cancelling git execution with ID: " + execution.getId(), e);
+    }
+  }
+
   private void updateExecutionStateToFail(GitOpExecution gitOpExecution) {
+    // Get latest execution object
+    Optional<GitOpExecution> optional = gitOpExecutionFacade.findByIdAndRepository(gitOpExecution.getRepository(),
+        gitOpExecution.getId());
+    gitOpExecution = optional.get();
+    if (gitOpExecution.getState() == GitOpExecutionState.CANCELLED) {
+      return;
+    }
     gitCommandOperationUtil.cleanUp(gitOpExecution.getRepository().getProject(),
         gitOpExecution.getUser(), gitOpExecution.getConfigSecret());
     gitOpExecutionFacade.updateState(gitOpExecution, GitOpExecutionState.FAILED, "Could not launch " +

hopsworks-common/src/main/java/io/hops/hopsworks/common/git/GitExecutionController.java

@@ -181,7 +181,8 @@ public GitOpExecution getExecutionInRepository(GitRepository repository, Integer
     return executionObj.get();
   }
 
-  public GitOpExecution updateGitExecutionState(Project project, Users hopsworksUser,
+
+  public synchronized GitOpExecution updateGitExecutionState(Project project, Users hopsworksUser,
                                                 GitCommandExecutionStateUpdateDTO stateDTO, Integer repositoryId,
                                                 Integer executionId)
       throws IllegalArgumentException, GitOpException {
@@ -191,31 +192,45 @@ public GitOpExecution updateGitExecutionState(Project project, Users hopsworksUs
     }
     LOGGER.log(Level.INFO, "Updating execution, Id = " + executionId + " to " + newState.getExecutionState());
     GitRepository repository = commandConfigurationValidator.verifyRepository(project, hopsworksUser, repositoryId);
-    GitOpExecution exec = getExecutionInRepository(repository, executionId);
-    exec.setCommandResultMessage(stateDTO.getMessage());
+    GitOpExecution execution = getExecutionInRepository(repository, executionId);
     if (newState.isFinalState()) {
       if (newState == GitOpExecutionState.SUCCESS) {
-        //Every successful operation should update the repository current commit and branch
-        repository.setCurrentBranch(stateDTO.getBranch());
-        repository.setCurrentCommit(stateDTO.getCommitHash());
-        GitCommandConfiguration executedCommandConfig = exec.getGitCommandConfiguration();
-        if (executedCommandConfig.getCommandType() == GitCommandType.DELETE_BRANCH) {
-          //if we deleted a branch then we should also delete all the commits for this branch
-          gitCommitsFacade.deleteAllInBranchAndRepository(executedCommandConfig.getBranchName(), repository);
-        }
-        if (executedCommandConfig.getCommandType() == GitCommandType.ADD_REMOTE ||
-            executedCommandConfig.getCommandType() == GitCommandType.DELETE_REMOTE) {
-          //Update the remotes which are in the execution final message
-          String remotesJson = exec.getCommandResultMessage();
-          if (!Strings.isNullOrEmpty(remotesJson)) {
-            gitRepositoryRemotesFacade.updateRepositoryRemotes(gitCommandOperationUtil.convertToRemote(repository,
-                remotesJson), repository);
-          }
-        }
+        finalizeSuccessfulExecution(execution, stateDTO);
+      }
+      if (newState == GitOpExecutionState.CANCELLED) {
+        gitCommandExecutor.cancelGitExecution(execution,  stateDTO.getMessage());
+        return execution;
+      }
+      execution = getExecutionInRepository(repository, executionId);
+      if (execution.getState() != GitOpExecutionState.CANCELLED) {
+        gitRepositoryFacade.updateRepositoryCid(repository, null);
+        gitCommandOperationUtil.cleanUp(project, hopsworksUser, execution.getConfigSecret());
+      }
+    }
+    if (execution.getState() == GitOpExecutionState.CANCELLED) {
+      return execution;
+    }
+    return gitOpExecutionFacade.updateState(execution, newState, stateDTO.getMessage());
+  }
+
+  private void finalizeSuccessfulExecution(GitOpExecution execution, GitCommandExecutionStateUpdateDTO stateDTO) {
+    //Every successful operation should update the repository current commit and branch
+    GitRepository repository = execution.getRepository();
+    repository.setCurrentBranch(stateDTO.getBranch());
+    repository.setCurrentCommit(stateDTO.getCommitHash());
+    GitCommandConfiguration executedCommandConfig = execution.getGitCommandConfiguration();
+    if (executedCommandConfig.getCommandType() == GitCommandType.DELETE_BRANCH) {
+      //if we deleted a branch then we should also delete all the commits for this branch
+      gitCommitsFacade.deleteAllInBranchAndRepository(executedCommandConfig.getBranchName(), repository);
+    }
+    if (executedCommandConfig.getCommandType() == GitCommandType.ADD_REMOTE ||
+        executedCommandConfig.getCommandType() == GitCommandType.DELETE_REMOTE) {
+      //Update the remotes which are in the execution final message
+      String remotesJson = execution.getCommandResultMessage();
+      if (!Strings.isNullOrEmpty(remotesJson)) {
+        gitRepositoryRemotesFacade.updateRepositoryRemotes(gitCommandOperationUtil.convertToRemote(repository,
+            remotesJson), repository);
       }
-      gitRepositoryFacade.updateRepositoryCid(repository, null);
-      gitCommandOperationUtil.cleanUp(project, hopsworksUser, exec.getConfigSecret());
     }
-    return gitOpExecutionFacade.updateState(exec, newState, stateDTO.getMessage());
   }
 }

hopsworks-common/src/main/java/io/hops/hopsworks/common/git/util/GitCommandOperationUtil.java

@@ -15,6 +15,7 @@
  */
 package io.hops.hopsworks.common.git.util;
 
+import io.hops.hopsworks.common.dao.git.GitOpExecutionFacade;
 import io.hops.hopsworks.common.dao.git.GitPaths;
 import io.hops.hopsworks.common.dao.git.GitRepositoryFacade;
 import io.hops.hopsworks.common.git.BasicAuthSecrets;
@@ -40,6 +41,7 @@
 import org.json.JSONArray;
 import org.json.JSONException;
 import org.json.JSONObject;
+import org.opensearch.common.Strings;
 
 import javax.ejb.EJB;
 import javax.ejb.Stateless;
@@ -66,9 +68,15 @@ public class GitCommandOperationUtil {
   public static final String COMMAND_LOG_FILE_NAME = "command_output.log";
   public static final String HOPSFS_MOUNT_LOG_FILE_NAME = "hopsfs_mount.log";
 
+  //we want to put some information to elastic via the logfile name. This is seperator for the attributes.
+  // Will work if the repository name will not contain this pattern
+  private static final String LOG_ATTRIBUTES_SEPERATOR = "--s--";
+
   @EJB
   private GitRepositoryFacade gitRepositoryFacade;
   @EJB
+  private GitOpExecutionFacade executionFacade;
+  @EJB
   private CertificateMaterializer certificateMaterializer;
   @EJB
   private Settings settings;
@@ -92,9 +100,13 @@ public void cleanUp(Project project, Users user, String configSecret) {
     FileUtils.deleteQuietly(new File(gitHomePath));
   }
 
-  public String getLogFileFullPath(String logDirPath, String hdfsUsername, Integer executionId,
+  public String getLogFileFullPath(GitOpExecution execution, String logDirPath, String hdfsUsername,
                                    String logFileName) {
-    return logDirPath + File.separator + hdfsUsername + "-" + executionId + "-" + logFileName;
+    return logDirPath + File.separator + hdfsUsername
+        + LOG_ATTRIBUTES_SEPERATOR  + execution.getId()
+        + LOG_ATTRIBUTES_SEPERATOR + execution.getRepository().getName()
+        + LOG_ATTRIBUTES_SEPERATOR + execution.getGitCommandConfiguration().getCommandType()
+        + LOG_ATTRIBUTES_SEPERATOR + logFileName;
   }
 
   public void generatePaths(GitPaths gitPaths) throws GitOpException {
@@ -129,6 +141,8 @@ public void generatePaths(GitPaths gitPaths) throws GitOpException {
       new File(gitPaths.getRunDirPath()).mkdirs();
       new File(gitPaths.getTokenPath()).mkdirs();
       new File(gitPaths.getConfDirPath()).mkdirs();
+
+      Files.setPosixFilePermissions(Paths.get(gitPaths.getLogDirPath()), perms);
     } catch (IOException ex) {
       removeProjectUserDirRecursive(gitPaths);
       throw new GitOpException(RESTCodes.GitOpErrorCode.GIT_PATHS_CREATION_ERROR, Level.SEVERE, "Failed to " +
@@ -229,16 +243,25 @@ public void shutdownCommandService(GitRepository repository, GitOpExecution exec
     } catch (Exception e) {
       LOGGER.log(Level.SEVERE, "Failed to update repository pid", e);
     }
+    killGitContainer(execution, cid);
+    cleanUp(repository.getProject(), execution.getUser(), getGitHome(execution.getConfigSecret()));
+  }
+
+  public void killGitContainer(GitOpExecution execution, String containerId) {
+    if (Strings.isNullOrEmpty(containerId)) {
+      return;
+    }
     String gitHomePath = getGitHome(execution.getConfigSecret());
-    String hdfsUsername = hdfsUsersController.getHdfsUserName(repository.getProject(), execution.getUser());
+    String hdfsUsername = hdfsUsersController.getHdfsUserName(execution.getRepository().getProject(),
+        execution.getUser());
     String prog = settings.getSudoersDir() + "/git.sh";
     int exitValue = 0;
     ProcessDescriptor.Builder pdBuilder = new ProcessDescriptor.Builder()
         .addCommand("/usr/bin/sudo")
         .addCommand(prog)
         .addCommand("kill")
         .addCommand(gitHomePath)
-        .addCommand(cid)
+        .addCommand(containerId)
         .addCommand(hdfsUsername)
         .redirectErrorStream(true)
         .setWaitTimeout(10L, TimeUnit.SECONDS);
@@ -255,7 +278,6 @@ public void shutdownCommandService(GitRepository repository, GitOpExecution exec
           "Exited with " + exitValue + "Failed to shutdown git container executing command for user "
               + hdfsUsername);
     }
-    cleanUp(repository.getProject(), execution.getUser(), gitHomePath);
   }
 
   public String getGitHome(String secret) {

hopsworks-common/src/main/java/io/hops/hopsworks/common/git/util/GitContainerArgumentsWriter.java

@@ -93,10 +93,10 @@ private GitContainerLaunchScriptArgumentsTemplate setUpTemplate(GitOpExecution g
     String hdfsUser = hdfsUsersController.getHdfsUserName(gitOpExecution.getRepository().getProject(),
         gitOpExecution.getUser());
     //Separate log file names for git logs
-    String fullCommandLogFilePath = gitCommandOperationUtil.getLogFileFullPath(gitPaths.getLogDirPath(), hdfsUser,
-        gitOpExecution.getId(), GitCommandOperationUtil.COMMAND_LOG_FILE_NAME);
-    String fullHopsfsMountLogFilePath = gitCommandOperationUtil.getLogFileFullPath(gitPaths.getLogDirPath(), hdfsUser,
-        gitOpExecution.getId(), GitCommandOperationUtil.HOPSFS_MOUNT_LOG_FILE_NAME);
+    String fullCommandLogFilePath = gitCommandOperationUtil.getLogFileFullPath(gitOpExecution, gitPaths.getLogDirPath(),
+        hdfsUser, GitCommandOperationUtil.COMMAND_LOG_FILE_NAME);
+    String fullHopsfsMountLogFilePath = gitCommandOperationUtil.getLogFileFullPath(gitOpExecution,
+        gitPaths.getLogDirPath(), hdfsUser, GitCommandOperationUtil.HOPSFS_MOUNT_LOG_FILE_NAME);
 
     String gitCommand = gitOpExecution.getGitCommandConfiguration().getCommandType().getGitCommand();
     return GitContainerLaunchScriptArgumentTemplateBuilder.newBuilder()

hopsworks-common/src/main/java/io/hops/hopsworks/common/opensearch/OpenSearchCleaner.java

@@ -110,6 +110,7 @@ public void deleteLogIndices(Timer timer) {
       String indexRegex = "(" + Settings.OPENSEARCH_LOG_INDEX_REGEX + ")" +
           "|(" + Settings.OPENSEARCH_SERVING_INDEX_REGEX + ")" +
           "|(" + Settings.OPENSEARCH_SERVICES_INDEX_REGEX + ")" +
+          "|(" + Settings.OPENSEARCH_GIT_INDEX_REGEX + ")" +
           "|(" + Settings.OPENSEARCH_ONLINEFS_INDEX_REGEX + ")";
       Map<String, Long> indices = elasticClientCtrl.mngIndicesGetByRegex(indexRegex, creationTimeParser);
       for (String index : indices.keySet()) {

hopsworks-common/src/main/java/io/hops/hopsworks/common/opensearch/OpenSearchController.java

@@ -373,7 +373,7 @@ public void createIndexPattern(Project project, Users user, String pattern)
   public void deleteProjectIndices(Project project) throws OpenSearchException {
     //Get all project indices
     String[] indices = elasticClientCtrl.mngIndicesGetByRegex(new String[]{project.getName() +
-        "_(((logs|serving)-\\d{4}.\\d{2}.\\d{2}))", "(((onlinefs)_" + project.getId() + "-\\d{4}.\\d{2}.\\d{2}))"});
+        "_(((logs|serving|git)-\\d{4}.\\d{2}.\\d{2}))", "(((onlinefs)_" + project.getId() + "-\\d{4}.\\d{2}.\\d{2}))"});
     for (String index : indices) {
       try {
         elasticClientCtrl.mngIndexDelete(index);

hopsworks-common/src/main/java/io/hops/hopsworks/common/project/ProjectController.java

@@ -2297,6 +2297,7 @@ public void addKibana(Project project, Users user) throws ProjectException {
 
     try {
       openSearchController.createIndexPattern(project, user, projectName + Settings.OPENSEARCH_LOGS_INDEX_PATTERN);
+      openSearchController.createIndexPattern(project, user, projectName + Settings.OPENSEARCH_GIT_INDEX_PATTERN);
     } catch (OpenSearchException e) {
       throw new ProjectException(RESTCodes.ProjectErrorCode.PROJECT_KIBANA_CREATE_INDEX_ERROR, Level.SEVERE, "Could " +
         "provision project on Kibana. Contact an administrator if problem persists. Reason: " + e.getUsrMsg(),

hopsworks-common/src/main/java/io/hops/hopsworks/common/util/Settings.java

@@ -1983,6 +1983,8 @@ public String getHopsworksPublicHost() {
   public static final String OPENSEARCH_PYPI_LIBRARIES_INDEX_PATTERN_PREFIX = "pypi_libraries_";
   public static final String OPENSEARCH_LOGS_INDEX_PATTERN = "_" + Settings.OPENSEARCH_LOGS_INDEX + "-*";
   public static final String OPENSEARCH_SERVING_INDEX = "serving";
+
+  public static final String OPENSEARCH_GIT_INDEX = "git";
   public static final String OPENSEARCH_SERVICES_INDEX = ".services";
   public static final String OPEN_SEARCH_ONLINEFS_INDEX = "onlinefs";
 
@@ -1992,6 +1994,8 @@ public String getHopsworksPublicHost() {
   public static final String OPENSEARCH_SERVICES_INDEX_REGEX = OPENSEARCH_SERVICES_INDEX + "-\\d{4}.\\d{2}.\\d{2}";
   public static final String OPENSEARCH_PYPI_LIBRARIES_INDEX_REGEX =
     OPENSEARCH_PYPI_LIBRARIES_INDEX_PATTERN_PREFIX + "*";
+  public static final String OPENSEARCH_GIT_INDEX_REGEX = ".*_" + OPENSEARCH_GIT_INDEX + "-\\d{4}.\\d{2}.\\d{2}";
+  public static final String OPENSEARCH_GIT_INDEX_PATTERN = "_" + OPENSEARCH_GIT_INDEX + "-*";
   public static final String OPENSEARCH_ONLINEFS_INDEX_REGEX =
       OPEN_SEARCH_ONLINEFS_INDEX + "_\\d+-\\d{4}.\\d{2}.\\d{2}";
 

hopsworks-persistence/src/main/java/io/hops/hopsworks/persistence/entity/git/config/GitOpExecutionState.java

@@ -38,7 +38,9 @@ public enum GitOpExecutionState {
   @XmlEnumValue("Submitted")
   SUBMITTED("Submitted"),
   @XmlEnumValue("Timedout")
-  TIMEDOUT("Timedout");
+  TIMEDOUT("Timedout"),
+  @XmlEnumValue("Cancelled")
+  CANCELLED("Cancelled");
 
   private final String executionState;
 
@@ -66,6 +68,7 @@ public boolean isFinalState() {
       case KILLED:
       case INITIALIZATION_FAILED:
       case TIMEDOUT:
+      case CANCELLED:
         return true;
       default:
         return false;
@@ -74,6 +77,6 @@ public boolean isFinalState() {
 
   public static Set<GitOpExecutionState> getFinalStates() {
     return EnumSet.
-        of(SUCCESS, FAILED, KILLED, INITIALIZATION_FAILED, TIMEDOUT);
+        of(SUCCESS, FAILED, KILLED, INITIALIZATION_FAILED, TIMEDOUT, CANCELLED);
   }
 }

hopsworks-rest-utils/src/main/java/io/hops/hopsworks/restutils/RESTCodes.java

@@ -2413,7 +2413,8 @@ public enum GitOpErrorCode implements RESTErrorCode {
             Response.Status.FORBIDDEN),
     READ_ONLY_REPOSITORY(36, "Repository is read only", Response.Status.BAD_REQUEST),
     ERROR_VALIDATING_REPOSITORY_PATH(37, "Error validating git repository path",
-        Response.Status.INTERNAL_SERVER_ERROR);
+        Response.Status.INTERNAL_SERVER_ERROR),
+    ERROR_CANCELLING_GIT_EXECUTION(38, "Failed to cancel git execution", Response.Status.BAD_REQUEST);
     private Integer code;
     private String message;
     private Response.Status respStatus;