MF-1342 - Use environment variables in docker-compose to use tagged version of image #1343

mteodor · 2021-01-28T16:59:19Z

Resolves #1342

manuio · 2021-01-28T18:00:15Z

provision/README.md

+| MF_PROVISION_CERTS_CA_KEY           | Mainflux CA cert private key                      | ""                               |
+| MF_PROVISION_CERTS_RSA_BITS         | Certificate RSA bits parameter                    | 4096                             |
+| MF_PROVISION_CERTS_HOURS_VALID      | Number of days that certificate is valid          | "2400h"                          |
+| MF_RELEASE_TAG                      | Docker image version to be deployed               | latest                |


The last one :)

manuio · 2021-01-28T18:04:36Z

users/README.md

@@ -99,7 +100,12 @@ make users
 make install

 # set the environment variables and run the service
-MF_USERS_LOG_LEVEL=[Users log level] MF_USERS_DB_HOST=[Database host address] MF_USERS_DB_PORT=[Database host port] MF_USERS_DB_USER=[Database user] MF_USERS_DB_PASS=[Database password] MF_USERS_DB=[Name of the database used by the service] MF_USERS_DB_SSL_MODE=[SSL mode to connect to the database with] MF_USERS_DB_SSL_CERT=[Path to the PEM encoded certificate file] MF_USERS_DB_SSL_KEY=[Path to the PEM encoded key file] MF_USERS_DB_SSL_ROOT_CERT=[Path to the PEM encoded root certificate file] MF_USERS_HTTP_PORT=[Service HTTP port] MF_USERS_SERVER_CERT=[Path to server certificate] MF_USERS_SERVER_KEY=[Path to server key] MF_JAEGER_URL=[Jaeger server URL] MF_EMAIL_DRIVER=[Mail server driver smtp] MF_EMAIL_HOST=[Mail server host] MF_EMAIL_PORT=[Mail server port] MF_EMAIL_USERNAME=[Mail server username] MF_EMAIL_PASSWORD=[Mail server password] MF_EMAIL_FROM_ADDRESS=[Email from address] MF_EMAIL_FROM_NAME=[Email from name] MF_EMAIL_TEMPLATE=[Email template file] MF_TOKEN_RESET_ENDPOINT=[Password reset token endpoint] $GOBIN/mainflux-users
+MF_USERS_LOG_LEVEL=[Users log level] MF_USERS_DB_HOST=[Database host address] MF_USERS_DB_PORT=[Database host port] MF_USERS_DB_USER=[Database user] MF_USERS_DB_PASS=[Database password] \


Add a new line for each envar:

MF_USERS_LOG_LEVEL=[Users log level] \ MF_USERS_DB_HOST=[Database host address] \ MF_USERS_DB_PORT=[Database host port] \ MF_USERS_DB_USER=[Database user] \ MF_USERS_DB_PASS=[Database password]

blokovi · 2021-01-28T18:15:20Z

auth/README.md

@@ -56,6 +56,7 @@ default values.
 | MF_AUTH_SERVER_KEY        | Path to server key in pem format                                         |               |
 | MF_AUTH_SECRET            | String used for signing tokens                                           | auth          |
 | MF_JAEGER_URL             | Jaeger server URL                                                        | localhost:6831|
+| MF_RELEASE_TAG            | Docker image version to be deployed                                      | latest         |


space in last column

manuio · 2021-01-29T10:22:26Z

auth/README.md

@@ -34,7 +34,7 @@ The following actions are supported:

 ## Configuration

-The service is configured using the environment variables presented in the
+The service is configured using the envir onment variables presented in the


envir onment

manuio · 2021-01-29T10:23:14Z

users/README.md

@@ -55,34 +55,34 @@ locally:
 version: "3.7"
 services:
  users:
-    image: mainflux/users:[MF_RELEASE_TAG]
+    image: mainflux/users:[\MF_RELEASE_TAG]


\MF_RELEASE_TAG

manuio · 2021-01-29T10:24:09Z

users/README.md

+      \MF_USERS_LOG_LEVEL: [Users log level]
+      \MF_USERS_DB_HOST: [Database host address]
+      \MF_USERS_DB_PORT: [Database host port]
+      \MF_USERS_DB_USER: [Database user]
+      \MF_USERS_DB_PASS: [Database password]
+      \MF_USERS_DB: [Name of the database used by the service]
+      \MF_USERS_DB_SSL_MODE: [SSL mode to connect to the database with]
+      \MF_USERS_DB_SSL_CERT: [Path to the PEM encoded certificate file]
+      \MF_USERS_DB_SSL_KEY: [Path to the PEM encoded key file]
+      \MF_USERS_DB_SSL_ROOT_CERT: [Path to the PEM encoded root certificate file]
+      \MF_USERS_HTTP_PORT: [Service HTTP port]
+      \MF_USERS_SERVER_CERT: [String path to server certificate in pem format]
+      \MF_USERS_SERVER_KEY: [String path to server key in pem format]
+      \MF_JAEGER_URL: [Jaeger server URL]
+      \MF_EMAIL_DRIVER: [Mail server driver smtp]
+      \MF_EMAIL_HOST: [\MF_EMAIL_HOST]
+      \MF_EMAIL_PORT: [\MF_EMAIL_PORT]
+      \MF_EMAIL_USERNAME: [\MF_EMAIL_USERNAME]
+      \MF_EMAIL_PASSWORD: [\MF_EMAIL_PASSWORD]
+      \MF_EMAIL_FROM_ADDRESS: [\MF_EMAIL_FROM_ADDRESS]
+      \MF_EMAIL_FROM_NAME: [\MF_EMAIL_FROM_NAME]
+      \MF_EMAIL_TEMPLATE: [\MF_EMAIL_TEMPLATE]
+      \MF_TOKEN_RESET_ENDPOINT: [\MF_TOKEN_RESET_ENDPOINT]


Looka like a wrong find a replace. Remove these \

manuio · 2021-01-29T10:24:16Z

users/README.md

 ```

-If `MF_EMAIL_TEMPLATE` doesn't point to any file service will function but password reset functionality will not work.
+If `\MF_EMAIL_TEMPLATE` doesn't point to any file service will function but password reset functionality will not work.


codecov-io · 2021-01-29T10:48:48Z

Codecov Report

Merging #1343 (66899cc) into master (1bf485b) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1343   +/-   ##
=======================================
  Coverage   60.05%   60.05%           
=======================================
  Files         113      113           
  Lines        8904     8904           
=======================================
  Hits         5347     5347           
  Misses       3088     3088           
  Partials      469      469

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1bf485b...66899cc. Read the comment docs.

manuio · 2021-01-29T10:49:29Z

docker/README.md

@@ -21,3 +21,4 @@ docker-compose -f docker/docker-compose.yml up
 docker-compose -f docker/addons/<path>/docker-compose.yml  up
 ```

+To start specific release you need to change the value of `MF_RELEASE_TAG` in `.env` before running these commands.


I'd use To pull docker images from a specific release...

nmarcetic · 2021-01-29T11:15:17Z

auth/README.md

@@ -67,7 +68,7 @@ locally:
 version: "3.7"
 services:
  auth:
-    image: mainflux/auth:[version]
+    image: mainflux/auth:[MF_RELEASE_TAG]


I don't get this, why it's wrapped with those brackets [MF_RELEASE_TAG]? I was expecting the same syntax as in Yaml ${MF_RELEASE_TAG} or at least <MF_RELEASE_TAG> a syntax used for variables in doc.

or maybe [Docker image version tag specified by MF_RELEASE_TAG]

I would go with ${MF_RELEASE_TAG} because its env var and exact syntax. But yea, it's probably better because those brackets are everywhere.

manuio · 2021-01-30T07:28:08Z

auth/README.md

-      MF_AUTH_SERVER_KEY: [String path to server key in pem format]
-      MF_JAEGER_URL: [Jaeger server URL]
-```
+The service itself is distributed as Docker container. Check the [`auth`](https://github.com/mainflux/mainflux/blob/master/docker/docker-compose.yml#L71-L94) service section in 


I'm not sure about using section here. If we change the something in thee docker-compose this will be wrong.

manuio

LGTM

drasko

I am generally not convinced by this PR.

I do not like removing documentation about ENV vars from README files and adding just links to docker-compose. I know this would be easier to maintain, but on the other hand hinders readability and quick understanding of deployment.

I also do not support adding docker-compose tag as an ENV var, which can be misleading that it is a part of a service, somebody might even think of using it in the Kubernetes. It is not, it is used only in docker-compose, and docker-compose is used ONLY in development.

I do not really want to support people giving them the wrong idea that docker-compose can be used for running stable releases. If they want to do it, I would rather let them change version by hand. We keep latest for a reason - to demotivate people using docker-compose for production, where Kubernetes should be used.

dborovcanin · 2021-01-31T21:03:04Z

I do not like removing documentation about ENV vars from README files and adding just links to docker-compose. I know this would be easier to maintain, but on the other hand hinders readability and quick understanding of deployment.

I don't agree with this.
We did not remove the env var table with all the explanation, we just removed unnecessary and quite useless "docker-compose-like" sections that we need to maintain with no benefit whatsoever (it does not explain the meaning of the variables better than the table in README, nor it explains usage better than the real compose file).

I also do not support adding docker-compose tag as an ENV var, which can be misleading that it is a part of a service, somebody might even think of using it in the Kubernetes. It is not, it is used only in docker-compose, and docker-compose is used ONLY in development.

I do agree with this.
This env var can be misinterpreted as a service-related configuration, rather than deployment-related stuff. That's both dangerous and wrong. However, I don't see how providing a version for docker-compose in any way implies that it's production-ready.

mteodor · 2021-02-01T08:21:46Z

I also do not support adding docker-compose tag as an ENV var, which can be misleading that it is a part of a service, somebody might even think of using it in the Kubernetes. It is not, it is used only in docker-compose, and docker-compose is used ONLY in development.

I dont think there is a place for confusion it just requires a basic knowledge of docker and kubernetes, it is clear that environment variables used for services are in the environment section of docker-compose.

People with such unclear knowledge of docker deployment will more likely checkout release, start it and be surprised that something isn't working as expected (since the services being deployed are latest rather than tagged release version ) and then ask for support, open an issue...

mteodor · 2021-02-01T08:25:55Z

I do not really want to support people giving them the wrong idea that docker-compose can be used for running stable releases. If they want to do it, I would rather let them change version by hand. We keep latest for a reason - to demotivate people using docker-compose for production, where Kubernetes should be used. <

Latest stays on master, this is just for the people that are experimenting with releases, when they checkout the release tag .env will be populated with appropriate version tag to start appropriate docker versions

nmarcetic · 2021-02-01T10:22:27Z

I am generally not convinced by this PR.

I do not like removing documentation about ENV vars from README files and adding just links to docker-compose. I know this would be easier to maintain, but on the other hand hinders readability and quick understanding of deployment.

I also do not support adding docker-compose tag as an ENV var, which can be misleading that it is a part of a service, somebody might even think of using it in the Kubernetes. It is not, it is used only in docker-compose, and docker-compose is used ONLY in development.

I do not really want to support people giving them the wrong idea that docker-compose can be used for running stable releases. If they want to do it, I would rather let them change version by hand. We keep latest for a reason - to demotivate people using docker-compose for production, where Kubernetes should be used.

I also disagree, as its already really well-argued by @dusanb94 and @mteodor here and here I don't have much to add, except that both root README and documentation should be updated with note or section which will say that docker-compose is meant and designed to be used in development environments, even if Docker is aiming that can be used in production and clustered with Swarm, We highly recommend Kubernetes for production.
I didn't saw any notice/recommendation in our documentation or readme, more like Mainflux can be easily deployed on Kubernetes platform than Should be. People who still want to use it to run it in production (even if it's possible), ignoring doc, readme, and recommendations, should be ignored also.

IMHO this PR is a nice improvement and it elimitantes development pain.

drasko · 2021-02-01T11:20:50Z

OK, I see that majority is for this PR, and makes sense. I would just need one more thought to consider: do we leave MF_RELEASE_TAG in the ENV var table (in the README), or we move it in the sentence below, adding information that docker-compose can be manipulated with this?

This was we could remove ambiguity that this ENV var is related to what service really accepts (in the table) and what is used for deployment via docker-compose.

@nmarcetic @dusanb94 @mteodor please give your thoughts on this.

nmarcetic · 2021-02-01T12:19:36Z

OK, I see that majority is for this PR, and makes sense. I would just need one more thought to consider: do we leave MF_RELEASE_TAG in the ENV var table (in the README), or we move it in the sentence below, adding information that docker-compose can be manipulated with this?

This was we could remove ambiguity that this ENV var is related to what service really accepts (in the table) and what is used for deployment via docker-compose.

@nmarcetic @dusanb94 @mteodor please give your thoughts on this.

Service actually does accept MF_RELEASE_TAG which is a docker image tag. I would put this NOTE in root README.md, the section that explains how to run compose file (and here I will also mention with a bolded label at the section bottom that docker-compose is meant and designed to be used for development and testing environments, and Kubernetes for production, Linkedin to K8s section if official doc). Same for an official doc, running the Mainflux section.
IMHO this way, everything is super clear. Don't spare words, mention it everywhere where you are mentioning Running Mainflux.

dborovcanin · 2021-02-01T12:24:25Z

@drasko @nmarcetic I think that we should remove this env var from the service README because it's explained elsewhere and has nothing to do with the service configuration. Root README sounds like a good candidate.

nmarcetic · 2021-02-01T13:39:55Z

@drasko @nmarcetic I think that we should remove this env var from the service README because it's explained elsewhere and has nothing to do with the service configuration. Root README sounds like a good candidate.

Agree. Let's put it in the root, make a note label for compose and K8s.

drasko · 2021-02-01T23:34:09Z

OK, this makes sense - we can put it in the root README, under the Install section.

I am not sure if we need as an additional note in microservice READMEs (not in the table, but in a separate sentece). I think not, it is redundant - people will usually deploy looking at the Install section of central README (or by looking in the docker-compose and figuring out). I let @mteodor decide on this.

drasko · 2021-02-01T23:35:51Z

On a second though - better just put it in the root README, because this is Mainflux-wide setting, not per-service setting.

Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com>

manuio

LGTM

drasko

LGTM

…ersion of image (absmach#1343) * add MF_RELEASE_TAG Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com> * update readme file for MF_RELEASE_TAG Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com> * update readme file for MF_RELEASE_TAG Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com> * fix explanation, space Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com> * fix bad search/replace Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com> * fix bad search/replace Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com> * minor changes Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com> * fix readme Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com> * improve explanation for tag Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com> * replace snippet with link to code section Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com> * improve explanation for tag Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com> * replace snippet with link to code section Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com> * remove MF_RELEASE_TAG from service readme Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com>

mteodor requested a review from a team as a code owner January 28, 2021 16:59

manuio suggested changes Jan 28, 2021

View reviewed changes

blokovi reviewed Jan 28, 2021

View reviewed changes

mteodor changed the title ~~MF-1342 - Use environment variables in docker-compose~~ MF-1342 - Use environment variables in docker-compose to use tagged version of image Jan 29, 2021

manuio suggested changes Jan 29, 2021

View reviewed changes

manuio reviewed Jan 29, 2021

View reviewed changes

nmarcetic reviewed Jan 29, 2021

View reviewed changes

dborovcanin previously approved these changes Jan 29, 2021

View reviewed changes

mteodor dismissed dborovcanin’s stale review via a45de2e January 29, 2021 11:46

mteodor force-pushed the tagged-docker branch from a45de2e to 5b5798c Compare January 29, 2021 12:01

manuio reviewed Jan 30, 2021

View reviewed changes

manuio previously approved these changes Jan 30, 2021

View reviewed changes

drasko requested changes Jan 31, 2021

View reviewed changes

mteodor dismissed manuio’s stale review via 474d292 February 1, 2021 13:24

mteodor added 4 commits February 2, 2021 09:27

add MF_RELEASE_TAG

9b15676

Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com>

update readme file for MF_RELEASE_TAG

1abaf1d

Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com>

update readme file for MF_RELEASE_TAG

ff7a24a

Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com>

fix explanation, space

28872fb

Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com>

mteodor added 9 commits February 2, 2021 09:27

fix bad search/replace

9a59b18

Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com>

fix bad search/replace

2bc092e

Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com>

minor changes

8851e9b

Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com>

fix readme

2aceda6

Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com>

improve explanation for tag

eb05577

Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com>

replace snippet with link to code section

4f21ce1

Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com>

improve explanation for tag

14a95fb

Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com>

replace snippet with link to code section

429c897

Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com>

remove MF_RELEASE_TAG from service readme

9077825

Signed-off-by: Mirko Teodorovic <mirko.teodorovic@gmail.com>

mteodor force-pushed the tagged-docker branch from 474d292 to 9077825 Compare February 2, 2021 08:27

nmarcetic approved these changes Feb 2, 2021

View reviewed changes

manuio approved these changes Feb 2, 2021

View reviewed changes

drasko approved these changes Feb 2, 2021

View reviewed changes

drasko merged commit 39a649c into absmach:master Feb 2, 2021

mteodor deleted the tagged-docker branch September 13, 2021 12:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MF-1342 - Use environment variables in docker-compose to use tagged version of image #1343

MF-1342 - Use environment variables in docker-compose to use tagged version of image #1343

mteodor commented Jan 28, 2021

manuio Jan 28, 2021

manuio Jan 28, 2021

blokovi Jan 28, 2021

manuio Jan 29, 2021

manuio Jan 29, 2021

manuio Jan 29, 2021

manuio Jan 29, 2021

codecov-io commented Jan 29, 2021 •

edited

Loading

manuio Jan 29, 2021

nmarcetic Jan 29, 2021

mteodor Jan 29, 2021

nmarcetic Jan 29, 2021

manuio Jan 30, 2021

manuio left a comment

drasko left a comment •

edited

Loading

dborovcanin commented Jan 31, 2021 •

edited

Loading

mteodor commented Feb 1, 2021 •

edited

Loading

mteodor commented Feb 1, 2021

nmarcetic commented Feb 1, 2021

drasko commented Feb 1, 2021 •

edited

Loading

nmarcetic commented Feb 1, 2021

dborovcanin commented Feb 1, 2021

nmarcetic commented Feb 1, 2021

drasko commented Feb 1, 2021

drasko commented Feb 1, 2021

manuio left a comment

drasko left a comment

MF-1342 - Use environment variables in docker-compose to use tagged version of image #1343

MF-1342 - Use environment variables in docker-compose to use tagged version of image #1343

Conversation

mteodor commented Jan 28, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented Jan 29, 2021 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manuio left a comment

Choose a reason for hiding this comment

drasko left a comment • edited Loading

Choose a reason for hiding this comment

dborovcanin commented Jan 31, 2021 • edited Loading

mteodor commented Feb 1, 2021 • edited Loading

mteodor commented Feb 1, 2021

nmarcetic commented Feb 1, 2021

drasko commented Feb 1, 2021 • edited Loading

nmarcetic commented Feb 1, 2021

dborovcanin commented Feb 1, 2021

nmarcetic commented Feb 1, 2021

drasko commented Feb 1, 2021

drasko commented Feb 1, 2021

manuio left a comment

Choose a reason for hiding this comment

drasko left a comment

Choose a reason for hiding this comment

codecov-io commented Jan 29, 2021 •

edited

Loading

drasko left a comment •

edited

Loading

dborovcanin commented Jan 31, 2021 •

edited

Loading

mteodor commented Feb 1, 2021 •

edited

Loading

drasko commented Feb 1, 2021 •

edited

Loading