diff --git a/api/allocations.go b/api/allocations.go index 9bd2d7aa656..2d859ffcc95 100644 --- a/api/allocations.go +++ b/api/allocations.go @@ -148,9 +148,6 @@ func (a *Allocations) GC(alloc *Allocation, q *QueryOptions) error { // Note: for cluster topologies where API consumers don't have network access to // Nomad clients, set api.ClientConnTimeout to a small value (ex 1ms) to avoid // long pauses on this API call. -// -// BREAKING: This method will have the following signature in 1.6.0 -// func (a *Allocations) Restart(allocID string, taskName string, allTasks bool, w *WriteOptions) (*WriteMeta, error) { func (a *Allocations) Restart(alloc *Allocation, taskName string, q *QueryOptions) error { req := AllocationRestartRequest{ TaskName: taskName, @@ -223,9 +220,6 @@ type AllocStopResponse struct { // Note: for cluster topologies where API consumers don't have network access to // Nomad clients, set api.ClientConnTimeout to a small value (ex 1ms) to avoid // long pauses on this API call. -// -// BREAKING: This method will have the following signature in 1.6.0 -// func (a *Allocations) Signal(allocID string, task string, signal string, w *WriteOptions) (*WriteMeta, error) { func (a *Allocations) Signal(alloc *Allocation, q *QueryOptions, task, signal string) error { req := AllocSignalRequest{ Signal: signal, diff --git a/api/api.go b/api/api.go index af960745d81..107457ce146 100644 --- a/api/api.go +++ b/api/api.go @@ -937,9 +937,8 @@ func (c *Client) query(endpoint string, out any, q *QueryOptions) (*QueryMeta, e return qm, nil } -// putQuery is used to do a PUT request when doing a read against an endpoint -// and deserialize the response into an interface using standard Nomad -// conventions. +// putQuery is used to do a PUT request when doing a "write" to a Client RPC. +// Client RPCs must use QueryOptions to allow setting AllowStale=true. func (c *Client) putQuery(endpoint string, in, out any, q *QueryOptions) (*QueryMeta, error) { r, err := c.newRequest("PUT", endpoint) if err != nil { @@ -969,6 +968,31 @@ func (c *Client) put(endpoint string, in, out any, q *WriteOptions) (*WriteMeta, return c.write(http.MethodPut, endpoint, in, out, q) } +// postQuery is used to do a POST request when doing a "write" to a Client RPC. +// Client RPCs must use QueryOptions to allow setting AllowStale=true. +func (c *Client) postQuery(endpoint string, in, out any, q *QueryOptions) (*QueryMeta, error) { + r, err := c.newRequest("POST", endpoint) + if err != nil { + return nil, err + } + r.setQueryOptions(q) + r.obj = in + rtt, resp, err := requireOK(c.doRequest(r)) + if err != nil { + return nil, err + } + defer resp.Body.Close() + + qm := &QueryMeta{} + parseQueryMeta(resp, qm) + qm.RequestTime = rtt + + if err := decodeBody(resp, out); err != nil { + return nil, err + } + return qm, nil +} + // post is used to do a POST request against an endpoint and // serialize/deserialized using the standard Nomad conventions. func (c *Client) post(endpoint string, in, out any, q *WriteOptions) (*WriteMeta, error) { diff --git a/api/node_meta.go b/api/node_meta.go index be851206bf7..4d5c4ffa590 100644 --- a/api/node_meta.go +++ b/api/node_meta.go @@ -30,9 +30,9 @@ func (n *Nodes) Meta() *NodeMeta { // Apply dynamic Node metadata updates to a Node. If NodeID is unset then Node // receiving the request is modified. -func (n *NodeMeta) Apply(meta *NodeMetaApplyRequest, qo *WriteOptions) (*NodeMetaResponse, error) { +func (n *NodeMeta) Apply(meta *NodeMetaApplyRequest, qo *QueryOptions) (*NodeMetaResponse, error) { var out NodeMetaResponse - _, err := n.client.post("/v1/client/metadata", meta, &out, qo) + _, err := n.client.postQuery("/v1/client/metadata", meta, &out, qo) if err != nil { return nil, err } diff --git a/command/agent/meta_endpoint.go b/command/agent/meta_endpoint.go index f07fe014a47..28250554944 100644 --- a/command/agent/meta_endpoint.go +++ b/command/agent/meta_endpoint.go @@ -59,7 +59,7 @@ func (s *HTTPServer) nodeMetaApply(resp http.ResponseWriter, req *http.Request) return nil, CodedError(http.StatusBadRequest, err.Error()) } - s.parseWriteRequest(req, &args.WriteRequest) + s.parse(resp, req, &args.QueryOptions.Region, &args.QueryOptions) parseNode(req, &args.NodeID) // Determine the handler to use diff --git a/contributing/checklist-rpc-endpoint.md b/contributing/checklist-rpc-endpoint.md index f8df1c0c8e7..a961d8ca00c 100644 --- a/contributing/checklist-rpc-endpoint.md +++ b/contributing/checklist-rpc-endpoint.md @@ -42,6 +42,14 @@ Prefer adding a new message to changing any existing RPC messages. upgraded, so use this to guard sending the new RPC, else send the old RPC * Version must match the actual release version! +* [ ] If implementing a Client RPC... + * Use `QueryOptions` instead of `WriteRequest` in the Request struct as + `WriteRequest` is only for *Raft* writes. + * Set `QueryOptions.AllowStale = true` in the *Server* RPC forwarder to avoid + an infinite loop between leaders and followers when a Client RPC is + forwarded through a follower. See + https://github.com/hashicorp/nomad/issues/16517 + ## Docs * [ ] Changelog diff --git a/nomad/client_meta_endpoint.go b/nomad/client_meta_endpoint.go index 80c7d237b77..1795a6abad0 100644 --- a/nomad/client_meta_endpoint.go +++ b/nomad/client_meta_endpoint.go @@ -25,6 +25,10 @@ func newNodeMetaEndpoint(srv *Server) *NodeMeta { func (n *NodeMeta) Apply(args *structs.NodeMetaApplyRequest, reply *structs.NodeMetaResponse) error { const method = "NodeMeta.Apply" + // Prevent infinite loop between leader and + // follower-with-the-target-node-connection. + args.QueryOptions.AllowStale = true + authErr := n.srv.Authenticate(nil, args) if done, err := n.srv.forward(method, args, args, reply); done { return err @@ -48,6 +52,10 @@ func (n *NodeMeta) Apply(args *structs.NodeMetaApplyRequest, reply *structs.Node func (n *NodeMeta) Read(args *structs.NodeSpecificRequest, reply *structs.NodeMetaResponse) error { const method = "NodeMeta.Read" + // Prevent infinite loop between leader and + // follower-with-the-target-node-connection. + args.QueryOptions.AllowStale = true + authErr := n.srv.Authenticate(nil, args) if done, err := n.srv.forward(method, args, args, reply); done { return err diff --git a/nomad/structs/node.go b/nomad/structs/node.go index f930b3385af..f025335ec94 100644 --- a/nomad/structs/node.go +++ b/nomad/structs/node.go @@ -357,7 +357,7 @@ func (di *DriverInfo) HealthCheckEquals(other *DriverInfo) bool { // NodeMetaApplyRequest is used to update Node metadata on Client agents. type NodeMetaApplyRequest struct { - WriteRequest + QueryOptions // Client RPCs must use QueryOptions to set AllowStale=true // NodeID is the node being targeted by this request (or the node // receiving this request if NodeID is empty).