hashicorp / nomad

Nomad is an easy-to-use, flexible, and performant workload orchestrator that can deploy a mix of microservice, batch, containerized, and non-containerized applications. Nomad is easy to operate and scale and has native Consul and Vault integrations.
https://www.nomadproject.io/
Other
14.81k stars 1.94k forks source link

"Missing" variable rules wrong when using `-var-file` #16136

Open jblachly opened 1 year ago

jblachly commented 1 year ago

Nomad version

Nomad v1.4.3 (f464aca721d222ae9c1f3df643b3c3aaa20e2da7)

Operating system and Environment details

Linux ip- 5.15.0-1028-aws #32-Ubuntu SMP Mon Jan 9 12:28:07 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux

(Ubuntu 22.04 LTS; base OS plus Nomad installed from Hashicorp repositories)

Issue

Variables defined in a variable file, but not in a jobspec file, result in a runtime error:

Error getting job struct: Error parsing job file from <hcl2 file>:
<nil>: Undefined variable; A "<variable name>" variable was set but was not found in known variables. To declare variable "<variable name>", place this block in your job files

The converse (variables defined in a jobspec file, but not in a variable file) also results in a runtime error, but this is desired behavior.

Reproduction steps

test.vars:

im_defined = "success"
im_undefined = "failure"

test.hcl:

variable "im_defined" {
  type = string
}
job "test" {
  group {
    task {
    }
  }
}

$ nomad job plan -var-file=test.vars test.hcl

Expected Result

Variables defined in a file -- just like variables defined in the environment -- but not a part of the job should be ignored.

RATIONALE: Some variables (database passwords, service URLs, etc.) may be shared among jobs, while other variables are unique. For consistency, it would be easiest to maintain a single variable file-per-environment (dev.vars, staging.vars, production.vars, etc.) and use it with all job files that belong to that environment.

With the current Nomad behavior, this is impossible unless all variables, including unused ones irrelevant for that particular job, are defined in the jobspec file, which makes the jobspec file cluttered.

Actual Result

Runtime error

Job file (if appropriate)

See repoduction, above

Nomad Server logs (if appropriate)

n/a

Nomad Client logs (if appropriate)

n/a

tgross commented 1 year ago

Hi @jblachly! This is intentional behavior, as it protects against typos in the variables file. For example, if you have foo defined in the job spec with a default value of "bar" and then accidentally set fooo = "baz" in the variables file, the job would run without error but you might not get what you intended.

But generally speaking we try to be consistent across products for this kind of thing, so I tried out what it looks like if I use an undeclared variable in Terraform, and I get the following warning (not error):

│ Warning: Value for undeclared variable │ │ The root module does not declare a variable named "whatever" but a value was found in file │ "terraform.tfvars". If you meant to use this value, add a "variable" block to the configuration. │ │ To silence these warnings, use TFVAR... environment variables to provide certain "global" settings to │ all configurations in your organization. To reduce the verbosity of these warnings, use the │ -compact-warnings option.

It might be nice if we could turn this error in to a warning in Nomad. I'm going to tag some folks who've thought about this kind of thing a lot like @mikenomitch and @angrycub and @lgfa29 to see what they think.

jblachly commented 1 year ago

ACK to warning; thank you for your thoughtful reply.

Warm regards

apollo13 commented 1 year ago

It might be nice if we could turn this error in to a warning in Nomad

isn't this -hcl2-strict=false? I remember complaining about this loudly so you lovely folks added it :þ

apollo13 commented 1 year ago

x-ref https://github.com/hashicorp/nomad/issues/11149

Please note that while I think that the idea of protecting against typos is great, I still think the current behavior makes for a bad default for plenty of usecases (like "group" variables supplied via CI etc…).

dani commented 1 year ago

I'm hitting the same issue. I have several jobfiles to manage, and some vars are shared among some (but not all) of them. Typical example : the URL of my keycloak instance. What I'd like to do is to pass my job a global var file (which has all the common settings of the current env, like in my example, my KC URL), and a job specific var, like

nomad run -var-file global.hcl -var-file job.hcl myjob.nomad.hcl

But it's not possible as the global.hcl file has variables not known to myjob.nomad.hcl. The very same issue affects nomad-pack, which makes it unusable for me (I have no idea how others deals with it).

To work around this issue, I've added this snippet in all my jobfiles

locals {
  defaults   = yamldecode(file("vars/defaults.yml"))
  global_env = yamldecode(file("../common/vars/global.yml"))
  job_env    = yamldecode(file("vars/job.yml"))
  conf       = merge(local.defaults, local.global_env, local.job_env)
}

(well, in fact it's a bit more complicated as I also set a variable to define my current env like dev, qa, prd and load the appropriate yaml file, but you get the idea)

Then I can use ${local.conf.foobar}. Unknown variable are silently ignored, and I can define them at the level I want. (default for the job, global env, override for the job). It's not very elegant, but it's the only solution I found to get what I want

dani commented 1 year ago

Any change to have a --ignore-missing-vars flag just like nomad-pack has now ? I still think it should be the default (and have a flag if we want to error on unknown variable), but at least, it'd make variables usable