Skip to content

Conversation

@gulducat
Copy link
Member

@gulducat gulducat commented Apr 21, 2025

Fixes #25672 - tl;dr NOMAD_REGION and NOMAD_NAMESPACE are ignored by nomad operator debug (edit: namespace does work, it just was not printed properly to the terminal)

I thought about having Meta.clientConfig() write Meta.region and Meta.namespace, but that felt off, so I opted for a couple of getters that can hopefully be easily discovered if needed for other cases.

Before:

$ NOMAD_REGION=global NOMAD_NAMESPACE=default nomad operator debug
Starting debugger...

Nomad CLI Version: Nomad v1.10.0
BuildDate 2025-04-09T16:40:54Z
Revision e26a2bd2acac2dcdcb623f4d293bac096beef478
           Region:
        Namespace:
          Servers: (1/1) ... etc etc ...

After:

$ NOMAD_REGION=global NOMAD_NAMESPACE=default nomad operator debug
Starting debugger...

Nomad CLI Version: Nomad v1.10.1-dev
BuildDate 2025-04-21T18:49:12Z
Revision c86d63da13145326ae69649fa9c51601dc16914a+CHANGES
           Region: global
        Namespace: default
          Servers: (1/1) ... etc etc ...

NOMAD_REGION and NOMAD_NAMESPACE were being ignored
@gulducat gulducat added type/bug theme/cli backport/ent/1.8.x+ent Changes are backported to 1.8.x+ent backport/ent/1.9.x+ent Changes are backported to 1.9.x+ent backport/1.10.x backport to 1.10.x release line labels Apr 21, 2025
@gulducat gulducat requested review from a team as code owners April 21, 2025 18:54
tgross
tgross previously approved these changes Apr 21, 2025

c.opts = &api.QueryOptions{
Region: c.Meta.region,
Region: c.Meta.Region(),
Copy link
Member

Choose a reason for hiding this comment

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

This PR fixes the reported bug, but why are we passing the region like this at all in an API request? I think operator debug is the only command to do so. This should already go to the right region by virtue of how the API client gets constructed.

Suggested change
Region: c.Meta.Region(),

Copy link
Member Author

Choose a reason for hiding this comment

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

good point. I believe here in QueryOptions an empty region is ignored, and specifying it is redundant, because yes it's a part of the API client, so may as well drop this line.

for clarity, the actual bugfix is passing the proper region into filterServerMembers below.

and now I'm realizing that NOMAD_NAMESPACE is unaffected by this bug, and its only shortcoming was not being c.Ui.Output properly.

@gulducat gulducat changed the title cli: operator debug: region and namespace env vars cli: operator debug: respect NOMAD_REGION env var Apr 21, 2025
Copy link
Member

@tgross tgross left a comment

Choose a reason for hiding this comment

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

LGTM!

@gulducat gulducat merged commit c46521a into main Apr 21, 2025
37 checks passed
@gulducat gulducat deleted the NET-12503-operator-debug-env-region-ns branch April 21, 2025 21:06
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, 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 Aug 20, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

backport/ent/1.8.x+ent Changes are backported to 1.8.x+ent backport/ent/1.9.x+ent Changes are backported to 1.9.x+ent backport/1.10.x backport to 1.10.x release line theme/cli type/bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

nomad operator debug ignores NOMAD_REGION

2 participants