You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@devlake.apache.org by "albinvass (via GitHub)" <gi...@apache.org> on 2023/05/15 11:27:40 UTC

[GitHub] [incubator-devlake-helm-chart] albinvass opened a new pull request, #130: Run devlake as a deployment instead of statefulset

albinvass opened a new pull request, #130:
URL: https://github.com/apache/incubator-devlake-helm-chart/pull/130

   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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-devlake-helm-chart] klesh commented on pull request #130: Run devlake as a deployment instead of statefulset

Posted by "klesh (via GitHub)" <gi...@apache.org>.
klesh commented on PR #130:
URL: https://github.com/apache/incubator-devlake-helm-chart/pull/130#issuecomment-1552380363

   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 
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-devlake-helm-chart] albinvass closed pull request #130: Run devlake as a deployment instead of statefulset

Posted by "albinvass (via GitHub)" <gi...@apache.org>.
albinvass closed pull request #130: Run devlake as a deployment instead of statefulset
URL: https://github.com/apache/incubator-devlake-helm-chart/pull/130


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-devlake-helm-chart] IronCore864 commented on pull request #130: Run devlake as a deployment instead of statefulset

Posted by "IronCore864 (via GitHub)" <gi...@apache.org>.
IronCore864 commented on PR #130:
URL: https://github.com/apache/incubator-devlake-helm-chart/pull/130#issuecomment-1550731630

   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.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-devlake-helm-chart] albinvass commented on a diff in pull request #130: Run devlake as a deployment instead of statefulset

Posted by "albinvass (via GitHub)" <gi...@apache.org>.
albinvass commented on code in PR #130:
URL: https://github.com/apache/incubator-devlake-helm-chart/pull/130#discussion_r1193702963


##########
charts/devlake/templates/deployments.yaml:
##########
@@ -206,3 +206,113 @@ spec:
             path: {{ .Values.option.localtime }}
             type: File
         {{- end }}
+---
+# devlake
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: {{ include "devlake.fullname" . }}-lake
+  labels:
+    {{- include "devlake.labels" . | nindent 4 }}
+spec:
+  replicas: 1
+  serviceName: {{ include "devlake.fullname" . }}-lake

Review Comment:
   ```suggestion
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-devlake-helm-chart] klesh commented on pull request #130: Run devlake as a deployment instead of statefulset

Posted by "klesh (via GitHub)" <gi...@apache.org>.
klesh commented on PR #130:
URL: https://github.com/apache/incubator-devlake-helm-chart/pull/130#issuecomment-1559289598

   It is described in [Upgrade](https://devlake.apache.org/docs/GettingStarted/Upgrade).
   
   Yes, I agree with you, hence #5235


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-devlake-helm-chart] albinvass closed pull request #130: Run devlake as a deployment instead of statefulset

Posted by "albinvass (via GitHub)" <gi...@apache.org>.
albinvass closed pull request #130: Run devlake as a deployment instead of statefulset
URL: https://github.com/apache/incubator-devlake-helm-chart/pull/130


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-devlake-helm-chart] ZhangNing10 commented on pull request #130: Run devlake as a deployment instead of statefulset

Posted by "ZhangNing10 (via GitHub)" <gi...@apache.org>.
ZhangNing10 commented on PR #130:
URL: https://github.com/apache/incubator-devlake-helm-chart/pull/130#issuecomment-1594211721

   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 https://github.com/apache/incubator-devlake-helm-chart/pull/144 about this. 
   Could you kindly have a look into the pr and help review it? thanks a ton!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-devlake-helm-chart] albinvass commented on pull request #130: Run devlake as a deployment instead of statefulset

Posted by "albinvass (via GitHub)" <gi...@apache.org>.
albinvass commented on PR #130:
URL: https://github.com/apache/incubator-devlake-helm-chart/pull/130#issuecomment-1559077111

   > 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
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-devlake-helm-chart] albinvass commented on pull request #130: Run devlake as a deployment instead of statefulset

Posted by "albinvass (via GitHub)" <gi...@apache.org>.
albinvass commented on PR #130:
URL: https://github.com/apache/incubator-devlake-helm-chart/pull/130#issuecomment-1596987078

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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org