grafana / k6-operator

An operator for running distributed k6 tests.
Apache License 2.0
576 stars 158 forks source link

Add Volume and VolumeMount support for runners #250

Closed andrei-trandafir closed 1 year ago

andrei-trandafir commented 1 year ago

Feature Description

Currently Volumes and VolumeMounts are set depending on the script configuration, which doesn't cover scenarios where we would want to fetch the script from other sources or bringing other artifacts. Adding option to customise the Volume and VolumeMounts to runners would solve this issue.

Suggested Solution (optional)

No response

Already existing or connected issues / PRs (optional)

https://github.com/grafana/k6-operator/pull/249

yorugac commented 1 year ago

Hi @andrei-trandafir! Thanks for opening the issue and the PR.

Looking at your PR, it seems like the main use case here is to have a place to store the script (or achive) downloaded by init container specifically while custom volumes in runners are more a way to reach that goal. Is this understanding correct or are there other cases which can be solved by defining custom volumes in runners?

I'd like to clarify this point because there are some overlaps between this issue and functionality developed in the PR https://github.com/grafana/k6-operator/pull/239 :sweat_smile: Over there, the implementation is very specific for init container case and certain type of test.

andrei-trandafir commented 1 year ago

Hey @yorugac , thanks for the quick response.

You're understanding is correct, we're using volumes in order to move data from the init container to the main container of the runner pod. This data would be either:

It might also be important to note that we output our real-time metrics to Datadog, rather than k6 cloud.

Is this something the that PR you mentioned adds as functionality and what are the timelines of the functionality being made available?

towens commented 1 year ago

Love this! It's quite common to mount a script from a configMap to a volume for use in initContainers. @yorugac can it get merged today? 😉

yorugac commented 1 year ago

@andrei-trandafir OK, thanks for clarifications. So, in the PR I linked, there's no way to specify volumes in K6 spec. However, there's a preliminary capability to specify a location of the remote script in K6 spec, i.e. a URL. Under the hood, both EmptyDir volume and init container are being created to download that script to that volume and then use it for the test itself. Right now that functionality is meant only for specific type of test run, but we could extend it to all test runs if it made sense for other use cases.

So far, it seems the main problem is to download script in init container. I imagine most people who need to download a script in init container really want to specify only the location of the script, i.e. they don't care about volumes themselves. That's why I wondered if there are use cases when volumes themselves should be specified in K6 API for other purposes, i.e. not for download of a remote script.

IOW, we have these two different approaches to changing K6 API, in order to achieve the main problem:

1) add volumes to K6 spec

I'm grokking this now but I'm inclined towards having volumes in K6 spec as you suggested, simply because I don't think its possible to figure out all possible use cases at the moment. So it's safer to have a more 'broad' solution, i.e. with volumes.

FYI, I'll tinker with this matter more on this week and will come back to your PR next week. And of course, if there are any additional thoughts, please share!

@towens thanks for the vote! Changes to API like a bit of thinking, esp. when they come in conflict between different approaches :smile: