Skip to content

Prevent worker to fail on missing key and secret keys #248

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Prevent worker to fail on missing key and secret keys #248

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
287c2c1
Prevent worker to fail on missing key and secret keys
d3flex Aug 8, 2025
3b706d2
Verify that worker register to the openqa_webui container
d3flex Sep 17, 2025
ef0c492
Update Docker build arguments and directories
d3flex Sep 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions tests/containers/single_container_webui.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,26 @@ use testapi;
use utils;

sub run {
assert_script_run(
"echo \"\$(cat <<EOF
[global]
# change to the URL the web UI will be available under so redirection for
# authentication works
base_url = http://openqa_webui

[auth]
method = Fake
Copy link
Member

@okurz okurz Sep 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should help you to not need to put this into the config file.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is this MR supposed to help here? What would be the value to set -m to? Reading https://open.qa/docs/#_fake it doesn't mention any other method to enable fake authentication. Or is something like https://github.com/os-autoinst/openQA/blob/05b96119875a37729e11d143ebbc6a402c96d50c/lib/OpenQA/Schema.pm#L39 also implemented in the auth-backend?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By setting the mode to "development" or "test" the fake authentication and log=debug is automatically enabled in https://github.com/os-autoinst/openQA/blob/05b96119875a37729e11d143ebbc6a402c96d50c/lib/OpenQA/Setup.pm#L270 . So instead of writing a config file one just needs to set the environment variable OPENQA_WEBUI_MODE=test when starting the openQA web UI container

EOF
)\" > /root/openQA/container/webui/conf/openqa.ini");

my $volumes = '-v "/root/data/factory:/data/factory" -v "/root/data/tests:/data/tests" -v "/root/openQA/container/webui/conf:/data/conf:ro"';
my $certificates = '-v "/root/server.crt:/etc/apache2/ssl.crt/server.crt" -v "/root/server.crt:/etc/apache2/ssl.crt/ca.crt" -v "/root/server.key:/etc/apache2/ssl.key/server.key"';

assert_script_run("openssl req -newkey rsa:4096 -x509 -sha256 -days 365 -nodes -subj '/CN=www.mydom.com/O=My Company Name LTD./C=DE' -out server.crt -keyout server.key");

assert_script_run("docker run -d --network testing $volumes $certificates -p 80:80 --name openqa_webui openqa_webui");
assert_script_run("docker run -d --network testing $volumes $certificates -p 80:80 --hostname openqa_webui --name openqa_webui openqa_webui");
wait_for_container_log('openqa_webui', 'Web application available at', 'docker');

assert_script_run('curl http://localhost');
assert_script_run('docker rm -f openqa_webui');
}

sub post_fail_hook {
Expand Down
32 changes: 28 additions & 4 deletions tests/containers/worker.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,34 @@ use testapi;
use utils;

sub run {
my $volumes = '-v "/root/data/factory:/data/factory" -v "/root/data/tests:/data/tests" -v "/root/openQA/container/openqa_data/data.template/conf/:/data/conf:ro"';
assert_script_run("docker run -d --network testing $volumes --name openqa_worker openqa_worker");
wait_for_container_log('openqa_worker', 'API key and secret are needed', 'docker');
clear_root_console;
my $confdir = '/tmp/openqa_worker_conf';
assert_script_run("mkdir -p $confdir");
assert_script_run(
"echo \"\$(cat <<EOF
[openqa_webui]
key = 1234567890ABCDEF
secret = 1234567890ABCDEF
EOF
)\" > $confdir/client.conf");
Comment on lines +9 to +14
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

os-autoinst/openQA#6767 should help so that we can set environment variables instead of needing to write config files


assert_script_run(
"echo \"\$(cat <<EOF
[global]
BACKEND = qemu
HOST = openqa_webui
WORKER_HOSTNAME = openqa_worker
EOF
)\" > $confdir/workers.ini");
my $volumes = qq{-v "/root/data/factory:/data/factory" -v "/root/data/tests:/data/tests" -v "$confdir:/data/conf:ro"};
assert_script_run('curl -v http://localhost/login');
assert_script_run(qq{docker run -d --network testing $volumes --hostname openqa_worker --name openqa_worker openqa_worker});
wait_for_container_log('openqa_worker', 'Registered and connected', 'docker');
clear_root_console;
}

sub post_run_hook {
script_run('docker rm -f openqa_worker');
script_run('docker rm -f openqa_webui');
}
Comment on lines +31 to 34
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it could be. But I went with script_run because the assert is overkill here. I usually expect assert in the sense of test validation.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to worry about performance. Please use it unless it's expected to fail.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think some comments have been mixed up here. I don't see the need for this post_run_hook. For sure the worker module shouldn't care to cleanup the webUI container

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The openqa_webui container is no longer being removed in tests/containers/single_container_webui.pm . That's why this is needed now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we wanted to avoid this dependency, we would need to setup & run a web UI container within the worker test. That was discussed before.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think originally the webUI container was removed to not influence any other test modules after that which is a good approach for testing. You now wanted to change the test from a "component test" to an "integration test" so the worker module relies on the webui test module. Fine for me. I would have not cared about removing the containers in this module then because the whole test flow became an integration test and left cleanup to the whole run, i.e. not call docker rm as afterwards the VM is destroyed anyway

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@d3flex Please fix this as well

Suggested change
sub post_run_hook {
script_run('docker rm -f openqa_worker');
script_run('docker rm -f openqa_webui');
}
assert_script_run('docker rm -f openqa_worker');
assert_script_run('docker rm -f openqa_webui');

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better just remove the complete post_run_hook. We don't need to make the test run longer to cleanup services as the whole VM will be destroyed anyway


1;