-
Notifications
You must be signed in to change notification settings - Fork 80
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
Update Dockerfile further and update version to 0.5.0 #299
Conversation
Signed-off-by: Ian Hoang <hoangia@amazon.com>
Signed-off-by: Ian Hoang <hoangia@amazon.com>
Signed-off-by: Ian Hoang <hoangia@amazon.com>
Signed-off-by: Ian Hoang <hoangia@amazon.com>
@@ -25,6 +24,8 @@ RUN mkdir -p /opensearch-benchmark/.benchmark && \ | |||
|
|||
USER 1000 | |||
|
|||
ARG BUILD_DATE |
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.
Have we updated the relevant docker build command to pass this argument? @IanHoang
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.
CI Jenkins build already passes this in but it's not necessary for users who want to experiment with the dockerfile since it just affects the label. I can add it to the documentation still though.
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.
Can you please share the CI job that will build this dockerfile?
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.
CI Job: https://build.ci.opensearch.org/job/opensearch-benchmark-release/
Previous Example of Docker Build: https://build.ci.opensearch.org/job/opensearch-benchmark-release/6/console.
Downloaded the previous Dockerhub release and it has the BUILD_DATE and VERSION already incorporated:
"Labels": {
"org.label-schema.build-date": "2023-04-28T00:47:36Z",
"org.label-schema.description": "A community driven, open source project to run performance tests for OpenSearch",
"org.label-schema.license": "Apache-2.0",
"org.label-schema.name": "opensearch-benchmark",
"org.label-schema.schema-version": "1.0",
"org.label-schema.url": "https://opensearch.org/",
"org.label-schema.vcs-url": "https://github.com/opensearch-project/OpenSearch-Benchmark",
"org.label-schema.vendor": "OpenSearch-Project",
"org.label-schema.version": "0.4.1"
}
},
LGTM! |
Should not affect this PR, but it would be good to know why the non-PyPI image is so much smaller. |
It was using stages in docker build, which only copies the necessary part from stage 0 to stage 1. |
@@ -16,7 +15,7 @@ RUN apt-get -y update && \ | |||
RUN groupadd --gid 1000 opensearch-benchmark && \ | |||
useradd -d /opensearch-benchmark -m -k /dev/null -g 1000 -N -u 1000 -l -s /bin/bash benchmark | |||
|
|||
RUN python3 -m pip install opensearch-benchmark==$VERSION | |||
RUN if [ -z "$VERSION" ] ; then python3 -m pip install opensearch-benchmark ; else python3 -m pip install opensearch-benchmark==$VERSION ; fi |
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.
Might be cleaner:
python3 -m pip install opensearch-benchmark${VERSION:+==}$VERSION
Description
Update Version.txt to prep for 0.5.0 release. Also, added some additional fixes based on Peter's comments in #298 PR
Issues Resolved
Testing
Tested with manual test in Dockerfile after buildilng image
Tested miscellaneous commands
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.