Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docker entrypoint: can override conf with envvars #191

Closed
wants to merge 9 commits into from

Conversation

hervenicol
Copy link

new feature:
you can override ArangoDB config with environment variables.
This to follow 12-factors principles: https://12factor.net/config

Here is the syntax:

env vars syntax:
 ARANGOCONF_SECTION_PARAMETER=VALUE
example:
 ARANGOCONF_LOG_LEVEL=debug
  => update [log] section with "level = debug"

Reasons for doing this:

  • have all conf set by env vars, rather than a part in 'command' and a part in 'environment'.
  • can lead to more systematic entrypoint code, where we don't manage ARANGO_ENCRYPTION_KEYFILE and ARANGO_STORAGE_ENGINE in a specific way (can be introduced later, and documented as var names would change).
  • var names can be determined from official documentation.

All in all, I believe it's more logical / simple from a user point of view.

@Simran-B
Copy link
Contributor

Do I understand correctly, that you want to be able to store any Arango configuration option as environment variables?

You can start any of the Arango binaries, e.g. docker run -it arangodb/arangodb arangosh, so a ARANGO_ prefix is insufficient IMO. It should be ARANGOD_ for server related config, ARANGOSH_ for the shell etc.

I believe it's more logical / simple from a user point of view

Is it really simpler to set up a bunch of environment variables instead of having a single configuration file? Also, what if you want to re-deploy to another machine? Copying the conf file seems much easier.

Are you aware that you can use environment variables in configuration files?

@hervenicol
Copy link
Author

hervenicol commented Oct 18, 2019

Do I understand correctly, that you want to be able to store any Arango configuration option as environment variables?

Yes, that's it.

You can start any of the Arango binaries, e.g. docker run -it arangodb/arangodb arangosh, so a ARANGO_ prefix is insufficient IMO. It should be ARANGOD_ for server related config, ARANGOSH_ for the shell etc.

That is why I used ARANGOCONF.

Is it really simpler to set up a bunch of environment variables instead of having a single configuration file? Also, what if you want to re-deploy to another machine? Copying the conf file seems much easier.

An external config file means a volume. It works well on a dev's laptop, but with an orchestrated docker (whether it's Swarm or K8S), it is a bit more complicated.

Are you aware that you can use environment variables in configuration files?

Using env vars in the config files requires changing the config file.
Also, I'd like to be able to use the vanilla Arangodb image rather than build / store my own, especially if it's just for a config file tweak.

On top of that, my script's behaviour avoids maintenance, as it's compatible with any future ArangoDB config sections / parameters.

@Simran-B
Copy link
Contributor

That is why I used ARANGOCONF.

It's ambiguous either way, wouldn't ARANGOD_CONF_ or something be better?

I guess it wouldn't hurt to have the option to configure everything through env vars, @dothebart ?

it's compatible with any future ArangoDB config sections / parameters.

There is a small chance that it breaks, e.g. if the following two options existed: --foo.bar-baz and foo-bar.baz (because both would translate to ARANGOD_CONF_FOO_BAR_BAZ).

@hervenicol
Copy link
Author

That is why I used ARANGOCONF.

It's ambiguous either way, wouldn't ARANGOD_CONF_ or something be better?

I'm bad at naming, but I don't see any technically problem with that.
You choose, I can patch accordingly.

it's compatible with any future ArangoDB config sections / parameters.

There is a small chance that it breaks, e.g. if the following two options existed: --foo.bar-baz and foo-bar.baz (because both would translate to ARANGOD_CONF_FOO_BAR_BAZ).

You've just spotted a bug; the current code lets underscores as underscores. In order to work with current config options, it should take ARANGOD_CONF_FOO_BAR_BAZ as --foo.bar-baz.
Section names have no special character (at the moment), and parameters have dashes rather than underscore.
I'll update my PR.

Apart from that, you're right in the way that, in future versions, if a section name contains a dash, it will be a problem. And I don't see an easy way to anticipate it.

@Simran-B Simran-B added the enhancement New feature or request label Oct 18, 2019
Copy link
Contributor

@dothebart dothebart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Copy link
Contributor

@fceller fceller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM apart from typo in comment

containers/arangodb35.docker/entrypoint.sh Outdated Show resolved Hide resolved
@dothebart
Copy link
Contributor

dothebart commented Oct 21, 2019

@hervenicol I have one issue: how would we handle multiple --log.level occurances? you can use that to switch on logging for separate facilities, i.e:

--log.level info 
--log.level communication=trace

which translates to:

[log]
level=info
level=communication=trace

@hervenicol
Copy link
Author

@hervenicol I have one issue: how would we handle multiple --log.level occurances? you can use that to switch on logging for separate facilities

I did not think about that, I thought parameter names in a section would be unique.
Your example is really good as I can easily imagine we would like to customize logging per platform.

Right now, I don't see how to manage it.
I wonder how it's managed when set in command-line.
Let me think about it...

@Simran-B
Copy link
Contributor

If you search https://www.arangodb.com/docs/stable/programs-arangod-options.html for the ellipsis symbol you will find 23 arangod options which allow multiple values.

An example for that in action can be found here:
https://www.arangodb.com/docs/stable/administration-configuration.html#options-with-multiple-values

arangod --log.level warning --log.level queries=trace --log.level startup=info
[log]
level = warning
level = queries=trace
level = startup=info

@hervenicol
Copy link
Author

My thoughts on this one.

  1. Arrays are not an option, as we need POSIX sh compatibility.

  2. We can act on the value, like by defining a split character (with spaces: LOG_LEVEL="info communication=trace), but I'm really not confident we can find a valid character here, that would not break values for any parameter.

  3. or act on the option name, like with some optional trailing characters. I guess underscores, would be OK.
    LOG_LEVEL____ would be interpreted as LOG_LEVEL. So we could define multiple 'LOG_LEVEL' vars.

  4. or have a specific treatment for these 23 (at the moment) options. That would limit the scope of a split character in the value. But require the script to know the list of such options, and be updated when this list changes.

I tend to think the "less dirty" would be option 3. But not really proud of it. Let me know your opinion.

@dothebart
Copy link
Contributor

maybe split the var value by ; and insert one line for each?

@OmarAyo
Copy link

OmarAyo commented Oct 21, 2019

CLA Available

@hervenicol
Copy link
Author

OK, I pushed a few updates.

I did a simple version for multiple values that splits on newlines. Do you think it is good enough?

I also changed the var prefix ARANGOCONF to ARANGOD_CONF as per @Simran-B 's wish.

Waiting for your feedback.

@KVS85
Copy link
Contributor

KVS85 commented Jul 17, 2024

Replaced by #590.

@KVS85 KVS85 closed this Jul 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants