action #130096
closedopenqa-clone-custom-git-refspec modifies PRODUCTDIR setting
Description
Observation¶
Tried to run a VR with the following command
$ openqa-clone-custom-git-refspec https://github.com/rakoenig/os-autoinst-distri-opensuse/tree/yast_cc_container https://openqa.opensuse.org/tests/3313986
This leds to a failure in the cloned job:
[2023-05-30T11:15:42.330782+02:00] [warn] [pid:32320] !!! main: PRODUCTDIR 'os-autoinst-distri-opensuseproducts/alp' invalid, could not be found at /usr/lib/os-autoinst/OpenQA/Isotovideo/Utils.pm line 269.
So I modified the script to ouput the cmd issued at the end and see what happens to PRODUCTDIR
on varius clone (dry runs):
test url | old_product_dir | productdir |
---|---|---|
https://openqa.opensuse.org/tests/3313986 | "products/alp" | "os-autoinst-distri-opensuseproducts/alp" |
https://openqa.opensuse.org/tests/3323239 | "opensuse/products/opensuse" | "os-autoinst-distri-opensuse/products/opensuse" |
https://openqa.suse.de/tests/11215764 | "sle/products/sle" | os-autoinst-distri-opensuse/products/sle" |
So in the first attempt we run into that error because we're missing a /
after os-autoinst-distri-opensuse
. Nevertheless: The resulting productdir setting is never the same as it is stored in the vars.json of the original job.
Additinal info¶
The lines to be investigated start here:
local old_productdir
old_productdir=$(echo "$json_data" | jq -r '.PRODUCTDIR') || throw_json_error "$json_url" "$json_data"
local old_casedir
old_casedir=$(echo "$json_data" | jq -r '.CASEDIR') || throw_json_error "$json_url" "$json_data"
if [[ ${old_productdir:0:1} == "/" ]]; then
local productdir="${productdir:-"${repo_name##*/}${old_productdir##"$old_casedir"}"}"
else
local productdir="${productdir:-"${repo_name##*/}${old_productdir#*"${old_casedir##*/}"}"}"
fi
The documentation for PRODUCTDIR
says:
Absolute or relative path to the optional "product directory" which includes the test schedule entry point main.pm as well as a needles subdirectory with the needles to load. Defaults to CASEDIR
. Relative paths are considered relative to CASEDIR
.
So it looks like if [[ ${old_productdir:0:1} == "/" ]]; then
tries to distinguish between absolute and relative paths. I just don't understand the use of it. We're cloning from an existing job that did this already and so from my understanding it would be enough just to copy the PRODUCTDIR
from the original job.
Even a line like local productdir="${productdir:-"${repo_name##*/}${old_productdir#*"${old_casedir##*/}"}"}"
does not seem to make sens because the initial check ${productdir:-
will result in false, since we just declared productdir as a local setting and so it should be empty, isn't it?
Proposed action¶
Think about the argements above and fix it in a way that does not make cloned tests fail.
Updated by livdywan over 1 year ago
- Category set to Feature requests
- Target version set to future
I assume it's not super critical so I'm not adding it to the backlog right away.
Updated by okurz over 1 year ago
- Target version changed from future to Ready
We can try to address it now
Updated by okurz over 1 year ago
rainerkoenig wrote:
So it looks like
if [[ ${old_productdir:0:1} == "/" ]]; then
tries to distinguish between absolute and relative paths. I just don't understand the use of it. We're cloning from an existing job that did this already and so from my understanding it would be enough just to copy thePRODUCTDIR
from the original job.
- No, we need to change productdir to refer to the git checked out version. the purpose of overwriting PRODUCTDIR is that we actually load the schedule from the git checked out test distribution, not the one from /var/lib/openqa/share/tests . 2. Since I created the script 5 years ago behaviour in openQA has changed significantly but I think we actually still need this line.
Even a line like local
productdir="${productdir:-"${repo_name##*/}${old_productdir#*"${old_casedir##*/}"}"}"
does not seem to make sens because the initial check${productdir:-
will result in false, since we just declared productdir as a local setting and so it should be empty, isn't it?
The inner ${productdir:-
gets the value from a variable $productdir
from an outer scope if defined, yielding an empty string otherwise. The local productdir
is a new locally scoped variable shadowing the potentially already defined outer scope variable.
I just tried it out to be sure with a script
foo=42
f() {
local foo=${foo:-43}
echo "foo in f(): $foo"
}
f
echo "foo in outer scope: $foo"
foo=
f
echo "foo in outer scope undefined: $foo"
which yields
foo in f(): 42
foo in outer scope: 42
foo in f(): 43
foo in outer scope undefined:
as expected where the first invocation of f()
assigns the outer scope value 42 to the local variable of same name. The second output line shows that the value in outer scope is still unaltered 42. After unassigning foo the function assigns the fallback value of 43 and shows that in the output while in the outer scope the variable has no value as shown by the last line of output.
Another important point is that PRODUCTDIR is never specified in settings and shouldn't be specified because isotovideo tries to look for main.pm in "$casedir/products/$distri" if not found in "$casedir". The interesting part is why that comes to a different conclusion for alp.
I don't understand which could would compute a product dir of "products/alp". In openQA OpenQA::Utils::productdir we have
146 my $dir = testcasedir($distri, $version, $rootfortests);
147 return $dir . "/products/$distri" if -e "$dir/products/$distri";
so I guess line 147 is not effective here as we have a string that starts with no "/". And os-autoinst doesn't know anything about the name part "products/", it just assumes that the main.pm is in PRODUCTDIR and PRODUCTDIR is set to CASEDIR if not set.
I added an extra debug worker instance on o3 worker openqaworker7, instance 42 and triggered special tests:
openqa-clone-job --within-instance https://openqa.opensuse.org/tests/3313986 TEST+=okurz-poo130096 _GROUP=0 BUILD= WORKER_CLASS=okurz_debug _EXIT_AFTER_SCHEDULE=1
1 job has been created:
- alp-bedrock-0.1-Default-x86_64-Build5.1-yast_1@uefi -> https://openqa.opensuse.org/tests/3340338
And with some custom debugging logs from the worker start:
Jun 06 10:56:29 openqaworker7 worker[28823]: PRODUCTDIR before: at /usr/share/openqa/script/../lib/OpenQA/Worker/Engines/isotovideo.pm line 398.
evaluated default productdir: /var/lib/openqa/cache/openqa1-opensuse/tests/alp/products/alp at /usr/share/openqa/script/../lib/OpenQA/Worker/Engines/isotovideo.pm line 400.
casedir ne target_name, PRODUCTDIR before: at /usr/share/openqa/script/../lib/OpenQA/Worker/Engines/isotovideo.pm line 411.
casedir ne target_name, PRODUCTDIR after products/alp at /usr/share/openqa/script/../lib/OpenQA/Worker/Engines/isotovideo.pm line 414.
The relevant code of concern seems to be:
$vars->{PRODUCTDIR} //= $absolute_paths && !$has_custom_dir ? $default_productdir : abs2rel($default_productdir, $default_casedir);
for the second branch when product dir is not set so effectively:
$vars->{PRODUCTDIR} = abs2rel($default_productdir, $default_casedir);
in the specific case:
abs2rel('/var/lib/openqa/cache/openqa1-opensuse/tests/alp/products/alp', '/var/lib/openqa/cache/openqa1-opensuse/tests/alp') -> `products/alp`
however for a standard opensuse tests when also using a custom git casedir that happens to be the same:
openqa-clone-job --within-instance https://openqa.opensuse.org/tests/3323239 TEST+=okurz-poo130096 _GROUP=0 BUILD= WORKER_CLASS=okurz_debug _EXIT_AFTER_SCHEDULE=1 _SKIP_CACHE_SYNC_TESTS=1 HDD_1= HDD_2= CASEDIR=https://github.com/os-autoinst/os-autoinst-distri-opensuse/
yielding:
casedir ne target_name, abs2rel(/var/lib/openqa/cache/openqa1-opensuse/tests/opensuse/products/opensuse, /var/lib/openqa/cache/openqa1-opensuse/tests/opensuse): products/opensuse
so I assume everything is actually fine regarding the generation of product dir in openQA worker and we should try to fix the concatenation in openqa-clone-custom-git-refspec
I tried out what a correct product dir should look like. I called
bash -ex $(which openqa-clone-custom-git-refspec) -n https://github.com/rakoenig/os-autoinst-distri-opensuse/tree/yast_cc_container https://openqa.opensuse.org/tests/3313986
and took over the openqa-clone-job command line and only added a slash in productdir and scheduled with WORKER_CLASS=okurz_debug
so that I could monitor how the pool dir looks like:
openqa-clone-job --skip-chained-deps --parental-inheritance --within-instance https://openqa.opensuse.org 3313986 _GROUP=0 TEST+=@rakoenig/os-autoinst-distri-opensuse#yast_cc_container BUILD=rakoenig/os-autoinst-distri-opensuse#yast_cc_container CASEDIR=https://github.com/rakoenig/os-autoinst-distri-opensuse.git#yast_cc_container PRODUCTDIR=os-autoinst-distri-opensuse/products/alp NEEDLES_DIR=needles WORKER_CLASS=okurz_debug
https://openqa.opensuse.org/tests/3340773 looks like it could properly find the schedule file.
Updated by okurz over 1 year ago
- Due date set to 2023-06-20
- Status changed from New to Feedback
Updated by okurz over 1 year ago
https://github.com/os-autoinst/openQA/pull/5192 merged.
@rainerkoenig can you try if the new version of openqa-clone-custom-git-refspec fixes tthe problem as you observed them? You can download the script directly from https://raw.githubusercontent.com/os-autoinst/openQA/master/script/openqa-clone-custom-git-refspec or wait some more minutes to have the package ready from devel:openQA or some days to have the updated package in Tumbleweed :)
Updated by okurz over 1 year ago
- Due date deleted (
2023-06-20) - Status changed from Feedback to Resolved
No response, assuming resolved