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

d3flex · 2025-08-08T08:15:23Z

The worker still fails but not due to misconfuration. But this time will throw an additional soft fail to make visible the issue with the containers' communication.

poo: https://progress.opensuse.org/issues/186651

okurz

language errors in commit message
disabled code
hardcoded credentials

tests/containers/worker.pm

The worker still fails but not due to misconfuration. But this time will throw an additional soft fail to make visible the issue with the containers' communication. poo: https://progress.opensuse.org/issues/186651 Signed-off-by: Ioannis Bonatakis <[email protected]>

tests/containers/single_container_webui.pm

tests/containers/worker.pm

perlpunk

Nice. I didn't know that we run apache in the webui container.

Maybe the curl http://openqa_webui/login can be done as part of the webui startup, then the entrypoint of the worker can be used as it is.

d3flex · 2025-09-18T04:54:22Z

Nice. I didn't know that we run apache in the webui container.

Maybe the curl http://openqa_webui/login can be done as part of the webui startup, then the entrypoint of the worker can be used as it is.

I should give it a go. But I presume there are nuances for this scenario?! what happens if the authentication is not using Fake?

Even with a proper client.conf the worker was unable to register with the webui. The redirection to the localhost is bypassed when the worker provides WORKER_HOSTNAME, so it was not the main problem. The webui container in the other side runs reserve proxy, however is the template. Overall the solution was a sync of the configuration files with the expected openqa host. To do that, specific files in the host are overriden instead of manipulated or created new ones. That was to keep the original files in the repo intact as it is likely already used as is in other cases which it is not known at that point. Main changes is to use Fake for the authentication and then override the ENTRYPOINT of the worker to request a /login before run the entry command. Other possible solution might be to edit the Dockerfile or maybe do a edit the db. Note that the containers now use the `--hostname` which it doesn't make any difference but it is nice to have the hostname set in the containers. poo: https://progress.opensuse.org/issues/186651 Signed-off-by: Ioannis Bonatakis <[email protected]>

okurz · 2025-09-18T06:48:52Z

Nice. I didn't know that we run apache in the webui container.
Maybe the curl http://openqa_webui/login can be done as part of the webui startup, then the entrypoint of the worker can be used as it is.

I should give it a go. But I presume there are nuances for this scenario?! what happens if the authentication is not using Fake?

I suggest to do what's necessary for the test to work in a clean and simple way so it's ok if the same approach wouldn't work in other scenarios

d3flex · 2025-09-18T16:13:10Z

Nice. I didn't know that we run apache in the webui container.

Maybe the curl http://openqa_webui/login can be done as part of the webui startup, then the entrypoint of the worker can be used as it is.

Sorry but so far I havent make it work and I try to focus how i can make the PR ready for review

d3flex · 2025-09-19T06:30:03Z

Likely the CI will fails.
os-autoinst/openQA#6751 is needed first.

The improvement is that the HOST is served during the runtime now, so no need to manipulate the config files.
In additional, the worker creates the files which are to be used as volumes, rather than the templates

okurz · 2025-09-23T07:34:04Z

tests/containers/worker.pm

+sub post_run_hook {
+  script_run('docker rm -f openqa_worker');
+  script_run('docker rm -f openqa_webui');
 }


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.

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

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

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

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.

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

@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');

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

tests/containers/worker.pm

tests/containers/single_container_webui.pm

okurz · 2025-09-24T10:18:18Z

tests/containers/single_container_webui.pm

+base_url = http://openqa_webui
+
+[auth]
+method = Fake


script: Allow to configure openQA mode openQA#6766

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

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?

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

perlpunk · 2025-09-24T10:50:33Z

tests/containers/worker.pm

+    script_run(
+        "echo  \"\$(cat <<EOF
+[global]
+BACKEND = qemu
+HOST = openqa_webui
+WORKER_HOSTNAME = openqa_worker
+EOF
+)\" > $confdir/workers.ini");


I think it would be a bit more readable like this (untested):

Suggested change

script_run(

"echo \"\$(cat <<EOF

[global]

BACKEND = qemu

HOST = openqa_webui

WORKER_HOSTNAME = openqa_worker

EOF

)\" > $confdir/workers.ini");

script_run(<<~EOSCRIPT);

cat > $confdir/workers.ini <<EOF

[global]

BACKEND = qemu

HOST = openqa_webui

WORKER_HOSTNAME = openqa_worker

EOF

EOSCRIPT

I havent managed to make this work. it makes the process hanging, so I will leave it out for now

