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

resource/job: set namespace on register #386

Merged
merged 3 commits into from
Dec 16, 2023
Merged

Conversation

FourLeafTec
Copy link
Contributor

When register jobs by Http api need to set the job namespace in the request.

coder/coder#10074

@hashicorp-cla
Copy link

hashicorp-cla commented Oct 6, 2023

CLA assistant check
All committers have signed the CLA.

@lgfa29
Copy link
Contributor

lgfa29 commented Dec 15, 2023

Hi @FourLeafTec 👋

Thank you for the PR, and apologies for the delay on getting this reviewed. I think the changes look good, but I'm curious to understand how you reached this problem.

The Nomad API should use the namespace from the job if none is set in the request:
https://github.com/hashicorp/nomad/blob/a6e164673ed5f37f010eafb8ff0adc92de195946/command/agent/job_endpoint.go#L1002-L1019

Since the provider doesn't set any WriteOptions I'm curious how you ended up with the request reported in coder/coder#10074 (comment), with the ?namespace=default query param.

[DEBUG] http: request complete: method=PUT path="/v1/jobs?namespace=default&region=global"

I have not use coder before (looks like a cool project!) but does it not use Terraform directly?

@lgfa29 lgfa29 self-assigned this Dec 15, 2023
@lgfa29
Copy link
Contributor

lgfa29 commented Dec 16, 2023

Ohh I see, I think it's the same problem as #274.

If coder is running the terraform CLI as a Nomad job, then it may be reading the NOMAD_NAMESPACE env var from that allocation.

We added the ignore_env_vars to fix this problem, so you can update your provider config to something like this:

provider "nomad" {
  ignore_env_vars = {
    "NOMAD_NAMESPACE" = true
    "NOMAD_REGION"    = true
  }
}

But setting the correct namespace sounds like a good thing to do regardless. Just watch out for the NOMAD_REGION env var 😅

Copy link
Contributor

@lgfa29 lgfa29 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pushed a commit to also set the namespace on PlanOpts and another one to add a changelog entry.

Thank you for the contribution!

@lgfa29 lgfa29 merged commit 8f20175 into hashicorp:main Dec 16, 2023
1 check passed
FourLeafTec added a commit to FourLeafTec/coder that referenced this pull request Dec 27, 2023
… coder's allocation.

If coder is running the terraform CLI as a Nomad job, then it may be reading the NOMAD_NAMESPACE env var from that allocation.

Added the ignore_env_vars to fix this problem.

hashicorp/terraform-provider-nomad#386
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.

None yet

3 participants