Repository: exasol/integration-test-docker-environment (language: python)

Issue:
Replace calls to docker_exec to support SSH access alternatively
Currently ITDE uses `docker_exec` to access the Docker Container, e.g. to analyze the content of some logfiles.  With version 8 and higher the format of the Docker Containers might change so that `docker_exec` is no longer possible. Instead ITDE will then need to use SSH access.

Ticket #301 already requested to detect the version number of the Exasol database inside the Docker Container and to decide whether to use `docker_exec` or SSH access.

The current ticket requests to replace all existing calls to `docker_exec` in ITDE by calls to an abstraction the enables to use either `docker_exec` or SSH access.  The abstraction needs to be aware about the current docker flavor detected in ticket #301  in order to use the appropriate access method.

AC
1. All calls to `docker_exec` are replaced by an abstraction
1. incl. `docker_db_log_based_bucket_sync_checker.py`
1. The abstraction uses the detected flavor of the current Docker Container detected in ticket #301



Discussion / hints:

Fix patch:
diff --git a/doc/changes/changes_2.0.0.md b/doc/changes/changes_2.0.0.md
index 729b5f0bb..219d1351f 100644
--- a/doc/changes/changes_2.0.0.md
+++ b/doc/changes/changes_2.0.0.md
@@ -32,3 +32,4 @@ If you need further versions, please open an issue.
 * #359: Fixed custom logging path not working if dir does not exist.
 * #304: Create SSH access replacements for calls to `docker.exec_run()`
 * #368: Removed versions 7.0.x of Exasol database
+* #305: Replaced calls to `DockerContainer.exec_run()` to support SSH access alternatively 


Test patch:
diff --git a/exasol_integration_test_docker_environment/lib/base/db_os_executor.py b/exasol_integration_test_docker_environment/lib/base/db_os_executor.py
index 7d13a7d7c..46c1c67b6 100644
--- a/exasol_integration_test_docker_environment/lib/base/db_os_executor.py
+++ b/exasol_integration_test_docker_environment/lib/base/db_os_executor.py
@@ -1,6 +1,7 @@
 from abc import abstractmethod
 import fabric
 import docker
+import time
 from docker import DockerClient
 from typing import Protocol, runtime_checkable
 from docker.models.containers import Container, ExecResult
@@ -10,6 +11,7 @@
     .lib.data.database_info import DatabaseInfo
 from exasol_integration_test_docker_environment.lib.docker \
     import ContextDockerClient
+from paramiko.ssh_exception import NoValidConnectionsError
 
 
 class DockerClientFactory:
@@ -39,6 +41,9 @@ class DbOsExecutor(Protocol):
     def exec(self, cmd: str) -> ExecResult:
         ...
 
+    def prepare(self):
+        pass
+
 
 class DockerExecutor(DbOsExecutor):
     def __init__(self, docker_client: DockerClient, container_name: str):
@@ -87,10 +92,22 @@ def __del__(self):
         self.close()
 
     def exec(self, cmd: str) -> ExecResult:
-        result = self._connection.run(cmd)
+        result = self._connection.run(cmd, warn=True, hide=True)
         output = result.stdout.encode("utf-8")
         return ExecResult(result.exited, output)
 
+    def prepare(self):
+        retry = 0
+        while retry < 20:
+            try:
+                retry += 1
+                self._connection.run("true", warn=True, hide=True)
+                break
+            except NoValidConnectionsError as ex:
+                if retry > 20:
+                    raise ex
+                time.sleep(1)
+
     def close(self):
         if self._connection is not None:
             self._connection.close()
