You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by GitBox <gi...@apache.org> on 2021/02/16 22:03:05 UTC

[GitHub] [fineract] Yann-OAF opened a new pull request #1623: feat: Helm chart

Yann-OAF opened a new pull request #1623:
URL: https://github.com/apache/fineract/pull/1623


   ## Description
   
   This is a proper Helm chart to run it on Kubernetes, as a more flexible alternative to the existing deployment definitions and setup scripts. It uses the bitnami mysql chart as a dependency to install MySQL, so all these values can be overridden.
   
   To install, just run:
   
   ```sh
   # Install mysql chart dependency
   helm dependency update
   
   # Install chart
   helm upgrade --install --set mysql.auth.rootPassword=xxxx my-fineract helm/fineract
   ```
   
   ## Checklist
   
   Please make sure these boxes are checked before submitting your pull request - thanks!
   
   - [x] Write the commit message as per https://github.com/apache/fineract/#pull-requests
   
   - [x] Acknowledge that we will not review PRs that are not passing the build _("green")_ - it is your responsibility to get a proposed PR to pass the build, not primarily the project's maintainers.
   
   - [ ] Create/update unit or integration tests for verifying the changes made.
   
   - [ ] Follow coding conventions at https://cwiki.apache.org/confluence/display/FINERACT/Coding+Conventions.
   
   - [ ] Add required Swagger annotation and update API documentation at fineract-provider/src/main/resources/static/api-docs/apiLive.htm with details of any API changes
   
   - [x] Submission is not a "code dump".  (Large changes can be made "in repository" via a branch.  Ask on the developer mailing list for guidance, if required.)
   
   FYI our guidelines for code reviews are at https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide.
   


----------------------------------------------------------------
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.

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



[GitHub] [fineract] BLasan commented on pull request #1623: feat: Helm chart

Posted by GitBox <gi...@apache.org>.
BLasan commented on pull request #1623:
URL: https://github.com/apache/fineract/pull/1623#issuecomment-800799405


   > Note that there would be a lot more we could do... in particular the next item on my list is to implement a `helm test` hook, as well as making the `mysql` dependency optional, in case you already have an external instance running.
   > 
   > This is really only a quickstart...
   
   Great. Shall we wait until @vorburger and @edcable  's reviews ?  


----------------------------------------------------------------
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.

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



[GitHub] [fineract] edcable commented on pull request #1623: feat: Helm chart

Posted by GitBox <gi...@apache.org>.
edcable commented on pull request #1623:
URL: https://github.com/apache/fineract/pull/1623#issuecomment-793046490


   @Yann-J It was nice speaking to you the other day. @vidakovic our release manager would love to include this in the forthcoming Fineract 1.5 release - are you able to address the failing build? 


----------------------------------------------------------------
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.

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



[GitHub] [fineract] BLasan commented on pull request #1623: feat: Helm chart

Posted by GitBox <gi...@apache.org>.
BLasan commented on pull request #1623:
URL: https://github.com/apache/fineract/pull/1623#issuecomment-793325538


   > @BLasan have you noticed this PR? It looks like something you could be interested in helping to review... 
   > 
   > I was thinking about something when reviewing #1653: That (resource limits) should be done here as well? More importantly, we're now maintaining Kubernetes YAML in 2 places (original raw Kube YAML, and Helm chart templates). That's perhaps not ideal... you guys should figure out how you want to do this going forward? Keep both? Replace the original YAML with the Helm templates, and subsequently remove the originals? Up to you - work together.
   
   It would be great if we could add resource limits in order to run the image using minimum number of resources in the cluster. (Cluster resources will not be over eaten by then) If someone wants to run the product in k8s without cloning it, they could use the helm charts directly (after releasing this) What if we could move these helm implementations to a new repository and maintain it there? Don't know whether this is a good idea. But we could maintain this and can do releases separately.


----------------------------------------------------------------
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.

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



[GitHub] [fineract] Yann-J commented on a change in pull request #1623: feat: Helm chart

Posted by GitBox <gi...@apache.org>.
Yann-J commented on a change in pull request #1623:
URL: https://github.com/apache/fineract/pull/1623#discussion_r595324760



##########
File path: helm/fineract/templates/fineract-secret.yaml
##########
@@ -0,0 +1,25 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+kind: Secret
+apiVersion: v1
+metadata:
+  name: {{ .Release.Name }}

Review comment:
       Actually, it's already Kind = Secret so I think that might be a bit redundant...




----------------------------------------------------------------
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.

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



[GitHub] [fineract] vorburger commented on pull request #1623: feat: Helm chart

Posted by GitBox <gi...@apache.org>.
vorburger commented on pull request #1623:
URL: https://github.com/apache/fineract/pull/1623#issuecomment-793082166


   @BLasan have you noticed this PR? It looks like something you could be interested in helping to review... 😄 
   
   I was thinking about something when reviewing #1653: That (resource limits) should be done here as well? More importantly, we're now maintaining Kubernetes YAML in 2 places (original raw Kube YAML, and Helm chart templates). That's perhaps not ideal... you guys should figure out how you want to do this going forward? Keep both? Replace the original YAML with the Helm templates, and subsequently remove the originals? Up to you - work together.


----------------------------------------------------------------
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.

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



[GitHub] [fineract] Yann-J commented on pull request #1623: feat: Helm chart

Posted by GitBox <gi...@apache.org>.
Yann-J commented on pull request #1623:
URL: https://github.com/apache/fineract/pull/1623#issuecomment-800791746


   Note that there would be a lot more we could do... in particular the next item on my list is to implement a `helm test` hook, as well as making the `mysql` dependency optional, in case you already have an external instance running.
   
   This is really only a quickstart...


----------------------------------------------------------------
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.

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



[GitHub] [fineract] github-actions[bot] commented on pull request #1623: feat: Helm chart

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #1623:
URL: https://github.com/apache/fineract/pull/1623#issuecomment-821732202


   This pull request seems to be stale.  Are you still planning to work on it?  We will automatically close it in 30 days.


-- 
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.

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



[GitHub] [fineract] Yann-J commented on a change in pull request #1623: feat: Helm chart

Posted by GitBox <gi...@apache.org>.
Yann-J commented on a change in pull request #1623:
URL: https://github.com/apache/fineract/pull/1623#discussion_r595322033



##########
File path: helm/fineract/templates/fineract-server-deployment.yml
##########
@@ -0,0 +1,87 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: {{ .Release.Name }}-backend
+  namespace: {{ .Release.Namespace }}
+  labels:
+    app: fineract
+    tier: backend
+spec:
+  replicas: {{ .Values.replicas }}
+  selector:
+    matchLabels:
+      app: fineract-backend
+      tier: backend
+      instance: {{ .Release.Name }}
+  template:
+    metadata:
+      labels:
+        app: fineract-backend
+        tier: backend
+        instance: {{ .Release.Name }}
+      annotations:
+        checksum/secrets: {{ include "secrets" . | sha256sum }}
+    spec:
+      initContainers:
+        - name: wait-db
+          image: jwilder/dockerize
+          imagePullPolicy: IfNotPresent
+          args:
+          - -wait
+          - tcp://{{ include "mysql_host" . }}:3306
+          - -timeout
+          - 300s
+      containers:
+        - name: fineract-backend
+          image: "{{ .Values.fineractServer.image.name }}:{{ .Values.fineractServer.image.tag }}"
+          envFrom:
+            - secretRef:
+                name: {{ .Release.Name | quote }}
+          env:
+            - name: FINERACT_DEFAULT_TENANTDB_HOSTNAME
+              value: {{ include "mysql_host" . }}
+            - name: fineract_tenants_url
+              value: {{ include "mysql_url" . }}
+            - name: fineract_tenants_uid
+              value: {{ include "mysql_user" . }}
+            - name: FINERACT_DEFAULT_TENANTDB_UID
+              value: {{ include "mysql_user" . }}
+            {{ if .Values.fineractServer.extraEnv }}
+            {{- range $key, $value := .Values.fineractServer.extraEnv }}
+            - name: {{ $key | quote}}
+              value: {{ $value | quote }}
+            {{- end }}
+            {{- end }}
+          ports:
+            - containerPort: 8080
+              name: http
+          readinessProbe:

