Skip to content
This repository was archived by the owner on Jun 6, 2024. It is now read-only.

added necessary files for building opensearch-2.x docker image #1

Merged
merged 1 commit into from
Sep 8, 2022
Merged

Conversation

rishabh6788
Copy link
Collaborator

Description

This PR adds necessary files for building Opensearch docker images.

Issues Resolved

opensearch-project/opensearch-build#780

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.

README.md Outdated

* Change the title in this README
* Edit your repository description on GitHub
Master repository where Dockerfiles for Opensearch are hosted. These docker files are used to build images for Opensearch Offical Images
Copy link
Member

@gaiksaya gaiksaya Jul 26, 2022

Choose a reason for hiding this comment

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

Maybe say

This repository hosts DockerFiles for OpenSearch and OpenSearch Dashboards. These docker files are used to build Offical docker Images for both the products.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Since we decided on creating a separate repo for dashboards docker files don't think we should include dashboards here.

Copy link
Member

Choose a reason for hiding this comment

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

My bad. Forgot about that but needs a little rewording to avoid terms like master.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated to use Main.

Comment on lines +123 to +126
# This script should exit with the same code as the opensearch command, but
# it would be a breaking change. Next line should be uncommented for the
# next major release.
# exit ${opensearch_exit_code}
Copy link
Member

Choose a reason for hiding this comment

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

Maybe create an issue for this? @peterzhuamazon ?

Copy link
Member

Choose a reason for hiding this comment

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

I have little memory of this where is this coming from @rishabh6788 ?

Copy link
Collaborator Author

@rishabh6788 rishabh6788 Jul 28, 2022

Choose a reason for hiding this comment

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

Even I do not have any context about this. @gaiksaya can you provide some background about this issue?

Copy link
Member

Choose a reason for hiding this comment

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

No idea hence asked @peterzhuamazon . Create an issue in this repo and keep. It can come as an enhancement

Comment on lines +57 to +68
function terminateProcesses {
if kill -0 $OPENSEARCH_PID >& /dev/null; then
echo "Killing opensearch process $OPENSEARCH_PID"
kill -TERM $OPENSEARCH_PID
wait $OPENSEARCH_PID
fi
if kill -0 $PA_PID >& /dev/null; then
echo "Killing performance analyzer process $PA_PID"
kill -TERM $PA_PID
wait $PA_PID
fi
}
Copy link
Member

Choose a reason for hiding this comment

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

We will need to de-couple these two tho.
Do you want to fix this now or in a follow-up PR?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Will take it up in the follow up PR.
Need some more information around this.

Signed-off-by: Rishabh Singh <sngri@amazon.com>
@rishabh6788 rishabh6788 requested a review from dblock July 28, 2022 18:24
@rishabh6788
Copy link
Collaborator Author

Adding @dblock for visibility.
We are working to publish our Opensearch and Opensearch Dashboards docker images as docker official images on docker hub. To know more about docker official images program please see https://github.com/docker-library/official-images#readme.

In a nutshell the docker expects us to raise a PR against their repo https://github.com/docker-library/official-images which will contain a spec file in a certain format. That spec file contains information regarding the public git repository that contains the Dockerfile, commit id and branch information to check out, directory path to Dockerfile to name a few.
Docker uses a tool called bashbrew that reads the provided spec file, then finds the Dockerfile, and then builds, tags and pushes it to docker hub.

This PR is to add the Dockerfile that will be built by bashbrew tool. The Dockerfile needs to be built for the latest version of the software. All the older versions that were built before is preserved by docker and listed under official repositories tag.

Please review the Dockerfile and all the supporting files and provide your comments.
@bbarani

Copy link
Member

@gaiksaya gaiksaya left a comment

Choose a reason for hiding this comment

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

Please create issues for follow up issues and unknown content. Also I think we should create an issue to add config of the components (like PA, security) only if the component is present

@rishabh6788 rishabh6788 merged commit 8be2d16 into opensearch-project:main Sep 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants