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

Container name for task, based on node config job_manager_host #21

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

nagualnodes
Copy link

Good day!

Glad to see v0.2.X as new mainline. Current PR is focused to reduce duplication of places where we specify jobmanager_host.

Current status

  1. We set static jobname container name in docker-compose.yml
  2. We job_manager_host which is same as hostname (formally must be servicename) in docker-compose.yml
  3. Then we use static container name in code, where this PR is focused to change logic, as step 1

Three points where something could go wrong. So main goal to reduce config points to minimal.

My propose in PR

  1. Use job_manager_host config value as variable for container_name, rather than current static value
  2. Reduce one point of failure where someday we reconfigure job_manager_host

After that we only need to setup docker-compose.yml, node.yaml without braking code at all. It's more logical to attach container name to node config, rather than using static value, which can broke by docker compose, which has weak relation to node service at all.

If this PR is acceptable, my next propose will be, move jobmanagers container_name and hostname to .env, to get single point of control for job_manager_host, besides node.yaml. This is valid for docker-compose, nomad, kubernetes, and can be well documented too.

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