Review comment:
       👍 done!




----------------------------------------------------------------
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.

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



[GitHub] [fineract] BLasan commented on a change in pull request #1623: feat: Helm chart

Posted by GitBox <gi...@apache.org>.
BLasan commented on a change in pull request #1623:
URL: https://github.com/apache/fineract/pull/1623#discussion_r595293764



##########
File path: helm/fineract/values.yaml
##########
@@ -0,0 +1,83 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+# TODO: add option to install ActiveMQ?
+
+fineractServer:
+  replicas: 1
+  image:
+    name: apache/fineract
+    tag: latest
+  # key/value pairs to pass as environment variables to the backend pods
+  extraEnv:
+    DRIVERCLASS_NAME: org.drizzle.jdbc.DrizzleDriver
+    PROTOCOL: jdbc
+    SUB_PROTOCOL: mysql:thin
+    fineract_tenants_driver: org.drizzle.jdbc.DrizzleDriver
+    FINERACT_DEFAULT_TENANTDB_PORT: "3306"
+  # sensitive key/value pairs to pass as environment variables to the backend pods as secrets
+  extraSecretEnv: {}
+
+fineractUI:
+  replicas: 1
+  image:
+    name: openmf/community-app
+    tag: latest
+
+# Globals will be available to subcharts
+global:
+  db:
+    tenantsDb: fineract_tenants
+    defaultDb: fineract_default
+
+# Any other parameter from https://artifacthub.io/packages/helm/bitnami/mysql
+mysql:
+  enabled: true
+  # Required because db driver doesn't support mysql 8...
+  image:
+    tag: '5.7'
+  auth:
+    # Please change these...
+    rootPassword: ozIRdODs8Jvs3BzZywgK
+    password: ihcqsCcsX6

Review comment:
       Agree with that. I think there was the same issue in mysql deployment in local cluster. I've fixed that. This issue was raised in a previous PR https://github.com/apache/fineract/pull/1583




----------------------------------------------------------------
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.

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



[GitHub] [fineract] Yann-J commented on pull request #1623: feat: Helm chart

Posted by GitBox <gi...@apache.org>.
Yann-J commented on pull request #1623:
URL: https://github.com/apache/fineract/pull/1623#issuecomment-800789574


   > @BLasan have you noticed this PR? It looks like something you could be interested in helping to review... 😄
   > 
   > I was thinking about something when reviewing #1653: That (resource limits) should be done here as well? More importantly, we're now maintaining Kubernetes YAML in 2 places (original raw Kube YAML, and Helm chart templates). That's perhaps not ideal... you guys should figure out how you want to do this going forward? Keep both? Replace the original YAML with the Helm templates, and subsequently remove the originals? Up to you - work together.
   
   Regarding the scripts vs Helm, I would say that Helm is probably the de-facto standard nowadays for deploying to k8s, but not everyone will have it, so it could make sense to keep supporting both.
   
   However, I would say that in the long run, once it's quite stable, the Helm chart probably should be in a separate repo so it can be versioned independently from the application, and then published into https://artifacthub.io/
   


----------------------------------------------------------------
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.

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



[GitHub] [fineract] BLasan commented on a change in pull request #1623: feat: Helm chart

Posted by GitBox <gi...@apache.org>.
BLasan commented on a change in pull request #1623:
URL: https://github.com/apache/fineract/pull/1623#discussion_r590165099



