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.

Sign up for GitHub

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

Jump to bottom

Nomad unix socket doesn't work. #944

Closed
jorgemarey opened this issue Aug 7, 2024 · 2 comments · Fixed by #950
Closed

Nomad unix socket doesn't work. #944

jorgemarey opened this issue Aug 7, 2024 · 2 comments · Fixed by #950
Assignees
@tgross

Comments

@jorgemarey
Copy link
Contributor

When setting the address to the nomad unix api socket, the autoscaler stops working correctly.

The following errors are shown constantly

2024-08-07T10:15:32.127Z [WARN]  policy_manager.policy_handler: failed to get target status: policy_id=798ce9d8-6074-3295-692b-281d2fa52fd6 error="Get \"http://127.0.0.1/v1/job/test-job/scale?namespace=test&region=mx&wait=300000ms\": dial tcp 127.0.0.1:80: connect: connection refused"
2024-08-07T10:15:32.127Z [ERROR] policy_manager.policy_handler: Get "http://127.0.0.1/v1/job/test-job/scale?namespace=test&region=mx&wait=300000ms": dial tcp 127.0.0.1:80: connect: connection refused: policy_id=798ce9d8-6074-3295-692b-281d2fa52fd6

It seems that the problem is that when setting the unix socket in the adress nomad mutates that Address and that is later used on the nomad target plugin instead of the unix address.
@jorgemarey jorgemarey mentioned this issue Aug 7, 2024
Closed
@tgross tgross self-assigned this Aug 8, 2024
tgross added a commit to hashicorp/nomad that referenced this issue Aug 9, 2024
@tgross
api: only set url field in config if previously unset
155e371 
In #16872 we added support for configuring the API client with a unix domain
socket. In order to set the host correctly, we parse the address before mutating
the Address field in the configuration. But this prevents the configuration from
being reused across multiple clients, as the next time we parse the address it
will no longer be pointing to the socket. This breaks consumers like the
autoscaler, which reuse the API config between plugins.

Update the `NewClient` constructor to only override the `url` field if it hasn't
already been parsed. Include a test demonstrating safe reuse with a unix domain
socket.

Ref: hashicorp/nomad-autoscaler#944
Ref: hashicorp/nomad-autoscaler#945
@tgross tgross mentioned this issue Aug 9, 2024
Merged
tgross added a commit to hashicorp/nomad that referenced this issue Aug 9, 2024
@tgross
api: only set url field in config if previously unset
c31a760 
In #16872 we added support for configuring the API client with a unix domain
socket. In order to set the host correctly, we parse the address before mutating
the Address field in the configuration. But this prevents the configuration from
being reused across multiple clients, as the next time we parse the address it
will no longer be pointing to the socket. This breaks consumers like the
autoscaler, which reuse the API config between plugins.

Update the `NewClient` constructor to only override the `url` field if it hasn't
already been parsed. Include a test demonstrating safe reuse with a unix domain
socket.

Ref: hashicorp/nomad-autoscaler#944
Ref: hashicorp/nomad-autoscaler#945
tgross added a commit to hashicorp/nomad that referenced this issue Aug 9, 2024
@tgross
api: only set url field in config if previously unset (#23785)
b7419bc 
In #16872 we added support for configuring the API client with a unix domain
socket. In order to set the host correctly, we parse the address before mutating
the Address field in the configuration. But this prevents the configuration from
being reused across multiple clients, as the next time we parse the address it
will no longer be pointing to the socket. This breaks consumers like the
autoscaler, which reuse the API config between plugins.

Update the `NewClient` constructor to only override the `url` field if it hasn't
already been parsed. Include a test demonstrating safe reuse with a unix domain
socket.

Ref: hashicorp/nomad-autoscaler#944
Ref: hashicorp/nomad-autoscaler#945
Merged
tgross added a commit that referenced this issue Aug 13, 2024
@tgross
deps: update Nomad API package to 1.8.3
49d0208 
Fixes: #944
@tgross tgross mentioned this issue Aug 13, 2024
Merged
tgross added a commit that referenced this issue Aug 13, 2024
@tgross
deps: update Nomad API package to 1.8.3
26b2c87 
Fixes: #944
tgross added a commit that referenced this issue Aug 13, 2024
@tgross
deps: update Nomad API package to 1.8.3
2ed8f9d 
Fixes: #944
@tgross tgross closed this as completed in 78aa924 Aug 13, 2024
@nwmqpa nwmqpa mentioned this issue Aug 20, 2024
Closed
@jorgemarey
Copy link
Contributor Author

Hi @tgross, this change is not working (also a new issue was opened related to this #955 ). I think the problem is that it's not the same configuration being used, a new configuration is being created here

@tgross
Copy link
Member

tgross commented Sep 4, 2024

Thanks @jorgemarey... taking a look. Not sure what happened there. 😦

tgross added a commit that referenced this issue Sep 5, 2024
@tgross
config: use URL method to get Nomad address for plugins
4d068d3 
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 mentioned this issue Sep 5, 2024
Merged
tgross added a commit that referenced this issue Sep 5, 2024
@tgross
config: use URL method to get Nomad address for plugins
b03bce5 
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 added a commit that referenced this issue Sep 5, 2024
@tgross
config: use URL method to get Nomad address for plugins
6d416bf 
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 added a commit that referenced this issue Sep 11, 2024
@tgross
config: use URL method to get Nomad address for plugins (#966)
180f107 
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@tgross tgross

Labels
None yet
Projects
Nomad - Community Issues Triage
Status: Done
Milestone
No milestone
2 participants
@tgross @jorgemarey