From 3bd793962e904ad6297f11e82a8315334f06b8ca Mon Sep 17 00:00:00 2001
From: tczekajlo <t.czekajlo@rasa.com>
Date: Tue, 3 Nov 2020 14:54:11 +0100
Subject: [PATCH] Changes based on the code review

---
 .github/workflows/ci.yml                         |  2 +-
 charts/rasa-x/templates/_helpers.tpl             | 16 ++++++++++------
 .../rasa-x/templates/db-migration-service.yaml   |  4 +---
 charts/rasa-x/values.yaml                        |  5 +++--
 4 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 52015de2..ed227961 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -105,7 +105,7 @@ jobs:
     - name: Validate that logging in works
       run: |
         TOKEN=$(curl --fail --data '{"username": "me", "password": "'${INITIAL_USER_PASSWORD}'"}' "http://localhost/api/auth" | jq -r '.access_token')
-        if [[ "${TOKEN}" == "" ]]; then
+        if [[ -z "${TOKEN}" ]]; then
           echo "Access token is empty: ${TOKEN}" && exit 1
         fi
         echo "Access token: ${TOKEN}"
diff --git a/charts/rasa-x/templates/_helpers.tpl b/charts/rasa-x/templates/_helpers.tpl
index 86446e36..b4eabb40 100644
--- a/charts/rasa-x/templates/_helpers.tpl
+++ b/charts/rasa-x/templates/_helpers.tpl
@@ -232,9 +232,7 @@ Return 'true' if required version to run the database migration service is corre
 If version is not valid semantic version then not use the DB migration service.
 */}}
 {{- define "db-migration-service.requiredVersion" -}}
-{{- if and (not (regexMatch "^(v[0-9]|0|[1-9]\\d*).(0|[1-9]\\d*)\\.(0|[1-9]\\d*)(?:-((?:0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\\.(?:0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?(?:\\+([0-9a-zA-Z-]+(?:\\.[0-9a-zA-Z-]+)*))?$" (include "db-migration-service.version" .))) (not .Values.dbMigrationService.ignoreVersionCheck) -}}
-{{- print "false" -}}
-{{- else if .Values.dbMigrationService.ignoreVersionCheck  -}}
+{{- if .Values.dbMigrationService.ignoreVersionCheck  -}}
 {{- print "true" -}}
 {{- else -}}
 {{- if semverCompare ">= 0.33.0" (include "db-migration-service.version" .) -}}
@@ -245,6 +243,12 @@ If version is not valid semantic version then not use the DB migration service.
 {{- end -}}
 {{- end -}}
 
+{{/*
+Returns the database migration service address
+*/}}
+{{- define "db-migration-service.address" -}}
+{{ .Release.Name }}{{- print "-db-migration-service-headless" -}}
+{{- end -}}
 {{/*
 Return an init container for database migration.
 */}}
@@ -258,9 +262,9 @@ initContainers:
   - '-c'
   - "apk update --no-cache && \
     apk add jq curl && \
-    until nslookup {{ .Release.Name }}-db-migration-service-headless 1> /dev/null; do echo Waiting for the database migration service; sleep 2; done && \
-    until [[ \"$(curl -s http://{{ .Release.Name }}-db-migration-service-headless:{{ .Values.dbMigrationService.port }} | jq -r .status)\" == \"completed\" ]]; do \
-    STATUS_JSON=$(curl -s http://{{ .Release.Name }}-db-migration-service-headless:{{ .Values.dbMigrationService.port }}); \
+    until nslookup {{ (include "db-migration-service.address" .) }} 1> /dev/null; do echo Waiting for the database migration service; sleep 2; done && \
+    until [[ \"$(curl -s http://{{ (include "db-migration-service.address" .) }}:{{ .Values.dbMigrationService.port }} | jq -r .status)\" == \"completed\" ]]; do \
+    STATUS_JSON=$(curl -s http://{{ (include "db-migration-service.address" .) }}:{{ .Values.dbMigrationService.port }}); \
     PROGRESS_IN_PERCENT=$(echo $STATUS_JSON | jq -r .progress_in_percent); \
     STATUS=$(echo $STATUS_JSON | jq -r .status); \
     echo The database migration status: ${STATUS}...${PROGRESS_IN_PERCENT}%; \
diff --git a/charts/rasa-x/templates/db-migration-service.yaml b/charts/rasa-x/templates/db-migration-service.yaml
index 08bcc787..4745530e 100644
--- a/charts/rasa-x/templates/db-migration-service.yaml
+++ b/charts/rasa-x/templates/db-migration-service.yaml
@@ -1,5 +1,4 @@
-{{ if eq "true" (include "db-migration-service.requiredVersion" .) }}
-{{ if .Values.separateDBMigrationService }}
+{{ if and (eq "true" (include "db-migration-service.requiredVersion" .)) .Values.separateDBMigrationService }}
 apiVersion: "v1"
 kind: "Service"
 metadata:
@@ -18,4 +17,3 @@ spec:
     {{- include "rasa-x.selectorLabels" . | nindent 4 }}
     app.kubernetes.io/component: db-migration-service
 {{end}}
-{{end}}
diff --git a/charts/rasa-x/values.yaml b/charts/rasa-x/values.yaml
index b0e3e893..46852eda 100644
--- a/charts/rasa-x/values.yaml
+++ b/charts/rasa-x/values.yaml
@@ -198,14 +198,15 @@ rasa:
 # dbMigrationService specifies settings for the database migration service
 # The database migration service requires Rasa X >= 0.33.0
 dbMigrationService:
-  # settings related to the init-db container used as a init container for services
+  # initContainer describes settings related to the init-db container used as a init container for deployments
   initContainer:
+    # image is the Docker image which is used by the init container
     image: alpine:latest
   # command overrides the default command to run in the container
   command: []
   # args overrides the default arguments to run in the container
   args: []
-  # name is a Docker image name
+  # name is the Docker image name which is used by the migration service
   name: "rasa/rasa-x"  # gcr.io/rasa-platform/rasa-x-ee
   # tag refers to the Rasa X image tag (uses `appVersion` by default)
   tag: ""