diff --git a/exasol_integration_test_docker_environment/lib/test_environment/database_setup/docker_db_log_based_bucket_sync_checker.py b/exasol_integration_test_docker_environment/lib/test_environment/database_setup/docker_db_log_based_bucket_sync_checker.py
index dcd8511ba..a1947d7e5 100644
--- a/exasol_integration_test_docker_environment/lib/test_environment/database_setup/docker_db_log_based_bucket_sync_checker.py
+++ b/exasol_integration_test_docker_environment/lib/test_environment/database_setup/docker_db_log_based_bucket_sync_checker.py
@@ -50,8 +50,7 @@ def found_new_log_line(self, exit_code, start_exit_code,
     def find_pattern_in_logfile(self):
         cmd = f"""grep '{self.pattern_to_wait_for}' {self.log_file_to_check}"""
         bash_cmd = f"""bash -c "{cmd}" """
-        exit_code, output = \
-            self.database_container.exec_run(bash_cmd)
+        exit_code, output = self.executor.exec(bash_cmd)
         return exit_code, output
 
     def output_happened_after_start_time(self, output, start_time):
diff --git a/exasol_integration_test_docker_environment/lib/test_environment/database_setup/find_exaplus_in_db_container.py b/exasol_integration_test_docker_environment/lib/test_environment/database_setup/find_exaplus_in_db_container.py
index 91e84c4fe..9a6ebe446 100644
--- a/exasol_integration_test_docker_environment/lib/test_environment/database_setup/find_exaplus_in_db_container.py
+++ b/exasol_integration_test_docker_environment/lib/test_environment/database_setup/find_exaplus_in_db_container.py
@@ -13,18 +13,19 @@ def find_exaplus(
     Tries to find path of exaplus in given container in directories where exaplus is normally installed.
     :db_container Container where to search for exaplus
     """
-    exit, output = db_container.exec_run(cmd="find /usr/opt -name 'exaplus' -type f")
+    exit, output = os_executor.exec("find /usr/opt -name 'exaplus' -type f")
     if exit != 0 or output == b"":
-        # Using /usr/opt and /opt together in one command doesn't work, because in Exasol 8, /usr/opt doesn't exist
+        # Using /usr/opt and /opt together in one command doesn't work,
+        # because in Exasol 8, /usr/opt doesn't exist
         # and as such the command fails, even if it finds exaplus in /opt
-        exit, output = db_container.exec_run(cmd="find /opt -name 'exaplus' -type f")
+        exit, output = os_executor.exec("find /opt -name 'exaplus' -type f")
     if exit != 0:
         raise RuntimeError(f"Exaplus not found on docker db! Output is {output}")
     found_paths = list(filter(None, output.decode("UTF-8").split("\n")))
     if len(found_paths) != 1:
         raise RuntimeError(f"Error determining exaplus path! Output is {output}")
     exaplus_path = PurePath(found_paths[0])
-    exit, output = db_container.exec_run(cmd=f"{exaplus_path} --help")
+    exit, output = os_executor.exec(f"{exaplus_path} --help")
     if exit != 0:
         raise RuntimeError(f"Exaplus not working as expected! Output is {output}")
     return exaplus_path
diff --git a/exasol_integration_test_docker_environment/lib/test_environment/database_setup/upload_file_to_db.py b/exasol_integration_test_docker_environment/lib/test_environment/database_setup/upload_file_to_db.py
index 2af0f5442..82534a14c 100644
--- a/exasol_integration_test_docker_environment/lib/test_environment/database_setup/upload_file_to_db.py
+++ b/exasol_integration_test_docker_environment/lib/test_environment/database_setup/upload_file_to_db.py
@@ -64,6 +64,7 @@ def run_task(self):
                 database_container = None
             if not self.should_be_reused(upload_target):
                 with self.executor_factory.executor() as executor:
+                    executor.prepare()
                     self.upload_and_wait(
                         database_container,
                         file_to_upload,
diff --git a/exasol_integration_test_docker_environment/lib/test_environment/database_waiters/is_database_ready_thread.py b/exasol_integration_test_docker_environment/lib/test_environment/database_waiters/is_database_ready_thread.py
index ac2893883..4c3676b28 100644
--- a/exasol_integration_test_docker_environment/lib/test_environment/database_waiters/is_database_ready_thread.py
+++ b/exasol_integration_test_docker_environment/lib/test_environment/database_waiters/is_database_ready_thread.py
@@ -43,6 +43,7 @@ def run(self):
                 db_connection_command = ""
                 bucket_fs_connection_command = ""
                 try:
+                    executor.prepare()
                     exaplus_path = find_exaplus(self._db_container, executor)
                     db_connection_command = self.create_db_connection_command(exaplus_path)
                     bucket_fs_connection_command = self.create_bucketfs_connection_command()
@@ -51,9 +52,9 @@ def run(self):
                     self.finish = True
                 while not self.finish:
                     (exit_code_db_connection, self.output_db_connection) = \
-                        self._db_container.exec_run(cmd=db_connection_command)
+                        executor.exec(db_connection_command)
                     (exit_code_bucketfs_connection, self.output_bucketfs_connection) = \
-                        self._db_container.exec_run(cmd=bucket_fs_connection_command)
+                        executor.exec(bucket_fs_connection_command)
                     if exit_code_db_connection == 0 and exit_code_bucketfs_connection == 0:
                         self.finish = True
                         self.is_ready = True
diff --git a/test/integration/conftest.py b/test/integration/conftest.py
index 4f523e584..32fa8fdaf 100644
--- a/test/integration/conftest.py
+++ b/test/integration/conftest.py
@@ -1,8 +1,9 @@
 import contextlib
-from typing import Any, Callable, Dict, Iterator, List, NewType, Optional
-
+import io
 import pytest
 
+from typing import Any, Callable, Dict, Iterator, List, NewType, Optional
+
 from exasol_integration_test_docker_environment.lib.docker import ContextDockerClient
 from test.integration.helpers import normalize_request_name
 from exasol_integration_test_docker_environment.testing import utils
@@ -102,3 +103,13 @@ def create_context(
         utils.close_environments(spawned)
 
     return create_context
+
+@pytest.fixture
+def fabric_stdin(monkeypatch):
+    """
+    Mock stdin to avoid ThreadException when reading from stdin while
+    stdout is captured by pytest: OSError: pytest: reading from stdin while
+    output is captured!  Consider using ``-s``.
+    See https://github.com/fabric/fabric/issues/2005
+    """
+    monkeypatch.setattr('sys.stdin', io.StringIO(''))
diff --git a/test/integration/helpers.py b/test/integration/helpers.py
index 53bf05874..506f4eec3 100644
--- a/test/integration/helpers.py
+++ b/test/integration/helpers.py
@@ -1,6 +1,6 @@
 import contextlib
 import re
-from typing import Any, cast
+from typing import Any, Union, cast
 from unittest.mock import Mock
 
 from exasol_integration_test_docker_environment.lib.docker import ContextDockerClient
@@ -39,7 +39,7 @@ def container_named(*names, matcher=None):
 
 def get_executor_factory(
     dbinfo: DatabaseInfo,
-    db_os_access: DbOsAccess=DbOsAccess.DOCKER_EXEC,
+    db_os_access: DbOsAccess = DbOsAccess.DOCKER_EXEC,
 ) -> DbOsExecFactory:
     if db_os_access == DbOsAccess.SSH:
         return SshExecFactory.from_database_info(dbinfo)
diff --git a/test/integration/test_bucketfs_upload.py b/test/integration/test_bucketfs_upload.py
index d4b196338..a4a8cf158 100644
--- a/test/integration/test_bucketfs_upload.py
+++ b/test/integration/test_bucketfs_upload.py
@@ -146,7 +146,8 @@ def validate(self, expected_content: str, expected_reuse: bool):
 
 @pytest.mark.parametrize("db_os_access", [DbOsAccess.DOCKER_EXEC, DbOsAccess.SSH])
 def test_upload_without_reuse(api_database, tmp_path, db_os_access):
-    with api_database() as db:
+    params = { "db_os_access": db_os_access.name }
+    with api_database(additional_parameters=params) as db:
         dbinfo = db.environment_info.database_info
         executor_factory = get_executor_factory(dbinfo, db_os_access)
         bucketfs = BucketFsAccess(db, executor_factory)
@@ -160,7 +161,8 @@ def test_upload_without_reuse(api_database, tmp_path, db_os_access):
 
 @pytest.mark.parametrize("db_os_access", [DbOsAccess.DOCKER_EXEC, DbOsAccess.SSH])
 def test_upload_with_reuse(api_database, tmp_path, db_os_access):
-    with api_database() as db:
+    params = { "db_os_access": db_os_access.name }
+    with api_database(additional_parameters=params) as db:
         dbinfo = db.environment_info.database_info
         executor_factory = get_executor_factory(dbinfo, db_os_access)
         bucketfs = BucketFsAccess(db, executor_factory)
diff --git a/test/integration/test_cli_environment.py b/test/integration/test_cli_environment.py
index 615daf6bd..1eb16caa9 100644
--- a/test/integration/test_cli_environment.py
+++ b/test/integration/test_cli_environment.py
@@ -81,14 +81,16 @@ def test_db_container_started(cli_database):
 
 
 @pytest.mark.parametrize("db_os_access", [DbOsAccess.DOCKER_EXEC, DbOsAccess.SSH])
-def test_db_available(cli_database, db_os_access):
-    with cli_database() as db:
+def test_db_available(cli_database, fabric_stdin, db_os_access):
+    params = [ "--db-os-access", db_os_access.name ]
+    with cli_database(additional_parameters=params) as db:
         with ContextDockerClient() as docker_client:
             dbinfo = db.on_host_docker_environment.environment_info.database_info
             db_container_name = dbinfo.container_info.container_name
             db_container = docker_client.containers.get(db_container_name)
             executor_factory = get_executor_factory(dbinfo, db_os_access)
             with executor_factory.executor() as executor:
+                executor.prepare()
                 exaplus = find_exaplus(db_container, executor)
                 command = smoke_test_sql(exaplus, db.on_host_docker_environment)
                 exit_code, output = db_container.exec_run(command)
diff --git a/test/integration/test_ssh_access.py b/test/integration/test_ssh_access.py
index b99813558..a3a20b11a 100644
--- a/test/integration/test_ssh_access.py
+++ b/test/integration/test_ssh_access.py
@@ -2,7 +2,6 @@
 import docker
 from docker.models.containers import Container as DockerContainer
 import fabric
-import io
 import os
 import pytest
 import time
@@ -29,17 +28,6 @@
 )
 
 
-@pytest.fixture
-def fabric_stdin(monkeypatch):
-    """
-    Mock stdin to avoid ThreadException when reading from stdin while
-    stdout is captured by pytest: OSError: pytest: reading from stdin while
-    output is captured!  Consider using ``-s``.
-    See https://github.com/fabric/fabric/issues/2005
-    """
-    monkeypatch.setattr('sys.stdin', io.StringIO(''))
-
-
 def test_generate_ssh_key_file(api_database):
     params = { "db_os_access": "SSH" }
     with api_database(additional_parameters=params) as db: