ENV variables are not straightforward to use

[sanderegg]

Problem

People do not understand how ENVs are defined.

example setting:
WEBSERVER_TRACING which is composed of at least TRACING_OPENTELEMETRY_COLLECTOR_ENDPOINT and TRACING_OPENTELEMETRY_COLLECTOR_PORT

1. if WEBSERVER_TRACING is not passed via docker-compose.yml, then defining TRACING_OPENTELEMETRY_COLLECTOR_ENDPOINT and TRACING_OPENTELEMETRY_COLLECTOR_PORT auto-enables the tracing --> this is ok
2. if WEBSERVER_TRACING is not passed via docker-compose.yml, it is not possible to disable tracing --> not good and not intuitive
3. to disable it would be necessary to delete TRACING_OPENTELEMETRY_COLLECTOR_ENDPOINT or TRACING_OPENTELEMETRY_COLLECTOR_PORT but it is not possible in the current system, and setting something like TRACING_OPENTELEMETRY_COLLECTOR_ENDPOINT= is understood as "value is set to empty" which pydantic-settings parser complain about --> problem
4. to disable one must pass WEBSERVER_TRACING via docker-compose.yml and set its value to 'null', which takes precedence over any value set in TRACING_OPENTELEMETRY_COLLECTOR_ENDPOINT and TRACING_OPENTELEMETRY_COLLECTOR_PORT --> works but not intuitive and also implies that osparc-simcore devs must add every composed ENV variables in the docker-compose.yml

Some side issues:

• repo.config.template are not sorted the same in every deployments, this is very annoying
• still a problem to have the repository with some data, and portainer having different values

a few requirements until now

• osparc-config defines every ENV explicitely, so that repo.config.template files may be compared and that no implicit values are set in the deployments (this is difficult for someone that does not know the service deployed)

Other problems while releasing

• the tests that check the compatibility of the ENV variables are allowed to fail,
• the CD (but the very last deployment step) should be run beforehand to ensure all ENV variables are in place,

[mrnicegyu11]

Since I was assigned I can add a few points:

• The validate_simcore2 test was originally written to assure that there are no implicit defaults and all env-vars are provided, but it was never green and we never reached the desired situation. At the advice of the releaseofficers we put allow-failure for both validation tests.
• Your points 1-4 seem to all require fixes in the simcore-settings library (so maybe we should file an associated issue there as well?). I will appropriate the mentioned criticism fully, I find the usage of enabling/disabling/passing env-vars very unintuitive tbh...
• repo.config.template are not sorted the same in every deployments, this is very annoying: since I have written and added the graph-sort based sorting pre-commit hook against some cirticism, I will politely state this statement is not true, at least in my book. If you have a better sorting in mind let's discuss the implemention, I'd be happy to improve here.
• still a problem to have the repository with some data, and portainer having different values: I find this annoying as well, and I would have very much liked to push out CI/CD so far that we could have this Git-Ops approach in place. It was rejected approx. 1 year ago as overkill, sadly... But it is at the end of the day just a choice, I find it very feasible if the proper patterns would be adopted. I am still a proponent, but for harmony's sake am quiet about it.

fyi @sanderegg

[sanderegg]

After a discussion with @pcrespov we came to the conclusion that some things could be improved rather swiftly:

• add a proper SENTINEL to make a complex ENV as being enabled or disabled:
  • instead of using WEBSERVER_TRACING={} use something like WEBSERVER_TRACING=ENABLED,
  • instead of using WEBSERVER_TRACING=null use something like WEBSERVER_TRACING=DISABLED

Other more involved changes are:

• Use explicit variables such as DIRECTOR_V2_POSTGRES__POSTGRES_DB=mydb instead of POSTGRES_DB=mydb:
  • allows for sorting ENVs per service instead of having sub settings in the middle,
  • this would allow to disable postgres only in the director-v2 by doing DIRECTOR_V2_POSTGRES__POSTGRES_DB= for example
  • no need to pass DIRECTOR_V2_POSTGRES at all

[sanderegg]

@mrnicegyu11 I used this repo because that is where this is mostly used.
--> there will be consequences in osparc-simcore for sure,
--> I can move it there if you like, but there will also be changes to be done in ops repo,
--> this needs to be discussed and in agreement among DevOps, release managers and backend

[mrnicegyu11]

thanks a lot, your points all sound very good, let me know if or how I can help :--)