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

HCL2 variables behave differently when passed via -var and -var-file #11149

Closed
apollo13 opened this issue Sep 8, 2021 · 7 comments · Fixed by #11165
Closed

HCL2 variables behave differently when passed via -var and -var-file #11149

apollo13 opened this issue Sep 8, 2021 · 7 comments · Fixed by #11165
Assignees
Labels
stage/accepted Confirmed, and intend to work on. No timeline committment though. theme/hcl theme/jobspec type/bug

Comments

@apollo13
Copy link
Contributor

apollo13 commented Sep 8, 2021

Nomad version

Nomad v1.2.0-dev (cf7675a+CHANGES)

Operating system and Environment details

Fedora 34

Issue

Planing a broken jobfile results in different error messages in the presence of a variable.

Reproduction steps

  • echo '{"TEST":"abc"}' > vars.json
  • nomad job plan -var-file vars.json example.nomad -- this one plans just fine
  • nomad job plan -var TEST=abc example.nomad -- fails

The second plan fails with:

Error getting job struct: Failed to parse using HCL 2. Use the HCL 1 parser with `nomad run -hcl1`, or address the following issues:
<nil>: Undefined -var variable; A "TEST" variable was passed in the command line but was not found in known variables. To declare variable "TEST", place this block in your job file

Is this a bug or by design? If it is by design, I think the docs could do with an update that variables specified via -var must be consumed. Behavior-wise I would prefer it to not error out (especially not in the case of a variable file) since it seems rather common to me to supply a set of base variables in CI scripts which are not necessarily used by all jobs.

Job file (if appropriate)

job "example" {
  datacenters = ["dc1"]

  group "cache" {
    network {
      port "db" {
        to = 6379
      }
    }

    service {
	name = "test"
	tags = [
		"a",
                "c",
		"b",
        ]
    }

    task "redis" {
      driver = "docker"

      config {
        image = "redis:3.2"

        ports = ["db"]
      }

      resources {
        cpu    = 500
        memory = 256
      }
    }
  }
}
@apollo13
Copy link
Contributor Author

apollo13 commented Sep 8, 2021

A second case with weird results; if I break the job file on purpose:

job "example" {
  datacenters = ["dc1"]

  group "cache" {
    network {
      port "db" {
        to = 6379
      }
    }

    service {
	name = "test"
	tags = [
		"a",
                "c"  ### Missing comma here ###
		"b",
        ]
    }

    task "redis" {
      driver = "docker"

      config {
        image = "redis:3.2"

        ports = ["db"]
      }

      resources {
        cpu    = 500
        memory = 256
      }
    }
  }
}

and execute

nomad job plan -var-file vars.json example.nomad

I now also get the error for the unused variable:

Error getting job struct: Error parsing job file from example.nomad:
example.nomad:16,3-4: Missing item separator; Expected a comma to mark the beginning of the next item.
<nil>: Undefined variable; A "TEST" variable was set but was not found in known variables. To declare variable "TEST", place this block in your job files

@apollo13
Copy link
Contributor Author

apollo13 commented Sep 8, 2021

As a sidenote: I also do not understand the error message To declare variable "TEST", place this block in your job files -- which block should I place where?

@jrasell
Copy link
Member

jrasell commented Sep 9, 2021

Hi @apollo13 and thanks for raising this issue with such detail.

Firstly, I would expect that both the plan command you describe in your first comment to behave in the same manner. This therefore indicates a bug to me in the nomad job plan -var-file vars.json example.nomad example command.

As detailed in the Nomad hcl2 documentation, "each input variable accepted by a job must be declared using a variable block". This is what the error message is referencing. This allows us to perform type assertion and checking when overriding variables as well as other safety checks. In order to therefore attempt to override a variable named "TEST" with a job specification file, you will need to define the variable, in the jobspec file, like variable "TEST" { type = string }.

it seems rather common to me to supply a set of base variables in CI scripts which are not necessarily used by all job
If there are use cases to support this, I would be interested in hearing them via a new issue, so we can keep this issue to track the difference in behaviour between override variables in files and passed via the CLI.

@jrasell jrasell added stage/accepted Confirmed, and intend to work on. No timeline committment though. theme/hcl theme/jobspec labels Sep 9, 2021
@jrasell jrasell added this to Needs Triage in Nomad - Community Issues Triage via automation Sep 9, 2021
@jrasell jrasell moved this from Needs Triage to Needs Roadmapping in Nomad - Community Issues Triage Sep 9, 2021
@apollo13
Copy link
Contributor Author

apollo13 commented Sep 9, 2021

Aside from the inconsistency, would you expect nomad job plan -var-file vars.json example.nomad to error out if TEST is not used by the job spec (asking because then I'd have to rethink about using nomad for templating -- I am fine with using a variable block to define types etc when I actually use the variable, otherwise it should just ignore it).

@jrasell
Copy link
Member

jrasell commented Sep 9, 2021

@apollo13 I personally would, but that is just how I would expect to run Nomad job files. I did a little further digging and it does look like there is a flag internally to the hcl2 jobspec parsing which can alter this behaviour as you desire. This parameter is not currently exposed to users to toggle. I will raise a separate issue to look into exposing this.

@schmichael
Copy link
Member

schmichael commented Sep 11, 2021

It appears Nomad mostly follows Terraform's lead here:

  • -var UNUSED_VAR=bar errors
  • -var-file file_with_unused_vars.json warns

Nomad lacks the warning in the second case but the end result appears to be the same.

That being said I don't really like Terraform's behavior. 😅 I feel like we should either allow or deny var files with undefined variables.

While it might be nice to allow people to stuff a json file full of variables and use them only as desired (maybe the variables are defined clusterwide and jobspec authors use them as desired), I think biasing for consistency, clarity, and failing-fast is fine.

We'll differ from Terraform in functionality, but not really in spirit since they discourage people from putting undefined variables in var files through their warning.

Update: In #11165 for Go api package users they can opt for Terraform's warning which seems like a fine escape hatch.

@github-actions
Copy link

I'm going to lock this issue because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
stage/accepted Confirmed, and intend to work on. No timeline committment though. theme/hcl theme/jobspec type/bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants