fix timeout inheritance in mirror.py (#3025)

fixes #3022

Author: Vladimir Kotal

opengrok-tools/src/main/python/opengrok_tools/utils/command.py

@@ -18,7 +18,7 @@
 #
 
 #
-# Copyright (c) 2017, 2019, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2017, 2020, Oracle and/or its affiliates. All rights reserved.
 #
 
 import logging
@@ -105,6 +105,7 @@ def terminate(self, p):
                 """
                 Make sure the process goes away.
                 """
+                self.logger.info("Terminating PID {}".format(p.pid))
                 p.terminate()
 
                 # The following code tries more methods to terminate

opengrok-tools/src/main/python/opengrok_tools/utils/mirror.py

@@ -19,7 +19,7 @@
 #
 
 #
-# Copyright (c) 2018, 2019, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2018, 2020, Oracle and/or its affiliates. All rights reserved.
 #
 
 import re
@@ -99,7 +99,7 @@ def get_repos_for_project(project_name, ignored_repos, uri, source_root,
                                   kwargs[COMMANDS_PROPERTY],
                                   kwargs[PROXY_PROPERTY],
                                   None,
-                                  kwargs['command_timeout'])
+                                  kwargs[CMD_TIMEOUT_PROPERTY])
         except (RepositoryException, OSError) as e:
             logger.error("Cannot get repository for {}: {}".
                          format(repo_path, e))
@@ -339,6 +339,11 @@ def mirror_project(config, project_name, check_changes, uri,
                                                project_name,
                                                config.get(HOOKDIR_PROPERTY))
 
+    if not command_timeout:
+        command_timeout = config.get(CMD_TIMEOUT_PROPERTY)
+    if not hook_timeout:
+        hook_timeout = config.get(HOOK_TIMEOUT_PROPERTY)
+
     proxy = None
     if use_proxy:
         proxy = config.get(PROXY_PROPERTY)

opengrok-tools/src/test/python/test_mirror.py

@@ -20,7 +20,7 @@
 #
 
 #
-# Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2019, 2020, Oracle and/or its affiliates. All rights reserved.
 #
 
 
@@ -37,10 +37,11 @@
 from opengrok_tools.utils.mirror import check_project_configuration, \
     check_configuration, mirror_project, run_command, \
     HOOKS_PROPERTY, PROXY_PROPERTY, IGNORED_REPOS_PROPERTY, \
-    PROJECTS_PROPERTY, DISABLED_CMD_PROPERTY, DISABLED_PROPERTY
+    PROJECTS_PROPERTY, DISABLED_CMD_PROPERTY, DISABLED_PROPERTY, \
+    CMD_TIMEOUT_PROPERTY, HOOK_TIMEOUT_PROPERTY
 import opengrok_tools.mirror
 from opengrok_tools.utils.exitvals import (
-    CONTINUE_EXITVAL,
+    CONTINUE_EXITVAL, FAILURE_EXITVAL
 )
 from opengrok_tools.utils.patterns import COMMAND_PROPERTY, PROJECT_SUBST
 from opengrok_tools.utils.command import Command
@@ -219,3 +220,63 @@ def test_disabled_command_run_args():
     project_name = "foo"
     run_command(cmd, project_name)
     verify(cmd).execute()
+
+
+def test_mirror_project_timeout(monkeypatch):
+    """
+    Test mirror_project() timeout inheritance/override from global
+    configuration to get_repos_for_project(). The test merely verifies
+    that the timeout values are passed between the expected functions,
+    not whether it actually affects the execution.
+    """
+    cmd_timeout = 3
+    hook_timeout = 4
+
+    def mock_get_repos(*args, **kwargs):
+        mock_get_repos.called = True
+
+        assert kwargs[CMD_TIMEOUT_PROPERTY] == cmd_timeout
+
+        # Technically this function should return list of Repository objects
+        # however for this test this is not necessary.
+        return ['foo']
+
+    def mock_process_hook(hook_ident, hook, source_root, project_name_arg,
+                          proxy, hook_timeout_arg):
+
+        assert hook_timeout_arg == hook_timeout
+
+        # We want to terminate mirror_project() once this function runs.
+        # This way mirror_project() will return FAILURE_EXITVAL
+        return False
+
+    def test_mirror_project(config):
+        retval = mirror_project(config, project_name, False,
+                                "http://localhost:8080/source", "srcroot")
+        assert retval == FAILURE_EXITVAL
+
+        # TODO: is there better way to ensure that get_repos_for_project()
+        #       was actually called ?
+        assert mock_get_repos.called
+
+    with monkeypatch.context() as m:
+        mock_get_repos.called = False
+        m.setattr("opengrok_tools.utils.mirror.get_repos_for_project",
+                  mock_get_repos)
+        m.setattr("opengrok_tools.utils.mirror.process_hook",
+                  mock_process_hook)
+
+        project_name = "foo"
+        # override testing
+        global_config_1 = {PROJECTS_PROPERTY:
+                           {project_name:
+                            {CMD_TIMEOUT_PROPERTY: cmd_timeout,
+                             HOOK_TIMEOUT_PROPERTY: hook_timeout}},
+                           CMD_TIMEOUT_PROPERTY: cmd_timeout * 2,
+                           HOOK_TIMEOUT_PROPERTY: hook_timeout * 2}
+        # inheritance testing
+        global_config_2 = {CMD_TIMEOUT_PROPERTY: cmd_timeout,
+                           HOOK_TIMEOUT_PROPERTY: hook_timeout}
+
+        test_mirror_project(global_config_1)
+        test_mirror_project(global_config_2)

opengrok-tools/src/test/python/test_repofactory.py

@@ -0,0 +1,47 @@
+#!/usr/bin/env python3
+
+#
+# CDDL HEADER START
+#
+# The contents of this file are subject to the terms of the
+# Common Development and Distribution License (the "License").
+# You may not use this file except in compliance with the License.
+#
+# See LICENSE.txt included in this distribution for the specific
+# language governing permissions and limitations under the License.
+#
+# When distributing Covered Code, include this CDDL HEADER in each
+# file and include the License file at LICENSE.txt.
+# If applicable, add the following below this CDDL HEADER, with the
+# fields enclosed by brackets "[]" replaced with your own identifying
+# information: Portions Copyright [yyyy] [name of copyright owner]
+#
+# CDDL HEADER END
+#
+
+#
+# Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
+#
+
+import os
+import tempfile
+from git import Repo
+
+from opengrok_tools.scm import GitRepository
+from opengrok_tools.scm.repofactory import get_repository
+
+
+def test_repofactory_timeout():
+    with tempfile.TemporaryDirectory() as source_root:
+        repo_name = "foo"
+        repo_path = os.path.join(source_root, repo_name)
+        Repo.init(repo_path)
+        timeout = 3
+
+        project_name = "foo"    # does not matter for this test
+        repo = get_repository(repo_path,
+                              "git", project_name,
+                              None, None, None, timeout)
+        assert repo is not None
+        assert isinstance(repo, GitRepository)
+        assert repo.timeout == timeout