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

HDDS-11590. Use JSON to pass config for docker #7672

Closed
wants to merge 1 commit into from

Conversation

ivanzlenko
Copy link
Contributor

@ivanzlenko ivanzlenko commented Jan 9, 2025

What changes were proposed in this pull request?

Instead of using env vars with non-compliant names we will use variables with JSON describing all necessary configuration files.

Please describe your PR in detail:
In this PR proposed to change our approach on describing cluster configuration for our Docker images.
Right now we are using a lot of environment variables with non-compliant names. Which in turn sometimes leads to situations where all our Docker infrastructure is not working thanks to changes on the Docker side.
Instead it is better to describe configurations using JSON format.
In the first implementation it is just a straight replacement of many env vars with one containing JSON. However later we can move all configurations into separate JSON files so we can have better control on what actually resides in all these configuration properties.
Changes are backward compatible with the old format.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-11590

How was this patch tested?

Patch tested using CI

Instead of using env vars with non-compliant names
we will use variables with JSON
describing all necessary configuration files
@adoroszlai
Copy link
Contributor

Thanks @ivanzlenko for the patch. Repeating part of my comment from the Jira issue.

I don't think we should wrap configs in JSON string.

  1. It adds unnecessary complexity. The Docker Compose bug only affects files references by env_file. We can include the same variables in the docker-compose.yaml file itself.
  2. It is error-prone. E.g. we have to come up with unique name in each config file.
  3. It also leads to unpredictable variable overriding behavior. With individual variables, any previous definitions are overridden by new ones, in the order they are defined, when Docker Compose reads the files. With wrapped variables, overriding happens in envtoconf, where order is not guaranteed to be the same.
  4. It is hard to debug (see sample SCM startup failure below -- which config file does it refer to, and which config variable?)
Traceback (most recent call last):
  File "/opt/hadoop/libexec/envtoconf.py", line 130, in <module>
    Simple(sys.argv[1:]).main()
  File "/opt/hadoop/libexec/envtoconf.py", line 117, in main
    self.process_envs()
  File "/opt/hadoop/libexec/envtoconf.py", line 56, in process_envs
    conf_variables=json.loads(os.environ[key])
  File "/usr/lib64/python3.9/json/__init__.py", line 346, in loads
    return _default_decoder.decode(s)
  File "/usr/lib64/python3.9/json/decoder.py", line 337, in decode
    obj, end = self.raw_decode(s, idx=_w(s, 0).end())
  File "/usr/lib64/python3.9/json/decoder.py", line 353, in raw_decode
    obj, end = self.scan_once(s, idx)
json.decoder.JSONDecodeError: Expecting ',' delimiter: line 7 column 1 (char 215)

@adoroszlai adoroszlai closed this Jan 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants