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

AllocDirStats field missing from api definition of HostStats #20246

Closed
tomqwpl opened this issue Mar 28, 2024 · 4 comments · Fixed by #20261
Closed

AllocDirStats field missing from api definition of HostStats #20246

tomqwpl opened this issue Mar 28, 2024 · 4 comments · Fixed by #20261
Assignees
@tgross
Labels
stage/accepted Confirmed, and intend to work on. No timeline committment though. theme/api HTTP API and SDK issues type/bug
Projects
Nomad - Community Issues Triage

Comments

@tomqwpl
Copy link

tomqwpl commented Mar 28, 2024

Nomad version

This is in git on main as of today.

Issue

In api/nodes.go there is a definition of HostStats. I believe that this declared what a user of the nomadapi sees as the definition of that struct.
However, in client/hosttast.go there is what I believe is the server side version of this struct. This version has AllocDirStats field in. I can see that AllocDirStats is returned in REST API responses, but the nomadapi version of HostStats doesn't declare that field so it's not available to users of nomadapi.
@tgross tgross added this to Needs Triage in Nomad - Community Issues Triage via automation Apr 1, 2024
@tgross
Copy link
Member

tgross commented Apr 1, 2024

Hi @tomqwpl! Yeah this looks like a bug in api/nodes.go. The HTTP server on the client node uses this version: client/hoststats/host.go#L25. Most of our structs get defined on the Nomad server under nomad/structs, and we have to keep them in sync as part of our development checklist for new fields. But client structs like that one don't have an equivalent checklist, so looks like we missed it.

Easy fix though. We'll get that done.

@tgross tgross self-assigned this Apr 1, 2024
@tgross tgross added stage/accepted Confirmed, and intend to work on. No timeline committment though. theme/api HTTP API and SDK issues labels Apr 1, 2024
@tgross tgross moved this from Needs Triage to Triaging in Nomad - Community Issues Triage Apr 1, 2024
tgross added a commit that referenced this issue Apr 2, 2024
@tgross
api: add missing AllocDirStats field in Go API
29504a1 
The JSON response for the Read Stats client API includes an `AllocDirStats`
field. This field is missing in the `api` package, so consumers of the Go API
can't use it to read the values we're getting back from the HTTP server.

Fixes: #20246
@tgross tgross mentioned this issue Apr 2, 2024
Merged
tgross added a commit that referenced this issue Apr 2, 2024
@tgross
api: add missing AllocDirStats field in Go API
b8856eb 
The JSON response for the Read Stats client API includes an `AllocDirStats`
field. This field is missing in the `api` package, so consumers of the Go API
can't use it to read the values we're getting back from the HTTP server.

Fixes: #20246
@tgross
Copy link
Member

tgross commented Apr 2, 2024

I've got a quick fix up here: #20261

tgross added a commit that referenced this issue Apr 3, 2024
@tgross
api: add missing AllocDirStats field in Go API (#20261)
78f9f17 
The JSON response for the Read Stats client API includes an `AllocDirStats`
field. This field is missing in the `api` package, so consumers of the Go API
can't use it to read the values we're getting back from the HTTP server.

Fixes: #20246
Nomad - Community Issues Triage automation moved this from Triaging to Done Apr 3, 2024
This was referenced Apr 3, 2024
Merged
Merged
Merged
@tgross
Copy link
Member

tgross commented Apr 3, 2024 •
edited

Fixed in #20261, which will ship in the next 1.7.x release, with backports to 1.6.x and 1.5.x. Thanks @tomqwpl!

@tomqwpl
Copy link
Author

tomqwpl commented Apr 4, 2024

@tgross Apologies I didn't get time to create the PR before you got there, since it was a trivial fix I had done locally (albeit I didn't have the test cases)

philrenaud pushed a commit that referenced this issue Apr 18, 2024
@tgross @philrenaud
api: add missing AllocDirStats field in Go API (#20261)
84e253a 
The JSON response for the Read Stats client API includes an `AllocDirStats`
field. This field is missing in the `api` package, so consumers of the Go API
can't use it to read the values we're getting back from the HTTP server.

Fixes: #20246
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@tgross tgross

Labels
stage/accepted Confirmed, and intend to work on. No timeline committment though. theme/api HTTP API and SDK issues type/bug
Projects
Nomad - Community Issues Triage
Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

api: add missing AllocDirStats field in Go API hashicorp/nomad
2 participants
@tgross @tomqwpl