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

Use config file for settings #50

Closed
jreadey opened this issue Apr 1, 2020 · 13 comments
Closed

Use config file for settings #50

jreadey opened this issue Apr 1, 2020 · 13 comments

Comments

@jreadey
Copy link
Member

jreadey commented Apr 1, 2020

Rather than relying on environment variables, use config file for settings. This would be mounted (for docker) or loaded as a secret (Kubernetes).

@jreadey jreadey mentioned this issue Apr 1, 2020
16 tasks
@jreadey
Copy link
Member Author

jreadey commented May 20, 2020

Changes to support config files are checked into jreadey-master. Posix, docker, and Kubernetes should all be supported.

If anyone can try out this branch before I merge into master, that would be appreciated.

@s004pmg
Copy link
Contributor

s004pmg commented May 20, 2020

Commit looks reasonable to me, I'll try to get some burn in time for this on DCOS.

@s004pmg
Copy link
Contributor

s004pmg commented May 20, 2020

@jreadey , I didn't expect that this would be exclusive. I.e., environment variables don't work at all as a configuration option now? I had thought that it would be either, or maybe both with a precedence of env variables.

@s004pmg
Copy link
Contributor

s004pmg commented May 20, 2020

Oh, I see now... The config.yml must exist in the config directory. It doesn't look like the paths align to find admin/config/config.yml in the docker build at the moment.

@jreadey
Copy link
Member Author

jreadey commented May 20, 2020

@s004pmg - there are 4 levels of config overrides (from lowest precedence to highest):

  1. value in config.yml
  2. value in override.yml
  3. environment variable
  4. command line switch

The problem with environment variables with Docker or Kubernetes is that they need to be explicitly passed in the docker-compose script or k8s yaml config. As the number of config keys increased, this got to be a bit tedious. So I've put most of the config option in config.yml and removed most of the ones in the yaml.

For kubernetes the config.yml is passed to the pods in a ConfigMap. I'm not exactly what the equivalent would be for DCOS.

Could you take a look at the changes in basenode.py? I made some changes in DCOS related code here, but don't have the ability to test it.

I'll check on the docker build now. This is for docker-compose.posix.yml?

@s004pmg
Copy link
Contributor

s004pmg commented May 21, 2020

I think that environment variables can be more common in DCOS, but either way works there.

I think I was too terse in my last message, here's the stack running a Docker container built off of master:

Traceback (most recent call last):
File "/usr/local/bin/hsds-datanode", line 8, in
sys.exit(main())
File "/usr/local/lib/python3.8/site-packages/hsds/datanode.py", line 230, in main
log.info("datanode start")
File "/usr/local/lib/python3.8/site-packages/hsds/hsds_logger.py", line 29, in info
if config.get("log_level") not in ("ERROR", "WARNING", "WARN"):
File "/usr/local/lib/python3.8/site-packages/hsds/config.py", line 111, in get
_load_cfg()
File "/usr/local/lib/python3.8/site-packages/hsds/config.py", line 30, in _load_cfg
raise FileNotFoundError(msg)
FileNotFoundError: unable to find config file

So that's a new stack I got by swapping in a new build. My point is that if we're going to still allow folks to primarily configure via environment variables, then the docker build should produce a stock config of defaults at that location. We shouldn't force them to mount in a blank config if they prefer to configure via environment variables.

@jreadey
Copy link
Member Author

jreadey commented May 21, 2020

Ok - got it.
Try out with this change: 5b7a0f1. If /config/config.yml is not found, the server will pull from /etc/config/config.yml (part of the docker image)

@s004pmg
Copy link
Contributor

s004pmg commented May 21, 2020

That gets the nodes starting, but now on to a new problem, looks like the node["host"] isn't getting set, so the cluster flails and doesn't self-organize.

I.e., this code in the headnode healthcheck fires:
if node["host"] is None: fail_count += 1 log.warn("Node found with missing host information.") continue

@s004pmg
Copy link
Contributor

s004pmg commented May 21, 2020

When I dump the node JSON, I get this:
{'node_number': 0, 'node_type': 'dn', 'host': None, 'port': None, 'id': None}
It's been a while since I've been in there, but I don't remember that being a valid node definition.

@s004pmg
Copy link
Contributor

s004pmg commented May 27, 2020

We've been testing successfully using commit 532565f for a few days now, I have no further concerns.

@jreadey
Copy link
Member Author

jreadey commented May 27, 2020

Are you still getting the strange state with node JSON?

@s004pmg
Copy link
Contributor

s004pmg commented May 27, 2020

Well, yes, though it gets past it. I get these in the head node for a while at start up:

WARN> Node found with missing host information.

Then it seems to go away after several minutes and the cluster finally turns ready (probably after several nodes turn over and restart). It's probably still worth debugging more because it may be delaying cluster startup, however it does settle in now.

@jreadey
Copy link
Member Author

jreadey commented May 28, 2020

I'll close this issue now. If anyone has questions/bugs with the config file usage, feel free to re-open

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

No branches or pull requests

2 participants