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

Basic and super-experimental support for distributed execution #3205

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

na--
Copy link
Member

@na-- na-- commented Jul 18, 2023

What?

This PR adds very basic and very experimental support for distributed execution, including packaging and sending the script to agents, and setup() and teardown() handling. It is another sliver of the newly refactored #2816 and built on top of #3191 and #3204.

This is done via the new k6 agent and k6 coordinator sub-commands and in a completely backwards-compatible manner. They have been marked as Hidden because the feature is definitely not ready for any kind of serious use and is very experimental. It currently doesn't have any tests or a lot of the required error handling...

This PR also doesn't add support distributed metric handling (i.e. centralized thresholds or the end-of-test summary). That part of the original proof-of-concept PR was refactored and moved in a separate commit and can be added as a separate pull request later, to make reviewing and potentially merging each PR easier 🎉

Another big missing element in this PR is any sort of mutual authentication and authorization of the gRPC connection between k6 agent and k6 coordinator. Given that the communication between these components could potentially happen over the internet, that feature is a must before the feature could be considered anywhere near ready for general use.

Finally, the --instance-count CLI flag of k6 coordinator is probably somewhat basic and insufficient for a lot of complex use cases. A more flexible configuration should probably should probably also be supported. For example, k6 coordinator can rely on the --execution-segment-sequence option to know the number of desired instances and their respective segments. If we add some new option to allow users to specify instance tags (e.g. key=value metadata pair every k6 agent instance is started with), k6 coordinator will be able to match individual instances to individual segments.

And now, after I've described all of the shortcomings and missing features of this PR above, it's a reasonable question to ask why it should be merged without them:

Why?

In short, because all of the missing features of this PR (including end-to-end tests) can be added later! Even though this is far from a polished and ready-to-use implementation of distributed execution, it is completely backwards compatible and makes no breaking k6 changes! Notice how no existing k6 tests have been disabled or even changed in any way! 🎉

I'd argue that it will be easier to actually get all of these things done in small increments once this PR is merged in master 🤞 And even if nothing from the missing things is prioritized, it will still be better to have this merged, to reduce the required upkeep of these PoCs (i.e. reduce merge conflicts when rebasing) over time. Having this in master also helps to avoid logical conflicts of other big features and refactorigns with it (e.g. #3112 had some issues).

Finally, even in its current super-basic form, this might actually be useful to some people for some use cases. For example, grafana/k6-operator#223 might be better implemented on top of this feature instead of from scratch 🤔

Checklist

  • I have performed a self-review of my code.
  • I have added tests for my changes.
  • I have run linter locally (make ci-like-lint) and all checks pass.
  • I have run tests locally (make tests) and all tests pass.
  • I have commented on my code, particularly in hard-to-understand areas.

Related PR(s)/Issue(s)

@codecov-commenter
Copy link

codecov-commenter commented Jul 18, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

❗ No coverage uploaded for pull request base (local-execution-controller@19a30e2). Click here to learn what that means.

❗ Current head 5d55aff differs from pull request most recent head 2443ac6. Consider uploading reports for the commit 2443ac6 to get more accurate results

Additional details and impacted files
@@                      Coverage Diff                      @@
##             local-execution-controller    #3205   +/-   ##
=============================================================
  Coverage                              ?   71.32%           
=============================================================
  Files                                 ?      275           
  Lines                                 ?    20720           
  Branches                              ?        0           
=============================================================
  Hits                                  ?    14778           
  Misses                                ?     5016           
  Partials                              ?      926           
Flag Coverage Δ
ubuntu 71.27% <0.00%> (?)
windows 71.18% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

This adds the `k6 agent` and `k6 coordinator` sub-commands and adds a very simple way to do distributed execution, including packaging and sending the script to agents, and setup() and teardown() handling. However, it doesn't include automatic metric handling (e.g. thresholds and the end-of-test summary).
@na-- na-- force-pushed the basic-distributed-execution branch from 1506bfd to 2443ac6 Compare December 13, 2023 12:37
Base automatically changed from local-execution-controller to master January 11, 2024 11:44
@na-- na-- requested a review from a team as a code owner January 11, 2024 11:44
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.

4 participants