##########
File path: helm/fineract/templates/fineract-server-deployment.yml
##########
@@ -0,0 +1,87 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: {{ .Release.Name }}-backend
+  namespace: {{ .Release.Namespace }}
+  labels:
+    app: fineract
+    tier: backend
+spec:
+  replicas: {{ .Values.replicas }}
+  selector:
+    matchLabels:
+      app: fineract-backend
+      tier: backend
+      instance: {{ .Release.Name }}
+  template:
+    metadata:
+      labels:
+        app: fineract-backend
+        tier: backend
+        instance: {{ .Release.Name }}
+      annotations:
+        checksum/secrets: {{ include "secrets" . | sha256sum }}
+    spec:
+      initContainers:
+        - name: wait-db
+          image: jwilder/dockerize
+          imagePullPolicy: IfNotPresent
+          args:
+          - -wait
+          - tcp://{{ include "mysql_host" . }}:3306
+          - -timeout
+          - 300s
+      containers:
+        - name: fineract-backend
+          image: "{{ .Values.fineractServer.image.name }}:{{ .Values.fineractServer.image.tag }}"
+          envFrom:
+            - secretRef:
+                name: {{ .Release.Name | quote }}
+          env:
+            - name: FINERACT_DEFAULT_TENANTDB_HOSTNAME
+              value: {{ include "mysql_host" . }}
+            - name: fineract_tenants_url
+              value: {{ include "mysql_url" . }}
+            - name: fineract_tenants_uid
+              value: {{ include "mysql_user" . }}
+            - name: FINERACT_DEFAULT_TENANTDB_UID
+              value: {{ include "mysql_user" . }}
+            {{ if .Values.fineractServer.extraEnv }}
+            {{- range $key, $value := .Values.fineractServer.extraEnv }}
+            - name: {{ $key | quote}}
+              value: {{ $value | quote }}
+            {{- end }}
+            {{- end }}
+          ports:
+            - containerPort: 8080
+              name: http
+          readinessProbe:

Review comment:
       You can use separate paths to check liveness and readiness probes. We have added configurations in to spring boot. See https://github.com/apache/fineract/pull/1649

##########
File path: helm/fineract/values.yaml
##########
@@ -0,0 +1,83 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+# TODO: add option to install ActiveMQ?
+
+fineractServer:
+  replicas: 1
+  image:
+    name: apache/fineract
+    tag: latest
+  # key/value pairs to pass as environment variables to the backend pods
+  extraEnv:
+    DRIVERCLASS_NAME: org.drizzle.jdbc.DrizzleDriver
+    PROTOCOL: jdbc
+    SUB_PROTOCOL: mysql:thin
+    fineract_tenants_driver: org.drizzle.jdbc.DrizzleDriver
+    FINERACT_DEFAULT_TENANTDB_PORT: "3306"
+  # sensitive key/value pairs to pass as environment variables to the backend pods as secrets
+  extraSecretEnv: {}
+
+fineractUI:
+  replicas: 1
+  image:
+    name: openmf/community-app
+    tag: latest
+
+# Globals will be available to subcharts
+global:
+  db:
+    tenantsDb: fineract_tenants
+    defaultDb: fineract_default
+
+# Any other parameter from https://artifacthub.io/packages/helm/bitnami/mysql
+mysql:
+  enabled: true
+  # Required because db driver doesn't support mysql 8...
+  image:
+    tag: '5.7'
+  auth:
+    # Please change these...
+    rootPassword: ozIRdODs8Jvs3BzZywgK
+    password: ihcqsCcsX6

Review comment:
       I don't know whether the following line would generate a proper random password. You can try this to generate the mysql password @Yann-J 
   
   `password: {{ randAlphaNum 64 | quote }}`
   
   Or else we can point a particular secret name to the get password. But in order to do that we need to create that secret first before installing helm chart
   Refer: https://stackoverflow.com/questions/56170052/how-not-to-overwrite-randomly-generated-secrets-in-helm-templates

