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

[Bug] [deploy on k8s] SPRING_DATASOURCE_DRIVER_CLASS_NAME need add in templates/_helpers.tpl #10103

Closed
2 of 3 tasks
zwZjut opened this issue May 18, 2022 · 23 comments · Fixed by #10128
Closed
2 of 3 tasks
Labels
backend bug Something isn't working
Milestone

Comments

@zwZjut
Copy link
Contributor

zwZjut commented May 18, 2022

Search before asking

  • I had searched in the issues and found no similar issues.

What happened

[Bug] [deploy on k8s] SPRING_DATASOURCE_DRIVER_CLASS_NAME need add in templates/_helpers.tpl

What you expected to happen

[Bug] [deploy on k8s] SPRING_DATASOURCE_DRIVER_CLASS_NAME need add in templates/_helpers.tpl

How to reproduce

[Bug] [deploy on k8s] SPRING_DATASOURCE_DRIVER_CLASS_NAME need add in templates/_helpers.tpl

Anything else

[Bug] [deploy on k8s] SPRING_DATASOURCE_DRIVER_CLASS_NAME need add in templates/_helpers.tpl

Version

dev

Are you willing to submit PR?

  • Yes I am willing to submit a PR!

Code of Conduct

@zwZjut zwZjut added bug Something isn't working Waiting for reply Waiting for reply labels May 18, 2022
@zwZjut
Copy link
Contributor Author

zwZjut commented May 18, 2022

add in values.yaml;
postgresql:
driver: org.postgresql.Driver

add in templates/_helpers.tpl ;

  • name: SPRING_DATASOURCE_DRIVER_CLASS_NAME
    {{- if .Values.postgresql.enabled }}
    value: {{ .Values.postgresql.driver }}
    {{- else }}
    value: {{ .Values.externalDatabase.driver }}
    {{- end }}

@github-actions
Copy link

Thank you for your feedback, we have received your issue, Please wait patiently for a reply.

  • In order for us to understand your request as soon as possible, please provide detailed information、version or pictures.
  • If you haven't received a reply for a long time, you can join our slack and send your question to channel #troubleshooting

@songjianet songjianet assigned songjianet and unassigned songjianet May 18, 2022
@songjianet songjianet added backend and removed Waiting for reply Waiting for reply labels May 18, 2022
@songjianet songjianet added this to the 3.0.0-beta-2 milestone May 18, 2022
@qingwli
Copy link
Member

qingwli commented May 19, 2022

Hi @zwZjut, I don't think we need to SPRING_DATASOURCE_DRIVER_CLASS_NAME.

Here is the reason.

You can see this define the DATABASE

And when you build the Docker image, will run the ./bin/start.sh on Api || Matset || Worker || Alert-Server.

The ./bin/start.sh will source $DOLPHINSCHEDULER_HOME/conf/dolphinscheduler_env.sh

And dolphinscheduler_env.sh will set the SPRING_PROFILES_ACTIVE , you can see the code here

Then you can see the driver-class-name will be chosen from the different data sources. postgresql and mysql

@qingwli
Copy link
Member

qingwli commented May 19, 2022

I have checked the yaml about Api || Matset || Worker || Alert-Server || tools, and they are two bugs.

  • tools module changes the this but not changed the yaml.

When you change the data source to Mysql but will still use data source postgresql

  • Woker module yaml does not contains Mysql config

@qingwli
Copy link
Member

qingwli commented May 19, 2022

And I have another question about running the Docker image.

$ docker run -d --name dolphinscheduler-tools \
    -e DATABASE="postgresql" \
    # Use "com.mysql.cj.jdbc.driver" if you use MySQL
    -e SPRING_DATASOURCE_DRIVER_CLASS_NAME="org.postgresql.Driver" \
    -e SPRING_DATASOURCE_URL="jdbc:postgresql://localhost:5432/<DATABASE>" \
    -e SPRING_DATASOURCE_USERNAME="<USER>" \
    -e SPRING_DATASOURCE_PASSWORD="<PASSWORD>" \
    --net host \
    apache/dolphinscheduler-tools:"${DOLPHINSCHEDULER_VERSION}" tools/bin/upgrade-schema.sh 

Do we need to config -e SPRING_DATASOURCE_DRIVER_CLASS_NAME="org.postgresql.Driver" \ ?

The reason please look commented above

WDYT @SbloodyS @caishunfeng

@SbloodyS
Copy link
Member

And I have another question about running the Docker image.

$ docker run -d --name dolphinscheduler-tools \
    -e DATABASE="postgresql" \
    # Use "com.mysql.cj.jdbc.driver" if you use MySQL
    -e SPRING_DATASOURCE_DRIVER_CLASS_NAME="org.postgresql.Driver" \
    -e SPRING_DATASOURCE_URL="jdbc:postgresql://localhost:5432/<DATABASE>" \
    -e SPRING_DATASOURCE_USERNAME="<USER>" \
    -e SPRING_DATASOURCE_PASSWORD="<PASSWORD>" \
    --net host \
    apache/dolphinscheduler-tools:"${DOLPHINSCHEDULER_VERSION}" tools/bin/upgrade-schema.sh 

Do we need to config -e SPRING_DATASOURCE_DRIVER_CLASS_NAME="org.postgresql.Driver" \ ?

The reason please look commented above

WDYT @SbloodyS @caishunfeng

Yes. It's needed.

@kezhenxu94
Copy link
Member

And I have another question about running the Docker image.

$ docker run -d --name dolphinscheduler-tools \
    -e DATABASE="postgresql" \
    # Use "com.mysql.cj.jdbc.driver" if you use MySQL
    -e SPRING_DATASOURCE_DRIVER_CLASS_NAME="org.postgresql.Driver" \
    -e SPRING_DATASOURCE_URL="jdbc:postgresql://localhost:5432/<DATABASE>" \
    -e SPRING_DATASOURCE_USERNAME="<USER>" \
    -e SPRING_DATASOURCE_PASSWORD="<PASSWORD>" \
    --net host \
    apache/dolphinscheduler-tools:"${DOLPHINSCHEDULER_VERSION}" tools/bin/upgrade-schema.sh 

Do we need to config -e SPRING_DATASOURCE_DRIVER_CLASS_NAME="org.postgresql.Driver" \ ?

SPRING_DATASOURCE_DRIVER_CLASS_NAME shouldn't be specified, DATABASE is aimed at configuring the database related info, when users set DATABASE=postgresql the driver should be set automatically

@kezhenxu94
Copy link
Member

kezhenxu94 commented May 20, 2022

@liqingwang please add a profile postgresql here and set correct driver in the yaml

---
spring:
config:
activate:
on-profile: mysql
datasource:
driver-class-name: com.mysql.jdbc.Driver
url: jdbc:mysql://127.0.0.1:3306/dolphinscheduler?useUnicode=true&characterEncoding=UTF-8

 --- 
 spring: 
   config: 
     activate: 
       on-profile: postgresql 
   datasource: 
     driver-class-name: org.postgresql.Driver

@qingwli
Copy link
Member

qingwli commented May 20, 2022

I agree.

If we don't need to specify the SPRING_DATASOURCE_DRIVER_CLASS_NAME, then we don't need this field when you start at Docker mode or K8s mode?

So this code doesn't need? @kezhenxu94

@kezhenxu94
Copy link
Member

I agree.

If we don't need to specify the SPRING_DATASOURCE_DRIVER_CLASS_NAME, then we don't need this field when you start at Docker mode or K8s mode?

So this code doesn't need? @kezhenxu94

Yes, we'd better keep the configurations minimal whenever they can be specified automatically, so users won't be overwhelming by so many configurations.

@qingwli
Copy link
Member

qingwli commented May 20, 2022

And we can't delete source "$DOLPHINSCHEDULER_HOME/conf/dolphinscheduler_env.sh" in docker mode, this sh will export SPRING field, am I correct?

@qingwli
Copy link
Member

qingwli commented May 20, 2022

Is my understanding of the driver set automatically correct? #10103 (comment)

@kezhenxu94
Copy link
Member

Is my understanding of the driver set automatically correct? #10103 (comment)

This is right

@kezhenxu94
Copy link
Member

And we can't delete source "$DOLPHINSCHEDULER_HOME/conf/dolphinscheduler_env.sh" in docker mode, this sh will export SPRING field, am I correct?

Hi @liqingwang , you don't need to delete source "$DOLPHINSCHEDULER_HOME/conf/dolphinscheduler_env.sh", if you set any env var in Kubernetes, it will override dolphinscheduler_env.sh, did you find anything different?

@qingwli
Copy link
Member

qingwli commented May 20, 2022

Yes, but only one difference, we don't define SPRING_DATASOURCE_DRIVER_CLASS_NAME in Kubernetes config file, but I define the database.

And source "$DOLPHINSCHEDULER_HOME/conf/dolphinscheduler_env.sh will export SPRING_PROFILES_ACTIVE=${DATABASE} , this will change the database driver, so we need this sh

@kezhenxu94
Copy link
Member

Yes, but only one difference, we don't define SPRING_DATASOURCE_DRIVER_CLASS_NAME in Kubernetes config file, but I define the database.

And source "$DOLPHINSCHEDULER_HOME/conf/dolphinscheduler_env.sh will export SPRING_PROFILES_ACTIVE=${DATABASE} , this will change the database driver, so we need this sh

right

@qingwli
Copy link
Member

qingwli commented May 20, 2022

I will follow the previous design pattern to change the code.

So here is another question. Pls see this #10128 (comment) , the active profile is to choose the database config, when we define the active profiles, and tools module can't change driver automatically. This is a bug. @SbloodyS

@SbloodyS
Copy link
Member

I will follow the previous design pattern to change the code.

So here is another question. Pls see this #10128 (comment) , the active profile is to choose the database config, when we define the active profiles, and tools module can't change driver automatically. This is a bug. @SbloodyS

Yes.

@kezhenxu94
Copy link
Member

I will follow the previous design pattern to change the code.

So here is another question. Pls see this #10128 (comment) , the active profile is to choose the database config, when we define the active profiles, and tools module can't change driver automatically. This is a bug. @SbloodyS

As I said here #10103 (comment), can you update this in #10128 and see whether that satisfy your requirement?

@qingwli
Copy link
Member

qingwli commented May 20, 2022

I'll have a try, thx @kezhenxu94

@qingwli
Copy link
Member

qingwli commented May 20, 2022

I am not sure If I use java -Dspring.profiles.active=upgrade to define the profile java used, and then export SPRING_PROFILES_ACTIVE=mysql, Spring will use both of them, or MySQL will overwritten upgrade

@kezhenxu94
Copy link
Member

I am not sure If I use java -Dspring.profiles.active=upgrade to define the profile java used, and then export SPRING_PROFILES_ACTIVE=mysql, Spring will use both of them, or MySQL will overwritten upgrade

It will be overridden

@qingwli
Copy link
Member

qingwli commented May 24, 2022

I used -Dspring.profiles.active=upgrade,${DATABASE} to use two profiles and the driver will be set automatically

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants