Deployment documentation #427

gusmith · 2019-09-11T08:04:56Z

Main changes:

adding CELERYD_CONCURRENCY environment variable
updating the helm charts and template to include a lot of required which should help debugging a deployment if not working
include some comments about recommended configurations

Before asking for review, need to deploy it for tests.

Makes a nice log when using `helm lint` or `helm install` and a value is absent.

Include removing some unused values.

It is now exactly similar to the `settings.py` in the entity-service. Should be kept up to date when veraibales are modified.

…fied.

… connections.

hardbyte

Unrequested review :-P

backend/entityservice/settings.py

deployment/entity-service/Chart.yaml

deployment/entity-service/README.md

hardbyte · 2019-09-11T23:40:24Z

deployment/entity-service/README.md

@@ -0,0 +1,29 @@
+Few helm tips:
+
+To check if the provided values are sufficient, remove/delete the `Charts` folder and run:


Suggested change

To check if the provided values are sufficient, remove/delete the `Charts` folder and run:

To check that all necessary values have been provided before deployment run:

Can this point at a released chart? e.g. something like lint my-values.yaml data61/anonlink-service==1.12?

Point out why the linter doesn't work with sub charts - is there an alternative to deleting the Charts folder if a user is deploying "properly" from our data61 chart repo with:

helm install data61/entity-service [--values...]

I do not have the impression it can point to a released chart. The help:

This command takes a path to a chart and runs a series of tests to verify that the chart is well-formed.

And when simply running helm lint from a random folder:

$ helm lint ==> Skipping . No chart found for linting (missing Chart.yaml) Error: 0 chart(s) linted, 1 chart(s) failed

which shows that it is searching for a Chart.yaml.

deployment/entity-service/templates/api-deployment.yaml

- by default, we do not have persistent volume claims (like that, doing `helm delete --purge` removes all the created resources) - by default, ingress is disabled (do not open the service to the oustide of the cluster, as may require domain name, certificates and all) - by default, request only one "normal" worker

It is already the default value file.

Create too much redundancy and hard to find documentation.

…uction-deployment documentation.

… already minimal.

gusmith · 2019-09-16T06:24:13Z

With all these changes, the default values.yaml file should be a minimal deployment.
I tested it, and run well.
Next step would be to create a new repository made specifically for the helm template/charts, instead of keeping it here. It will include splitting well the documentation too.

gusmith · 2019-09-17T02:16:59Z

The force-push was to remove two commits at the end which shouldn't have been directly in this PR.
I opened a new PR #429 because two tests were sometime failing because of timing issues: one was simply timing out, for the other one, the database update was taking longer to observe the error.

hardbyte

Should make debugging a deployment easier!

hardbyte · 2019-09-17T05:46:52Z

deployment/entity-service/templates/api-deployment.yaml

@@ -77,6 +77,9 @@ spec:
                secretKeyRef:
                  name: {{ template "es.fullname" . }}
                  key: minioSecretKey
+            # Flask should get access to more connections if required. It is often asking for run status directly into the database.
+            - name: DATABASE_MAX_CONNECTIONS


This could be in the config map as FLASK_DB_MAX_CONNECTIONS instead of being the only non secret config passed in as an env var.

Created FLASK_DB... and CELERY_DB_... env vars.

hardbyte · 2019-09-17T05:47:59Z

deployment/entity-service/templates/configmap.yaml

@@ -5,39 +5,65 @@ metadata:
  labels:
    {{- include "es.release_labels" . | indent 4 }}
 data:
+  DEBUG: {{ required "workers.debug is required." .Values.workers.debug | quote }}


FYI: I think we should be transitioning away from using debug

I agree, even more seeing how much work was done on the log file.

hardbyte · 2019-09-17T05:58:57Z

deployment/entity-service/values.yaml

+    ## Maximum number of tasks a pool worker process can execute before it’s replaced with a new one
+    ## Low number is recommended, otherwise the celery workers may exhaust the available memory and threads.
+    ## Cf issue https://github.com/data61/anonlink-entity-service/issues/410
+    MAX_TASKS_PER_CHILD: "30"


that is much much lower than 4096

deployment/entity-service/values.yaml

…mponents. By default, we are using 10 max connection for flask, but 3 for celery.

Includes more than just my PR.

Guillaume Smith added 12 commits September 11, 2019 17:46

Use environment variable to set celery concurrency.

9b0cae6

Split the celery option -Ofair to inlcude a space.

50dbc48

In most of the templates, add some requirements on values.

49bfc21

Makes a nice log when using `helm lint` or `helm install` and a value is absent.

Change the deployment configmap.

555654e

Include removing some unused values.

Re-order and add commented out values.

7000f3d

It is now exactly similar to the `settings.py` in the entity-service. Should be kept up to date when veraibales are modified.

Was convenient to have, but may be confusing.

89fcf7e

Update chart value.

bd071e1

Comment in code to ask for dev to mention what env variables are modi…

cb85002

…fied.

A tiny help read me for the deployment.

f8bb8a9

Comment the helm values.yaml file.

38b9a22

Update docker-compose file to include limitation on number of datbase…

b8b77a5

… connections.

Not sure what we want in this documentation...

ba3eadb

gusmith self-assigned this Sep 11, 2019

hardbyte reviewed Sep 12, 2019

View reviewed changes

Guillaume Smith added 12 commits September 12, 2019 17:00

Docstring to comments.

4e09872

This template is not working and may not be usable.

778a863

Some update in a readme which needs to move.

7472f23

git should ignore the charts.

a76b937

Remove minimal value.

f41a14c

It is already the default value file.

Delete a README not updated.

27f6ae8

Create too much redundancy and hard to find documentation.

Update the production-deployment docs.

236f2f7

Remove this documentation which is now redundant as added to the prod…

4157f34

…uction-deployment documentation.

Azure pipeline: remove using values.yaml file as it is always used.

7136571

Remove using the minimalvalues.yaml file as the default deployment is…

e415cd5

… already minimal.

Remove all mention of value files in azure pipeline.

ae6e5a9

gusmith requested a review from hardbyte September 16, 2019 06:51

gusmith force-pushed the deployment-documentation branch from c3c38bf to ae6e5a9 Compare September 17, 2019 02:06

gusmith mentioned this pull request Sep 17, 2019

Slow tests #429

Merged

Merge branch 'develop' into deployment-documentation

617a5c3

gusmith mentioned this pull request Sep 17, 2019

Issues with celery and workers - Documentation of configuration required. #410

Closed

hardbyte approved these changes Sep 17, 2019

View reviewed changes

Guillaume Smith added 5 commits September 17, 2019 16:37

disable celery monitring by default

7e2eee9

Split DATABASE_MIN_CONNECTIONS and max into their celery and flask co…

a45d98a

…mponents. By default, we are using 10 max connection for flask, but 3 for celery.

Update the methods initializaing the database connection pool.

b9abf3e

Update config map to use the new environment variables.

91d67bb

Remove an unnecessary env var from the api-deployment template.

f24e966

hardbyte approved these changes Sep 17, 2019

View reviewed changes

Guillaume Smith added 2 commits September 18, 2019 10:19

Update the docker-compose file with the new env variables.

8253102

Forgot to update the changelog.

d8c1bf6

Includes more than just my PR.

gusmith merged commit 6ba4af2 into develop Sep 18, 2019

gusmith deleted the deployment-documentation branch September 18, 2019 01:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deployment documentation #427

Deployment documentation #427

gusmith commented Sep 11, 2019

hardbyte left a comment

hardbyte Sep 11, 2019

gusmith Sep 16, 2019

gusmith commented Sep 16, 2019

gusmith commented Sep 17, 2019 •

edited

Loading

hardbyte left a comment

hardbyte Sep 17, 2019

gusmith Sep 17, 2019

hardbyte Sep 17, 2019

gusmith Sep 17, 2019

hardbyte Sep 17, 2019

		@@ -0,0 +1,29 @@
		Few helm tips:

		To check if the provided values are sufficient, remove/delete the `Charts` folder and run:

	To check if the provided values are sufficient, remove/delete the `Charts` folder and run:
	To check that all necessary values have been provided before deployment run:

Deployment documentation #427

Deployment documentation #427

Conversation

gusmith commented Sep 11, 2019

hardbyte left a comment

Choose a reason for hiding this comment

hardbyte Sep 11, 2019

Choose a reason for hiding this comment

gusmith Sep 16, 2019

Choose a reason for hiding this comment

gusmith commented Sep 16, 2019

gusmith commented Sep 17, 2019 • edited Loading

hardbyte left a comment

Choose a reason for hiding this comment

hardbyte Sep 17, 2019

Choose a reason for hiding this comment

gusmith Sep 17, 2019

Choose a reason for hiding this comment

hardbyte Sep 17, 2019

Choose a reason for hiding this comment

gusmith Sep 17, 2019

Choose a reason for hiding this comment

hardbyte Sep 17, 2019

Choose a reason for hiding this comment

gusmith commented Sep 17, 2019 •

edited

Loading