Include system env vars into Initializer job

[ausias-armesto]

As part of my specific deployment, I needed to add the system environment variables into the Initializer Job.

[yorugac]

Hi @ausias-armesto, thanks for the PR.
We've had and have issues on this topic:
#207
#375

Could you please describe details of your use case over there first?

On the subject of the change itself: ATM, I don't think it's a good idea to hard-code this option to all test runs in k6-operator. I'd suggest to look for a workaround first. If there is no workaround, it'd be good to understand why.

[ausias-armesto]

Hi @ausias-armesto, thanks for the PR. We've had and have issues on this topic: #187 #375

Could you please describe details of your use case over there first?

On the subject of the change itself: ATM, I don't think it's a good idea to hard-code this option to all test runs in k6-operator. I'd suggest to look for a workaround first. If there is no workaround, it'd be good to understand why.

Hi @yorugac ,

Thanks for the reply. I totally understand that the change may have impact on other test runs and I didn't consider that.

My repo is public and you can have a look: https://github.com/hoprnet/hoprd-test/tree/main/k6
I think that the reason was because it was needed to built the options variable which was created dynamically based on input environment variables. So I could use the same script and customise the environment variables named: ENVIRONMENT_NAME, WORKLOAD_NAME, SCENARIO_ITERATIONS, SCENARIO_DURATION, etc to the type of load testing execution that I need at any time.
Perhaps there is a better alternative to implement that kind of requirements.

This is the pipeline triggering the tests: https://github.com/hoprnet/hoprnet/blob/master/.github/workflows/load-tests.yaml

[ausias-armesto]

This is definitely related to #375

[yorugac]

In my prev comment: I meant issue #207 and not #187 🤦

Thanks for sharing details @ausias-armesto! AFAIS at the moment, it should be working fine assuming workloads are part of the image 🤔 What is the error you're getting when you try to run this?

[ausias-armesto]

Hi @yorugac,

I've checked again with the current helm chart version 3.5.0 and it works fine without adding the system environment variables. I think that I tested with 3.3.0 and it failed. But it's good that it's not needed anymore.

Thanks.

[ausias-armesto]

Hi @yorugac
I've come up with the reason why it was working. I added a default value to the environment variables, and then those values were taken by the initialiser job but the issue still exists.
If the k6 script requires some environment variables to build the options then it requires that those environment variables are set with a default value at the code level

This snippet of code would fail.

const TEST_TYPE= __ENV.TEST_TYPE
var options
if(TEST_TYPE ="TEST_TYPE1") {
 options = ....
} else {
 options = ....
}
export options

But if the env variable has a default value like in the following snippet, then it works

const TEST_TYPE= __ENV.TEST_TYPE || "DEFAULT_TEST_TYPE"
var options
if(TEST_TYPE ="TEST_TYPE1") {
 options = ....
} else {
 options = ....
}
export options

Adding the parameter as implemented in the PR would solve the issue as well, but the developer needs to be aware of that restriction.

[yorugac]

@ausias-armesto thanks for the update!

const TEST_TYPE= __ENV.TEST_TYPE || "DEFAULT_TEST_TYPE"

This way of using env vars repeats very often actually. For instance, it's probably present in all examples in quickpizza. Like a "k6 good practice" thing.

Perhaps, it could be pointed out more, esp. in the main docs 🤔 I can also add an additional emphasis on it in k6-operator docs for env vars, but it sounds like it is a proper workaround without having to pass system env vars to the jobs.

[yorugac]

I've just added the emphasis about env var defintion in k6-operator doc.

As an additional source of recommendations, we recently published this blog too: https://grafana.com/blog/2024/04/30/organizing-your-grafana-k6-performance-testing-suite-best-practices-to-get-started/
It also contains definition of env var in k6 script with the default value.
Hopefully, we'll have a better centralized documentation about such things one day.

I'm closing this PR after all, as the workaround exists and is basically a standard practice for k6 scripts. Thank you again @ausias-armesto, for looking into the issue!