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 dockerize to Dockerfile for grpc-server #376

Merged
merged 2 commits into from
Nov 22, 2021
Merged

Conversation

miseyu
Copy link
Contributor

@miseyu miseyu commented Nov 17, 2021

Summary

  • Add dockerize to Dockerfile

Why?

  • Since we have a use case where we want to be able to configure the database with environment variables.

Usage

$ docker run --env SCALAR_DB_CONTACT_POINTS=cassandra --env SCALAR_DB_CONTACT_PORT=9042 --env SCALAR_DB_USERNAME=cassandra --env SCALAR_DB_PASSWORD=cassandra --env SCALAR_DB_STORAGE=cassandra -d -p 60051:60051 -p 8080:8080 ghcr.io/scalar-labs/scalardb-server:<version> dockerize -template database.properties.tmpl:database.properties /scalardb/server/bin/scalardb-server --config=database.properties


ENV SCALARDB_SERVER_OPTS -Dlog4j.configurationFile=file:log4j2.properties

ENTRYPOINT ["./bin/scalardb-server"]
CMD ["--config", "database.properties"]
CMD ["./bin/scalardb-server", "--config", "database.properties"]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

To support backward compatibility

@miseyu miseyu self-assigned this Nov 17, 2021
@feeblefakie
Copy link
Contributor

@miseyu Thank you! Is this needed for the scalability test?
Maybe we should discuss what to change based on what is needed.

@miseyu
Copy link
Contributor Author

miseyu commented Nov 18, 2021

@feeblefakie
This is to support helm charts for converting database users and passwords to secrets.
Yes, Please let us discuss this separately.

@miseyu miseyu closed this Nov 18, 2021
@miseyu miseyu deleted the support-dockerize branch November 18, 2021 12:03
@miseyu miseyu restored the support-dockerize branch November 19, 2021 06:43
@miseyu miseyu reopened this Nov 19, 2021
feeblefakie
feeblefakie previously approved these changes Nov 19, 2021
Copy link
Contributor

@feeblefakie feeblefakie left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you!

WORKDIR /scalardb/server

COPY log4j2.properties .
# Support for use cases where environment variables are used to configure the database
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
# Support for use cases where environment variables are used to configure the database
# Support use cases where environment variables are used to configure the database

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have corrected the problem.
b66edbc

server/README.md Outdated
@@ -41,6 +41,9 @@ $ docker run -v <your local configuration file path>:/scalardb/server/database.p
# For custom log configuration
$ docker run -v <your local configuration file path>:/scalardb/server/database.properties -v <your custom log4j2 configuration file path>:/scalardb/server/log4j2.properties -d -p 60051:60051 -p 8080:8080 ghcr.io/scalar-labs/scalardb-server:<version>

# For set in environment variables
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
# For set in environment variables
# Use environment variables

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have corrected the problem.
b66edbc

thongdk8
thongdk8 previously approved these changes Nov 19, 2021
Copy link
Contributor

@thongdk8 thongdk8 left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you!

@miseyu
Copy link
Contributor Author

miseyu commented Nov 19, 2021

@feeblefakie
We have made corrections to your comment.
I'm sorry for the inconvenience, but could you please check again? 🙇
PTAL

Copy link
Collaborator

@brfrn169 brfrn169 left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you!

Copy link
Contributor

@feeblefakie feeblefakie left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you!

@feeblefakie feeblefakie merged commit 2044bcc into master Nov 22, 2021
@feeblefakie feeblefakie deleted the support-dockerize branch November 22, 2021 07:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants