exasol / integration-test-docker-environment

A docker-based environment for integration tests with the EXASOL DB.
https://exasol.github.io/integration-test-docker-environment/
MIT License
6 stars 2 forks source link

Forward SSH port #329

Closed ckunki closed 1 year ago

ckunki commented 1 year ago

ITDE currently 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.

The current ticket requests to to forward the SSH port outside the Docker Container in order to enable accessing the operating system hosting the database via SSH from outside the Docker Container.

Acceptance Criteria

  1. ITDE offers a cli option --ssh-port-forward to specify the port mapping
  2. user guide describes new cli option
  3. ITDE implementation handles the command line option in API, test environment, and luigi task
  4. If the container does not exist already and user did not specify a port then ITDE selects a random port from the list of free ports available on the host system outside the docker
  5. If the container exists already then ITDE
    • reuses the existing container
    • inquires the forwarded port from the container
    • ignores the command line option
ckunki commented 1 year ago

Affected are

  1. cli/options/test_environment_options.py defines new cli option
  2. lib/test_environment/parameter/docker_db_test_environment_parameter.py defines new task parameter
  3. lib/test_environment/spawn_test_database.py forwards the SSH port accordingly
  4. cli/commands/spawn_test_environment.py
  5. lib/api/spawn_test_environment_with_test_container.py
  6. lib/api/spawn_test_environment.py
  7. testing/exaslct_test_environment.py

Proposed changes for (3):

SSH_PORT = "22"
if self.ssh_port_forward is not None:
    ports[f"{SSH_PORT}/tcp"] = ('0.0.0.0', int(self.ssh_port_forward))
ckunki commented 1 year ago

Discussion with @tkilias:

For reuse:

or

def get_mapped_port(container: Container, port: int) -> int:
    try:
        return int(
            container.attrs
            ["HostConfig"]
            ["PortBindings"]
            [f"{port}/tcp"]
            [0]
            ["HostPort"]
        )
    except (KeyError, IndexError):
        return None

Example from container.attrs:

   "PortBindings": {
     "6583/tcp": [{"HostIp": "0.0.0.0", "HostPort": "36263"}],
     "8888/tcp": [{"HostIp": "0.0.0.0", "HostPort": "50033"}]
   },
ckunki commented 1 year ago

See exasol_integration_test_docker_environment.testing.utils.find_free_ports.

Used in exasol_integration_test_docker_environment/testing/api_test_environment.py:

database_port, bucketfs_port = find_free_ports(2)

Implementation options

I opt for option (c).

ckunki commented 1 year ago

Observations indicate that some logic in Docker seems to overwrite file /root/.ssh/authorized_keys.

Which alternatives exist?

For A1 and A2 feasibility is unknown. A3 and A4 require to know either password or the secret key in advance.

tkilias commented 1 year ago

Extend docker db EXAConf template with

https://github.com/exasol/integration-test-docker-environment/blob/74396e4211287b451977684943fe46b17334de28/docker_db_config_template/8.17.0/EXAConf#L86

add authorized_keys config to root user with like that

authorized_keys = {{ authorized_keys }}

ckunki commented 1 year ago

After changing one of the templates, e.g. docker_db_config_template/7.1.0/EXAConf you might need to copy the templates to folder exasol_integration_test_docker_environment/docker_db_config with the following command:

bash ./githooks/update_packaging.sh

Inside the Docker Container the following commands might be helpful:

tkilias commented 1 year ago

the issue is, in the c4 container, you can't access the exaconf and exaconf changes the config file, but not the applied config, for that exainit needs to run again.

tkilias commented 1 year ago

if you want to change the user in a running container, you probably need to use confd.

ckunki commented 1 year ago

No worries, currently I have some progress and I already could find out that the key in EXAConf is not authorized_keys but AuthorizedKeys. 🙂

ckunki commented 1 year ago

Open questions

ID File Question Proposal
Q1 eitde/lib/test_environment/parameter/external_test_environment_parameter.py Which default value to use for SSH port for external exasol database? 22
Q2 eitde/cli/options/test_environment_options.py Should cli option --external-exasol-ssh-port be mandatory or optional? optional
Q3 eitde/lib/test_environment/spawn_test_environment.py See question Q2, Should missing SSH port raise an exception here, too? No
Q4 pytestitde/__init_\.py Which port should be used for ssh_port_forward? 22
Q6 eitde/lib/api/spawn_test_environment.py Do we accept a breaking change to API? ?
Q7 eitde/lib/api/spawn_test_environment_with_test_container.py See Q6 ?
Q8 eitde/lib/data/database_info.py See Q6 ?
Q9 doc/user_guide/user_guide.rst eitde/lib/test_environment/abstract_spawn_test_environment.py user_guide.rst describes file environment_info.sh to contain "export" statement for each variable, while implemention does not generate "export" statements. Which file should be updated? user_guide.rst
ckunki commented 1 year ago

Dear reviewers: I updated the PR #339 and fixed most review findings. Only remaining (to my knowledge) is discussion #1219777763.

ckunki commented 1 year ago

Dear reviewers I fixed findings from round No. 2. Please review again.

ckunki commented 1 year ago

Blocked by #351

ckunki commented 1 year ago

blocking issue #351 has been implemented and released with version 1.7.1 - removing label blocked:yes

ckunki commented 1 year ago

Ready for Review again.