Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds support options.cloud #373

Merged
merged 2 commits into from
Feb 26, 2024
Merged

Conversation

olegbespalov
Copy link
Contributor

@olegbespalov olegbespalov commented Feb 19, 2024

What?

This change adds a new/actual way of defining cloud options (options.cloud) to the k6-operator. A logic mimics the one in the k6-core; the prior is for defining cloud options with a fallback to the load impact.

I haven't touched the README in that PR. It could be updated along with updates to the k6-docs (once we are confident to do that).

How did I test

TBH, I haven't done any testing apart from the running unit tests (make test). I also quickly searched for any existing tests that could be adjusted by introducing a new logic. I could add a simple test case for the unit testing the fallback logic of the new TestName and ProjectID functions if you think it's necessary or any other place that could be better for keeping those tests.

Why?

The options.cloud refactor has been merged (grafana/k6#3348) into k6-core, and the new way of defining options could appear soon.

Closes: #309

@olegbespalov olegbespalov self-assigned this Feb 19, 2024
Copy link
Collaborator

@yorugac yorugac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR, @olegbespalov.

This change looks like it should be backwards compatible so I assume it would work with any version of k6.

There is a question about exact format: please see my comment.

Regarding the unit test: given the long deprecation period (years over here), yes, a small unit test(s) for the new logic would be nice.

pkg/cloud/types.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@yorugac yorugac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the update, @olegbespalov. The change seems to be working as expected 👍

ℹ️ Until new release, the full impact of this change is noticeable only with image: "grafana/k6:master".

@yorugac yorugac merged commit df39311 into grafana:main Feb 26, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support options.cloud
2 participants