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

QE-746 As a QE member, I want to containerize Postgres Database clust… #38

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

egeaksoz-zz
Copy link

…er for XLD testing, so I can include this platform at every release

egeaksoz and others added 3 commits November 3, 2021 13:48
…er for XLD testing, so I can include this platform at every release
# Conflicts:
#	src/main/kotlin/ai/digital/integration/server/common/domain/Cluster.kt
#	src/main/kotlin/ai/digital/integration/server/deploy/internals/cluster/DeployDockerClusterHelper.kt
#	src/main/kotlin/ai/digital/integration/server/deploy/tasks/StartDeployIntegrationServerTask.kt
@@ -71,6 +77,14 @@ open class DeployDockerClusterHelper(val project: Project) {
return "${worker.dockerImage}:${getClusterVersion()}"
}

private fun getWorkerJdbcUrl(): String {
var workerJdbcUrl = "postgresql:5432/xld-db"
Copy link
Contributor

Choose a reason for hiding this comment

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

This is already not correct, as it depends what database is used, and this config has to be configurable.

private fun getWorkerJdbcUrl(): String {
var workerJdbcUrl = "postgresql:5432/xld-db"
if (isDatabaseLoadBalancerEnabled()) {
workerJdbcUrl = "haproxydb:5000/postgres"
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here, hard coded url is not acceptable.


services:
zoo1:
image: zookeeper:3.4
Copy link
Contributor

Choose a reason for hiding this comment

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

has to be configurable

zoo1:
image: zookeeper:3.4
hostname: zoo1
container_name: zoo1
Copy link
Contributor

Choose a reason for hiding this comment

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

Not the best names even for default value, and they should be also configurable

hostname: zoo1
container_name: zoo1
ports:
- 2191:2181
Copy link
Contributor

Choose a reason for hiding this comment

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

In all this file, all ports which are exposed have to be configurable.

depends_on:
- zoo1

zoo3:
Copy link
Contributor

Choose a reason for hiding this comment

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

The number of nodes and partitions have to be configurable.
I might want not 3 but 5 nodes.
Should be some simple way to specify it in config.
Like partitions: 5, and all this file is generated. Exposed ports have to be random, and the function used which identifies which port is free now. Check in the code, we have it.

- XL_CLUSTER_MODE=default
- XL_DB_PASSWORD=admin
- XL_DB_USERNAME=postgres
- XL_DB_URL=jdbc:postgresql://haproxydb:5000/postgres
Copy link
Contributor

Choose a reason for hiding this comment

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

not good

@@ -8,6 +8,7 @@ import org.gradle.kotlin.dsl.property
open class Cluster(objects: ObjectFactory) {
Copy link
Contributor

Choose a reason for hiding this comment

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

There is no documentation about it

Copy link
Contributor

Choose a reason for hiding this comment

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

And please create a blog post for that to explain on the high level how it works.

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.

2 participants