[global] BACKEND = qemu HOST = openqa_webui WORKER_HOSTNAME = openqa_worker EOF ' timed out at /usr/lib/os-autoinst/autotest.pm line 515.```

Ok, it doesn't work because assert_script_run appends the; echo ~hel1-\$?- stuff, but that doesn't work on its own line after the EOF line.
However, this works:

type_string(<<~"EOSCRIPT"); cat > $confdir/workers.ini <<EOF [global] BACKEND = qemu HOST = openqa_webui WORKER_HOSTNAME = openqa_worker EOF EOSCRIPT

and we already do this kind of command in tests/install/openqa_worker.pm for example.

I find this more readable, but I don't insist on it.

okurz · 2025-09-24T10:51:20Z

tests/containers/worker.pm

+        "echo  \"\$(cat <<EOF
+[openqa_webui]
+key = 1234567890ABCDEF
+secret = 1234567890ABCDEF
+EOF
+)\" > $confdir/client.conf");


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

tests/containers/single_container_webui.pm

The templates are almost entirely modified and in order to provide a clean configuration for the test case, it is better to not use them at all. This, hopefully descreases the complexity and provides a clean overview of the minimal configuration that the container is needed. Furthermore, this commit is depended on the webui Dockerfile, which should be provide the proxy configuration on the webui container for the build. Depends-on: os-autoinst/openQA#6751 Signed-off-by: Ioannis Bonatakis <[email protected]>

d3flex requested a review from kalikiana August 8, 2025 08:15

okurz requested changes Aug 15, 2025

View reviewed changes

kalikiana reviewed Aug 15, 2025

View reviewed changes

tests/containers/worker.pm Outdated Show resolved Hide resolved

d3flex force-pushed the feat/test_reg_worker branch from ac668b3 to e1eded8 Compare August 18, 2025 07:39

d3flex force-pushed the feat/test_reg_worker branch from e1eded8 to 65e6213 Compare September 17, 2025 15:19

d3flex added the not-ready label Sep 17, 2025

okurz requested changes Sep 17, 2025

View reviewed changes

perlpunk reviewed Sep 17, 2025

View reviewed changes

d3flex force-pushed the feat/test_reg_worker branch from 65e6213 to 09e5e89 Compare September 18, 2025 05:47

d3flex force-pushed the feat/test_reg_worker branch from 09e5e89 to 3b706d2 Compare September 18, 2025 05:48

d3flex force-pushed the feat/test_reg_worker branch from 50149c1 to 0a622da Compare September 22, 2025 14:50

okurz reviewed Sep 23, 2025

View reviewed changes

okurz requested changes Sep 23, 2025

View reviewed changes

tests/containers/worker.pm Outdated Show resolved Hide resolved

tests/containers/single_container_webui.pm Outdated Show resolved Hide resolved

okurz requested changes Sep 24, 2025

View reviewed changes

tests/containers/single_container_webui.pm Outdated Show resolved Hide resolved

tests/containers/single_container_webui.pm Outdated Show resolved Hide resolved

okurz reviewed Sep 24, 2025

View reviewed changes

perlpunk reviewed Sep 24, 2025

View reviewed changes

okurz reviewed Sep 24, 2025

View reviewed changes

d3flex force-pushed the feat/test_reg_worker branch from 0a622da to 3e58d6f Compare September 25, 2025 04:28

d3flex marked this pull request as ready for review September 25, 2025 04:29

okurz reviewed Sep 25, 2025

View reviewed changes

tests/containers/single_container_webui.pm Outdated Show resolved Hide resolved

tests/containers/single_container_webui.pm Show resolved Hide resolved

d3flex force-pushed the feat/test_reg_worker branch from 3e58d6f to d7d5628 Compare September 25, 2025 07:01

d3flex force-pushed the feat/test_reg_worker branch from d7d5628 to ef0c492 Compare September 26, 2025 11:37

kalikiana approved these changes Sep 26, 2025

View reviewed changes

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

Are you sure you want to change the base?

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

Uh oh!

Conversation

d3flex commented Aug 8, 2025

Uh oh!

okurz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

perlpunk left a comment

Choose a reason for hiding this comment

Uh oh!

d3flex commented Sep 18, 2025

Uh oh!

okurz commented Sep 18, 2025

Uh oh!

d3flex commented Sep 18, 2025

Uh oh!

d3flex commented Sep 19, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

okurz Sep 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

okurz Sep 24, 2025 •

edited

Loading