##########
File path: helm/fineract/templates/fineract-server-deployment.yml
##########
@@ -0,0 +1,87 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: {{ .Release.Name }}-backend
+  namespace: {{ .Release.Namespace }}
+  labels:
+    app: fineract
+    tier: backend
+spec:
+  replicas: {{ .Values.replicas }}
+  selector:
+    matchLabels:
+      app: fineract-backend
+      tier: backend
+      instance: {{ .Release.Name }}
+  template:
+    metadata:
+      labels:
+        app: fineract-backend
+        tier: backend
+        instance: {{ .Release.Name }}
+      annotations:
+        checksum/secrets: {{ include "secrets" . | sha256sum }}
+    spec:
+      initContainers:
+        - name: wait-db
+          image: jwilder/dockerize
+          imagePullPolicy: IfNotPresent
+          args:
+          - -wait
+          - tcp://{{ include "mysql_host" . }}:3306
+          - -timeout
+          - 300s
+      containers:
+        - name: fineract-backend
+          image: "{{ .Values.fineractServer.image.name }}:{{ .Values.fineractServer.image.tag }}"
+          envFrom:
+            - secretRef:
+                name: {{ .Release.Name | quote }}
+          env:
+            - name: FINERACT_DEFAULT_TENANTDB_HOSTNAME
+              value: {{ include "mysql_host" . }}
+            - name: fineract_tenants_url
+              value: {{ include "mysql_url" . }}
+            - name: fineract_tenants_uid
+              value: {{ include "mysql_user" . }}
+            - name: FINERACT_DEFAULT_TENANTDB_UID
+              value: {{ include "mysql_user" . }}
+            {{ if .Values.fineractServer.extraEnv }}
+            {{- range $key, $value := .Values.fineractServer.extraEnv }}
+            - name: {{ $key | quote}}
+              value: {{ $value | quote }}
+            {{- end }}
+            {{- end }}

Review comment:
       I think you have missed this env.
   ```
           - name: FINERACT_DEFAULT_TENANTDB_PWD
             valueFrom:
               secretKeyRef:
                 name: fineract-tenants-db-secret
                 key: password
   ```




----------------------------------------------------------------
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.

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



[GitHub] [fineract] github-actions[bot] closed pull request #1623: feat: Helm chart

Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #1623:
URL: https://github.com/apache/fineract/pull/1623


   


-- 
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.

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



[GitHub] [fineract] BLasan commented on a change in pull request #1623: feat: Helm chart

Posted by GitBox <gi...@apache.org>.
BLasan commented on a change in pull request #1623:
URL: https://github.com/apache/fineract/pull/1623#discussion_r595316650



##########
File path: helm/fineract/templates/fineract-server-deployment.yml
##########
@@ -0,0 +1,87 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: {{ .Release.Name }}-backend
+  namespace: {{ .Release.Namespace }}
+  labels:
+    app: fineract
+    tier: backend
+spec:
+  replicas: {{ .Values.replicas }}
+  selector:
+    matchLabels:
+      app: fineract-backend
+      tier: backend
+      instance: {{ .Release.Name }}
+  template:
+    metadata:
+      labels:
+        app: fineract-backend
+        tier: backend
+        instance: {{ .Release.Name }}
+      annotations:
+        checksum/secrets: {{ include "secrets" . | sha256sum }}
+    spec:
+      initContainers:
+        - name: wait-db
+          image: jwilder/dockerize
+          imagePullPolicy: IfNotPresent
+          args:
+          - -wait
+          - tcp://{{ include "mysql_host" . }}:3306
+          - -timeout
+          - 300s
+      containers:
+        - name: fineract-backend
+          image: "{{ .Values.fineractServer.image.name }}:{{ .Values.fineractServer.image.tag }}"
+          envFrom:
+            - secretRef:
+                name: {{ .Release.Name | quote }}

Review comment:
       Use the secret name (this is bit confusing)

##########
File path: helm/fineract/templates/fineract-secret.yaml
##########
@@ -0,0 +1,25 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+kind: Secret
+apiVersion: v1
+metadata:
+  name: {{ .Release.Name }}

Review comment:
       Use `{{ .Release.Name }}-secret`.




----------------------------------------------------------------
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.

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



[GitHub] [fineract] Yann-J commented on a change in pull request #1623: feat: Helm chart

Posted by GitBox <gi...@apache.org>.
Yann-J commented on a change in pull request #1623:
URL: https://github.com/apache/fineract/pull/1623#discussion_r595296740



##########
File path: helm/fineract/templates/fineract-server-deployment.yml
##########
@@ -0,0 +1,87 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: {{ .Release.Name }}-backend
+  namespace: {{ .Release.Namespace }}
+  labels:
+    app: fineract
+    tier: backend
+spec:
+  replicas: {{ .Values.replicas }}
+  selector:
+    matchLabels:
+      app: fineract-backend
+      tier: backend
+      instance: {{ .Release.Name }}
+  template:
+    metadata:
+      labels:
+        app: fineract-backend
+        tier: backend
+        instance: {{ .Release.Name }}
+      annotations:
+        checksum/secrets: {{ include "secrets" . | sha256sum }}
+    spec:
+      initContainers:
+        - name: wait-db
+          image: jwilder/dockerize
+          imagePullPolicy: IfNotPresent
+          args:
+          - -wait
+          - tcp://{{ include "mysql_host" . }}:3306
+          - -timeout
+          - 300s
+      containers:
+        - name: fineract-backend
+          image: "{{ .Values.fineractServer.image.name }}:{{ .Values.fineractServer.image.tag }}"
+          envFrom:
+            - secretRef:
+                name: {{ .Release.Name | quote }}
+          env:
+            - name: FINERACT_DEFAULT_TENANTDB_HOSTNAME
+              value: {{ include "mysql_host" . }}
+            - name: fineract_tenants_url
+              value: {{ include "mysql_url" . }}
+            - name: fineract_tenants_uid
+              value: {{ include "mysql_user" . }}
+            - name: FINERACT_DEFAULT_TENANTDB_UID
+              value: {{ include "mysql_user" . }}
+            {{ if .Values.fineractServer.extraEnv }}
+            {{- range $key, $value := .Values.fineractServer.extraEnv }}
+            - name: {{ $key | quote}}
+              value: {{ $value | quote }}
+            {{- end }}
+            {{- end }}

Review comment:
       I don't think so, this one is handled with:
   
   ```
             envFrom:
               - secretRef:
                   name: {{ .Release.Name | quote }}
   ```




----------------------------------------------------------------
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.

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



[GitHub] [fineract] awasum commented on pull request #1623: feat: Helm chart

Posted by GitBox <gi...@apache.org>.
awasum commented on pull request #1623:
URL: https://github.com/apache/fineract/pull/1623#issuecomment-822228433


   @Yann-J  There are some Apache Rat failures in the build on Travis: https://travis-ci.com/github/apache/fineract/builds/220288308
   
   You could either add the licenses or ignore the files. After that @vorburger may give his go ahead or more feedback...


-- 
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.

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



[GitHub] [fineract] vorburger commented on a change in pull request #1623: feat: Helm chart

Posted by GitBox <gi...@apache.org>.
vorburger commented on a change in pull request #1623:
URL: https://github.com/apache/fineract/pull/1623#discussion_r589743950



##########
File path: helm/fineract/values.yaml
##########
@@ -0,0 +1,83 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+# TODO: add option to install ActiveMQ?
+
+fineractServer:
+  replicas: 1
+  image:
+    name: apache/fineract
+    tag: latest
+  # key/value pairs to pass as environment variables to the backend pods
+  extraEnv:
+    DRIVERCLASS_NAME: org.drizzle.jdbc.DrizzleDriver
+    PROTOCOL: jdbc
+    SUB_PROTOCOL: mysql:thin
+    fineract_tenants_driver: org.drizzle.jdbc.DrizzleDriver
+    FINERACT_DEFAULT_TENANTDB_PORT: "3306"
+  # sensitive key/value pairs to pass as environment variables to the backend pods as secrets
+  extraSecretEnv: {}
+
+fineractUI:
+  replicas: 1
+  image:
+    name: openmf/community-app
+    tag: latest
+
+# Globals will be available to subcharts
+global:
+  db:
+    tenantsDb: fineract_tenants
+    defaultDb: fineract_default
+
+# Any other parameter from https://artifacthub.io/packages/helm/bitnami/mysql
+mysql:
+  enabled: true
+  # Required because db driver doesn't support mysql 8...
+  image:
+    tag: '5.7'
+  auth:
+    # Please change these...
+    rootPassword: ozIRdODs8Jvs3BzZywgK
+    password: ihcqsCcsX6

