Docker tools timeout redux

Dockerfile

@@ -96,7 +96,7 @@ ENV CLASSPATH /usr/local/tomcat/bin/bootstrap.jar:/usr/local/tomcat/bin/tomcat-j
 COPY docker/logging.properties /usr/local/tomcat/conf/logging.properties
 RUN sed -i -e 's/Valve/Disabled/' /usr/local/tomcat/conf/server.xml
 
-# add our scripts
+# add our scripts and configuration
 COPY docker /scripts
 RUN chmod -R +x /scripts
 

docker/start.py

@@ -76,6 +76,7 @@
 OPENGROK_JAR = os.path.join(OPENGROK_LIB_DIR, 'opengrok.jar')
 
 NOMIRROR_ENV_NAME = 'NOMIRROR'
+API_TIMEOUT_ENV_NAME = 'API_TIMEOUT'
 
 expected_token = None
 periodic_timer = None
@@ -481,16 +482,16 @@ def main():
         setup_redirect_source(logger, url_root)
 
     api_timeout = 8
-    if os.environ.get('API_TIMEOUT'):
-        api_timeout = int(os.environ.get('API_TIMEOUT'))
-    extra_indexer_options = "--connectTimeout " + str(api_timeout)
+    if os.environ.get(API_TIMEOUT_ENV_NAME):
+        api_timeout = int(os.environ.get(API_TIMEOUT_ENV_NAME))
+    else:
+        os.environ[API_TIMEOUT_ENV_NAME] = str(api_timeout)
 
     env = {}
-    indexer_opt = os.environ.get('INDEXER_OPT', '')
-    if indexer_opt:
-        extra_indexer_options += " " + indexer_opt
-    logger.info("extra indexer options: '{}'".format(extra_indexer_options))
-    env['OPENGROK_INDEXER_OPTIONAL_ARGS'] = extra_indexer_options
+    extra_indexer_options = os.environ.get('INDEXER_OPT', '')
+    if extra_indexer_options:
+        logger.info("extra indexer options: '{}'".format(extra_indexer_options))
+        env['OPENGROK_INDEXER_OPTIONAL_ARGS'] = extra_indexer_options
 
     if os.environ.get(NOMIRROR_ENV_NAME):
         env[OPENGROK_NO_MIRROR_ENV] = os.environ.get(NOMIRROR_ENV_NAME)

docker/sync.yml

@@ -7,12 +7,17 @@ commands:
       duration: PT1H
       tags: ['%PROJECT%']
       text: resync + reindex in progress
-- command: [opengrok-mirror, -c, '/opengrok/etc/mirror.yml', -I, -U, '%URL%', '%PROJECT%']
-- command: [opengrok-reindex-project, --printoutput,
-    --jar, /opengrok/lib/opengrok.jar, -U, '%URL%', -P, '%PROJECT%', --,
-    -r, dirbased, -G, -m, '256', --leadingWildCards, 'on',
-    -c, /usr/local/bin/ctags, -U, '%URL%', -H, '%PROJECT%']
-  limits: {RLIMIT_NOFILE: 1024}
+- command:
+    args: [opengrok-mirror, -c, '/opengrok/etc/mirror.yml', -I, -U, '%URL%', '%PROJECT%']
+- command:
+    args: [opengrok-reindex-project, --printoutput,
+      --api_timeout, '%API_TIMEOUT%',
+      --jar, /opengrok/lib/opengrok.jar, -U, '%URL%', -P, '%PROJECT%', --,
+      --connectTimeout, '%API_TIMEOUT%',
+      -r, dirbased, -G, -m, '256', --leadingWildCards, 'on',
+      -c, /usr/local/bin/ctags, -U, '%URL%', -H, '%PROJECT%']
+    limits: {RLIMIT_NOFILE: 1024}
+    args_subst: {"%API_TIMEOUT%": "$API_TIMEOUT"}
 - call:
     uri: '%URL%api/v1/messages?tag=%PROJECT%'
     method: DELETE

tools/src/main/python/opengrok_tools/sync.py

@@ -19,7 +19,7 @@
 #
 
 #
