-
Notifications
You must be signed in to change notification settings - Fork 2k
cli: operator debug: respect NOMAD_REGION env var #25716
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
Conversation
NOMAD_REGION and NOMAD_NAMESPACE were being ignored
command/operator_debug.go
Outdated
|
|
||
| c.opts = &api.QueryOptions{ | ||
| Region: c.Meta.region, | ||
| Region: c.Meta.Region(), |
There was a problem hiding this comment.
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.
| Region: c.Meta.Region(), |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
|
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. |
Fixes #25672 - tl;dr
NOMAD_REGIONandare ignored byNOMAD_NAMESPACEnomad operator debug(edit: namespace does work, it just was not printed properly to the terminal)I thought about having
Meta.clientConfig()writeMeta.regionandMeta.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:
After: