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

Adding Benchmark Testing Environment #161

Closed
wants to merge 5 commits into from

Conversation

okhasawn
Copy link
Contributor

Description

This PR adds the ability to create an environment (using docker) where JMeter tests can be run repeatedly, where its' requests are sent to the traffic capture proxy server before going to a webserver (nginx). The purpose of this is to see the performance impact of our tool.

Issues Resolved

Migrations-1079

Testing

Check List

  • New functionality includes testing
    • All tests pass, including unit test, integration test and doctest
  • New functionality has been documented
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Omar Khasawneh <[email protected]>
Signed-off-by: Omar Khasawneh <[email protected]>
Signed-off-by: Omar Khasawneh <[email protected]>
@codecov
Copy link

codecov bot commented May 18, 2023

Codecov Report

Merging #161 (f37a357) into main (0783d7e) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #161   +/-   ##
=======================================
  Coverage   91.51%   91.51%           
=======================================
  Files          25       25           
  Lines        1155     1155           
  Branches      122      122           
=======================================
  Hits         1057     1057           
  Misses         83       83           
  Partials       15       15           

Copy link
Collaborator

@gregschohn gregschohn left a comment

Choose a reason for hiding this comment

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

A large percentage of these changes have been absorbed into the latest changes on the mainline that had been held in stasis for a couple weeks, but are now in. Those changes, however, don't include changes to the JMeter test code (JCommander) and the 3 container test rig wasn't included.

We need some of these changes and not others. The changes that we do need should be done in a way that's consistent w/ the rest of the docker solution.

  1. Containers should not build java code. They should pickup the artifacts from the project build directories. See the dockerSolution build.gradle file for examples for how to build the JMeter image. There's already an image being built for the proxy container that would be perfect to use (since that's what we'd be deploying). The nginx container can be built using an "external" Dockerfile, like the Elasticsearch or OpenSearch Benchmark images that the dockerSolution gradle project builds.
  2. That also means that you don't need to create zips/tars and extract them. You should never do build file operations outside of your build directories. Right now, you extract the file contents to $PROJECT/extracted. That's dangerous and creates for sloppier workspaces. Devs will already have build in their git ignore files - so everything that goes under will be ignored. Anything outside of that tells the developer - hey, this is configuration INTO my builds. If they try to make a change there, it will be wiped out next build. Lastly, you could end up accidentally writing over top some other configs/code.
  3. Command line arguments could be passed via a compose file/run command - but that will depend somewhat on how we find out how to deploy onto remote containers. Unless there's some gotcha that we don't know about for mapping entrypoint/run arguments to Fargate, we should likely just pass them as arguments. See https://docs.docker.com/engine/reference/builder/#cmd for how docker will pass run arguments appended to the ENTRYPOINT. Those can be set in the compose file directly as well.
  4. The nginx dockerfile and config should be moved from their old location. I'd expect to see either a move or a delete on those. From my read, there will now be two versions in the repo, which will just create confusion.

@okhasawn
Copy link
Contributor Author

okhasawn commented Jul 5, 2023

Closing. Created PR #231 instead, due to going through a whole new method of creating the containers.

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.

2 participants