-# Copyright (c) 2017, 2022, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2017, 2023, Oracle and/or its affiliates. All rights reserved.
 #
 
 """

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

@@ -18,7 +18,7 @@
 #
 
 #
-# Copyright (c) 2017, 2022, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2017, 2023, Oracle and/or its affiliates. All rights reserved.
 #
 
 import logging
@@ -297,7 +297,7 @@ def close(self):
         finally:
             if self.timeout != 0 and timeout_thread:
                 with time_condition:
-                    time_condition.notifyAll()
+                    time_condition.notify_all()
 
             # The subprocess module does not close the write pipe descriptor
             # it fetched via OutputThread's fileno() so in order to gracefully
@@ -344,10 +344,14 @@ def fill_arg(self, args_append=None, args_subst=None):
                 newarg = cmdarg
                 for pattern in args_subst.keys():
                     if pattern in newarg and args_subst[pattern]:
+                        value = args_subst[pattern]
+                        if value.startswith("$"):
+                            self.logger.debug(f"treating {value} as environment variable")
+                            value = os.environ.get(value[1:], "")
                         self.logger.debug("replacing '{}' in '{}' with '{}'".
                                           format(pattern, newarg,
-                                                 args_subst[pattern]))
-                        newarg = newarg.replace(pattern, args_subst[pattern])
+                                                 value))
+                        newarg = newarg.replace(pattern, value)
                         self.logger.debug("replaced argument with {}".
                                           format(newarg))
                         subst_done = i

tools/src/main/python/opengrok_tools/utils/commandsequence.py

@@ -18,7 +18,7 @@
 #
 
 #
-# Copyright (c) 2017, 2022, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2017, 2023, Oracle and/or its affiliates. All rights reserved.
 #
 
 import logging
@@ -41,6 +41,10 @@
 HEADERS_PROPERTY = "headers"
 METHOD_PROPERTY = "method"
 URI_PROPERTY = "uri"
+ARGS_SUBST_PROPERTY = "args_subst"
+LIMITS_PROPERTY = "limits"
+ENV_PROPERTY = "env"
+ARGS_PROPERTY = "args"
 
 
 class CommandConfigurationException(Exception):
@@ -97,8 +101,8 @@ def check_command_property(command):
     if command_value is None and call_value is None:
         raise CommandConfigurationException(f"command dictionary has unknown key: {command}")
 
-    if command_value and not isinstance(command_value, list):
-        raise CommandConfigurationException("command value not a list: {}".
+    if command_value and not isinstance(command_value, dict):
+        raise CommandConfigurationException("command value not a dictionary: {}".
                                             format(command_value))
     if call_value:
         check_call_config(call_value)
@@ -179,6 +183,9 @@ def __init__(self, name, commands, loglevel=logging.INFO, cleanup=None,
 
         self.url = url
 
+        self.args_subst = {PROJECT_SUBST: self.name,
+                           URL_SUBST: self.url}
+
     def __str__(self):
         return str(self.name)
 
@@ -245,8 +252,7 @@ def run(self):
             if command.get(CALL_PROPERTY):
                 try:
                     call_rest_api(ApiCall(command.get(CALL_PROPERTY)),
-                                  {PROJECT_SUBST: self.name,
-                                   URL_SUBST: self.url},
+                                  self.args_subst,
                                   self.http_headers,
                                   self.api_timeout,
                                   self.async_api_timeout)
@@ -258,13 +264,16 @@ def run(self):
 
                     break
             elif command.get(COMMAND_PROPERTY):
-                command_args = command.get(COMMAND_PROPERTY)
+                command = command.get(COMMAND_PROPERTY)
+                command_args = command.get(ARGS_PROPERTY)
+                args_subst = self.args_subst
+                if command.get(ARGS_SUBST_PROPERTY):
+                    args_subst.update(command.get(ARGS_SUBST_PROPERTY))
                 command = Command(command_args,
-                                  env_vars=command.get("env"),
+                                  env_vars=command.get(ENV_PROPERTY),
                                   logger=self.logger,
-                                  resource_limits=command.get("limits"),
-                                  args_subst={PROJECT_SUBST: self.name,
-                                              URL_SUBST: self.url},
+                                  resource_limits=command.get(LIMITS_PROPERTY),
+                                  args_subst=args_subst,
                                   args_append=[self.name], excl_subst=True)
                 ret_code = self.run_command(command)
 
@@ -316,13 +325,13 @@ def run_cleanup(self):
                     self.logger.error("API call {} failed: {}".
                                       format(cleanup_cmd, e))
             elif cleanup_cmd.get(COMMAND_PROPERTY):
-                command_args = cleanup_cmd.get(COMMAND_PROPERTY)
+                cleanup_cmd = cleanup_cmd.get(COMMAND_PROPERTY)
+                command_args = cleanup_cmd.get(ARGS_PROPERTY)
                 self.logger.debug("Running cleanup command '{}'".
                                   format(command_args))
                 cmd = Command(command_args,
                               logger=self.logger,
-                              args_subst={PROJECT_SUBST: self.name,
-                                          URL_SUBST: self.url},
+                              args_subst=self.args_subst,
                               args_append=[self.name], excl_subst=True)
                 cmd.execute()
                 if cmd.getretcode() != SUCCESS_EXITVAL:

tools/src/test/python/test_command.py

@@ -20,7 +20,7 @@
 #
 
 #
-# Copyright (c) 2017, 2020, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2017, 2023, Oracle and/or its affiliates. All rights reserved.
 # Portions Copyright (c) 2020, Krystof Tulinger <[email protected]>
 #
 
@@ -75,6 +75,17 @@ def test_subst_multiple():
     assert cmd.cmd == ['foo', 'aaablahbbbxyzccc', 'bar']
 
 
+def test_subst_env():
+    """
+    Test substitution with value treated as environment variable.
+    """
+    os.environ["FOO"] = "foo"
+    cmd = Command(['foo', 'aaa%ARG%bbb', 'bar'],
+                  args_subst={"%ARG%": "$FOO"})
+    assert cmd.cmd == ['foo', 'aaafoobbb', 'bar']
+    os.environ.pop("FOO")
+
+
 # On Windows the return code is actually 1.
 @pytest.mark.skipif(platform.system() == 'Windows',
                     reason="broken on Windows")