two fixes for inspect on connect proxy #7690
Conversation
There was a problem hiding this comment.
Is there a backward compatibility concern with struct renaming? Might need use tags rather than renaming field. Also, are these values set by user in the API or just read-only fields?
Given that nomad/structs are persisted, changing the field name would mean that future versions will silently drop 0.11.0 Expose field.
FWIWI, struct persistence use msgpack with codec tags, while api interaction uses json tag - so probably a clever use of tags can preserve compatibility while solving the issue.
shoenig
force-pushed
the
b-inspect-proxy-output
branch
2 times, most recently
from
35bd50f to
b956062
Compare
Before, the proxy stanza did not parse non-object fields `local_service_port` and `local_service_address` from the connect `proxy` stanza. This change fixes that.
…tions The field names within the structs representing the Connect proxy definition were not the same (nomad/structs/ vs api/), causing the values to be lost in translation for the 'nomad job inspect' command. Since the field names already shipped in v0.11.0 we cannot simply fix the names. Instead, use the json struct tag on the structs/ structs to remap the name to match the publicly expose api/ package on json encoding. This means existing jobs from v0.11.0 will continue to work, and the JSON API for job submission will remain backwards compatible.
shoenig
force-pushed
the
b-inspect-proxy-output
branch
from
b956062 to
eef81c3
Compare
|
Gah! yep, we need to preserve the field names as they're persisted in 0.11. Following your suggestion, I've now left the field names alone, and only applied |
|
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. |
This PR contains 2 commits, each with a fix around the response of
nomad job inspecton jobs containing aproxystanza. More details in each commit, but basicallyproxy.local_service_addressandproxy.local_service_portwas missingproxyrepresentation withinapiandnomad/structswere different, causing them to be lost in translation (i.e. JSON marshaling)Fixes #7397