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

Add support for gatling 3.0 #19

Merged
merged 4 commits into from
Nov 21, 2018
Merged

Conversation

CucumisSativus
Copy link
Contributor

First of all, thanks for you awesome work.

Recently I updated my tests to gatling 3.0 and to still use them with your image i needed those changes. Hopefully they will be useful for others :)

libc6-compat is required since gatling tries to load libnetty, which fails, because there is no libcrypt.so.1 in the system

libc6-compat is required since gatling tries to load libnetty, which fails
because there is no libcrypt.so.1 in the system
WORKDIR /opt

# gating version
ENV GATLING_VERSION 3.0.0-RC4
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
ENV GATLING_VERSION 3.0.0-RC4
ENV GATLING_VERSION 3.0.0

Since the stable version was just released, maybe we can use the stable version instead?

RUN mkdir -p gatling

# install gatling
RUN apk add --update wget bash && \
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be better to move the installation of the libgc-compact dependency here

@CucumisSativus
Copy link
Contributor Author

So since 3.0.0 is released I changed the name of the directory, added mention about 3.0.0 to readme and updated link to the latest release

3.0.0/Dockerfile Outdated
@@ -0,0 +1,39 @@
# Gatling is a highly capable load testing tool.
#
# Documentation: https://gatling.io/docs/2.3/
Copy link

Choose a reason for hiding this comment

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

Is now redirected to https://gatling.io/docs/current/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point! I will put there https://gatling.io/docs/3.0/cheat-sheet/ to make sure it will be still valid even after release of new gatling version. Thanks for pointing that out

3.0.0/Dockerfile Outdated
# Gatling is a highly capable load testing tool.
#
# Documentation: https://gatling.io/docs/2.3/
# Cheat sheet: http://gatling.io/#/cheat-sheet/2.3
Copy link

Choose a reason for hiding this comment

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

@guikcd
Copy link

guikcd commented Oct 30, 2018

Also gatling is now compatible with Java 9, 10 and 11(see gatling/gatling#3359)

But the openjdk image seems to have dropped alpine support and have now a slim version:

  • 8-jdk-alpine 73 MB
  • 10-jre-slim 105 MB
  • 10-jdk-slim 301 MB

If the switch to more modern images is done, the first instruction of packages need to be changed to use apt instead of apk, for example:

FROM openjdk:11-jre-slim
[...]
RUN apt update && apt -y install wget unzip && \
  rm -rf /var/cache/apt/* /var/lib/apt/lists/* && \
[...]

I've used the gatling default provided simulation: gatling.sh --simulation computerdatabase.BasicSimulation --run-description test

@CucumisSativus
Copy link
Contributor Author

@soedar what do you think about changing the base image? If you think that update is needed, should we do it in this PR or create a separate one for this?

@soedar
Copy link
Contributor

soedar commented Nov 5, 2018

@CucumisSativus I'm not sure actually, and I do not have merge permission to the repository. Ping @denvazh what do you think?

@CucumisSativus
Copy link
Contributor Author

@soedar sorry my mistake. Let's ping @denvazh for the second time, just in case ;)

@denvazh
Copy link
Owner

denvazh commented Nov 21, 2018

@CucumisSativus @soedar thank you for pull request and sorry for the delay. I had a lot of stuff on my plate, but I was almost on time to prepare release for rc1 to 3 (#20), thus I will still keep them (and also include 4 while at it) and also merge this PR on top of these changes.

@denvazh
Copy link
Owner

denvazh commented Nov 21, 2018

@CucumisSativus @guikcd for the base java image I think it makes more sense to merge this PR as-is and release and then work on switch.
Personally, I don't really care which base version is used. Alpine was used mainly because of its small image size.
In any case, I have created an issue regarding the base image switch: #21

@denvazh denvazh merged commit 3525f43 into denvazh:master Nov 21, 2018
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