Repository: cekit/cekit

Issue:
Allow to override the default timeout for OSBS builds
**Describe the solution you'd like**
There are some cases where the OSBS build can take a long time, specially when there is a long queue on the system.

It seems currently there is one non-documented `DOCKER_TIMEOUT` environment variable timeout, but there is nothing related to OSBS.

We could add environment variables `OSBS_TIMEOUT` / `ODCS_TIMEOUT` to match the existing `DOCKER_TIMEOUT` and also document the usage.

Discussion:
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/cekit/cekit/pull/913?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cekit) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 90.10%. Comparing base [(`962949a`)](https://app.codecov.io/gh/cekit/cekit/commit/962949a4ef3f0182d6dcc3d0f3e52db02539a962?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cekit) to head [(`7d563c4`)](https://app.codecov.io/gh/cekit/cekit/pull/913?dropdown=coverage&src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cekit).


<details><summary>Additional details and impacted files</summary>


```diff
@@             Coverage Diff             @@
##           develop     #913      +/-   ##
===========================================
- Coverage    90.15%   90.10%   -0.06%     
===========================================
  Files           42       42              
  Lines         3252     3255       +3     
===========================================
+ Hits          2932     2933       +1     
- Misses         320      322       +2     
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/cekit/cekit/pull/913?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cekit).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cekit).


Patch:
diff --git a/cekit/builders/docker_builder.py b/cekit/builders/docker_builder.py
index 1623160..e6b5b95 100644
--- a/cekit/builders/docker_builder.py
+++ b/cekit/builders/docker_builder.py
@@ -6,6 +6,7 @@ import traceback
 from cekit.builder import Builder
 from cekit.cekit_types import DependencyDefinition
 from cekit.errors import CekitError
+from cekit.tools import parse_env_timeout
 
 LOGGER = logging.getLogger("cekit")
 
@@ -179,21 +180,7 @@ class DockerBuilder(Builder):
         # Default Docker daemon connection timeout 10 minutes
         # It needs to be high enough to allow Docker daemon to export the
         # image for squashing.
-        try:
-            timeout = int(os.getenv("DOCKER_TIMEOUT", "600"))
-        except ValueError:
-            raise CekitError(
-                "Provided timeout value: '{}' cannot be parsed as integer, exiting.".format(
-                    os.getenv("DOCKER_TIMEOUT")
-                )
-            )
-
-        if timeout <= 0:
-            raise CekitError(
-                "Provided timeout value needs to be greater than zero, currently: '{}', exiting.".format(
-                    timeout
-                )
-            )
+        timeout = parse_env_timeout("DOCKER_TIMEOUT", "600")
 
         params = {"version": DOCKER_API_VERSION}
         params.update(docker.utils.kwargs_from_env())
diff --git a/cekit/builders/osbs.py b/cekit/builders/osbs.py
index e59c759..d9250c0 100644
--- a/cekit/builders/osbs.py
+++ b/cekit/builders/osbs.py
@@ -23,7 +23,7 @@ from cekit.descriptor.resource import (
     _UrlResource,
 )
 from cekit.errors import CekitError
-from cekit.tools import Chdir, copy_recursively, run_wrapper
+from cekit.tools import Chdir, copy_recursively, parse_env_timeout, run_wrapper
 
 if TYPE_CHECKING:
     from cekit.descriptor.osbs import Repository
@@ -182,9 +182,7 @@ class OSBSBuilder(Builder):
             else:
                 LOGGER.info("No changes made to the code, committing skipped")
 
-    def _wait_for_osbs_task(
-        self, task_id: str, current_time: int = 0, timeout: int = 7200
-    ):
+    def _wait_for_osbs_task(self, task_id: str, timeout: int, current_time: int = 0):
         """Default timeout is 2hrs"""
 
         LOGGER.debug(f"Checking if task {task_id} is finished...")
@@ -231,7 +229,7 @@ class OSBSBuilder(Builder):
         ):
             # It's not necessary to query the API so often
             time.sleep(sleep_time)
-            return self._wait_for_osbs_task(task_id, current_time + sleep_time, timeout)
+            return self._wait_for_osbs_task(task_id, timeout, current_time + sleep_time)
 
         # In all other cases (failed, cancelled) task did not finish successfully
         raise CekitError(
@@ -335,7 +333,9 @@ class OSBSBuilder(Builder):
                 if self.params.nowait:
                     return
 
-                self._wait_for_osbs_task(task_id)
+                self._wait_for_osbs_task(
+                    task_id, timeout=parse_env_timeout("OSBS_TIMEOUT", "7200")
+                )
 
                 LOGGER.info("Image was built successfully in OSBS!")
 
diff --git a/cekit/generator/base.py b/cekit/generator/base.py
index f5d9b5e..1dce90a 100644
--- a/cekit/generator/base.py
+++ b/cekit/generator/base.py
@@ -29,7 +29,13 @@ from cekit.errors import CekitError
 from cekit.generator import legacy_version
 from cekit.generator.legacy_version import LegacyVersion
 from cekit.template_helper import TemplateHelper
-from cekit.tools import DependencyDefinition, Map, download_file, load_descriptor
+from cekit.tools import (
+    DependencyDefinition,
+    Map,
+    download_file,
+    load_descriptor,
+    parse_env_timeout,
+)
 from cekit.version import __version__ as cekit_version
 
 if TYPE_CHECKING:
@@ -534,7 +540,9 @@ class Generator(object):
 
         LOGGER.debug(f"Waiting for compose {compose_id} to finish...")
 
-        compose = odcs.wait_for_compose(compose_id, timeout=600)
+        compose = odcs.wait_for_compose(
+            compose_id, timeout=parse_env_timeout("ODCS_TIMEOUT", "600")
+        )
         state = compose.get("state", None)
 
         if not state:
diff --git a/cekit/tools.py b/cekit/tools.py
index ce084ae..997b0b2 100644
--- a/cekit/tools.py
+++ b/cekit/tools.py
@@ -372,6 +372,25 @@ def locate_binary(executable: str) -> str:
     return path
 
 
+def parse_env_timeout(name: str, default: str) -> int:
+    try:
+        timeout = int(os.getenv(name, default))
+    except ValueError:
+        raise CekitError(
+            "Provided timeout value: '{}' cannot be parsed as integer, exiting.".format(
+                os.getenv(name)
+            )
+        )
+
+    if timeout <= 0:
+        raise CekitError(
+            "Provided timeout value needs to be greater than zero, currently: '{}', exiting.".format(
+                timeout
+            )
+        )
+    return timeout
+
+
 class Chdir(object):
     """Context manager for changing the current working directory"""
 
diff --git a/docs/handbook/cli.rst b/docs/handbook/cli.rst
index 8fabe00..376b3d0 100644
--- a/docs/handbook/cli.rst
+++ b/docs/handbook/cli.rst
@@ -10,6 +10,9 @@ The ``cekit`` CLI tool supports the following common options:
 ``-v`` ``--verbose``
     Enable verbose logging
 
+``--trace``
+    Enable trace logging
+
 ``--nocolor``
     Disable color output. This is also implicitly activated when the environment variable ``NO_COLOR`` is set.
 
diff --git a/docs/handbook/configuration.rst b/docs/handbook/configuration.rst
index 8856771..bc77d06 100644
--- a/docs/handbook/configuration.rst
+++ b/docs/handbook/configuration.rst
@@ -1,3 +1,13 @@
+Environment Variables
+======================
+
+There are several environment variables that may be set to override default timeouts:
+
+* ``DOCKER_TIMEOUT`` - represents the Docker daemon connection timeout (default: 600)
+* ``OSBS_TIMEOUT`` - represents the OSBS timeout (default: 7200)
+* ``ODCS_TIMEOUT`` - represents the ODCS compose timeout (default: 600)
+
+
 Configuration file
 =========================
 


Test patch:
diff --git a/tests/test_builder.py b/tests/test_builder.py
index 12cd7fa..74b2de5 100644
--- a/tests/test_builder.py
+++ b/tests/test_builder.py
@@ -253,7 +253,7 @@ def test_osbs_builder_run_brew_stage(mocker):
         ]
     )
 
-    builder._wait_for_osbs_task.assert_called_once_with("12345")
+    builder._wait_for_osbs_task.assert_called_once_with("12345", timeout=7200)
 
 
 def test_osbs_builder_run_brew(mocker):
@@ -312,7 +312,7 @@ def test_osbs_builder_run_brew(mocker):
         ]
     )
 
-    builder._wait_for_osbs_task.assert_called_once_with("12345")
+    builder._wait_for_osbs_task.assert_called_once_with("12345", timeout=7200)
 
 
 def test_osbs_builder_run_koji(mocker):
@@ -371,7 +371,7 @@ def test_osbs_builder_run_koji(mocker):
         ]
     )
 
-    builder._wait_for_osbs_task.assert_called_once_with("12345")
+    builder._wait_for_osbs_task.assert_called_once_with("12345", timeout=7200)
 
 
 def test_osbs_builder_run_brew_nowait(mocker):
@@ -510,7 +510,7 @@ def test_osbs_wait_for_osbs_task_finished_successfully(mocker):
         ],
     )
 
-    assert builder._wait_for_osbs_task("12345") is True
+    assert builder._wait_for_osbs_task("12345", timeout=7200) is True
 
     run.assert_called_with(
         ["brew", "call", "--json-output", "getTaskInfo", "12345"],
@@ -564,7 +564,7 @@ def test_osbs_wait_for_osbs_task_in_progress(mocker):
         ],
     )
 
-    assert builder._wait_for_osbs_task("12345") is True
+    assert builder._wait_for_osbs_task("12345", timeout=7200) is True
 
     run.assert_has_calls(
         [
@@ -618,7 +618,7 @@ def test_osbs_wait_for_osbs_task_failed(mocker):
         CekitError,
         match="Task 12345 did not finish successfully, please check the task logs!",
     ):
-        builder._wait_for_osbs_task("12345")
+        builder._wait_for_osbs_task("12345", timeout=7200)
 
     run.assert_called_with(
         ["brew", "call", "--json-output", "getTaskInfo", "12345"],
diff --git a/tests/test_unit_tools.py b/tests/test_unit_tools.py
index c37538a..0a80de9 100644
--- a/tests/test_unit_tools.py
+++ b/tests/test_unit_tools.py
@@ -946,3 +946,32 @@ def test_locate_binary(caplog):
     assert "/bin/ls" in locate_binary("ls")
     with pytest.raises(CekitError):
         locate_binary("no-ls")
+
+
+@mock.patch.dict(os.environ, {"DOCKER_TIMEOUT": "10"})
+def test_parse_env_timeout():
+    timeout = tools.parse_env_timeout("DOCKER_TIMEOUT", "600")
+    assert timeout == 10
+
+
+def test_parse_env_timeout_2():
+    timeout = tools.parse_env_timeout("DOCKER_TIMEOUT", "600")
+    assert timeout == 600
+
+
+@mock.patch.dict(os.environ, {"DOCKER_TIMEOUT": "timeout"})
+def test_parse_env_timeout_3():
+    with pytest.raises(
+        CekitError,
+        match="cannot be parsed as integer, exiting",
+    ):
+        tools.parse_env_timeout("DOCKER_TIMEOUT", "600")
+
+
+@mock.patch.dict(os.environ, {"OSBS_TIMEOUT": "0"})
+def test_parse_env_timeout_4():
+    with pytest.raises(
+        CekitError,
+        match="Provided timeout value needs to be greater than zero",
+    ):
+        tools.parse_env_timeout("OSBS_TIMEOUT", "600")