Review comment:
       These default password here scare me, a lot... people will NOT change these - they never do.
   
   There must a better way to do this, with Helm; what would be the Helm equivalent of what we're doing in https://github.com/apache/fineract/blob/develop/kubernetes/kubectl-startup.sh#L22 with `kubectl create secret generic fineract-tenants-db-secret --from-literal=username=root --from-literal=password=$(head /dev/urandom | tr -dc A-Za-z0-9 | head -c 16)` ?




----------------------------------------------------------------
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.

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



[GitHub] [fineract] github-actions[bot] commented on pull request #1623: feat: Helm chart

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #1623:
URL: https://github.com/apache/fineract/pull/1623#issuecomment-844597720


   This pull request seems to be stale.  Are you still planning to work on it?  We will automatically close it in 30 days.


-- 
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.

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



[GitHub] [fineract] Yann-J commented on pull request #1623: feat: Helm chart

Posted by GitBox <gi...@apache.org>.
Yann-J commented on pull request #1623:
URL: https://github.com/apache/fineract/pull/1623#issuecomment-822202469


   Is there anything else you folks need from me on this one?


-- 
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.

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



[GitHub] [fineract] Yann-J commented on pull request #1623: feat: Helm chart

Posted by GitBox <gi...@apache.org>.
Yann-J commented on pull request #1623:
URL: https://github.com/apache/fineract/pull/1623#issuecomment-800404924


   OK, I've made an update that addresses most comments above (I think), in particular avoid setting default passwords, and raising an error if they're empty.
   
   Some tuning along the way, in particular all extra variables (Secret or not) are now evaluated as templates - don't know if that's useful here but it doesn't hurt, and I've seen many cases where it is. They're also all supporting multiline values. Again, not sure if it's super useful here but you never know.


----------------------------------------------------------------
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.

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



[GitHub] [fineract] Yann-J commented on a change in pull request #1623: feat: Helm chart

Posted by GitBox <gi...@apache.org>.
Yann-J commented on a change in pull request #1623:
URL: https://github.com/apache/fineract/pull/1623#discussion_r595710395



##########
File path: helm/fineract/values.yaml
##########
@@ -0,0 +1,83 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+# TODO: add option to install ActiveMQ?
+
+fineractServer:
+  replicas: 1
+  image:
+    name: apache/fineract
+    tag: latest
+  # key/value pairs to pass as environment variables to the backend pods
+  extraEnv:
+    DRIVERCLASS_NAME: org.drizzle.jdbc.DrizzleDriver
+    PROTOCOL: jdbc
+    SUB_PROTOCOL: mysql:thin
+    fineract_tenants_driver: org.drizzle.jdbc.DrizzleDriver
+    FINERACT_DEFAULT_TENANTDB_PORT: "3306"
+  # sensitive key/value pairs to pass as environment variables to the backend pods as secrets
+  extraSecretEnv: {}
+
+fineractUI:
+  replicas: 1
+  image:
+    name: openmf/community-app
+    tag: latest
+
+# Globals will be available to subcharts
+global:
+  db:
+    tenantsDb: fineract_tenants
+    defaultDb: fineract_default
+
+# Any other parameter from https://artifacthub.io/packages/helm/bitnami/mysql
+mysql:
+  enabled: true
+  # Required because db driver doesn't support mysql 8...
+  image:
+    tag: '5.7'
+  auth:
+    # Please change these...
+    rootPassword: ozIRdODs8Jvs3BzZywgK
+    password: ihcqsCcsX6

Review comment:
       Yeah... I would argue that the situation is a bit different in a Helm context since overriding parameters is dead simple (`helm install --set mysql.auth.password=...`), and really part of the Helm workflow, so the resistance is probably a bit lower there...
   
   Anyway my latest commit is addressing this, and provides an error message in case the values aren't provided.




----------------------------------------------------------------
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.

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



[GitHub] [fineract] BLasan commented on a change in pull request #1623: feat: Helm chart

Posted by GitBox <gi...@apache.org>.
BLasan commented on a change in pull request #1623:
URL: https://github.com/apache/fineract/pull/1623#discussion_r595719337



