Add support for environment variable

[crazoes]

This change will allow us to run some kselftests which requires to pass parameters

[nuclearcat]

This patch wont work as env not defined in parameters. Also if it is missing, it is breaking template.
kernelci/kernelci-pipeline#750

[crazoes]

@nuclearcat we already got the parameter support merged in the Linaro's test-definition here Linaro/test-definitions#511

Why will this not work?

[nuclearcat]

2 problems:

1. config/lava/kselftest/kselftest.jinja2 belongs to legacy system
2. If parameter "env" is not defined in yaml file in params sections, it will corrupt yaml file generated by template. (and this is what happened in referenced issue)

[nfraprado]

FWIW I had this as part of #2577 . Haven't had time to re-test get it merged yet. I'd be fine with adding staging-skip to my PR and letting you merge the ENV functionality through this PR first. Also Denys' comment about the case when ENV is undefined is also not handled in my PR, so that'd need fixing.

[JenySadadia]

Hello @crazoes
I think you need to define env inside params section of job definition.
For instance here https://github.com/kernelci/kernelci-pipeline/blob/main/config/pipeline.yaml#L890

[crazoes]

@nuclearcat @JenySadadia I already created a PR for adding env into the pipeline.yaml file
kernelci/kernelci-pipeline#743

Does that look good? I'll change it to point to generic.jinja2 instead of kselftest.jinja2

[JenySadadia]

@nuclearcat @JenySadadia I already created a PR for adding env into the pipeline.yaml file kernelci/kernelci-pipeline#743

Does that look good? I'll change it to point to generic.jinja2 instead of kselftest.jinja2

Yes, after renaming the template, it should work.

[crazoes]

@a-wai does this patch look good to you?
kernelci/kernelci-pipeline#743 depends on this one.

[a-wai]

I would drop this change, as this template is only used for the legacy system and therefore not relevant for your use-case. Otherwise LGTM.

[crazoes]

Thanks, I updated the commit

[a-wai]

LGTM, but it seems something is still wrong: weird output in LAVA

[crazoes]

@a-wai this usually happens when the kernel has not been updated. If the kernel doesn't have the patch to add support for new parameters then test will not work as expected.
Do you know if we are using latest kernel image from the KernelCI repo?

[nuclearcat]

We are almost always using tip of tree images (latest).
We need to make sure this patches works on all kernels, including stable-rc trees.

[a-wai]

This job was running on 6.11-rc4, so not exactly an old kernel... Are additional patches required?

[crazoes]

@a-wai yes we need additional patch in the kernel in order to run the suspend and hibernation test with RTC.
Shuah is going to merge it in the current merge window but we didn't want to wait for it so @laura-nao had added the required patch to our linux repo https://github.com/kernelci/linux/tree/staging-mainline

Btw, do you know which branch do we use from the above? Because I don't see my patch added to the staging mainline branch.

https://lore.kernel.org/lkml/20240715192634.19272-1-shreeya.patel@collabora.com/

[a-wai]

Ah, then you need to add rules to this job so it only runs on the staging-mainline branch; the LAVA job I linked to ran on Linus' tree, so without your patches.

[crazoes]

If everything looks good then can we merged this now @nuclearcat @a-wai ?

[laura-nao]

@laura-nao had added the required patch to our linux repo https://github.com/kernelci/linux/tree/staging-mainline

@crazoes just for clarity, I didn't add the patch to staging-mainline but to the collabora-next tree. Fwiw you can run the test on the next tree now since the patch already landed there.

[nfraprado]

If this is ready can it be merged? I'm waiting on it to resume working on #2577. Thanks.

[nuclearcat]

I verified one of latest jobs, https://staging.kernelci.org:9000/viewer?node_id=66e2bc615785c698a0038303, can spot

      parameters:
        ENV: ''
        SKIPFILE: /dev/null
        SKIP_INSTALL: true
        TESTPROG_URL: https://kciapistagingstorage1.file.core.windows.net/staging/kbuild-gcc-12-x86-66e2b3ea5785c698a0037dda/kselftest.tar.gz?sv=2022-11-02&ss=f&srt=sco&sp=r&se=2024-10-17T19:19:12Z&st=2023-10-17T11:19:12Z&spr=https&sig=sLmFlvZHXRrZsSGubsDUIvTiv%2BtzgDq6vALfkrtWnv8%3D

So ENV variable seems present and properly formatted, even it is empty.
Merging, thanks.