-
Notifications
You must be signed in to change notification settings - Fork 20
Modify Gatling Runner Pod’s Multi-Containers Structure to make gatling-runner run as main container #65
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
Conversation
Signed-off-by: Yoichi Kawasaki <yokawasa@gmail.com>
Signed-off-by: Yoichi Kawasaki <yokawasa@gmail.com>
Signed-off-by: Yoichi Kawasaki <yokawasa@gmail.com>
Signed-off-by: Yoichi Kawasaki <yokawasa@gmail.com>
Signed-off-by: Yoichi Kawasaki <yokawasa@gmail.com>
@@ -43,4 +43,4 @@ Gatling Runner Pod runs multiple containers. The diagram below shows how those c | |||
- gatling-result-transferer | |||
- Uploads the simulation.log file to to user-selected Cloud Storage (can be configured in `.spec.cloudStorageSpec`) | |||
|
|||
📝 It is noted that gatling-waiter and gatling-runner run as init containers and gatling-result-transferer as a main container in Gatling Runner Pod in the case of generating an aggregated Gatling result report. However it's also noted that gatling-waitner runs as an init container and gatling-runner run as a main container in the case of not generating the report. | |||
📝 It is noted that **from gatling-operator-v0.8.1** gatling-waiter runs as an init container and both gatling-runner and gatling-result-transferer runs as main containers in a Gatling Runner Pod. However, **before gatling-operator-v0.8.1** both gatling-waiter and gatling-runner run as init containers and gatling-result-transferer as a main container in the case of generating an aggregated Gatling result report while gatling-runner runs as a main container in the case of not generating the report. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
v0.8.1 is going to be the next version to be released( which of course hasn't been released yet at this point)
Signed-off-by: Yoichi Kawasaki <yokawasa@gmail.com>
pkg/commands/commands.go
Outdated
@@ -66,6 +66,10 @@ if [ ! -d ${RESULTS_DIR_PATH} ]; then | |||
mkdir -p ${RESULTS_DIR_PATH} | |||
fi | |||
gatling.sh -sf ${SIMULATIONS_DIR_PATH} -s %s -rsf ${RESOURCES_DIR_PATH} -rf ${RESULTS_DIR_PATH} %s | |||
|
|||
if [ $? -eq 0 ]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yokawasa
I feel that if the gatling-runner container fails, the COMPLETED file is not created and the gatling-result-transfer container does not terminate.
How about creating a FAILED file if gatling-runner fails and a SUCCEEDED file if it succeeds?
(I'm sure there are other ways to do this)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ksudate
That's a great point ❤️ !!
Yes, like you said, it's better to terminate the gatling-result-transfer container as quickly as possible when the runner fails. I'll take your idea to create FAILED
file.
Thanks for the comment!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the fix.
Looks like it's all good!
Signed-off-by: Yoichi Kawasaki <yokawasa@gmail.com>
@ksudate I would go ahead to merge this if there is no more review comment within today |
Description
Summary
The PR includes the following 3 updates:
About Gatling Runner Pods multi-container structure update
What's achieved with the PR is that gatling-waiter runs as an init container and both gatling-runner and gatling-result-transferer should run as main containers in a Gatling Runner Pod.
Currently, as described in architecture diagram (see Gatling Operator Architecture and Design), gatling-waiter and gatling-runner container run as init containers and gatling-result-transferer as a main container in the case of generating an aggregated Gatling result report while gatling-runner runs as a main container in the case of not generating the report.
For easy Pod architecture comparison, here is before and after image
Before
After
Testing
Test1 - to see if gatling benchmark run and the report is generated as expected
base gatling CR is this. here i enabled
generateReport
like thisapply the manifest and see the result
See the report URL
then accessing reportUrl with your browser. the result page was generated successfully
Test2 - to see if gatling runner container run as main container
Here are dump output. As you can see the gatling runner container run as main container .
More specifically gatling-waiter run as init container, and both gatling-runner & gatling-result-transferer as main containers
Test3 - to see if gatling-result-transferer container ends as quickly as gatling-runner container fails
run the gatling runner pod in the same way as Testing1
Then fails the gatling runner container while it's running
You can force the gatling runner container to fail with the following script
It is observed that gatling-result-transferer container ends as quickly as gatling-runner container fails
Checklist
Relevant issue