##########
File path: helm/fineract/templates/fineract-secret.yaml
##########
@@ -0,0 +1,25 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+kind: Secret
+apiVersion: v1
+metadata:
+  name: {{ .Release.Name }}

Review comment:
       Yuh, but you have used the prefix `{{ .Release.Name }}` for other resources. So instead of having just the `{{ .Release.Name }}` I would suggest to use `{{ .Release.Name }}-secret` as the secret name. That'll be something meaningful. But it's not a requirement with high priority. Have you done a release for this implementation? Hope this works fine :) The configurations you've done look good to me




----------------------------------------------------------------
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.

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



[GitHub] [fineract] BLasan commented on pull request #1623: feat: Helm chart

Posted by GitBox <gi...@apache.org>.
BLasan commented on pull request #1623:
URL: https://github.com/apache/fineract/pull/1623#issuecomment-800798473


   > Note that there would be a lot more we could do... in particular the next item on my list is to implement a `helm test` hook, as well as making the `mysql` dependency optional, in case you already have an external instance running.
   > 
   > This is really only a quickstart...
   
   
   
   > > @BLasan have you noticed this PR? It looks like something you could be interested in helping to review... smile
   > > I was thinking about something when reviewing #1653: That (resource limits) should be done here as well? More importantly, we're now maintaining Kubernetes YAML in 2 places (original raw Kube YAML, and Helm chart templates). That's perhaps not ideal... you guys should figure out how you want to do this going forward? Keep both? Replace the original YAML with the Helm templates, and subsequently remove the originals? Up to you - work together.
   > 
   > Regarding the scripts vs Helm, I would say that Helm is probably the de-facto standard nowadays for deploying to k8s, but not everyone will have it, so it could make sense to keep supporting both.
   > 
   > However, I would say that in the long run, once it's quite stable, the Helm chart probably should be in a separate repo so it can be versioned independently from the application, and then published into https://artifacthub.io/
   
   +1 @vorburger I think this is ideal as we can maintain the helm chart implmentations separately and do releases accordingly


----------------------------------------------------------------
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.

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



[GitHub] [fineract] Yann-J commented on a change in pull request #1623: feat: Helm chart

Posted by GitBox <gi...@apache.org>.
Yann-J commented on a change in pull request #1623:
URL: https://github.com/apache/fineract/pull/1623#discussion_r595285462



##########
File path: helm/fineract/values.yaml
##########
@@ -0,0 +1,83 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+# TODO: add option to install ActiveMQ?
+
+fineractServer:
+  replicas: 1
+  image:
+    name: apache/fineract
+    tag: latest
+  # key/value pairs to pass as environment variables to the backend pods
+  extraEnv:
+    DRIVERCLASS_NAME: org.drizzle.jdbc.DrizzleDriver
+    PROTOCOL: jdbc
+    SUB_PROTOCOL: mysql:thin
+    fineract_tenants_driver: org.drizzle.jdbc.DrizzleDriver
+    FINERACT_DEFAULT_TENANTDB_PORT: "3306"
+  # sensitive key/value pairs to pass as environment variables to the backend pods as secrets
+  extraSecretEnv: {}
+
+fineractUI:
+  replicas: 1
+  image:
+    name: openmf/community-app
+    tag: latest
+
+# Globals will be available to subcharts
+global:
+  db:
+    tenantsDb: fineract_tenants
+    defaultDb: fineract_default
+
+# Any other parameter from https://artifacthub.io/packages/helm/bitnami/mysql
+mysql:
+  enabled: true
+  # Required because db driver doesn't support mysql 8...
+  image:
+    tag: '5.7'
+  auth:
+    # Please change these...
+    rootPassword: ozIRdODs8Jvs3BzZywgK
+    password: ihcqsCcsX6

Review comment:
       Hello there, sorry for my delay here!
   
   We can't use `randAlphaNum` inside the chart because that would cause the value to be re-generated each time we deploy. However there's no problem to keep that value empty and implement a runtime warning to instruct users to provide a value.
   
   I'm not so sure that "people will NOT change these" is really true any more with Helm since it's just so easy to do... but for extra protection it makes sense to force it, I can do that.




----------------------------------------------------------------
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.

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