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

config: use URL method to get Nomad address for plugins #966

Merged
merged 1 commit into from
Sep 11, 2024

Conversation

tgross
Copy link
Member

@tgross tgross commented Sep 5, 2024

For #944 we fixed the Nomad API package so that it no longer mutated the private url field if previously set, which allowed reusing an api.Config object between clients when a unix domain socket was in use.

However, the autoscaler plugins for Nomad strategy and target don't use the api.Config object we parse directly and instead get a map of string->string derived from that config so it can be passed over the go-plugin interface. This mapping did not account for the Address field being mutated when unix domain sockets are in use, so the bug was not actually fixed.

Update the mapping to use the safe URL() method on the config, rather than reading the Address field.

Fixes: #955
Ref: hashicorp/nomad#23785

@tgross
Copy link
Member Author

tgross commented Sep 5, 2024

Needs a test fix. Done!

For #944 we fixed the Nomad API package so that it no longer mutated the private
`url` field if previously set, which allowed reusing an `api.Config` object
between clients when a unix domain socket was in use.

However, the autoscaler plugins for Nomad strategy and target don't use the
`api.Config` object we parse directly and instead get a map of string->string
derived from that config so it can be passed over the go-plugin interface. This
mapping did not account for the `Address` field being mutated when unix domain
sockets are in use, so the bug was not actually fixed.

Update the mapping to use the safe `URL()` method on the config, rather than
reading the `Address` field.

Fixes: #955
Ref: hashicorp/nomad#23785
@tgross tgross marked this pull request as ready for review September 5, 2024 17:00
@tgross tgross requested review from gulducat and removed request for gulducat September 10, 2024 20:11
Copy link
Member

@Juanadelacuesta Juanadelacuesta left a comment

Choose a reason for hiding this comment

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

LGTM

@tgross tgross merged commit 180f107 into main Sep 11, 2024
33 checks passed
@tgross tgross deleted the nomad-config-url branch September 11, 2024 12:48
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.

Autoscaler can't connect to UNIX domain socket.
2 participants