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

Run devlake as a deployment instead of statefulset #130

Closed
wants to merge 2 commits into from

Conversation

albinvass
Copy link
Contributor

Currently the devlake pods are deployed with a statefulset and uses a persistent volume to generate a .env file on start.
This creates seemingly unecessary limits on how the deployment configuration can be updated, and limits how pods can be scheduled in a cluster.
Instead of generate a configmap for the .env file and run devlake as a deployment.

@IronCore864
Copy link
Member

Hi @albinvass, I think DevLake would need a .env file to work: https://github.com/apache/incubator-devlake/blob/main/backend/core/config/config_viper.go#L37.

I agree that this isn't following the 12-factor app rules, nor is it cloud-native.

I'd propose creating an issue in the main repo to support loading configurations from ENV vars; then the chart update would be easy:

  • statefulset to deployment
  • no xxx-devlake-config configmap as .env file mounted as a volume
  • directly envFrom/configMapRef from the xxx-devlake-config configmap in the deployment

Are you good with Golang? If so, maybe you can contribute a PR to add this small feature :) But it seems other deployment methods (like docker-compose) still rely on the .env file, so bear in mind to support both .env file and ENV vars.

@klesh
Copy link
Contributor

klesh commented May 18, 2023

To clarify, devlake supports both .env and ENV vars already, in fact, ENV vars are overriding the settings from the .env file.

The only reason .env is needed is to allow devlake to generate a random ENCODE_KEY on the first boot for sensitive information encryption such as github token in case the database is compromised.

Theoretically, the ENCODE_KEY can be generated by the user when deploying devlake, then we don't need the .env file at all. However, there are some concerns:

  1. Is it acceptable for our users to set up such an Encryption Key manually, I feel like it is hard for most of them to understand the concept.
  2. Can we semi-automate this process? maybe we could accept an manual input or generate one for them, and even prompt them to store the Key somewhere safe in case they want to redeploy it in the future. We have to support helm and docker-compose at the same time in a similar manner.

@hezyin @Startrekzky

@albinvass
Copy link
Contributor Author

Theoretically, the ENCODE_KEY can be generated by the user when deploying devlake, then we don't need the .env file at all. However, there are some concerns:

I'm assuming the ENCODE_KEY is used to encrypt anything stored in the database? In that case losing the encode key could lead to loss of data. So in my opinion it's better if devlake requires a bit more work from the user than hiding a secret that can create issues for the user long-term.

I did a quick scan of the docs and can't find any mention of the encode key in the documentation, maybe I missed it? In any case I feel like the importance of the secret should be more pronounced, and the easiest way to do it is to fail on start if it's not available. The devlake executable could even have a command or tool to generate the secret for the user.
https://devlake.apache.org/docs/Overview/Introduction

@klesh
Copy link
Contributor

klesh commented May 23, 2023

It is described in Upgrade.

Yes, I agree with you, hence #5235

@albinvass albinvass closed this Jun 15, 2023
@albinvass albinvass deleted the devlake-as-deployment branch June 15, 2023 07:50
@albinvass albinvass restored the devlake-as-deployment branch June 15, 2023 12:06
@albinvass albinvass reopened this Jun 15, 2023
@albinvass albinvass marked this pull request as ready for review June 15, 2023 12:12
@ZhangNing10
Copy link
Contributor

hi @albinvass , thanks for looking into the issue and creating the pr!
As stated by klesh, devlake would generate a random ENCODE_KEY to the .env file, and the path is specified via the environment variable ENV_PATH which is /app/config/.env.
If mounting /app/config/.env as a configmap, then devlake can't write the file.
So we plan to remove this behavior in v0.18, rename the ENCODE_KEY to ENCRYPTION_KEY which would be an environment variable, and let users generate it manually.
I have created a pr #144 about this.
Could you kindly have a look into the pr and help review it? thanks a ton!

@albinvass
Copy link
Contributor Author

Looks good to me, thanks! I'll close this PR in favour of yours.

@albinvass albinvass closed this Jun 19, 2023
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.

4 participants